Skip to content
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.

Sign up for GitHub

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

core/{.,state,vm},miner,ethr/tracers: implement 7709 #31015

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+39 −18
Open

core/{.,state,vm},miner,ethr/tracers: implement 7709 #31015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/chain_makers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse
blockContext := NewEVMBlockContext(b.header, cm, &b.header.Coinbase)
blockContext.Random = &common.Hash{} // enable post-merge instruction set
evm := vm.NewEVM(blockContext, statedb, cm.config, vm.Config{})
ProcessParentBlockHash(b.header.ParentHash, evm)
ProcessParentBlockHash(b.header.ParentHash, evm, false)
}

// Execute any user modifications to the block
Expand Down Expand Up @@ -492,7 +492,7 @@ func GenerateVerkleChain(config *params.ChainConfig, parent *types.Block, engine
// EIP-2935
blockContext := NewEVMBlockContext(b.header, cm, &b.header.Coinbase)
evm := vm.NewEVM(blockContext, statedb, cm.config, vm.Config{})
ProcessParentBlockHash(b.header.ParentHash, evm)
ProcessParentBlockHash(b.header.ParentHash, evm, false)
}

// Execute any user modifications to the block.
Expand Down
4 changes: 4 additions & 0 deletions core/state/statedb_hooked.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ func (s *hookedStateDB) Witness() *stateless.Witness {
return s.inner.Witness()
}

func (s *hookedStateDB) AccessEvents() *AccessEvents {
return s.inner.AccessEvents()
}

