Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
41 changes: 28 additions & 13 deletions cmd/evm/internal/t8ntool/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package t8ntool

import (
"fmt"
stdmath "math"
"math/big"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -43,8 +44,9 @@ import (
)

type Prestate struct {
Env stEnv `json:"env"`
Pre types.GenesisAlloc `json:"pre"`
Env stEnv `json:"env"`
Pre types.GenesisAlloc `json:"pre"`
BT map[common.Hash]hexutil.Bytes `json:"vkt,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Please elaborate the use case of using the specific binaryTrie data as the pre-state.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought mostly formatting and ease of use/test.

But can definitely change my mind 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.

same thing as the other comment: it's impossible to go from a tree leaf to an account, so you need to rebuild the tree from scratch at each new block.

Copy link
Member

Choose a reason for hiding this comment

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

How about we use a more general name? TreeLeaves ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did the renaming, but I also looked into the possibility of caching the accounts/slots and I don't think it makes sense to do that: I would have to cache the data at some global variable since the tree is opened independently multiple times (e.g. in system contracts), which means I would have to pass that global cache all over the code and that is a very invasive change for something we hope can disappear.

}

//go:generate go run github.com/fjl/gencodec -type ExecutionResult -field-override executionResultMarshaling -out gen_execresult.go
Expand Down Expand Up @@ -142,7 +144,8 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
return h
}
var (
statedb = MakePreState(rawdb.NewMemoryDatabase(), pre.Pre)
isEIP4762 = chainConfig.IsVerkle(big.NewInt(int64(pre.Env.Number)), pre.Env.Timestamp)
statedb = MakePreState(rawdb.NewMemoryDatabase(), pre.Pre, isEIP4762)
signer = types.MakeSigner(chainConfig, new(big.Int).SetUint64(pre.Env.Number), pre.Env.Timestamp)
gaspool = new(core.GasPool)
blockHash = common.Hash{0x13, 0x37}
Expand Down Expand Up @@ -191,7 +194,6 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
Time: pre.Env.ParentTimestamp,
ExcessBlobGas: pre.Env.ParentExcessBlobGas,
BlobGasUsed: pre.Env.ParentBlobGasUsed,
BaseFee: pre.Env.ParentBaseFee,
}
header := &types.Header{
Time: pre.Env.Timestamp,
Expand Down Expand Up @@ -301,6 +303,10 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
// Amount is in gwei, turn into wei
amount := new(big.Int).Mul(new(big.Int).SetUint64(w.Amount), big.NewInt(params.GWei))
statedb.AddBalance(w.Address, uint256.MustFromBig(amount), tracing.BalanceIncreaseWithdrawal)

if isEIP4762 {
statedb.AccessEvents().AddAccount(w.Address, true, stdmath.MaxUint64)
}
}

// Gather the execution-layer triggered requests.
Expand Down Expand Up @@ -361,8 +367,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
execRs.Requests = requests
}

// Re-create statedb instance with new root upon the updated database
// for accessing latest states.
// Re-create statedb instance with new root for MPT mode
statedb, err = state.New(root, statedb.Database())
if err != nil {
return nil, nil, nil, NewError(ErrorEVM, fmt.Errorf("could not reopen state: %v", err))
Expand All @@ -371,12 +376,17 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
return statedb, execRs, body, nil
}

func MakePreState(db ethdb.Database, accounts types.GenesisAlloc) *state.StateDB {
tdb := triedb.NewDatabase(db, &triedb.Config{Preimages: true})
func MakePreState(db ethdb.Database, accounts types.GenesisAlloc, isBintrie bool) *state.StateDB {
tdb := triedb.NewDatabase(db, &triedb.Config{Preimages: true, IsVerkle: isBintrie})
sdb := state.NewDatabase(tdb, nil)
statedb, err := state.New(types.EmptyRootHash, sdb)

root := types.EmptyRootHash
if isBintrie {
root = types.EmptyBinaryHash
}
statedb, err := state.New(root, sdb)
if err != nil {
panic(fmt.Errorf("failed to create initial state: %v", err))
panic(fmt.Errorf("failed to create initial statedb: %v", err))
}
for addr, a := range accounts {
statedb.SetCode(addr, a.Code, tracing.CodeChangeUnspecified)
Expand All @@ -387,18 +397,23 @@ func MakePreState(db ethdb.Database, accounts types.GenesisAlloc) *state.StateDB
}
}
// Commit and re-open to start with a clean state.
root, err := statedb.Commit(0, false, false)
mptRoot, err := statedb.Commit(0, false, false)
Copy link
Member

Choose a reason for hiding this comment

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

why change the var name 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.

to differentiate between the MPT root and a verkle root, this is not apparent here but for the transition there will be two concurrent roots. But I will revert for now, since there's not going to be any "rebase".

if err != nil {
panic(fmt.Errorf("failed to commit initial state: %v", err))
}
statedb, err = state.New(root, sdb)
// If bintrie mode started, check if conversion happened
if isBintrie {
Copy link
Member

Choose a reason for hiding this comment

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

In the MPT, the statedb re-init is essential, as after the commit, the mutated nodes are all converted as the hash and de-reference from the trie.

The only way to access the them is re-open the trie with new root hash.

I am not sure how it's handled in the binaryTrie. I guess it somehow still reference the new nodes after the commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

After Commit(), all mutated nodes are converted to hash nodes (line 763 in trie.go: t.root = newCommitter(...).Commit(...)).

  1. The committer replaces all dirty nodes with their hash representations
  2. The trie becomes unusable because nodes are now just hashes, not full nodes
    That's why we need to re-open the trie with state.New(root, sdb) to get a fresh trie that can load nodes from the database.

In Binary Trie (Verkle/Binary):

  • The ``Commit()` method collects nodes but doesn't convert them to hashes
  • The trie maintains its in-memory node structure after commit
  • The trie remains usable after commit because nodes are still accessible

So you are right, we should check if we're in Bintrie EIP. And if so, do this, otherwise, if we are using MPT we should avoid doing that.

Does that make sense @gballet ?

return statedb
}
// For MPT mode, reopen the state with the committed root
statedb, err = state.New(mptRoot, sdb)
if err != nil {
panic(fmt.Errorf("failed to reopen state after commit: %v", err))
}
return statedb
}

