overlay transition #198

gballet · 2023-04-21T12:43:14Z

Implement the general structure of the overlay transition from an EL client point of view. The details of the optimization and insertion in the tree are left to ethereum/go-verkle#314.

TL;DR; This creates a series of "adapter" patterns that, based on context, will forward requests either to the verkle (overlay) backend, or to the base (MPT) backend.

For example, this implements a TransitionTrie that implements the state.Trie interface. When (*TransitionTrie).TryGet is called, it will try to read from the verkle tree, and if no value is found there, it will search the MPT.

NOTE This is just the general structure, it compiles but probably won't work

TODO

agree with @jsign and integrate with that PR
Fix all the loose parts and use it for replay
Rework the OpenStorageTrie interface

gballet · 2023-04-21T12:46:51Z

core/blockchain.go

+		conversionBlock := uint64(17000) // random block number for now
+		if parent.Number.Uint64() == conversionBlock {
+			bc.StartVerkleTransition(parent.Root, emptyVerkleRoot)
+		}


This is where the blockchain realizes that the transition should start, and signal a ForkedDB/TransitionTrie adapter should be used.

trie/transition.go

jsign · 2023-04-21T14:25:58Z

trie/transition.go

+		return t.overlay.GetKey(key)
+	}
+	return t.base.GetKey(key)
+}


Why we need to do the t.overlay.GetKey(...) call if resolving this is only related to a query to the preimages database? Shouldn't we always ask and know we'll get a correct answer from the base tree?

Also, I looked what GetKey(...) means for verkle and I'm not sure that implementation is correct. As in, not really resolving what is expected and actually making the logic here work incorrectly.

yup it's all buggy. To be fair, this function isn't used very often, if at all, and there's a proposal by Vitalik to also put the preimage in the C-level polynomial so we never have to worry about converting again. I don't think it makes sense to spend time implementing this function at all, tbh, so I'll replace it with a panic.

jsign · 2023-04-21T14:34:03Z

trie/transition.go

+	if t.overlay.GetKey(key) != nil {
+		return t.overlay.TryGet(addr, key)
+	}


Uhm, maybe you're planning to give t.overlay.GetKey(key) other semantics.
Can't we do t.overlay.TryGet and if that returns an empty value (not present), then call the base?

no that's completely incorrect and most likely the result of some dumb copy/paste.

jsign · 2023-04-21T14:34:40Z

trie/transition.go

+		return t.overlay.TryGet(addr, key)
+	}
+	// TODO also insert value into overlay
+	return t.base.TryGet(nil, key)


Is this nil intentional?

yes, because that parameter isn't needed for the MPT. But we can pass the address if you're more comfortable with it.

jsign · 2023-04-21T14:37:52Z

trie/transition.go

+	if t.overlay.GetKey(key) != nil {
+		return t.overlay.TryGet(addr, key)
+	}
+	// TODO also insert value into overlay


Tangent thought: doing this vs not doing may change the gas fee dynamics for users. Not sure if that's relevant for deciding doing this or not (from my perspective it would be better doing it, as we talked). But... just sharing the thought if UX could be impacted in a negative way.

well technically no gas will be charged at this layer. So if we change the gas costs during the transition (we should), then we should charge a blanket cost of : reading into vkt + reading into mpt

core/state/trie_prefetcher.go

jsign · 2023-04-21T14:45:41Z

core/state/database.go

