-
Notifications
You must be signed in to change notification settings - Fork 137
feat(tests): add benchmark for the worst initcode jumpdest analysis #1646
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: main
Are you sure you want to change the base?
Conversation
6b017c7
to
f9b75b4
Compare
Benchmarking EIP-7907: Meter Contract Code Size And Increase LimitThis test can be used to benchmark EIP-7907. The implementation must allow initcodes longer than the Prague limit. Generated state test: worst_initcode_jumpdest_analysis.json ResultsevmoneIt looks like increasing the limit causes the test to be executed ~6x longer, but this is still ~192Mgas/s in the worst case.
Geth[
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-605b5b-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 102958028,
"allocs": 110530,
"bytesAllocated": 123316072,
"gasUsed": 72000000
}
},
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-615b5b-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 97807776,
"allocs": 110570,
"bytesAllocated": 123322833,
"gasUsed": 72000000
}
},
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-615b5b5b-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 89937716,
"allocs": 110541,
"bytesAllocated": 123318193,
"gasUsed": 72000000
}
},
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-00-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 71727318,
"allocs": 110613,
"bytesAllocated": 123327866,
"gasUsed": 72000000
}
},
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-5b-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 69766863,
"allocs": 110608,
"bytesAllocated": 123328256,
"gasUsed": 72000000
}
},
{
"name": "tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Osaka-state_test-605b-initcode_size_49152]",
"pass": true,
"stateRoot": "0x6ce6519d6ba9af1d21bfff604bdd25e2912a5f79a5cd7ee35abc7db1a1c76b31",
"fork": "Osaka",
"benchStats": {
"time": 102013648,
"allocs": 110550,
"bytesAllocated": 123313963,
"gasUsed": 72000000
}
}
] |
f9b75b4
to
5f1d2b2
Compare
Transaction, | ||
While, | ||
compute_create2_address, | ||
) | ||
from ethereum_test_tools.vm.opcode import Opcodes as Op | ||
from pytest_plugins.execute.pre_alloc import MAX_BYTECODE_SIZE, MAX_INITCODE_SIZE |
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.
I think we need to put this in ethereum_test_forks
as a property of the fork so we can adapt the test automatically.
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.
I put this in #1649 😄 👍
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.
Yes, make sense to me.
@chfast if there is a 6x performance degradation does this mean the initcode pricing from 3860 is too cheap? One would assume this extra initcode analysis is paid for? (2 gas per 32 bytes is thus too cheap for initcode analysis?) |
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.
Apart from the #1649 discussion, LGTM!
Cycles:
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-615b5b-initcode_size_49152]-1 255986171
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-605b-initcode_size_49152]-1 330872313
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-615b5b5b-initcode_size_49152]-1 381150402
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-00-initcode_size_49152]-1 406269727
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-605b5b-initcode_size_49152]-1 473544495
tests/zkevm/test_worst_bytecode.py::test_worst_initcode_jumpdest_analysis[fork_Cancun-blockchain_test_from_state_test-5b-initcode_size_49152]-1 758025210
Op.PUSH1[len(initcode_prefix)] | ||
+ Op.MSTORE |
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.
Clever use of the returned address!
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.
Yes this is so well disguised that I initially thought it did not edit the initcode 😄 👍 Love this!
sender=pre.fund_eoa(), | ||
) | ||
|
||
state_test( |
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.
@jsign Note that I downgraded to state_test
. I think we should do this for all benchmarks with single transaction. EEST will generate a blockchain test for it anyway.
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.
Good point!
@@ -204,3 +204,100 @@ def test_worst_bytecode_single_opcode( | |||
], | |||
exclude_full_post_state_in_output=True, | |||
) | |||
|
|||
|
|||
@pytest.mark.valid_from("Cancun") |
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_from("Cancun") | |
@pytest.mark.valid_from("Paris") |
Should this be Paris to also include pre-EIP-3860 forks? Test also seems to imply this by swapping out PUSH0 for PUSH1 pre-Shanghai
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.
This was the original idea, but because in Paris there is also no metering for initcode the benchmark behaves very differently. We rather need the Osaka implementation to test it on.
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.
As a lateral comment, targeting the latest fork should be the primary goal. While many of these tests could be generalized further to cover previous forks, I would prefer not to overcomplexify them since we aren't interested in benchmarking proof generations for older forks. If the generalization to cover older forks doesn't make the test more complex, I guess it's fine. Just mentioning this for some extra context.
cc @kevaundray
🗒️ Description
Add a benchmark where we place a very long initcode in the memory and then invoke
CREATE
instructions with this initcode up to the block gas limit. The initcode itself have minimal execution time but forces the EVM to perform the full jumpdest analysis on the parametrized byte pattern. The initicode is slightly modified betweenCREATE
invocations to prevent caching.🔗 Related Issues
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.