func rlpHash(x interface{}) (h common.Hash) {
func rlpHash(x any) (h common.Hash) {
hw := sha3.NewLegacyKeccak256()
rlp.Encode(hw, x)
hw.Sum(h[:0])
Expand Down
21 changes: 21 additions & 0 deletions cmd/evm/internal/t8ntool/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ var (
"\t<file> - into the file <file> ",
Value: "block.json",
}
OutputBTFlag = &cli.StringFlag{
Name: "output.vkt",
Copy link
Member

Choose a reason for hiding this comment

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

output.vkt is confusing, it essentially refers to the file storing the trie leaves right? It doesn't matter it's verkle/bintree/mpt

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed, but the reason why we keep this is because we are dependent on a branch of execution spec tests that is no longer maintained and it has all these fields hardcoded.

Copy link
Member Author

Choose a reason for hiding this comment

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

so I'm banking on the fact this code can be removed soon.

Usage: "Determines where to put the `BT` of the post-state.\n" +
"\t`stdout` - into the stdout output\n" +
"\t`stderr` - into the stderr output\n" +
"\t<file> - into the file <file> ",
Value: "vkt.json",
}
OutputWitnessFlag = &cli.StringFlag{
Copy link
Member

Choose a reason for hiding this comment

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

It's not used, maybe remove it from this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

True! Plus we aren't even testing witgen at all when comparing against Besu.

Name: "output.witness",
Usage: "Determines where to put the `witness` of the post-state.\n" +
"\t`stdout` - into the stdout output\n" +
"\t`stderr` - into the stderr output\n" +
"\t<file> - into the file <file> ",
Value: "witness.json",
}
InputAllocFlag = &cli.StringFlag{
Name: "input.alloc",
Usage: "`stdin` or file name of where to find the prestate alloc to use.",
Expand Down Expand Up @@ -123,6 +139,11 @@ var (
Usage: "`stdin` or file name of where to find the transactions list in RLP form.",
Value: "txs.rlp",
}
// TODO(@CPerezz): rename `Name` of the file in a follow-up PR (relays on EEST -> https://github.com/ethereum/execution-spec-tests/tree/verkle/main)
InputBTFlag = &cli.StringFlag{
Name: "input.vkt",
Usage: "`stdin` or file name of where to find the prestate BT.",
}
SealCliqueFlag = &cli.StringFlag{
Name: "seal.clique",
Usage: "Seal block with Clique. `stdin` or file name of where to find the Clique sealing data.",
Expand Down
Loading
Loading