blockchain: add execution witness validation #515

jsign · 2024-10-21T16:58:28Z

This PR adds the required logic to add an extra validation when processing a block. We must verify that the execution witness generated while re-executing the block matches the one present in the block. This is a similar check as checking the stateRoot and usedGas.

It also adds the parentRoot to the witness when filling.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

.github/workflows/stable-spec-tests.yml

jsign · 2024-10-24T15:21:58Z

go.mod

-go 1.19
+go 1.22
+
+toolchain go1.22.1


This was forced by the Go compiler since we updated to the lastest go-verkle, see its go.mod.

go.mod

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

cmd/evm/internal/t8ntool/execution.go

jsign · 2024-10-25T18:25:26Z

cmd/evm/internal/t8ntool/execution.go

-	case *trie.TransitionTrie:
-		vtrpre = tr.Overlay().Copy()


We stop doing witness generation for Overlay Tree.

jsign · 2024-10-25T18:29:29Z

cmd/evm/internal/t8ntool/execution.go

+		root, err := statedb.Commit(0, false)
+		if err != nil {
+			panic(err)
+		}


Apart from collecting the root, we were also ignoring a potential error.

jsign · 2024-10-25T18:30:32Z

cmd/evm/internal/t8ntool/execution.go

+		// If the VKT prestate is empty, this means that the "parent" root is the MPT.
+		// If we don't do this, we'd consider the "parent" to be an empty VKT which isn't
+		// correct.
+		if pre.VKT != nil {
+			parentRoot = root
+		}


I added a comment since this if might not be entirely obvious. Practically, the if is irrelevant since we don't create witnesses now for Overlay Tree.

But, this method doesn't know about this decision and should return the "parent" root for all cases.

jsign · 2024-10-25T18:33:31Z

consensus/beacon/consensus.go

-		if chain.Config().ProofInBlocks {
-			preTrie, err := state.Database().OpenTrie(parent.Root)
-			if err != nil {
-				return nil, fmt.Errorf("error opening pre-state tree root: %w", err)
-			}
-
-			var okpre, okpost bool
-			var vtrpre, vtrpost *trie.VerkleTrie
-			switch pre := preTrie.(type) {
-			case *trie.VerkleTrie:
-				vtrpre, okpre = preTrie.(*trie.VerkleTrie)
-				switch tr := state.GetTrie().(type) {
-				case *trie.VerkleTrie:
-					vtrpost = tr
-					okpost = true
-				// This is to handle a situation right at the start of the conversion:
-				// the post trie is a transition tree when the pre tree is an empty
-				// verkle tree.
-				case *trie.TransitionTrie:
-					vtrpost = tr.Overlay()
-					okpost = true
-				default:
-					okpost = false
-				}
-			case *trie.TransitionTrie:
-				vtrpre = pre.Overlay()
-				okpre = true
-				post, _ := state.GetTrie().(*trie.TransitionTrie)
-				vtrpost = post.Overlay()
-				okpost = true
-			default:
-				// This should only happen for the first block of the
-				// conversion, when 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
-			}
-			if okpre && okpost {
-				if len(keys) > 0 {
-					p, k, err = trie.ProveAndSerialize(vtrpre, vtrpost, keys, vtrpre.FlatdbNodeResolver)
-					if err != nil {
-						return nil, fmt.Errorf("error generating verkle proof for block %d: %w", header.Number, err)
-					}
-				}
-			}


This deleted block chunk is only extracted to a new method below. I did this because now apart from creating witnesses in block generation, we also create the witness in block processing... validating the witness is now part of the block validation (more on that below).

If you look at 517, this function will be split back into smaller elements. I suggest not doing that.

jsign · 2024-10-25T18:33:52Z

consensus/beacon/consensus.go

 	}
 	return block, nil
 }

