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 t8n tool errors #797

Merged
merged 5 commits into from
Jul 12, 2023
Merged

fix t8n tool errors #797

merged 5 commits into from
Jul 12, 2023

Conversation

gurukamath
Copy link
Collaborator

@gurukamath gurukamath commented Jun 20, 2023

What was wrong?

Some edge cases were not handled properly by t8n

Related to Issue #773

How was it fixed?

  1. Fix bug to accept state fork ConstantinopleFix instead of Constantinople
  2. Update evm tools tests to use GeneralStateTests fixtures
  3. Handle invalid transactions without crashing the t8n tool.
  4. Handle a couple of testing edge cases.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@gurukamath gurukamath marked this pull request as draft June 20, 2023 17:50
@gurukamath gurukamath marked this pull request as ready for review June 22, 2023 07:38
@gurukamath gurukamath requested a review from SamWilsn June 22, 2023 07:50
This commit handles a couple of edge cases

1. There are some error messages in the test fixtures that could raise RLPDecodingError or InvalidBlock depending on the context. For example TR_TypeNotSupported. If we try to include a typed transaction in Istanbul, it causes an RLPDecodingError. However, if we include a type 2 transaction in Berlin, it won't raise a RLPDecodingError since type1 and type2 are indistinguishable while decoding the rlp. In this case, it raises an InvalidBlock Exception. Therefore we stop making a distinction between these cases in the context of testing.

2. The test HighGasPrice raises a ValueError. We just raise a InvalidBlock from it.
src/ethereum/arrow_glacier/fork.py Outdated Show resolved Hide resolved
src/ethereum_spec_tools/evm_tools/t8n/t8n_types.py Outdated Show resolved Hide resolved
src/ethereum_spec_tools/evm_tools/utils.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 80.21% and no project coverage change.

Comparison is base (03f0204) 74.06% compared to head (d4cde00) 74.06%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #797   +/-   ##
=======================================
  Coverage   74.06%   74.06%           
=======================================
  Files         571      571           
  Lines       31721    31726    +5     
=======================================
+ Hits        23494    23498    +4     
- Misses       8227     8228    +1     
Flag Coverage Δ
unittests 74.06% <80.21%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ethereum/arrow_glacier/fork.py 0.00% <0.00%> (ø)
src/ethereum/arrow_glacier/utils/message.py 0.00% <ø> (ø)
src/ethereum/arrow_glacier/vm/__init__.py 0.00% <0.00%> (ø)
...ereum/arrow_glacier/vm/instructions/environment.py 0.00% <0.00%> (ø)
...c/ethereum/arrow_glacier/vm/instructions/system.py 0.00% <0.00%> (ø)
src/ethereum/arrow_glacier/vm/interpreter.py 0.00% <0.00%> (ø)
src/ethereum/berlin/fork.py 93.83% <ø> (ø)
src/ethereum/berlin/utils/message.py 96.00% <ø> (ø)
src/ethereum/byzantium/fork.py 95.67% <ø> (ø)
src/ethereum/byzantium/utils/message.py 94.73% <ø> (ø)
... and 95 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@SamWilsn SamWilsn merged commit 2da1ad3 into ethereum:master Jul 12, 2023
4 checks passed
@gurukamath gurukamath deleted the t8n-tool-errors branch August 16, 2023 09:37
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.

4 participants