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

Refactor transition post genesis #311

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Refactor transition post genesis #311

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff855a8
rewrite per-block conversion pointer management
gballet Nov 24, 2023
0cf1dfb
remove unused method
gballet Nov 24, 2023
13b84f3
fix: a branch that can verge at genesis or post genesis (#314)
gballet Nov 30, 2023
0da3cd3
fix: import cycle in conversion refactor (#315)
gballet Nov 30, 2023
4ee2726
fix shadowfork panic in OpenStorageTrie
gballet Nov 30, 2023
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
18 changes: 17 additions & 1 deletion consensus/beacon/consensus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/misc/eip1559"
"github.com/ethereum/go-ethereum/consensus/misc/eip4844"
"github.com/ethereum/go-ethereum/core"
Copy link
Owner Author

Choose a reason for hiding this comment

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

this causes a cycle in tests

"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"
Expand Down Expand Up @@ -368,6 +369,12 @@ func (beacon *Beacon) Finalize(chain consensus.ChainHeaderReader, header *types.
state.Witness().TouchAddressOnWriteAndComputeGas(header.Coinbase[:], uint256.Int{}, utils.NonceLeafKey)
state.Witness().TouchAddressOnWriteAndComputeGas(header.Coinbase[:], uint256.Int{}, utils.CodeKeccakLeafKey)
state.Witness().TouchAddressOnWriteAndComputeGas(header.Coinbase[:], uint256.Int{}, utils.CodeSizeLeafKey)

if chain.Config().IsPrague(header.Number, header.Time) {
fmt.Println("at block", header.Number, "performing transition?", state.Database().InTransition())
Copy link
Owner Author

Choose a reason for hiding this comment

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

Suggested change
fmt.Println("at block", header.Number, "performing transition?", state.Database().InTransition())

parent := chain.GetHeaderByHash(header.ParentHash)
core.OverlayVerkleTransition(state, parent.Root)
Copy link
Owner Author

Choose a reason for hiding this comment

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

this PR does the same thing as #309, but it was easier to reason about when called in a single location. It should still be possible to move it to e.g. Prepare but that's definitely enough work for a single PR.

}
}

// FinalizeAndAssemble implements consensus.Engine, setting the final state and
Expand All @@ -392,6 +399,7 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea

// Assign the final state root to header.
header.Root = state.IntermediateRoot(true)
state.Database().SaveTransitionState(header.Root)

var (
p *verkle.VerkleProof
Expand All @@ -405,6 +413,7 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
return nil, fmt.Errorf("nil parent header for block %d", header.Number)
}

state.Database().LoadTransitionState(parent.Root)
Copy link
Owner Author

Choose a reason for hiding this comment

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

When producing the proof, the pre-tree is re-opened. Unfortunately, the OpenTrie function needs the pointers, who have to be reloaded. This is a bit tacky and could be avoided if the pre tree was opened before, e.g. in Prepare.

preTrie, err := state.Database().OpenTrie(parent.Root)
if err != nil {
return nil, fmt.Errorf("error opening pre-state tree root: %w", err)
Expand Down Expand Up @@ -435,7 +444,14 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
vtrpost = post.Overlay()
okpost = true
default:
panic("invalid tree type")
// This should only happen for the first block,
// so the previous tree is a merkle tree. Logically,
// the "previous" verkle tree is an empty tree.
okpre = true
vtrpre = trie.NewVerkleTrie(verkle.New(), state.Database().TrieDB(), utils.NewPointCache(), false)
post := state.GetTrie().(*trie.TransitionTrie)
vtrpost = post.Overlay()
okpost = true
Comment on lines +447 to +454
Copy link
Owner Author

Choose a reason for hiding this comment

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

the change caused this panic to trigger, because OpenTrie needs the parameters to decide if it's a verkle or merkle tree. But I don't think it was correct to begin with, because for the first block, the "pre" tree is technically a MPT. But if we want to prove against something, that something we prove against is an empty verkle tree. So this is what this code does: it returns an empty verkle tree so that the proof can be built against it.

For some reason I haven't determined yet, the first proof is still empty. That is an investigation for later though.

}
if okpre && okpost {
if len(keys) > 0 {
Expand Down
40 changes: 20 additions & 20 deletions core/overlay_transition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
mpt = tt.Base()
vkt = tt.Overlay()
hasPreimagesBin = false
preimageSeek = migrdb.GetCurrentPreimageOffset(root)
preimageSeek = migrdb.GetCurrentPreimageOffset()
Copy link
Owner Author

Choose a reason for hiding this comment

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

This PR reverts the actions of the base branch, since it has the concept of "current" set of pointers.

fpreimages *bufio.Reader
)

Expand All @@ -65,15 +65,15 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
hasPreimagesBin = true
}

accIt, err := statedb.Snaps().AccountIterator(mpt.Hash(), migrdb.GetCurrentAccountHash(root))
accIt, err := statedb.Snaps().AccountIterator(mpt.Hash(), migrdb.GetCurrentAccountHash())
if err != nil {
return err
}
defer accIt.Release()
accIt.Next()

// If we're about to start with the migration process, we have to read the first account hash preimage.
if migrdb.GetCurrentAccountAddress(root) == nil {
if migrdb.GetCurrentAccountAddress() == nil {
var addr common.Address
if hasPreimagesBin {
if _, err := io.ReadFull(fpreimages, addr[:]); err != nil {
Expand All @@ -85,8 +85,8 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
return fmt.Errorf("addr len is zero is not 32: %d", len(addr))
}
}
migrdb.SetCurrentAccountAddress(addr, root)
if migrdb.GetCurrentAccountHash(root) != accIt.Hash() {
migrdb.SetCurrentAccountAddress(addr)
if migrdb.GetCurrentAccountHash() != accIt.Hash() {
return fmt.Errorf("preimage file does not match account hash: %s != %s", crypto.Keccak256Hash(addr[:]), accIt.Hash())
}
preimageSeek += int64(len(addr))
Expand All @@ -108,7 +108,7 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
log.Error("Invalid account encountered during traversal", "error", err)
return err
}
vkt.SetStorageRootConversion(*migrdb.GetCurrentAccountAddress(root), acc.Root)
vkt.SetStorageRootConversion(*migrdb.GetCurrentAccountAddress(), acc.Root)

// Start with processing the storage, because once the account is
// converted, the `stateRoot` field loses its meaning. Which means
Expand All @@ -120,7 +120,7 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
// to during normal block execution. A mitigation strategy has been
// introduced with the `*StorageRootConversion` fields in VerkleDB.
if acc.HasStorage() {
stIt, err := statedb.Snaps().StorageIterator(mpt.Hash(), accIt.Hash(), migrdb.GetCurrentSlotHash(root))
stIt, err := statedb.Snaps().StorageIterator(mpt.Hash(), accIt.Hash(), migrdb.GetCurrentSlotHash())
if err != nil {
return err
}
Expand All @@ -132,7 +132,7 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
// processing the storage for that account where we left off.
// If the entire storage was processed, then the iterator was
// created in vain, but it's ok as this will not happen often.
for ; !migrdb.GetStorageProcessed(root) && count < maxMovedCount; count++ {
for ; !migrdb.GetStorageProcessed() && count < maxMovedCount; count++ {
var (
value []byte // slot value after RLP decoding
safeValue [32]byte // 32-byte aligned value
Expand Down Expand Up @@ -160,12 +160,12 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
}
preimageSeek += int64(len(slotnr))

mkv.addStorageSlot(migrdb.GetCurrentAccountAddress(root).Bytes(), slotnr, safeValue[:])
mkv.addStorageSlot(migrdb.GetCurrentAccountAddress().Bytes(), slotnr, safeValue[:])

// advance the storage iterator
migrdb.SetStorageProcessed(!stIt.Next(), root)
if !migrdb.GetStorageProcessed(root) {
migrdb.SetCurrentSlotHash(stIt.Hash(), root)
migrdb.SetStorageProcessed(!stIt.Next())
if !migrdb.GetStorageProcessed() {
migrdb.SetCurrentSlotHash(stIt.Hash())
}
}
stIt.Release()
Expand All @@ -178,20 +178,20 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
if count < maxMovedCount {
count++ // count increase for the account itself

mkv.addAccount(migrdb.GetCurrentAccountAddress(root).Bytes(), acc)
vkt.ClearStrorageRootConversion(*migrdb.GetCurrentAccountAddress(root))
mkv.addAccount(migrdb.GetCurrentAccountAddress().Bytes(), acc)
vkt.ClearStrorageRootConversion(*migrdb.GetCurrentAccountAddress())

// Store the account code if present
if !bytes.Equal(acc.CodeHash, types.EmptyCodeHash[:]) {
code := rawdb.ReadCode(statedb.Database().DiskDB(), common.BytesToHash(acc.CodeHash))
chunks := trie.ChunkifyCode(code)

mkv.addAccountCode(migrdb.GetCurrentAccountAddress(root).Bytes(), uint64(len(code)), chunks)
mkv.addAccountCode(migrdb.GetCurrentAccountAddress().Bytes(), uint64(len(code)), chunks)
}

// reset storage iterator marker for next account
migrdb.SetStorageProcessed(false, root)
migrdb.SetCurrentSlotHash(common.Hash{}, root)
migrdb.SetStorageProcessed(false)
migrdb.SetCurrentSlotHash(common.Hash{})

// Move to the next account, if available - or end
// the transition otherwise.
Expand All @@ -212,7 +212,7 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
return fmt.Errorf("preimage file does not match account hash: %s != %s", crypto.Keccak256Hash(addr[:]), accIt.Hash())
}
preimageSeek += int64(len(addr))
migrdb.SetCurrentAccountAddress(addr, root)
migrdb.SetCurrentAccountAddress(addr)
} else {
// case when the account iterator has
// reached the end but count < maxCount
Expand All @@ -221,9 +221,9 @@ func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash) error {
}
}
}
migrdb.SetCurrentPreimageOffset(preimageSeek, root)
migrdb.SetCurrentPreimageOffset(preimageSeek)

log.Info("Collected key values from base tree", "count", count, "duration", time.Since(now), "last account", statedb.Database().GetCurrentAccountHash(root))
log.Info("Collected key values from base tree", "count", count, "duration", time.Since(now), "last account", statedb.Database().GetCurrentAccountHash())

// Take all the collected key-values and prepare the new leaf values.
// This fires a background routine that will start doing the work that
Expand Down
Loading