Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add multi-owner modular account updateOwners test #1160

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −0
Open

test: add multi-owner modular account updateOwners test #1160

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions account-kit/smart-contracts/src/msca/client/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,64 @@ describe("Modular Account Multi Owner Account Tests", async () => {
`);
}, 100000);

it("should update ownership successfully", async () => {
const provider = await givenConnectedProvider({
signer: signer2,
owners,
});
await setBalance(instance.getClient(), {
address: provider.getAddress(),
value: parseEther("1000"),
});
const signer3 = LocalAccountSigner.mnemonicToAccountSigner(
MODULAR_MULTIOWNER_ACCOUNT_OWNER_MNEMONIC,
{ accountIndex: 2 }
);

// Deploy account and update owners. Remove signer1 and add signer3.
let result = await provider.updateOwners({
args: [[await signer3.getAddress()], [await signer1.getAddress()]],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is signer1 already on the account? or is this part of the test -- it should not revert if a signer is not on the account?

I ask because you attach signer2 above to this and wanted to make sure this was intentional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signer1 and signer2 are the initial owners. So here we deploy, set the initial owners, and update them at the same time. This one doesn't revert.

});
await provider.waitForUserOperationTransaction(result);

let newOwnerAddresses = await provider.readOwners({
pluginAddress: "0xcE0000007B008F50d762D155002600004cD6c647",
});
expect(newOwnerAddresses).not.toContain(await signer1.getAddress());
expect(newOwnerAddresses).toHaveLength(2);
expect(newOwnerAddresses).toEqual(
expect.arrayContaining([
await signer2.getAddress(),
await signer3.getAddress(),
])
);

// Update owners again. This time, remove signer3 and add signer1.
result = await provider.updateOwners({
args: [[await signer1.getAddress()], [await signer3.getAddress()]],
overrides: {
maxFeePerGas: 1_851_972_078n * 2n,
maxPriorityFeePerGas: 1_000_000_000n * 2n,
callGasLimit: 43_960n * 2n,
verificationGasLimit: 92_326n * 2n,
preVerificationGas: 46_248n * 2n,
},
Comment on lines +203 to +209
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to figure out why these values need to be increased. Doubling the derived values got the UO to succeed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heads up: each of these fields supports a {multiplier: 2n} instead of raw bigint

Copy link
Collaborator

@moldy530 moldy530 Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm that's strange... we still need to merge in wevm/prool#26 because our tests use rundler 0.2.0 and we're on 0.4.0 already... I know there's been some changes to the pvg logic in later versions of rundler, so I wonder if that's related

maybe we should fork prool and use that instead

});
await provider.waitForUserOperationTransaction(result);

newOwnerAddresses = await provider.readOwners({
pluginAddress: "0xcE0000007B008F50d762D155002600004cD6c647",
});
expect(newOwnerAddresses).not.toContain(await signer3.getAddress());
expect(newOwnerAddresses).toHaveLength(2);
expect(newOwnerAddresses).toEqual(
expect.arrayContaining([
await signer1.getAddress(),
await signer2.getAddress(),
])
);
}, 100000);

const givenConnectedProvider = ({
signer,
accountAddress,
Expand Down
Loading