EIP-7623 #8093
EIP-7623 #8093
Conversation
Signed-off-by: Daniel Lehrner <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
daniellehrner
force-pushed
the
feat/issue-7994/eip-7623
branch
from
348ad34 to
2d2cfaf
Compare
Signed-off-by: Daniel Lehrner <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
There was a problem hiding this comment.
I think there's a bug in when the floor gets applied for tx processing, see comments.
I like the way you structured the refactor 👍
Perhaps not a functional issue, but I'm a little confused by the Long boundary checks:
- Their use appears inconsistent which makes me wonder if one or more of them were left in by accident?
- I understand checking MAX_VALUE before an add (although doesn't clampedAdd already handle this?) but I don't understand why we return early if the intermediate cost == MIN_VALUE. In the intermediate calculations, won't the subsequent addition usually mean cost > MIN_VALUE?
| * | ||
| * @param payload the call data payload | ||
| * @param isContractCreation whether the transaction is a contract creation | ||
| * @param baselineGas how much gas is used by access lists and code delegations |
There was a problem hiding this comment.
nit: baselineGas isn't the most descriptive name but this is a hard one to name. IMO we should try to match the spec with our names (which is still in flux so good luck 😆)
cc. @jflo as just noticed you had the opposite opinion 😅 #7997 (comment)
| if (cost == Long.MIN_VALUE || cost == Long.MAX_VALUE) { | ||
| return cost; | ||
| } |
There was a problem hiding this comment.
Why do we do check MIN_VALUE and return here, potentially bypassing adding more to the cost below?
To me, this doesn't appear to be equivalent behaviour to https://github.com/hyperledger/besu/blob/2aadbfcb0a6c22734ef3ae8cc2ae3823512c5419/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/FrontierGasCalculator.java
| final long totalCostFloor = | ||
| tokensInCallData(payload.size(), zeroBytes(payload)) * TOTAL_COST_FLOOR_PER_TOKEN; | ||
|
|
||
| return clampedAdd(TX_BASE_COST, Math.max(dynamicIntrinsicGasCost, totalCostFloor)); |
There was a problem hiding this comment.
I believe this is a bug because we shouldn't consider the totalCostFloor as part of the intrinsic gas, rather apply the floor after tx execution.
Note, this code does still work for validating the tx (where the floor is only compared against the intrinsic gas).
According to the latest spec correction, the floor should also be applied after the refund: https://github.com/ethereum/execution-specs/pull/1076/files
In short:
gas_used -= refund
gas_used = max(gas_used, floor_cost)
where gas_used is the post-execution value rather than intrinsic gas.
PR description
Implements EIP-7623 (calldata cost increase)
Fixed Issue(s)
fixes #7994
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests