-
Notifications
You must be signed in to change notification settings - Fork 390
feat: add mainnet tests for osaka #1806
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
base: forks/osaka
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||
| """ | ||||||||
| EIP-7883 ModExp gas cost increase tests. | ||||||||
|
|
||||||||
| Tests for ModExp gas cost increase in | ||||||||
| [EIP-7883: ModExp Gas Cost Increase](https://eips.ethereum.org/EIPS/eip-7883). | ||||||||
| """ | ||||||||
|
|
||||||||
| from typing import Dict | ||||||||
|
|
||||||||
| import pytest | ||||||||
| from execution_testing import ( | ||||||||
| Alloc, | ||||||||
| StateTestFiller, | ||||||||
| Transaction, | ||||||||
| ) | ||||||||
|
|
||||||||
| from ...byzantium.eip198_modexp_precompile.helpers import ModExpInput | ||||||||
| from .spec import ref_spec_7883 | ||||||||
|
|
||||||||
| REFERENCE_SPEC_GIT_PATH = ref_spec_7883.git_path | ||||||||
| REFERENCE_SPEC_VERSION = ref_spec_7883.version | ||||||||
|
|
||||||||
|
|
||||||||
| @pytest.mark.parametrize( | ||||||||
| "modexp_input,modexp_expected", | ||||||||
| [ | ||||||||
| pytest.param( | ||||||||
| ModExpInput( | ||||||||
| base="ff" * 31 + "fc", | ||||||||
| exponent="02", | ||||||||
| modulus="05", | ||||||||
| declared_base_length=32, | ||||||||
| declared_exponent_length=1, | ||||||||
| declared_modulus_length=1, | ||||||||
| ), | ||||||||
| bytes([4]), | ||||||||
| id="32-bytes-long-base", | ||||||||
| ), | ||||||||
| pytest.param( | ||||||||
| ModExpInput( | ||||||||
| base="ff" * 32 + "fb", | ||||||||
| exponent="02", | ||||||||
| modulus="05", | ||||||||
| declared_base_length=33, | ||||||||
| declared_exponent_length=1, | ||||||||
| declared_modulus_length=1, | ||||||||
| ), | ||||||||
| bytes([1]), | ||||||||
| id="33-bytes-long-base", # higher cost than 32 bytes | ||||||||
|
Comment on lines
42
to
53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove this test and add 1/2 extras. These both currently test the minimum gas cost increase of 200 to 500 from the EIP.
Please see below for modexp equation snippets: They shouldn't cost too much more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The link you have put shows that these two test do NOT both test the same thing. The 32 bytes base test is cheaper than the 33 bytes test. If you remove one of them you lose coverage of that case. But I agree that we can add another test we an even larger number, e.g. the two cases u mentioned |
||||||||
| ), | ||||||||
| ], | ||||||||
| ) | ||||||||
| @pytest.mark.valid_at("Osaka") | ||||||||
|
||||||||
| @pytest.mark.valid_at("Osaka") | |
| @pytest.mark.valid_at("Osaka") | |
| @pytest.mark.mainnet |
We only want to fill/execute these specifically for mainnet validation.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||
| """ | ||||||
| Tests for [EIP-7951: Precompile for secp256r1 Curve Support](https://eips.ethereum.org/EIPS/eip-7951). | ||||||
|
||||||
| Tests for [EIP-7951: Precompile for secp256r1 Curve Support](https://eips.ethereum.org/EIPS/eip-7951). | |
| Mainnet Marked Checklist Tests for [EIP-7951: Precompile for secp256r1 Curve Support](https://eips.ethereum.org/EIPS/eip-7951). |
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.
If we add this here we can remove Osaka mark's below.
| pytestmark = [pytest.mark.valid_at("Osaka"), pytest.mark.mainnet] |
Outdated
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.
| @pytest.mark.valid_at("Osaka") |
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. My understanding is that this is a valid tx but with an invalid signature?
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.
it is a valid ecdsa signature over a different curve, so the signature verification is expected to fail (but this is a good test because the other curve is also used by clients)
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.
Could we make it clear in the docstring that these are specifically for mainnet execute use only (fill is a sanity check).