Add support for EIP-7623 #389
Conversation
|
Thanks! I'll also try to cherry-pick this changes once you finished to v1.0 branch. |
This reverts commit e129161.
gasometer/src/lib.rs
Outdated
| used_gas: 0, | ||
| refunded_gas: 0, | ||
| config, | ||
| metrics: GasMetrics::new(), |
There was a problem hiding this comment.
Metrics is probably a bad name. Could we do something similar to bluealloy/revm#1744?
There was a problem hiding this comment.
Much better solution! Addressed in dc1f984
Since v1.0 branch has access to execution layer vector tests for pectra, I would prefer that we do the first implementation there. @manuelmauro Could you give it a try? |
| gas_per_empty_account_cost: u64, | ||
| has_eip_7623: bool, | ||
| gas_calldata_zero_floor: u64, | ||
| gas_calldata_non_zero_floor: u64, |
There was a problem hiding this comment.
To avoid having many fields in DerivedConfigInputs, we could have a single field, which is specify to eip-7623
pub struct EIP7623Config {
enabled: bool,
standard_token_cost: u64,
total_cost_floor_per_token: u64
}| gas_calldata_non_zero_floor: u64, | |
| eip_7623: EIP7623Config |
There was a problem hiding this comment.
Also, we already 2 constants that are used for this.
gas_transaction_zero_data: 4,
gas_transaction_non_zero_data: 16,
There was a problem hiding this comment.
eip-7623 changes gas_transaction_non_zero_data from 16 to 40 and gas_transaction_zero_data from 4 to 10.
There was a problem hiding this comment.
I think we need to keep both, because the 10/40 is applied only in the case the 4/16 plus execution costs do not reach the floor.
tx.gasUsed = (
21000
+
max(
STANDARD_TOKEN_COST * tokens_in_calldata
+ execution_gas_used
+ isContractCreation * (32000 + INITCODE_WORD_COST * words(calldata)),
TOTAL_COST_FLOOR_PER_TOKEN * tokens_in_calldata
)
)
The STANDARD_TOKEN_COST stays 4, while we add TOTAL_COST_FLOOR_PER_TOKEN which is 10.
There was a problem hiding this comment.
To avoid having many fields in DerivedConfigInputs, we could have a single field, which is specify to eip-7623
I like this, but maybe better to evaluate it as a refactoring in another PR?
Draft PR here: #391 |
* feat: ✨ add support for EIP-7623 * feat: ✨ add post_execution method to gasometer * fix: 🐛 check for invalid gas limit * feat: ✨ add gas tracker * style: 🎨 fmt * fix: 🐛 add check for recorded transaction * Revert "fix: 🐛 add check for recorded transaction" This reverts commit e129161. * fix: 🐛 use saturanting arithmetic in execution gas calculation * refactor: 🔥 remove apply_eip_7623_adjustment function * fix: 🐛 fix computation of EIP-7623 cost * fix: 🐛 fix execution cost computation * refactor: ♻️ move cost computation inside GasTracker * perf: ⚡ add caching to GasTracker * test: ✅ add proper testing * refactor: 🚨 clippy * refactor: ♻️ rename GasTracker to GasMetrics and move it to a new module * refactor: ♻️ rename execution_cost to non_intrinsic_cost * refactor: ♻️ rename tracker to metrics * docs: 📝 improve docs * fix: 🐛 add instrinsic gas cost metric * docs: 📝 document intrinsic_cost_metrics method * refactor: ♻️ add init method to GasMetrics * refactor: ♻️ reduce code duplication * refactor: ♻️ improve if condition * fix: 🐛 add contract creation component to intrinsic gas calculation * fix: 🐛 remove duplicated init_code_cost component * refactor: 🔥 remove intrinsic cost metrics * test: ✅ test invalid transaction with gas lower than floor/intrinsic cost * refactor: ♻️ wildly simplify the solution * revert: ⏪ remove refactoring
3 participants
Description
Adds support for EIP-7623