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.

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

fix: fix paymaster override for multisig plugin #676

Merged
merged 9 commits into from
Jun 4, 2024
Merged

Conversation

Zer0dot
Copy link
Collaborator

@Zer0dot Zer0dot commented May 21, 2024

Pull Request Checklist

This PR loosens the restrictions on the paymaster middleware to allow overriding with nonzero paymaster-related fields. This allows accounts with the multisig plugin to sign over the correctly built userOp.

TODO

  • Adjust the Alchemy GasManager to handle the case where the override is nonzero

Let me know if there's anything missing to get this over the line.


PR-Codex overview

This PR updates the middleware logic for bypassing paymaster and paymaster data handling, adds new utility methods, and refactors multisig account execution flow.

Detailed summary

  • Added bypassPaymasterAndDataEmptyHex utility method
  • Updated middleware logic in alchemyGasManagerMiddleware
  • Refactored multisig account execution flow in multisig-account.e2e.test.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

moldy530
moldy530 previously approved these changes May 24, 2024
Copy link
Collaborator

@moldy530 moldy530 left a comment

Choose a reason for hiding this comment

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

nice! yea LGTM as long as all of the multi-sig e2e tests pass.

@Zer0dot
Copy link
Collaborator Author

Zer0dot commented May 27, 2024

nice! yea LGTM as long as all of the multi-sig e2e tests pass.

One's still failing with AA23 or OOG, debugging that and we'll see if it's connected to this change!

@Zer0dot Zer0dot marked this pull request as ready for review June 3, 2024 14:00
@Zer0dot
Copy link
Collaborator Author

Zer0dot commented Jun 3, 2024

Update wrt the error-- adding the first signature generated when the userOp is first built to the signatures array fixes it. I modified the test to run both as a normal 3/3, and then as a 3/3 with paymaster.

Should be good to go!

@Zer0dot Zer0dot requested a review from moldy530 June 3, 2024 16:14
@rthomare
Copy link
Collaborator

rthomare commented Jun 4, 2024

Nice work!

Great Job GIF by HockeyDiversityAlliance

Copy link

vercel bot commented Jun 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
aa-sdk-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 4, 2024 3:36pm

@Zer0dot Zer0dot merged commit 6db5bfc into main Jun 4, 2024
5 checks passed
@Zer0dot Zer0dot deleted the fix/msig-paymaster branch June 4, 2024 15:47
Dan-Nolan pushed a commit that referenced this pull request Jun 6, 2024
* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test
moldy530 added a commit that referenced this pull request Jun 6, 2024
* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: adds streamlined quick start

* docs: drops the file extension on local quick start links

* docs: fix relative routes across quick start