+// CopyTrie implements Database
+func (fdb *ForkingDB) CopyTrie(t Trie) Trie {
+	mpt := fdb.cachingDB.CopyTrie(t)
+	overlay := fdb.VerkleDB.CopyTrie(t)


Not sure how expensive is overlay := fdb.VerkleDB.CopyTrie(t), but if !fdb.started we don't use the result; so maybe we can move it inside the if statement below.

core/state_processor.go

jsign · 2023-04-21T14:51:34Z

core/state_processor.go

+			// move N=500 accounts into the verkle tree, starting with the
+			// slots from the previous account.


Should we skip always the first result of the iterator? Asking since LastAccHash/LastSlotHash I think point to the last migrated value; so in theory we'd like continue with the next?

jsign · 2023-04-21T14:56:01Z

core/state_processor.go

+			for ; stIt.Next() && count < 500; count++ {
+				slotnr := rawdb.ReadPreimage(statedb.Database().DiskDB(), stIt.Hash())
+
+				// @jsign: do your magic here adding the slot `slotnr`
+			}
+
+			// if less than 500 slots were moved, move to the next account
+			for count < 500 {
+				if accIt.Next() {
+					acc, err := snapshot.FullAccount(accIt.Account())
+					if err != nil {
+						log.Error("Invalid account encountered during traversal", "error", err)
+						return err
+					}
+					addr := rawdb.ReadPreimage(statedb.Database().DiskDB(), accIt.Hash())
+
+					// @jsign: do your magic here adding the account at `addr
+
+					// Store the account code if present
+					if !bytes.Equal(acc.CodeHash, emptyCode) {
+						code := rawdb.ReadCode(statedb.Database().DiskDB(), common.BytesToHash(acc.CodeHash))
+						chunks := trie.ChunkifyCode(code)
+
+						// @jsign: do your magic here with the code chunks
+					}
+
+					if !bytes.Equal(acc.Root, emptyRoot[:]) {
+						for ; stIt.Next() && count < 500; count++ {
+							slotnr := rawdb.ReadPreimage(statedb.Database().DiskDB(), stIt.Hash())
+
+							// @jsign do your magic here with extra slots


gballet · 2023-04-21T15:46:34Z

core/state/database.go

+	return mpt
+}
+
+// OpenStorageTrie implements Database


So this implementation is somewhat tacky: there are several trees in the MPT world, but only one in the verkle world. I usually worked around the problem by making sure this would never get called, and it's probably the right approach still.

But I'm also thinking that this function should disappear from the interface in upstream altogether: one can see all these trees as only one tree, after all. I need to think it over, but this hack should work for now.

gballet · 2023-04-21T15:48:02Z

core/state/database.go

+func (fdb *ForkingDB) StartTransition(originalRoot, translatedRoot common.Hash) {
+	fmt.Println(`
+	__________.__                       .__                .__                   __       .__                               .__          ____         
+	\__    ___|  |__   ____        ____ |  |   ____ ______ |  |__ _____    _____/  |_     |  |__ _____    ______    __  _  _|__| ____   / ___\ ______


you don't want to know where this comes from 🤣

trie/transition.go

Fix some bugs identified in the code review Co-authored-by: Ignacio Hagopian <[email protected]> Include base -> overlay key-values migration logic (#199) * mod: add go-verkle version with key-value migration new apis Signed-off-by: Ignacio Hagopian <[email protected]> * core/stateprocessor: use constant for max number of migrated key-values Signed-off-by: Ignacio Hagopian <[email protected]> * core: add base->overlay key-values migration logic Signed-off-by: Ignacio Hagopian <[email protected]> * core: fix some compiler errors Signed-off-by: Ignacio Hagopian <[email protected]> * trie: consider removing transition trie api in the future Signed-off-by: Ignacio Hagopian <[email protected]> * mod: use latest go-verkle Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> fix some unit tests errors get convresion block from file fix compilation issues fix initialization issue in migrator fix: changes needed to run the first 28 blocks important sutff: fix the banner fix: use nonce instead of balance in nonce leaf (#202) fixes for performing the overlay transition (#203) * fixes for performing the overlay transition * fixes for the full replay * fix: deletion-and-recreation of EoA * fixes to replay 2M+ blocks * upgrade to go-verkle@master * fix: proper number of chunk evals * rewrite conversion loop to fix known issues changes to make replay work with the overlay method (#216) * fixes for performing the overlay transition fixes for the full replay fix: deletion-and-recreation of EoA fixes to replay 2M+ blocks upgrade to go-verkle@master fix: proper number of chunk evals rewrite conversion loop to fix known issues changes to make replay work with the overlay method fixes to replay 2M+ blocks update to latest go-verkle@master * use a PBSS-like scheme for internal nodes (#221) * use a PBSS-like scheme for internal nodes * a couple of fixes coming from debugging replay * fix: use an error to notify the transition tree that a deleted account was found in the overlay tree (#222) * fixes for pbss replay (#227) * fixes for pbss replay * trie/verkle: use capped batch size (#229) * trie/verkle: use capped batch size Signed-off-by: Ignacio Hagopian <[email protected]> * trie/verkle: avoid path variable allocation per db.Put Signed-off-by: Ignacio Hagopian <[email protected]> * don't keep more than 32 state root conversions in RAM (#230) --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Guillaume Ballet <[email protected]> * cleanup some code * mod: update go-verkle Signed-off-by: Ignacio Hagopian <[email protected]> * re-enable snapshot (#231) * re-enable cancun block / snapshot (#226) * clear storage conversion key upon translating account (#234) * clear storage conversion key upon translating account * mod: use latest go-verkle Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> * fix: self-deadlock with translated root map mutex (#236) * return compressed commitment as root commitment (#237) --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> fix first panic in *TransitionTrie.Copy() upgrade go-verkle to latest master mod: update go-verkle (#239) Signed-off-by: Ignacio Hagopian <[email protected]> core: print state root every 100 blocks (#240) Signed-off-by: Ignacio Hagopian <[email protected]> fix: only Commit the account trie (#242) fixes to get TestProcessVerkle to work with the overlay branch (#238) * fixes to get TestProcessVerkle to work with the overlay branch * fix all panics in verkle state processor test * fix proof verification

gballet commented Apr 21, 2023

View reviewed changes

jsign reviewed Apr 21, 2023

View reviewed changes

gballet commented Apr 21, 2023

View reviewed changes

trie/transition.go Outdated Show resolved Hide resolved

gballet mentioned this pull request May 12, 2023

read from tree in state object if the snapshot is nil #205

Merged

gballet force-pushed the overlay-implementation branch from b3d393f to 54dd265 Compare July 26, 2023 12:36

gballet closed this Mar 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

overlay transition #198

overlay transition #198

gballet commented Apr 21, 2023 •

edited

Loading

gballet Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023 •

edited

Loading

jsign Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023

jsign Apr 21, 2023

gballet Apr 21, 2023

gballet Apr 21, 2023

		// move N=500 accounts into the verkle tree, starting with the
		// slots from the previous account.

overlay transition #198

overlay transition #198

Conversation

gballet commented Apr 21, 2023 • edited Loading

NOTE This is just the general structure, it compiles but probably won't work

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gballet Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gballet commented Apr 21, 2023 •

edited

Loading

gballet Apr 21, 2023 •

edited

Loading