-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Panic error support #2986
Conversation
8951d47
to
44d0895
Compare
This should be ready for review now |
There was a problem hiding this 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.
@@ -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: |
There was a problem hiding this comment.
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!
44d0895
to
52673dd
Compare
- 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.
52673dd
to
dbc58db
Compare
There was a problem hiding this 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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
What was wrong?
closes #2985
How was it fixed?
_utils
file for better code readabilityPanic
errorsEthereumTesterProvider
and integration tests testing againstgeth
shanghaiTime
(also added the glacier blocks to the config though those are less important since it was just difficulty bomb timing).** 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