* feat: run store hydration on window focus (#695)

* run store hydration on window focus

* remove logs

* feat: update button colors (#696)

* feat: update button colors

* fix: update test

* fix: plugins were missing mainnet config (#692)

* fix: account hydration was missing state (#693)

* chore: enable version syncing when publishing (#694)

* chore(release): publish v3.18.0 [skip-ci]

* fix: fix paymaster override for multisig plugin (#676)

* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test

* chore: rename button "type" to "variant"  (#699)

* feat: switch type => variant and use type submit

* fix: add custom focus state

* fix: update the version of the store so that we don't have clashes (#703)

* chore(release): publish v3.18.1 [skip-ci]

* chore(release): publish v3.18.2 [skip-ci]

* feat: add updated error UI + custom error rendering  (#705)

* feat: add error boundary by id

* fix: remove unused classnames

* feat: use error from hook

* fix: remove unused variables

* fix: remove console log

* fix: add new colors to test

* feat: rename to useAuthError

* docs: add vercel json (#700)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json

* docs: add build script

* docs: add rewrites for .html`

* docs: try cleanUrls

* docs: remove rewrites

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>

* chore: remove the docs publishing github action

* docs: run locally meets dashboard quickstart

* chore: update the gh actions to checkout submodules

* chore: add embedded accounts submodule

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>
Co-authored-by: Tina <59578595+tinaszheng@users.noreply.github.com>
Co-authored-by: Alchemy Bot <alchemy-bot@alchemy.com>
Co-authored-by: Zer0dot <zer0dot.dev@gmail.com>
Co-authored-by: Cody McCabe <cmccabe524@gmail.com>
moldy530 added a commit that referenced this pull request Jun 6, 2024
* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: adds streamlined quick start

* docs: drops the file extension on local quick start links

* docs: fix relative routes across quick start

* feat: run store hydration on window focus (#695)

* run store hydration on window focus

* remove logs

* feat: update button colors (#696)

* feat: update button colors

* fix: update test

* fix: plugins were missing mainnet config (#692)

* fix: account hydration was missing state (#693)

* chore: enable version syncing when publishing (#694)

* chore(release): publish v3.18.0 [skip-ci]

* fix: fix paymaster override for multisig plugin (#676)

* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test

* chore: rename button "type" to "variant"  (#699)

* feat: switch type => variant and use type submit

* fix: add custom focus state

* fix: update the version of the store so that we don't have clashes (#703)

* chore(release): publish v3.18.1 [skip-ci]

* chore(release): publish v3.18.2 [skip-ci]

* feat: add updated error UI + custom error rendering  (#705)

* feat: add error boundary by id

* fix: remove unused classnames

* feat: use error from hook

* fix: remove unused variables

* fix: remove console log

* fix: add new colors to test

* feat: rename to useAuthError

* docs: add vercel json (#700)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json

* docs: add build script

* docs: add rewrites for .html`

* docs: try cleanUrls

* docs: remove rewrites

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>

* chore: remove the docs publishing github action

* docs: run locally meets dashboard quickstart

* chore: update the gh actions to checkout submodules

* chore: add embedded accounts submodule

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>
Co-authored-by: Tina <59578595+tinaszheng@users.noreply.github.com>
Co-authored-by: Alchemy Bot <alchemy-bot@alchemy.com>
Co-authored-by: Zer0dot <zer0dot.dev@gmail.com>
Co-authored-by: Cody McCabe <cmccabe524@gmail.com>
moldy530 added a commit that referenced this pull request Jun 13, 2024
* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: adds streamlined quick start

* docs: drops the file extension on local quick start links

* docs: fix relative routes across quick start

* feat: run store hydration on window focus (#695)

* run store hydration on window focus

* remove logs

* feat: update button colors (#696)

* feat: update button colors

* fix: update test

* fix: plugins were missing mainnet config (#692)

* fix: account hydration was missing state (#693)

* chore: enable version syncing when publishing (#694)

* chore(release): publish v3.18.0 [skip-ci]

* fix: fix paymaster override for multisig plugin (#676)

* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test

* chore: rename button "type" to "variant"  (#699)

* feat: switch type => variant and use type submit

* fix: add custom focus state

* fix: update the version of the store so that we don't have clashes (#703)

* chore(release): publish v3.18.1 [skip-ci]

* chore(release): publish v3.18.2 [skip-ci]

* feat: add updated error UI + custom error rendering  (#705)

* feat: add error boundary by id

* fix: remove unused classnames

* feat: use error from hook

* fix: remove unused variables

* fix: remove console log

* fix: add new colors to test

* feat: rename to useAuthError

* docs: add vercel json (#700)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json

* docs: add build script

* docs: add rewrites for .html`

* docs: try cleanUrls

* docs: remove rewrites

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>

* chore: remove the docs publishing github action

* docs: run locally meets dashboard quickstart

* chore: update the gh actions to checkout submodules

* chore: add embedded accounts submodule

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>
Co-authored-by: Tina <59578595+tinaszheng@users.noreply.github.com>
Co-authored-by: Alchemy Bot <alchemy-bot@alchemy.com>
Co-authored-by: Zer0dot <zer0dot.dev@gmail.com>
Co-authored-by: Cody McCabe <cmccabe524@gmail.com>
moldy530 added a commit that referenced this pull request Jun 13, 2024
* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json (#700)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json

* docs: add build script

* docs: add rewrites for .html`

* docs: try cleanUrls

* docs: remove rewrites

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>

* chore: remove the docs publishing github action

* docs: adds streamlined quick start (#697)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: adds streamlined quick start

* docs: drops the file extension on local quick start links

* docs: fix relative routes across quick start

* feat: run store hydration on window focus (#695)

* run store hydration on window focus

* remove logs

* feat: update button colors (#696)

* feat: update button colors

* fix: update test

* fix: plugins were missing mainnet config (#692)

* fix: account hydration was missing state (#693)

* chore: enable version syncing when publishing (#694)

* chore(release): publish v3.18.0 [skip-ci]

* fix: fix paymaster override for multisig plugin (#676)

* fix: allow nonzero paymaster override

* refactor: update to use Hex type for paymasterAndData/paymasterData override

* fix: add checks for paymasterData and paymaster field nonzero overrides

* fix: add paymaster address override type

* feat: gas manager only bypasses paymaster middleware when the UserOp paying for its own gas

* fix: add signatures and overrides to fix 3/3 msig test and add paymaster test

* chore: rename button "type" to "variant"  (#699)

* feat: switch type => variant and use type submit

* fix: add custom focus state

* fix: update the version of the store so that we don't have clashes (#703)

* chore(release): publish v3.18.1 [skip-ci]

* chore(release): publish v3.18.2 [skip-ci]

* feat: add updated error UI + custom error rendering  (#705)

* feat: add error boundary by id

* fix: remove unused classnames

* feat: use error from hook

* fix: remove unused variables

* fix: remove console log

* fix: add new colors to test

* feat: rename to useAuthError

* docs: add vercel json (#700)

* docs: revert revert to use vocs for docs

* docs: address some of the broken things on the site (#689)

* docs: update the logos for AK

* docs: add vercel json

* docs: add build script

* docs: add rewrites for .html`

* docs: try cleanUrls

* docs: remove rewrites

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>

* chore: remove the docs publishing github action

* docs: run locally meets dashboard quickstart

* chore: update the gh actions to checkout submodules

* chore: add embedded accounts submodule

---------

Co-authored-by: moldy <michael.moldoveanu@alchemy.com>
Co-authored-by: Tina <59578595+tinaszheng@users.noreply.github.com>
Co-authored-by: Alchemy Bot <alchemy-bot@alchemy.com>
Co-authored-by: Zer0dot <zer0dot.dev@gmail.com>
Co-authored-by: Cody McCabe <cmccabe524@gmail.com>

---------

Co-authored-by: Cody McCabe <cmccabe524@gmail.com>
Co-authored-by: Dan Nolan <dan.nolan@alchemy.com>
Co-authored-by: Tina <59578595+tinaszheng@users.noreply.github.com>
Co-authored-by: Alchemy Bot <alchemy-bot@alchemy.com>
Co-authored-by: Zer0dot <zer0dot.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants