test: add multi-owner modular account updateOwners test #1160
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Your org has enabled the Graphite merge queue for merging into mainAdd the label “graphite-merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
jaypaik
requested review from
moldy530,
rthomare,
dancoombs,
mokok123,
dphilipson and
linnall
as code owners
jaypaik
force-pushed
the
11-17-test_add_multi-owner_modular_account_updateowners_test
branch
from
1983a71
to
908d16e
Compare
| 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, | ||
| }, |
There was a problem hiding this comment.
Need to figure out why these values need to be increased. Doubling the derived values got the UO to succeed.
There was a problem hiding this comment.
heads up: each of these fields supports a {multiplier: 2n} instead of raw bigint
There was a problem hiding this comment.
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
|
|
||
| // Deploy account and update owners. Remove signer1 and add signer3. | ||
| let result = await provider.updateOwners({ | ||
| args: [[await signer3.getAddress()], [await signer1.getAddress()]], |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Debugging inaccurate gas/fee estimation for modular account's
updateOwners. Bumped values withoverridesto get test to pass. Putting this up for 👀 .Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
This PR adds a test case to verify the functionality of updating ownership in a smart contract. It checks the successful removal and addition of owners using different signers.
Detailed summary
should update ownership successfully.signer2and set an initial balance.signer1and addingsigner3.signer1was removed andsigner2,signer3were present.signer3and addsigner1.