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

Panic error support #2986

Merged
merged 7 commits into from
Jun 8, 2023
Merged

Panic error support #2986

merged 7 commits into from
Jun 8, 2023

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Jun 7, 2023

What was wrong?

closes #2985

How was it fixed?

  • Refactor contract error handling into its own _utils file for better code readability
  • Add support for Solidity >= 0.8.0 Panic errors
  • Create a test contract that tests all known cases directly
  • Add core tests testing against EthereumTesterProvider and integration tests testing against geth
  • Update config for go-ethereum integration test suite to turn on Shanghai by specifying a shanghaiTime (also added the glacier blocks to the config though those are less important since it was just difficulty bomb timing).
  • Regenerate test fixture so the compiled contracts are updated**

** It turns out we were a few geth versions behind though the filename was misleading. This PR regenerates the test fixture with 1.11.6 and bumps the CI's py-geth version and geth version as well.

Todo:

Cute Animal Picture

20230607_160333

fselmo added a commit to fselmo/web3.py that referenced this pull request Jun 7, 2023
@fselmo fselmo marked this pull request as ready for review June 7, 2023 22:08
@fselmo fselmo requested review from kclowes, pacrob and reedsa June 7, 2023 22:10
fselmo added a commit to fselmo/web3.py that referenced this pull request Jun 7, 2023
@fselmo fselmo force-pushed the panic-error-support branch from 8951d47 to 44d0895 Compare June 7, 2023 22:16
@fselmo
Copy link
Collaborator Author

fselmo commented Jun 7, 2023

This should be ready for review now

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Nice refactor! This looks good to me! I just left one nit that you can definitely ignore.

web3/_utils/contract_error_handling.py Outdated Show resolved Hide resolved
@@ -698,95 +691,6 @@ def apply_list_to_array_formatter(formatter: Any) -> Callable[..., Any]:
STANDARD_NORMALIZERS, RPC_ABIS
)

# the first 4 bytes of keccak hash for:
Copy link
Collaborator

Choose a reason for hiding this comment

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

So much better. Nice cleanup!

fselmo added a commit to fselmo/web3.py that referenced this pull request Jun 8, 2023
@fselmo fselmo force-pushed the panic-error-support branch from 44d0895 to 52673dd Compare June 8, 2023 16:03
fselmo added 6 commits June 8, 2023 10:17
- Refactor contract error handling to its own file for better readability. Some re-structuring of the ``raise_contract_logic_error_on_revert()`` method could probably also be done but for now just move this over to its own spot in the ``_utils``.
- Add support for Solidity Panic errors and provide clear messaging for each defined error code
- Create and compile a Solidity contract for testing, PanicErrorsContract.sol
- Add core tests for use with EthereumTesterProvider / eth-tester + py-evm
- The test fixture had not been updated to 1.11.5 thought he name stated so. This commit adds the ``shanghaiBlock`` as well as the last few glacier blocks which only changed the difficulty bomb timing so shouldn't have made much of a difference on the API.
- Regeneration of the fixture is now guaranteed to be using 1.11.6

Related changes to ethereum#2985

- Add integration tests for Solidity Panic errors and, with the regeneration of the fixture above, make sure they work against Geth.
@fselmo fselmo force-pushed the panic-error-support branch from 52673dd to dbc58db Compare June 8, 2023 16:19
web3/exceptions.py Outdated Show resolved Hide resolved
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

lgtm

def __init__(
self,
message: Optional[str] = None,
data: Optional[Union[str, Dict[str, str]]] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@fselmo fselmo merged commit ab1644b into ethereum:main Jun 8, 2023
fselmo added a commit that referenced this pull request Jun 8, 2023
@fselmo fselmo deleted the panic-error-support branch June 8, 2023 17:10
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.

Support the Solidity Panic error
3 participants