rebase verkle tree PR over kiln changes #24561
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
verkle: Implement Trie, NodeIterator and Database ifs Fix crash in TestDump Fix TestDump Fix TrieCopy remove unnecessary traces fix: Error() returned errIteratorEnd in verkle node iterator rewrite the iterator and change the signature of OpenStorageTrie add the adapter to reuse the account trie for storage don't try to deserialize a storage leaf into an account Fix statedb unit tests (#14) * debug code * Fix more unit tests * remove traces * Go back to the full range One tree to rule them all remove updateRoot, there is no root to update store code inside the account leaf fix build save current state for Sina Update go-verkle to latest Charge WITNESS_*_COST gas on storage loads Add witness costs for SSTORE as well Charge witness gas in the case of code execution corresponding code deletion add a --verkle flag to separate verkle experiments from regular geth operations use the snapshot to get data stateless execution from block witness AccessWitness functions Add block generation test + genesis snapshot generation test stateless block execution (#18) * test stateless block execution * Force tree resolution before generating the proof increased coverage in stateless test execution (#19) * test stateless block execution * Force tree resolution before generating the proof * increase coverage in stateless test execution ensure geth compiles fix issues in tests with verkle trees deactivated Ensure stateless data is available when executing statelessly (#20) * Ensure stateless data is available when executing statelessly * Actual execution of a statless block * bugfixes in stateless block execution * code cleanup - Reduce PR footprint by reverting NewEVM to its original signature - Move the access witness to the block context - prepare for a change in AW semantics Need to store the initial values. - Use the touch helper function, DRY * revert the signature of MustCommit to its original form (#21) fix leaf proofs in stateless execution (#22) * Fixes in witness pre-state * Add the recipient's nonce to the witness * reduce PR footprint and investigate issue in root state calculation * quick build fix cleanup: Remove extra parameter in ToBlock revert ToBlock to its older signature fix import cycle in vm tests fix linter issue fix appveyor build fix nil pointers in tests Add indices, yis and Cis to the block's Verkle proof upgrade geth dependency to drop geth's common dep fix cmd/devp2p tests fix rebase issues quell an appveyor warning fix address touching in SLOAD and SSTORE fix access witness for code size touch target account data before calling make sure the proper locations get touched in (ext)codecopy touch all code pages in execution add pushdata to witness remove useless code in genesis snapshot generation testnet: fix some of the rebase/drift issues Fix verkle proof generation in block fix an issue occuring when chunking past the code size fix: ensure the code copy doesn't extend past the code size Upgrade go-verkle to its IPA version (#24) fixes for the IPA testnet upgrade to latest go-verkle update go-verkle to get more fixes simplify code by removing all stateless references (#25) fix verkle proof test by enforcing values alignment to 32 bytes remove unneeded KZG tag fix the stateless test Move AccessWitness into StateDB (#27) * move AccessWitness into StateDB * set Accesses in TxContext constructor * Ensures that a statedb is initialized with a witness * copy AccessWitness in StateDB.Copy. use copied state in miner worker.commit. * remove redundant line Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Fix contract creation issue enable verkle on cancun block: take 2 (#28) * enable verkle on cancun block: take 2 * fix typo. make unreachable line panic message more clear fix rebase issues fix linter issue merge undefined instead of panicking (#30) initialize the new access witness if not already present fix boundary check in PUSH fix bound check in code chunking fix boundary condition check in PUSH32 add circleci support (#32) * add circleci support * disable linter, which is broken again * actually run tests remove unnecessary cancun block declaration in tests (#33) upgrade go version (#34) fix calculation in get_tree_key_for_storage_slot (#35) use the witness in statedb, revert applyTx signature (#36) * use the witness in statedb, revert applyTx signature * fix miner tests * fix catalyst build Remove access witness from the signature of Process (#38) consensus/ethash: ensure uncle accounts are included in block witness (#40) consensus/ethash: move accumulation of coinbase witness before coinbase account is credited (#41) remove outdated comment miner: embed verkle proof in sealing block (#39) * miner: embed verkle proof in sealing block * add test to ensure that verkle proof is present in mined blocks Refactor witness-accumulation in EVM (#42) * make push dynamically-charged. charge witness gas costs for push. refactor evm witness gas charging to move logic for touching a range of bytecode into a helper method 'touchEachChunksAndChargeGas' * add witness gas calculation for CodeCopy, ExtCodeCopy, SLoad back to gas_table.go * witness gas charging for CALL * remove explicit reference to evm.TxContext * core/vm: make touchEachChunksAndCharge gas handle nil code value * core/vm: call implementation, separate out witnesses into touch/set * some fixes * remove witness touching from opCall: this will go in evm.go * remove witness touching for call from gas_table.go * (hopefully) fix tests * add SSTORE witness charging that was removed mistakenly * charge witness gas for call * clean up and comment touchEachChunksAndChargeGas * make suggested changes * address remaining points * fix build issues * remove double-charging for contract creation witness gas charging call onleaf in verkle commit (#45) replace sha256 with pedersen_hash in get_key (#46) * replace sha256 with pedersen_hash * fix: prevent an OOB * workaround timeout in unit test * update go-ipa and reduce the timeout * fix for unit tests: do not call NewAccessWitness in NewEVMTxContext (#49) * potential fix: do not call NewAccessWitness in NewEVMTxContext * more fixes: check for the existence of Accesses * fix absence of witness in copy * fix another witness issue * workaround: ensure the prefetcher is off in verkle mode * fix the remaining issues in tests * review feedback * fix witness allocation in stateless test reactivate working lines in test fix: don't create settings each time a key is calculated (#53) fix: don't use rlp in the tree for slot values (#51) * fix: don't use rlp in the tree for slot values * fix timeout after rebase core/vm: don't include contract deployer bytecode in AccessWitness or charge witness access costs for it (#54) Verkle EXTCODECOPY implementation (#55) * core/vm: verkle extcodecopy naive way (do jumpdest analysis on target contract every EXTCODECOPY) * no double-charge * address edge-case in touchEachChunksAndChargeGas * simplify line Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Use IsCancun where applicable (#56) * replace Accesses != nil with IsCancun(...) * fix Charge witness gas when calling/creating a contract (#60) * Charge witness gas when calling/creating a contract Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> * gofmt * replace checks with evm.Access!=nil with IsCancun * remove double-charging of witness access costs for contract creation initialization Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> verkle proof deserialization (#61) * use proof serialization * remove cruft * save current state * fix most issues up to this point * fix remaining build issues * update the go.mod to use the right branch * remove custom-defined set type * update go-verkle to get merged PRs * extract key, value data from proof * only activate precomp calculations if this is a verkle chain Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> fix: build more than one block in stateless test (#66) * reproduce the bug * fix the nil AccessWitness when Resetting * fix nonce management in blocks * fix: make sure the snapshot is reused during the chain generation Fix GetTreeKey (#65) * trie/utils: fix GetTreeKey * disable the faulty test, tracking issue opened Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Implement latest stateless gas charging spec (#63) * latest stateless gas-charging spec: add write event charging * rename gas calculation function so that it doesn't seem it's charging gas Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Various fixes for rust-verkle proof format compatibility (#67) * code to extract the block * separate proof from keys in block * display state root of block 0 * change file name to reflect the correct block number * use RLP instead of flat binary for keyvals in block * update go-verkle to fix build * fix rebase issues * make test pass * fix issue in map copy Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> Fix contract creation aw (#68) * fix contract creation AccessWitness * remove tracing * remove tracing * remove block2.rlp * move repeated error message into its own error type fix a couple linter issues Sload fix (#69) * move SLOAD gas calculation to gasSLoadEIP2929 * re-add gasSLoad, used by SSTORE * make requested changes fix: witness gas costs being charged more than once (#70) * fix: witness gas costs being charged more than one * update test case Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> re-enable VerkleTrie::ToDot core/state: implement GetCodeSize for VerkleDB (#75) implement core/state: implement GetCodeSize for VerkleDB copy the pre-state, use an untouched copy for the proof (#72) test to compare the proof format with that of rust verkle (#73) * test to compare the proof format with that of rust verkle * comment block extraction code check proof in verkle tree test don't delete from the trie when we encounter empty state object if verkle is enabled (#74) core/state: write code to rawdb under codehash key when verkle is enabled (#76) force the 32-byte alignment of nonce and balance (#71) * force the 32-byte alignment of nonce and balance * review feedback: fix endianness in output core/state - ensure storage value is set properly in snapshot when verkle is enabled (#78) fixes for the return type change in go-verkle (#77) * fixes for the return type change in go-verkle * update go-verkle * fix endianness issue + workaround for witness fix * add changes to miner Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com> calculate tree index correctly for SLOAD access event (#79) core/vm: witness write event for SSTORE (#80) * core/vm: witness write event for SSTORE * remove TODO that is addressed by this branch fix estimateGas (handle ErrInsufficientBalanceWitness). Return proper error message when ErrInsufficientBalanceWitness is encountered (#81) Verkle/fix/perdersen hash (#84) * remove ineffective statement * fix endianness in pedersen_hash refactor: set witness account values in getStateObject() (#83) * enforce 32-byte alignment * save current state * write account values to witness from getObject * code cleanup + sanity checks * fix RLP serialization of missing keys * remove code redundancy for GetTreeKeyCodeChunk * fix stem calculation issue for code and storage * remove redundant SetLeafValue calls * Add a contract creation tx to the test * fix botched module version update * detail gas calculation cost in verkle contract deployment fix verkle trie iterator to not miss leaf values in the first index of a leaf node (#88) creation-time CODECOPY adds no code to the witness (#86) * creation-time CODECOPY adds no code to the witness * Add a more complex test to ensure EXTCODECOPY is called * Fix offset calculations in the slot function * minor tweaks * fix initial value capture for storage * fix build * fix: get the proper byte in get_tree_key_for_storage_slot * fix length calculation in getDataAndAdjustedBounds * fix verkle test small simplification in order to facilitate rebase upgrade go-ipa + go-verkle: faster precomp loading fix: place slot value, not its rlp encoding in witness fix: code chunification with incorrect header (#89) * fix: code chunification with incorrect header * add a test from rust-verkle and fix offset bug * fix ci build * complete coverage of rust-verkle chunking tests * fix: make sure the witness value is set * add another code chunking test from rust-verkle spec: don't add the trailing pushdata for a final PUSHn (#90)
|
note to self: the original state isn't retained in |
gballet
commented
Mar 18, 2022
| if config.IsCancun(header.Number) { | ||
| uncleCoinbase := utils.GetTreeKeyBalance(uncle.Coinbase.Bytes()) | ||
| state.Witness().TouchAddressOnReadAndComputeGas(uncleCoinbase) | ||
| state.Witness().SetLeafValue(uncleCoinbase, state.GetBalance(uncle.Coinbase).Bytes()) |
There was a problem hiding this comment.
Note to self: this should happen in GetBalance/AddBalance and no change should appear in this file
gballet
commented
Mar 18, 2022
| receipts types.Receipts | ||
| logs []*types.Log | ||
| ) | ||
| receipts, logs, usedGas, err = bc.processor.Process(block, statedb, bc.vmConfig) |
There was a problem hiding this comment.
looks like a leftover merge issue 🤔
gballet
commented
Mar 18, 2022
|
|
||
| // AccessWitness lists the locations of the state that are being accessed | ||
| // during the production of a block. | ||
| // TODO(@gballet) this doesn't fully support deletions |
gballet
commented
Mar 18, 2022
| // Get the operation from the jump table and validate the stack to ensure there are | ||
| // enough stack items available to perform the operation. | ||
| // TODO how can we tell if we are in stateless mode here and need to get the op from the witness |
There was a problem hiding this comment.
check TODO still makes sense, I don't think it does
gballet
commented
Mar 18, 2022
| *meter += time.Since(readStart) | ||
| } | ||
| }() | ||
| } |
gballet
commented
Mar 18, 2022
|
|
||
| return out[:] | ||
| } | ||
|
|
There was a problem hiding this comment.
function no longer needed
gballet
commented
Mar 18, 2022
| binary.LittleEndian.PutUint64(out[:8], s.data.Nonce) | ||
| return out[:] | ||
| } | ||
|
|
There was a problem hiding this comment.
function no longer needed
gballet
commented
Mar 18, 2022
| root, err := state.Commit(false) | ||
| if err != nil { | ||
| panic(err) | ||
| } |
There was a problem hiding this comment.
useful for debug, but it need not be part of this PR
gballet
commented
Mar 18, 2022
| state, err := New(common.Hash{}, db, nil) | ||
| if err != nil { | ||
| panic("nil stte") | ||
| } |
There was a problem hiding this comment.
useful for debug, but it need not be part of the PR
gballet
commented
Mar 18, 2022
| @@ -17,8 +17,11 @@ | |||
| package core | |||
|
|
|||
| import ( | |||
| //"bytes" | |||
There was a problem hiding this comment.
TODO: remove the block extraction code, maybe extract and move this to an external utility
gballet
commented
Mar 18, 2022
| @@ -63,6 +63,18 @@ func getData(data []byte, start uint64, size uint64) []byte { | |||
| return common.RightPadBytes(data[start:end], int(size)) | |||
| } | |||
|
|
|||
| func getDataAndAdjustedBounds(data []byte, start uint64, size uint64) (codeCopyPadded []byte, actualStart uint64, sizeNonPadded uint64) { | |||
gballet
commented
Mar 18, 2022
gballet
commented
Mar 18, 2022
| return nil, nil | ||
| } | ||
|
|
||
| // touchEachChunksAndChargeGas is a helper function to touch every chunk in a code range and charge witness gas costs | ||
| func touchEachChunksOnReadAndChargeGas(offset, size uint64, address []byte, code []byte, contract AnalyzedContract, accesses *types.AccessWitness, deployment bool) uint64 { |
There was a problem hiding this comment.
This can be rewritten to use ChunkifyCode instead of the AnalyzedContract interface, and is coupled with the PR in which the chunking is saved into the DB so that it's not done each time.
gballet
commented
Mar 18, 2022
| if evm.chainConfig.IsCancun(evm.Context.BlockNumber) { | ||
| addr := contract.Address() | ||
| index := trieUtils.GetTreeKeyStorageSlot(addr[:], x) | ||
| cost += evm.Accesses.TouchAddressOnWriteAndComputeGas(index) |
There was a problem hiding this comment.
TODO: check this isn't redundant with the call to GetState. It should be.
gballet
commented
Mar 18, 2022
| addr := contract.Address() | ||
| index := trieUtils.GetTreeKeyStorageSlot(addr[:], where) | ||
| gasUsed += evm.Accesses.TouchAddressOnReadAndComputeGas(index) | ||
| } |
There was a problem hiding this comment.
🤔 this change is a bit weird because access lists are supposed to be removed with the verkle tree change, and yet we are charging gas for this... If anything, this funciton should simply not be called in a verkle tree context.
gballet
commented
Mar 18, 2022
gballet
commented
Mar 18, 2022
| @@ -235,14 +235,17 @@ func TestEnterExit(t *testing.T) { | |||
| if _, err := newJsTracer("{step: function() {}, fault: function() {}, result: function() { return null; }, enter: function() {}, exit: function() {}}", new(tracers.Context)); err != nil { | |||
| t.Fatal(err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
TODO: remove extra lines
gballet
commented
Mar 18, 2022
| IsBerlin, IsLondon bool | ||
| IsMerge bool | ||
| IsBerlin, IsLondon, IsCancun, bool | ||
| IsMerge bool |
gballet
commented
Mar 18, 2022
| const ( | ||
| PUSH1 = 0x60 | ||
| PUSH32 = 0x7f | ||
| ) |
There was a problem hiding this comment.
see if I can replace this with their vm.PUSHn equivalent. This used not to be possible, but it's possible in tests, so the dependency cycle might have been fixed.
|
Superseded by #25253, closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this PR is a test run to see what problems occur when rebasing the verkle tree PR over kiln. Disgregard.