+func BuildVerkleProof(header *types.Header, state *state.StateDB, parentRoot common.Hash) (verkle.StateDiff, *verkle.VerkleProof, error) {


This method is just the old code. No code lines where changed.

jsign · 2024-10-25T18:34:43Z

core/block_validator.go

@@ -131,6 +132,24 @@ func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateD
 	if root := statedb.IntermediateRoot(v.config.IsEIP158(header.Number)); header.Root != root {
 		return fmt.Errorf("invalid merkle root (remote: %x local: %x) dberr: %w", header.Root, root, statedb.Error())
 	}
+	if blockEw := block.ExecutionWitness(); blockEw != nil {


Finally, we reached the main goal of the PR. ValidateState is an existing method that has the usual stateRoot and gasUsage checks when validating blocks.

Now I added logic to also check the execution witness (if the proposed block has one).

jsign · 2024-10-25T18:34:59Z

core/chain_makers.go

-			err = trie.DeserializeAndVerifyVerkleProof(block.ExecutionWitness().VerkleProof, block.ExecutionWitness().ParentStateRoot[:], block.Root().Bytes(), block.ExecutionWitness().StateDiff)
+			err = verkle.Verify(block.ExecutionWitness().VerkleProof, block.ExecutionWitness().ParentStateRoot[:], block.Root().Bytes(), block.ExecutionWitness().StateDiff)


Sideffects of upgrading go-verkle.

jsign · 2024-10-25T18:35:30Z

trie/verkle.go

@@ -300,60 +300,6 @@ func ProveAndSerialize(pretrie, posttrie *VerkleTrie, keys [][]byte, resolver ve
 	return p, kvps, nil
 }

-func DeserializeAndVerifyVerkleProof(vp *verkle.VerkleProof, preStateRoot []byte, postStateRoot []byte, statediff verkle.StateDiff) error {


This is gone, since was moved to go-verkle.

actually it's coming back in #517 but yeah let's sort this out when we get there.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign · 2024-10-26T15:58:44Z

.github/workflows/stable-spec-tests.yml

-  FIXTURES_TAG: "verkle@v0.0.6"
+  FIXTURES_TAG: "verkle@v0.0.7-alpha-4"


Whenever we merge this PR to kaustinen-with-shapella, I'd be able to create the final v0.0.7. That is because execution-spec-tests targets kaustinen-with-shapella by default, and I need this PR changes to be there first.

Kind of the usual chicken and egg problems. I can do a later quick PR to change this to v0.0.7 when that exists.

cmd/evm/__debug_bin3936612204

trie/verkle_test.go

gballet · 2024-10-28T11:25:31Z

trie/verkle.go

@@ -300,60 +300,6 @@ func ProveAndSerialize(pretrie, posttrie *VerkleTrie, keys [][]byte, resolver ve
 	return p, kvps, nil
 }

-func DeserializeAndVerifyVerkleProof(vp *verkle.VerkleProof, preStateRoot []byte, postStateRoot []byte, statediff verkle.StateDiff) error {


actually it's coming back in #517 but yeah let's sort this out when we get there.

core/state_processor_test.go

cmd/evm/internal/t8ntool/execution.go

gballet · 2024-10-28T16:50:10Z

consensus/beacon/consensus.go

-		if chain.Config().ProofInBlocks {
-			preTrie, err := state.Database().OpenTrie(parent.Root)
-			if err != nil {
-				return nil, fmt.Errorf("error opening pre-state tree root: %w", err)
-			}
-
-			var okpre, okpost bool
-			var vtrpre, vtrpost *trie.VerkleTrie
-			switch pre := preTrie.(type) {
-			case *trie.VerkleTrie:
-				vtrpre, okpre = preTrie.(*trie.VerkleTrie)
-				switch tr := state.GetTrie().(type) {
-				case *trie.VerkleTrie:
-					vtrpost = tr
-					okpost = true
-				// This is to handle a situation right at the start of the conversion:
-				// the post trie is a transition tree when the pre tree is an empty
-				// verkle tree.
-				case *trie.TransitionTrie:
-					vtrpost = tr.Overlay()
-					okpost = true
-				default:
-					okpost = false
-				}
-			case *trie.TransitionTrie:
-				vtrpre = pre.Overlay()
-				okpre = true
-				post, _ := state.GetTrie().(*trie.TransitionTrie)
-				vtrpost = post.Overlay()
-				okpost = true
-			default:
-				// This should only happen for the first block of the
-				// conversion, when 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
-			}
-			if okpre && okpost {
-				if len(keys) > 0 {
-					p, k, err = trie.ProveAndSerialize(vtrpre, vtrpost, keys, vtrpre.FlatdbNodeResolver)
-					if err != nil {
-						return nil, fmt.Errorf("error generating verkle proof for block %d: %w", header.Number, err)
-					}
-				}
-			}


If you look at 517, this function will be split back into smaller elements. I suggest not doing that.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

gballet

LGTM. Maybe wait for the testnet relaunch before merging, but otherwise it's good to go.

jsign mentioned this pull request Oct 22, 2024

add proof Equal methods ethereum/go-verkle#458

Merged

jsign added 7 commits October 24, 2024 12:16

blockchain: add execution witness validation

9beccc2

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

more fixes

86c9305

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

update go-ipa

7ee97cc

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

update go-ipa

20fa694

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

add parentRoot to dispatched output

b0521df

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

remove todo

7ee8902

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

fix compilation

47e17d1

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign-fixtures-witness-check branch from 260d1b1 to 043792d Compare October 24, 2024 15:18

jsign changed the base branch from jsign-witness-fix to kaustinen-with-shapella October 24, 2024 15:19

target tenatative version

fd59830

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign-fixtures-witness-check branch from 043792d to fd59830 Compare October 24, 2024 15:19

jsign commented Oct 24, 2024

View reviewed changes

.github/workflows/stable-spec-tests.yml Outdated Show resolved Hide resolved

jsign commented Oct 24, 2024

View reviewed changes

fix

0406d61

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign-fixtures-witness-check branch from 5fe94aa to 813f9ea Compare October 24, 2024 16:53

generate parentRoot for witness check

a588006

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign-fixtures-witness-check branch from 813f9ea to a588006 Compare October 24, 2024 17:07

jsign added 12 commits October 24, 2024 15:58

progress

b08b06b

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

more fixes

dcaffc6

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

change order of execution witness checks

3ca9415

fill: remove witness generation in overlay tree when

c781d4b

cleanup

034a01c

fix

b738092

fixes

c229a56

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

cleanup

551b0c8

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

mod: use latest go-verkle

606e682

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

more fixes

4afce0a

update go-verkle

f4a98cf

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

add comment

8461f91

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

cleanup

e9f1c83

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign-fixtures-witness-check branch from 9ca31e6 to e9f1c83 Compare October 25, 2024 18:32

jsign commented Oct 25, 2024

View reviewed changes

jsign added 3 commits October 25, 2024 17:42

ci: test fixtures alpha

bf7a55b

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

ci change branch

8790b40

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

restore target branch

3a17341

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign commented Oct 26, 2024

View reviewed changes

jsign requested a review from gballet October 26, 2024 18:15

jsign marked this pull request as ready for review October 26, 2024 18:15

gballet reviewed Oct 28, 2024

View reviewed changes

jsign and others added 4 commits October 28, 2024 13:57

pr feedback

1ac865c

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

fix nit

ff97fbe

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

disable linter warnings, won't fix them in this branch

d134ccf

disable more linter checks

f0cfa2e

gballet approved these changes Oct 28, 2024

View reviewed changes

jsign mentioned this pull request Oct 28, 2024

update go-verkle #491

Closed

gballet merged commit 0f986ab into kaustinen-with-shapella Oct 29, 2024
12 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blockchain: add execution witness validation #515

blockchain: add execution witness validation #515

jsign commented Oct 21, 2024 •

edited

Loading

jsign Oct 24, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

gballet Oct 28, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

jsign Oct 25, 2024

gballet Oct 28, 2024

jsign Oct 26, 2024

gballet Oct 28, 2024

gballet Oct 28, 2024

gballet left a comment

		err = trie.DeserializeAndVerifyVerkleProof(block.ExecutionWitness().VerkleProof, block.ExecutionWitness().ParentStateRoot[:], block.Root().Bytes(), block.ExecutionWitness().StateDiff)
		err = verkle.Verify(block.ExecutionWitness().VerkleProof, block.ExecutionWitness().ParentStateRoot[:], block.Root().Bytes(), block.ExecutionWitness().StateDiff)

		FIXTURES_TAG: "verkle@v0.0.6"
		FIXTURES_TAG: "verkle@v0.0.7-alpha-4"

blockchain: add execution witness validation #515

blockchain: add execution witness validation #515

Conversation

jsign commented Oct 21, 2024 • 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

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 left a comment

Choose a reason for hiding this comment

jsign commented Oct 21, 2024 •

edited

Loading