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: entrypoint does not revert even first postOp reverts with short … #293

Merged
merged 5 commits into from
Jun 25, 2023

Conversation

leekt
Copy link
Contributor

@leekt leekt commented May 30, 2023

…revert reason

Entrypoint should retry the postOp when first postOp reverts, but it throws the whole bundle when firts postOp reverted with revert reason shorter than 32 bytes because entrypoint tries to mload the 32 bytes.

This is not a critical bug that requires Entrypoint to be re-deployed, But i'm still posting the PR to acknowledge the paymaster builders.

To not run into this issue, make sure your paymaster does not revert with short revert data on postOp

  1. do not use custom opcode that does not have any parameter.
  2. do not use revert()/require() without data

If future entrypoint still has retry logic on postOp, I think this should be handled properly.

fixes #289

@drortirosh
Copy link
Contributor

This is true.
But we will not merge it in until we have some other reason to update the entrypoint.
Just note that this is required for testing only: on GETHreturndatacopy silently return if there is no data. only on hardhat it reverts.

@drortirosh drortirosh added the defer-to-next-version Should be merged when we have some other breaking change. label Jun 7, 2023
@leekt
Copy link
Contributor Author

leekt commented Jun 7, 2023

That is weird 🤔 Maybe this is a bug in hardhat side

@leekt
Copy link
Contributor Author

leekt commented Jun 8, 2023

I have tested with localgeth docker image and got error, seems geth is reverting too.

     return data out of bounds
  ProviderError: HttpProviderError
      at HttpProvider.request (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/hardhat/src/internal/core/providers/http.ts:78:19)
      at AutomaticSenderProvider.request (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/hardhat/src/internal/core/providers/accounts.ts:344:34)
      at AutomaticGasProvider.request (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/hardhat/src/internal/core/providers/gas-providers.ts:135:34)
      at AutomaticGasPriceProvider.request (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/hardhat/src/internal/core/providers/gas-providers.ts:153:36)
      at BackwardsCompatibilityProviderAdapter.send (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/hardhat/src/internal/core/providers/backwards-compatibility.ts:36:27)
      at EthersProviderWrapper.send (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/@nomiclabs/hardhat-ethers/src/internal/ethers-provider-wrapper.ts:13:48)
      at EthersProviderWrapper.<anonymous> (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/@ethersproject/providers/src.ts/json-rpc-provider.ts:640:31)
      at step (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/@ethersproject/providers/lib/json-rpc-provider.js:48:23)
      at Object.next (/Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/@ethersproject/providers/lib/json-rpc-provider.js:29:53)
      at /Users/leekt/workspace/leekt/leekt-account-abstraction/node_modules/@ethersproject/providers/lib/json-rpc-provider.js:23:71

@forshtat forshtat changed the base branch from develop to next_v0.7 June 25, 2023 19:57
@forshtat forshtat merged commit 74a3c37 into eth-infinitism:next_v0.7 Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-to-next-version Should be merged when we have some other breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle 'postOp' revert with <32 bytes size
3 participants