func (s *hookedStateDB) SubBalance(addr common.Address, amount *uint256.Int, reason tracing.BalanceChangeReason) uint256.Int {
prev := s.inner.SubBalance(addr, amount, reason)
if s.hooks.OnBalanceChange != nil && !amount.IsZero() {
Expand Down
17 changes: 14 additions & 3 deletions core/state_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package core

import (
"encoding/binary"
"fmt"
"math/big"

Expand Down Expand Up @@ -86,7 +87,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
ProcessBeaconBlockRoot(*beaconRoot, evm)
}
if p.config.IsPrague(block.Number(), block.Time()) {
ProcessParentBlockHash(block.ParentHash(), evm)
ProcessParentBlockHash(block.ParentHash(), evm, p.config.IsVerkle(block.Number(), block.Time()))
}

// Iterate over and process the individual transactions
Expand Down Expand Up @@ -234,14 +235,24 @@ func ProcessBeaconBlockRoot(beaconRoot common.Hash, evm *vm.EVM) {
}

// ProcessParentBlockHash stores the parent block hash in the history storage contract
// as per EIP-2935.
func ProcessParentBlockHash(prevHash common.Hash, evm *vm.EVM) {
// as per EIP-2935/7709.
func ProcessParentBlockHash(prevHash common.Hash, evm *vm.EVM, is7709 bool) {
if tracer := evm.Config.Tracer; tracer != nil {
onSystemCallStart(tracer, evm.GetVMContext())
if tracer.OnSystemCallEnd != nil {
defer tracer.OnSystemCallEnd()
}
}
if is7709 {
// currently 8192 for kautinen7, to be changed to 8 (and made a constant) for kaustinen8
ringIndex := (evm.Context.BlockNumber.Uint64() - 1) % params.HistoryServeWindow
var key common.Hash
binary.BigEndian.PutUint64(key[24:], ringIndex)
evm.StateDB.SetState(params.SystemAddress, key, prevHash)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to use SystemAddress here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because that's what is used on the testnet, it's older than the current 2935 address.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the devnet was launched even before the 2935 contract deployment?

evm.StateDB.AccessEvents().SlotGas(params.SystemAddress, key, true)
evm.StateDB.Finalise(false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if it's necessary.

If Verkle is enabled, SStore will be replaced with SStore4762, the slot gas charging and warming will be done automatically if we register the block hash via invoking the contract, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this doesn't actually invoke any contract, just does the ugly and sets the slot. Which necessitates also setting the access-event.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why not invoking the contract? The gas charging, slot warming can all be done automatically in this way.

Copy link
Member Author

@gballet gballet Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you might have missed the gist of conversations I had over this topic. There are several issues with invoking a contract:

  1. it's slower and uglier as you have to instantiate an evm each time, and copy/paste the code to do so all over the place
  2. it's more brittle, since it can revert. Also, what if there is a bug in the contract? Not as easy to fix as a bug in a client.
  3. it's not extensible, if you need to change the behavior, you need to deploy another contract
  4. you must filter the keys that bloat the witness a posteriori
  5. You need to keep a list of system contracts and check that list for each call, as they have a different behavior (see eip 4762)

Only 3 and 4 are relevant to this issue, but system contracts are generally a bad idea in my view.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conclusion was that, we would deprecate calling system contracts in verkle.

return
}
msg := &Message{
From: params.SystemAddress,
GasLimit: 30_000_000,
Expand Down
17 changes: 10 additions & 7 deletions core/verkle_witness_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func TestProcessParentBlockHash(t *testing.T) {
// block 1 parent hash is 0x0100....
// block 2 parent hash is 0x0200....
// etc
checkBlockHashes := func(statedb *state.StateDB) {
checkBlockHashes := func(statedb *state.StateDB, isVerkle bool) {
statedb.SetNonce(params.HistoryStorageAddress, 1)
statedb.SetCode(params.HistoryStorageAddress, params.HistoryStorageCode)
// Process n blocks, from 1 .. num
Expand All @@ -226,35 +226,38 @@ func TestProcessParentBlockHash(t *testing.T) {
header := &types.Header{ParentHash: common.Hash{byte(i)}, Number: big.NewInt(int64(i)), Difficulty: new(big.Int)}
vmContext := NewEVMBlockContext(header, nil, new(common.Address))
evm := vm.NewEVM(vmContext, statedb, params.MergedTestChainConfig, vm.Config{})
ProcessParentBlockHash(header.ParentHash, evm)
ProcessParentBlockHash(header.ParentHash, evm, isVerkle)
}
// Read block hashes for block 0 .. num-1
for i := 0; i < num; i++ {
have, want := getContractStoredBlockHash(statedb, uint64(i)), common.Hash{byte(i + 1)}
have, want := getContractStoredBlockHash(statedb, uint64(i), isVerkle), common.Hash{byte(i + 1)}
if have != want {
t.Errorf("block %d, have parent hash %v, want %v", i, have, want)
t.Errorf("block %d, verkle=%v, have parent hash %v, want %v", i, isVerkle, have, want)
}
}
}
t.Run("MPT", func(t *testing.T) {
statedb, _ := state.New(types.EmptyRootHash, state.NewDatabaseForTesting())
checkBlockHashes(statedb)
checkBlockHashes(statedb, false)
})
t.Run("Verkle", func(t *testing.T) {
db := rawdb.NewMemoryDatabase()
cacheConfig := DefaultCacheConfigWithScheme(rawdb.PathScheme)
cacheConfig.SnapshotLimit = 0
triedb := triedb.NewDatabase(db, cacheConfig.triedbConfig(true))
statedb, _ := state.New(types.EmptyVerkleHash, state.NewDatabase(triedb, nil))
checkBlockHashes(statedb)
checkBlockHashes(statedb, true)
})
}

// getContractStoredBlockHash is a utility method which reads the stored parent blockhash for block 'number'
func getContractStoredBlockHash(statedb *state.StateDB, number uint64) common.Hash {
func getContractStoredBlockHash(statedb *state.StateDB, number uint64, isVerkle bool) common.Hash {
ringIndex := number % params.HistoryServeWindow
var key common.Hash
binary.BigEndian.PutUint64(key[24:], ringIndex)
if isVerkle {
return statedb.GetState(params.SystemAddress, key)
}
return statedb.GetState(params.HistoryStorageAddress, key)
}

Expand Down
3 changes: 3 additions & 0 deletions core/vm/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"math/big"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/stateless"
"github.com/ethereum/go-ethereum/core/tracing"
"github.com/ethereum/go-ethereum/core/types"
Expand Down Expand Up @@ -98,6 +99,8 @@ type StateDB interface {

Witness() *stateless.Witness

AccessEvents() *state.AccessEvents

// Finalise must be invoked at the end of a transaction
Finalise(bool)
}
Expand Down
2 changes: 1 addition & 1 deletion eth/state_accessor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block,
}
// If prague hardfork, insert parent block hash in the state as per EIP-2935.
if eth.blockchain.Config().IsPrague(block.Number(), block.Time()) {
core.ProcessParentBlockHash(block.ParentHash(), evm)
core.ProcessParentBlockHash(block.ParentHash(), evm, eth.blockchain.Config().IsVerkle(block.Number(), block.Time()))
}
if txIndex == 0 && len(block.Transactions()) == 0 {
return nil, vm.BlockContext{}, statedb, release, nil
Expand Down
8 changes: 4 additions & 4 deletions eth/tracers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed
}
// Insert parent hash in history contract.
if api.backend.ChainConfig().IsPrague(next.Number(), next.Time()) {
core.ProcessParentBlockHash(next.ParentHash(), evm)
core.ProcessParentBlockHash(next.ParentHash(), evm, api.backend.ChainConfig().IsVerkle(next.Number(), next.Time()))
}
// Clean out any pending release functions of trace state. Note this
// step must be done after constructing tracing state, because the
Expand Down Expand Up @@ -541,7 +541,7 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config
core.ProcessBeaconBlockRoot(*beaconRoot, evm)
}
if chainConfig.IsPrague(block.Number(), block.Time()) {
core.ProcessParentBlockHash(block.ParentHash(), evm)
core.ProcessParentBlockHash(block.ParentHash(), evm, api.backend.ChainConfig().IsVerkle(block.Number(), block.Time()))
}
for i, tx := range block.Transactions() {
if err := ctx.Err(); err != nil {
Expand Down Expand Up @@ -605,7 +605,7 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac
core.ProcessBeaconBlockRoot(*beaconRoot, evm)
}
if api.backend.ChainConfig().IsPrague(block.Number(), block.Time()) {
core.ProcessParentBlockHash(block.ParentHash(), evm)
core.ProcessParentBlockHash(block.ParentHash(), evm, api.backend.ChainConfig().IsVerkle(block.Number(), block.Time()))
}

// JS tracers have high overhead. In this case run a parallel
Expand Down Expand Up @@ -782,7 +782,7 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block
core.ProcessBeaconBlockRoot(*beaconRoot, evm)
}
if chainConfig.IsPrague(block.Number(), block.Time()) {
core.ProcessParentBlockHash(block.ParentHash(), evm)
core.ProcessParentBlockHash(block.ParentHash(), evm, api.backend.ChainConfig().IsVerkle(block.Number(), block.Time()))
}
for i, tx := range block.Transactions() {
// Prepare the transaction for un-traced execution
Expand Down
2 changes: 1 addition & 1 deletion miner/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (miner *Miner) prepareWork(genParams *generateParams, witness bool) (*envir
core.ProcessBeaconBlockRoot(*header.ParentBeaconRoot, env.evm)
}
if miner.chainConfig.IsPrague(header.Number, header.Time) {
core.ProcessParentBlockHash(header.ParentHash, env.evm)
core.ProcessParentBlockHash(header.ParentHash, env.evm, false)
}
return env, nil
}
Expand Down
Loading