-
Notifications
You must be signed in to change notification settings - Fork 13
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
witness: only include elements if there's enough gas for it #495
Changes from all commits
9f44b79
8b45fb7
d46fa01
d9bab66
3d79a1d
59720c4
49d84a2
ed4e5e5
4ecd713
95f771d
81940b9
09be3aa
f147cdc
b2e2fb8
e98688e
f29b148
05e6092
fdb9c50
9ea37be
83aca33
ffc1f2d
a00d50a
85196cc
8b106f2
efabf95
51dca93
b32bca9
26452f4
4d4eabc
f4733ce
4b91d84
9635cc1
1ec805c
b2b96a2
8057173
59426d9
4694243
c329550
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 |
---|---|---|
|
@@ -4,11 +4,11 @@ on: | |
push: | ||
branches: [master] | ||
pull_request: | ||
branches: [master, kaustinen-with-shapella] | ||
branches: [master, kaustinen-with-shapella, jsign-witness-fix] | ||
workflow_dispatch: | ||
|
||
env: | ||
FIXTURES_TAG: "[email protected].3" | ||
FIXTURES_TAG: "[email protected].5" | ||
|
||
jobs: | ||
setup: | ||
|
@@ -71,13 +71,8 @@ jobs: | |
extract: true | ||
- name: Clone execution-spec-tests and consume tests | ||
run: | | ||
git clone https://github.com/ethereum/execution-spec-tests | ||
curl -LsSf https://astral.sh/uv/install.sh | sh | ||
git clone https://github.com/ethereum/execution-spec-tests -b ${{ env.FIXTURES_TAG }} --depth 1 | ||
cd execution-spec-tests | ||
git checkout 250a064ba38cd92cad02691f4f7c2ecbefedd954 | ||
python3 -m venv venv | ||
. venv/bin/activate | ||
pip install --upgrade pip | ||
pip install -e ".[docs,lint,test]" | ||
solc-select use 0.8.24 --always-install | ||
consume direct --input=../fixtures -n auto | ||
uv run consume direct --evm-bin="${{ github.workspace }}/bin/evm" --input=../fixtures -n auto | ||
shell: bash |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ | |
|
||
import ( | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/common/math" | ||
"github.com/ethereum/go-ethereum/params" | ||
"github.com/ethereum/go-ethereum/trie/utils" | ||
"github.com/holiman/uint256" | ||
|
@@ -30,6 +29,9 @@ | |
// * the second bit is set if the branch has been read | ||
type mode byte | ||
|
||
// UseGasFn is a function that can be used to charge gas for a given amount. | ||
type UseGasFn func(uint64) bool | ||
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. Now Many Before I tried another idea which was not passing this callback, but asking for
The caller should:
The above logic should be done in all the callers. This proposed API where we pass a callback function, avoids all this logic in all callers. The API call will already charge for whatever was allowed, and simply return false signaling that the available gas was insufficient and let the caller fail (i.e: the caller shouldn't deal with charging stuff since that was already done in the API call). That's the "TLDR" of this approach. I know was mentioned this still sounds like can get some pushback from the geth team, so we can try again with the other approach... but I found it a bit annoying. |
||
|
||
const ( | ||
AccessWitnessReadFlag = mode(1) | ||
AccessWitnessWriteFlag = mode(2) | ||
|
@@ -89,132 +91,132 @@ | |
return naw | ||
} | ||
|
||
func (aw *AccessWitness) TouchFullAccount(addr []byte, isWrite bool) uint64 { | ||
var gas uint64 | ||
func (aw *AccessWitness) TouchFullAccount(addr []byte, isWrite bool, useGasFn UseGasFn) bool { | ||
for i := utils.BasicDataLeafKey; i <= utils.CodeHashLeafKey; i++ { | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, byte(i), isWrite) | ||
if _, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, byte(i), isWrite, useGasFn); !ok { | ||
return false | ||
} | ||
} | ||
return gas | ||
return true | ||
} | ||
|
||
func (aw *AccessWitness) TouchAndChargeMessageCall(addr []byte) uint64 { | ||
var gas uint64 | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, false) | ||
return gas | ||
func (aw *AccessWitness) TouchAndChargeMessageCall(addr []byte, useGasFn UseGasFn) bool { | ||
chargedGas, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, false, useGasFn) | ||
return ok && (chargedGas > 0 || useGasFn(params.WarmStorageReadCostEIP2929)) | ||
} | ||
|
||
func (aw *AccessWitness) TouchAndChargeValueTransfer(callerAddr, targetAddr []byte) uint64 { | ||
var gas uint64 | ||
gas += aw.touchAddressAndChargeGas(callerAddr, zeroTreeIndex, utils.BasicDataLeafKey, true) | ||
gas += aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.BasicDataLeafKey, true) | ||
return gas | ||
func (aw *AccessWitness) TouchAndChargeValueTransfer(callerAddr, targetAddr []byte, useGasFn UseGasFn) bool { | ||
chargedGas1, ok := aw.touchAddressAndChargeGas(callerAddr, zeroTreeIndex, utils.BasicDataLeafKey, true, useGasFn) | ||
if !ok { | ||
return false | ||
} | ||
chargedGas2, ok := aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.BasicDataLeafKey, true, useGasFn) | ||
return ok && (chargedGas1+chargedGas2 > 0 || useGasFn(params.WarmStorageReadCostEIP2929)) | ||
} | ||
|
||
// TouchAndChargeContractCreateCheck charges access costs before | ||
// a contract creation is initiated. It is just reads, because the | ||
// address collision is done before the transfer, and so no write | ||
// are guaranteed to happen at this point. | ||
func (aw *AccessWitness) TouchAndChargeContractCreateCheck(addr []byte) uint64 { | ||
var gas uint64 | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, false) | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, false) | ||
return gas | ||
func (aw *AccessWitness) TouchAndChargeContractCreateCheck(addr []byte, useGasFn UseGasFn) bool { | ||
if _, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, false, useGasFn); !ok { | ||
return false | ||
} | ||
_, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, false, useGasFn) | ||
return ok | ||
} | ||
|
||
// TouchAndChargeContractCreateInit charges access costs to initiate | ||
// a contract creation. | ||
func (aw *AccessWitness) TouchAndChargeContractCreateInit(addr []byte) uint64 { | ||
var gas uint64 | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, true) | ||
gas += aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, true) | ||
return gas | ||
func (aw *AccessWitness) TouchAndChargeContractCreateInit(addr []byte, useGasFn UseGasFn) bool { | ||
if _, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, true, useGasFn); !ok { | ||
return false | ||
} | ||
_, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, true, useGasFn) | ||
return ok | ||
} | ||
|
||
func (aw *AccessWitness) TouchTxOriginAndComputeGas(originAddr []byte) uint64 { | ||
func (aw *AccessWitness) TouchTxOriginAndComputeGas(originAddr []byte) { | ||
for i := utils.BasicDataLeafKey; i <= utils.CodeHashLeafKey; i++ { | ||
aw.touchAddressAndChargeGas(originAddr, zeroTreeIndex, byte(i), i == utils.BasicDataLeafKey) | ||
aw.touchAddressAndChargeGas(originAddr, zeroTreeIndex, byte(i), i == utils.BasicDataLeafKey, nil) | ||
} | ||
|
||
// Kaustinen note: we're currently experimenting with stop chargin gas for the origin address | ||
// so simple transfer still take 21000 gas. This is to potentially avoid breaking existing tooling. | ||
// This is the reason why we return 0 instead of `gas`. | ||
// Note that we still have to touch the addresses to make sure the witness is correct. | ||
return 0 | ||
} | ||
|
||
func (aw *AccessWitness) TouchTxExistingAndComputeGas(targetAddr []byte, sendsValue bool) uint64 { | ||
aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.BasicDataLeafKey, sendsValue) | ||
aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.CodeHashLeafKey, false) | ||
|
||
// Kaustinen note: we're currently experimenting with stop chargin gas for the origin address | ||
// so simple transfer still take 21000 gas. This is to potentially avoid breaking existing tooling. | ||
// This is the reason why we return 0 instead of `gas`. | ||
// Note that we still have to touch the addresses to make sure the witness is correct. | ||
return 0 | ||
func (aw *AccessWitness) TouchTxTarget(targetAddr []byte, sendsValue bool) { | ||
aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.BasicDataLeafKey, sendsValue, nil) | ||
aw.touchAddressAndChargeGas(targetAddr, zeroTreeIndex, utils.CodeHashLeafKey, false, nil) | ||
} | ||
|
||
func (aw *AccessWitness) TouchSlotAndChargeGas(addr []byte, slot common.Hash, isWrite bool) uint64 { | ||
func (aw *AccessWitness) TouchSlotAndChargeGas(addr []byte, slot common.Hash, isWrite bool, useGasFn UseGasFn, warmCostCharging bool) bool { | ||
treeIndex, subIndex := utils.GetTreeKeyStorageSlotTreeIndexes(slot.Bytes()) | ||
return aw.touchAddressAndChargeGas(addr, *treeIndex, subIndex, isWrite) | ||
} | ||
|
||
func (aw *AccessWitness) touchAddressAndChargeGas(addr []byte, treeIndex uint256.Int, subIndex byte, isWrite bool) uint64 { | ||
stemRead, selectorRead, stemWrite, selectorWrite, selectorFill := aw.touchAddress(addr, treeIndex, subIndex, isWrite) | ||
|
||
var gas uint64 | ||
if stemRead { | ||
gas += params.WitnessBranchReadCost | ||
} | ||
if selectorRead { | ||
gas += params.WitnessChunkReadCost | ||
} | ||
if stemWrite { | ||
gas += params.WitnessBranchWriteCost | ||
} | ||
if selectorWrite { | ||
gas += params.WitnessChunkWriteCost | ||
} | ||
if selectorFill { | ||
gas += params.WitnessChunkFillCost | ||
} | ||
|
||
return gas | ||
chargedGas, ok := aw.touchAddressAndChargeGas(addr, *treeIndex, subIndex, isWrite, useGasFn) | ||
return ok && (!warmCostCharging || chargedGas > 0 || useGasFn(params.WarmStorageReadCostEIP2929)) | ||
} | ||
|
||
// touchAddress adds any missing access event to the witness. | ||
func (aw *AccessWitness) touchAddress(addr []byte, treeIndex uint256.Int, subIndex byte, isWrite bool) (bool, bool, bool, bool, bool) { | ||
func (aw *AccessWitness) touchAddressAndChargeGas(addr []byte, treeIndex uint256.Int, subIndex byte, isWrite bool, useGasFn UseGasFn) (uint64, bool) { | ||
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. This is the new "base" fundamental method that charges for stuff.
(Continue to read below comments for further bullets) |
||
branchKey := newBranchAccessKey(addr, treeIndex) | ||
chunkKey := newChunkAccessKey(branchKey, subIndex) | ||
|
||
// Read access. | ||
var branchRead, chunkRead bool | ||
if _, hasStem := aw.branches[branchKey]; !hasStem { | ||
branchRead = true | ||
aw.branches[branchKey] = AccessWitnessReadFlag | ||
} | ||
if _, hasSelector := aw.chunks[chunkKey]; !hasSelector { | ||
chunkRead = true | ||
aw.chunks[chunkKey] = AccessWitnessReadFlag | ||
} | ||
|
||
// Write access. | ||
var branchWrite, chunkWrite, chunkFill bool | ||
if isWrite { | ||
if (aw.branches[branchKey] & AccessWitnessWriteFlag) == 0 { | ||
branchWrite = true | ||
aw.branches[branchKey] |= AccessWitnessWriteFlag | ||
} | ||
|
||
chunkValue := aw.chunks[chunkKey] | ||
if (chunkValue & AccessWitnessWriteFlag) == 0 { | ||
chunkWrite = true | ||
aw.chunks[chunkKey] |= AccessWitnessWriteFlag | ||
} | ||
} | ||
|
||
// TODO: charge chunk filling costs if the leaf was previously empty in the state | ||
var gas uint64 | ||
if branchRead { | ||
gas += params.WitnessBranchReadCost | ||
} | ||
if chunkRead { | ||
gas += params.WitnessChunkReadCost | ||
} | ||
if branchWrite { | ||
gas += params.WitnessBranchWriteCost | ||
} | ||
if chunkWrite { | ||
gas += params.WitnessChunkWriteCost | ||
} | ||
if chunkFill { | ||
gas += params.WitnessChunkFillCost | ||
} | ||
Comment on lines
+182
to
+197
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.
|
||
|
||
if useGasFn != nil { | ||
if ok := useGasFn(gas); !ok { | ||
return 0, false | ||
} | ||
} | ||
Comment on lines
+199
to
+203
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.
If wasn't possible, return early. Notice that we haven't technically added anything to the witness. This is important. |
||
|
||
if branchRead { | ||
aw.branches[branchKey] = AccessWitnessReadFlag | ||
} | ||
if branchWrite { | ||
aw.branches[branchKey] |= AccessWitnessWriteFlag | ||
} | ||
if chunkRead { | ||
aw.chunks[chunkKey] = AccessWitnessReadFlag | ||
} | ||
if chunkWrite { | ||
chunkWrite = true | ||
aw.chunks[chunkKey] |= AccessWitnessWriteFlag | ||
} | ||
Comment on lines
+205
to
217
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.
|
||
|
||
return branchRead, chunkRead, branchWrite, chunkWrite, chunkFill | ||
return gas, true | ||
} | ||
|
||
type branchAccessKey struct { | ||
|
@@ -242,15 +244,15 @@ | |
} | ||
|
||
// touchCodeChunksRangeOnReadAndChargeGas is a helper function to touch every chunk in a code range and charge witness gas costs | ||
func (aw *AccessWitness) TouchCodeChunksRangeAndChargeGas(contractAddr []byte, startPC, size uint64, codeLen uint64, isWrite bool) uint64 { | ||
func (aw *AccessWitness) TouchCodeChunksRangeAndChargeGas(contractAddr []byte, startPC, size uint64, codeLen uint64, isWrite bool, useGasFn UseGasFn) bool { | ||
// note that in the case where the copied code is outside the range of the | ||
// contract code but touches the last leaf with contract code in it, | ||
// we don't include the last leaf of code in the AccessWitness. The | ||
// reason that we do not need the last leaf is the account's code size | ||
// is already in the AccessWitness so a stateless verifier can see that | ||
// the code from the last leaf is not needed. | ||
if size == 0 || startPC >= codeLen { | ||
return 0 | ||
return true | ||
} | ||
|
||
endPC := startPC + size | ||
|
@@ -261,25 +263,23 @@ | |
endPC -= 1 // endPC is the last bytecode that will be touched. | ||
} | ||
|
||
var statelessGasCharged uint64 | ||
for chunkNumber := startPC / 31; chunkNumber <= endPC/31; chunkNumber++ { | ||
treeIndex := *uint256.NewInt((chunkNumber + 128) / 256) | ||
subIndex := byte((chunkNumber + 128) % 256) | ||
gas := aw.touchAddressAndChargeGas(contractAddr, treeIndex, subIndex, isWrite) | ||
var overflow bool | ||
statelessGasCharged, overflow = math.SafeAdd(statelessGasCharged, gas) | ||
if overflow { | ||
panic("overflow when adding gas") | ||
if _, ok := aw.touchAddressAndChargeGas(contractAddr, treeIndex, subIndex, isWrite, useGasFn); !ok { | ||
return false | ||
} | ||
} | ||
|
||
return statelessGasCharged | ||
return true | ||
} | ||
|
||
func (aw *AccessWitness) TouchBasicData(addr []byte, isWrite bool) uint64 { | ||
return aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, isWrite) | ||
func (aw *AccessWitness) TouchBasicData(addr []byte, isWrite bool, useGasFn UseGasFn, warmCostCharging bool) bool { | ||
chargedGas, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, isWrite, useGasFn) | ||
return ok && (!warmCostCharging || chargedGas > 0 || useGasFn(params.WarmStorageReadCostEIP2929)) | ||
} | ||
|
||
func (aw *AccessWitness) TouchCodeHash(addr []byte, isWrite bool) uint64 { | ||
return aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, isWrite) | ||
func (aw *AccessWitness) TouchCodeHash(addr []byte, isWrite bool, useGasFn UseGasFn, warmCostCharging bool) bool { | ||
chargedGas, ok := aw.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, isWrite, useGasFn) | ||
return ok && (!warmCostCharging || chargedGas > 0 || useGasFn(params.WarmStorageReadCostEIP2929)) | ||
} |
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.
For any
Touch*(...)
call that doesn't care about charging gas, we send thenil
UseGas(...)
function.This is the equivalent action of the previous API where we ignored the returned value (since we didn't charge that gas to anyone).