WIP: feat(Osaka): eip7907 Meter Contract Code Size And Increase Limit #2550
Conversation
CodSpeed Performance ReportMerging #2550 will degrade performances by 4.7%Comparing Summary
Benchmarks breakdown
|
|
Appreciate the effort! Lets do this more holistically. Change this field revm/crates/context/src/cfg.rs Line 24 in f8a9630 fn max_code_size(&self) -> usize to CfgEnv that would give us a runtime limit depending on SpecId, or if concrete value is set
|
will do! any and all feedback and suggestions is much appreciated |
Added specId conditional to Also, I'm planning to work on the gas metering and additional code warmed (account code loaded) state next. I can tag you when I think this PR (and the EIP is more finalized) if you'd like, otherwise any reviews in the meantime are appreciated. One question: How should I think about backwards compatibility? For example, if L1 and op-stack adopts this EIP, would the plan be to remove any eip-170 code (eip7907 replaces it)? |
Co-authored-by: rakita <rakita@users.noreply.github.com>
| @@ -290,6 +290,53 @@ impl<T> StateLoad<T> { | |||
| } | |||
| } | |||
|
|
|||
| /// Duplicates StateLoad, adding code cold status. | |||
There was a problem hiding this comment.
@rakita I needed to return is_code_cold from account load calls to use it for gas calculations. Is there a better way to extend StateLoad without duplicating the rest of the code here? StateLoad is widely used and was hesitant to change it
crates/handler/src/validation.rs
Outdated
| let block_and_txn_gas_limit_cap = gas_limit.unwrap_or(eip7825::TX_GAS_LIMIT_CAP); | ||
| cfg.tx_gas_limit_cap = Some(block_and_txn_gas_limit_cap); | ||
|
|
||
| let ctx = Context::mainnet().with_db(CacheDB::<EmptyDB>::default()).with_cfg(cfg); |
There was a problem hiding this comment.
@rakita I was thinking to add more tests to check the journal/db if code is warmed properly and txns with multiple CALL and CREATE. Should this be done in this repo and if so are there any good examples of this? or should these scenarios be covered in the EEST tests?
crates/interpreter/src/gas/calc.rs
Outdated
| warm_cold_cost(is_cold) | ||
| } else if spec_id.is_enabled_in(SpecId::TANGERINE) { | ||
| 700 | ||
| } else { | ||
| 20 | ||
| }; | ||
| if spec_id.is_enabled_in(SpecId::OSAKA) && is_code_cold { | ||
| let large_contract_cost = large_contract_code_size_cost(len); | ||
| base_gas += large_contract_cost; |
There was a problem hiding this comment.
Saw code around this doing safe adds, but was getting an error
|
@jgresham we had ian nternal discussion and are probably changing it to just increase of contract limit increase. Will take this over to push it over the line. |
EIP: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7907.md
create/create2EXTCODESIZErelated changesexecution specs reference implementation https://github.com/ethereum/execution-specs/pull/1231/files
I'll try to get more of this ready and tested in advance of fusaka devnet-2 testing