From 546b27ea62aaeb0f90e1dd96f1eee115712ffd03 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:54:43 +0100 Subject: [PATCH 1/6] test: check witness when contract creation fails --- core/state_processor_test.go | 71 ++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 8ed660a5042e..01773a0389de 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -542,3 +542,74 @@ func TestProcessVerkle(t *testing.T) { } } } + +func TestProcessVerkleiInvalidContractCreation(t *testing.T) { + var ( + config = ¶ms.ChainConfig{ + ChainID: big.NewInt(69420), + HomesteadBlock: big.NewInt(0), + EIP150Block: big.NewInt(0), + EIP155Block: big.NewInt(0), + EIP158Block: big.NewInt(0), + ByzantiumBlock: big.NewInt(0), + ConstantinopleBlock: big.NewInt(0), + PetersburgBlock: big.NewInt(0), + IstanbulBlock: big.NewInt(0), + MuirGlacierBlock: big.NewInt(0), + BerlinBlock: big.NewInt(0), + LondonBlock: big.NewInt(0), + Ethash: new(params.EthashConfig), + ShanghaiTime: u64(0), + PragueTime: u64(0), + TerminalTotalDifficulty: common.Big0, + TerminalTotalDifficultyPassed: true, + ProofInBlocks: true, + } + bcdb = rawdb.NewMemoryDatabase() // Database for the blockchain + gendb = rawdb.NewMemoryDatabase() // Database for the block-generation code, they must be separate as they are path-based. + coinbase = common.HexToAddress("0x71562b71999873DB5b286dF957af199Ec94617F7") + account1 = common.HexToAddress("0x687704DB07e902e9A8B3754031D168D46E3D586e") + account2 = common.HexToAddress("0x6177843db3138ae69679A54b95cf345ED759450d") + gspec = &Genesis{ + Config: config, + Alloc: GenesisAlloc{ + coinbase: GenesisAccount{ + Balance: big.NewInt(1000000000000000000), // 1 ether + Nonce: 0, + }, + account1: GenesisAccount{ + Balance: big.NewInt(1000000000000000000), // 1 ether + Nonce: 2, + }, + account2: GenesisAccount{ + Balance: big.NewInt(1000000000000000000), // 1 ether + Nonce: 0, + }, + }, + } + ) + // Verkle trees use the snapshot, which must be enabled before the + // data is saved into the tree+database. + genesis := gspec.MustCommit(bcdb) + overrideProofInBlock := true + conversionStride := uint64(0) + blockchain, _ := NewBlockChain(bcdb, nil, gspec, &ChainOverrides{OverrideProofInBlock: &overrideProofInBlock, OverrideOverlayStride: &conversionStride}, beacon.New(ethash.NewFaker()), vm.Config{}, nil, nil) + defer blockchain.Stop() + + // Commit the genesis block to the block-generation database as it + // is now independent of the blockchain database. + gspec.MustCommit(gendb) + + _, _, _, statediff := GenerateVerkleChain(gspec.Config, genesis, beacon.New(ethash.NewFaker()), gendb, 1, func(i int, gen *BlockGen) { + gen.SetPoS() + + var tx types.Transaction + txpayload := common.Hex2Bytes("01f8d683010f2c028443ad7d0e830186a08080b880b00e7fa3c849dce891cce5fae8a4c46cbb313d6aec0c0ffe7863e05fb7b22d4807674c6055527ffbfcb0938f3e18f7937aa8fa95d880afebd5c4cec0d85186095832d03c85cf8a60755260ab60955360cf6096536066609753606e60985360fa609953609e609a53608e609b536024609c5360f6609d536072609e5360a4609fc080a08fc6f7101f292ff1fb0de8ac69c2d320fbb23bfe61cf327173786ea5daee6e37a044c42d91838ef06646294bf4f9835588aee66243b16a66a2da37641fae4c045f") + if err := tx.UnmarshalBinary(txpayload); err != nil { + t.Fatal(err) + } + gen.AddTx(&tx) + }) + + t.Log(statediff) +} From ebdd6c23201427795e660a4e48463a52445260ab Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 17 Jan 2024 16:11:10 +0100 Subject: [PATCH 2/6] check no suffix goes above 4 --- core/state_processor_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 01773a0389de..cd44a0d8ea4e 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -591,9 +591,7 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { // Verkle trees use the snapshot, which must be enabled before the // data is saved into the tree+database. genesis := gspec.MustCommit(bcdb) - overrideProofInBlock := true - conversionStride := uint64(0) - blockchain, _ := NewBlockChain(bcdb, nil, gspec, &ChainOverrides{OverrideProofInBlock: &overrideProofInBlock, OverrideOverlayStride: &conversionStride}, beacon.New(ethash.NewFaker()), vm.Config{}, nil, nil) + blockchain, _ := NewBlockChain(bcdb, nil, gspec, nil, beacon.New(ethash.NewFaker()), vm.Config{}, nil, nil) defer blockchain.Stop() // Commit the genesis block to the block-generation database as it @@ -611,5 +609,12 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { gen.AddTx(&tx) }) - t.Log(statediff) + // Check that no account has a value above 4 + for _, stemStateDiff := range statediff[0] { + for _, suffixDiff := range stemStateDiff.SuffixDiffs { + if suffixDiff.Suffix > 4 { + t.Fatalf("invalid suffix diff found: %d\n", suffixDiff.Suffix) + } + } + } } From f7273dc267660582edb7835eb1d512d5a775a569 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:35:00 +0100 Subject: [PATCH 3/6] add more transactions to test + fix slot computation --- core/state_processor_test.go | 75 ++++++++++++++++++++++++++++++------ trie/utils/verkle.go | 29 +++----------- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index cd44a0d8ea4e..cceba475f5c3 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -17,7 +17,7 @@ package core import ( - //"bytes" + "bytes" "crypto/ecdsa" //"fmt" @@ -579,11 +579,11 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { }, account1: GenesisAccount{ Balance: big.NewInt(1000000000000000000), // 1 ether - Nonce: 2, + Nonce: 0, }, account2: GenesisAccount{ Balance: big.NewInt(1000000000000000000), // 1 ether - Nonce: 0, + Nonce: 1, }, }, } @@ -598,22 +598,75 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { // is now independent of the blockchain database. gspec.MustCommit(gendb) - _, _, _, statediff := GenerateVerkleChain(gspec.Config, genesis, beacon.New(ethash.NewFaker()), gendb, 1, func(i int, gen *BlockGen) { + // Create two blocks that reproduce what is happening on kaustinen. + // - The first block contains two failing contract creation transactions, that write to storage before they revert. + // - The second block contains a single failing contract creation transaction, that fails right off the bat. + _, _, _, statediff := GenerateVerkleChain(gspec.Config, genesis, beacon.New(ethash.NewFaker()), gendb, 2, func(i int, gen *BlockGen) { gen.SetPoS() - var tx types.Transaction - txpayload := common.Hex2Bytes("01f8d683010f2c028443ad7d0e830186a08080b880b00e7fa3c849dce891cce5fae8a4c46cbb313d6aec0c0ffe7863e05fb7b22d4807674c6055527ffbfcb0938f3e18f7937aa8fa95d880afebd5c4cec0d85186095832d03c85cf8a60755260ab60955360cf6096536066609753606e60985360fa609953609e609a53608e609b536024609c5360f6609d536072609e5360a4609fc080a08fc6f7101f292ff1fb0de8ac69c2d320fbb23bfe61cf327173786ea5daee6e37a044c42d91838ef06646294bf4f9835588aee66243b16a66a2da37641fae4c045f") - if err := tx.UnmarshalBinary(txpayload); err != nil { - t.Fatal(err) + if i == 0 { + var tx1, tx2, tx3 types.Transaction + tx1payload := common.Hex2Bytes("f8d48084479c2c18830186a08080b8806000602955bda3f9600060ca55600060695523b360006039551983576000601255b0620c2fde2c592ac2600060bc55e0ac6000606455a63e22600060e655eb607e605c5360a2605d5360c7605e53601d605f5360eb606053606b606153608e60625360816063536079606453601e60655360fc60665360b7606753608b60685383021e7ca0cc20c65a97d2e526b8ec0f4266e8b01bdcde43b9aeb59d8bfb44e8eb8119c109a07a8e751813ae1b2ce734960dbc39a4f954917d7822a2c5d1dca18b06c584131f") + if err := tx1.UnmarshalBinary(tx1payload); err != nil { + t.Fatal(err) + } + gen.AddTx(&tx1) + + tx2payload := common.Hex2Bytes("02f8db83010f2c01843b9aca0084479c2c18830186a08080b88060006085553fad6000600a55600060565555600060b55506600060cf557f1b8b38183e7bd1bdfaa7123c5a4976e54cce0e42049d841411978fd3595e25c66019527f0538943712953cf08900aae40222a40b2d5a4ac8075ad8cf0870e2be307edbb96039527f9f3174ff85024747041ae7a611acffb987c513c088d90ab288aec080a0cd6ac65ce2cb0a912371f6b5a551ba8caffc22ec55ad4d3cb53de41d05eb77b6a02e0dfe8513dfa6ec7bfd7eda6f5c0dac21b39b982436045e128cec46cfd3f960") + if err := tx2.UnmarshalBinary(tx2payload); err != nil { + t.Fatal(err) + } + gen.AddTx(&tx2) + + // this one is a simple transfer that succeeds, necessary to get the correct nonce in the other block. + tx3payload := common.Hex2Bytes("f8e80184479c2c18830186a094bbbbde4ca27f83fc18aa108170547ff57675936a80b8807ff71f7c15faadb969a76a5f54a81a0117e1e743cb7f24e378eda28442ea4c6eb6604a527fb5409e5718d44e23bfffac926e5ea726067f772772e7e19446acba0c853f62f5606a526020608a536088608b536039608c536004608d5360af608e537f7f7675d9f210e0a61564e6d11e7cd75f5bc9009ac9f6b94a0fc63035441a83021e7ba04a4a172d81ebb02847829b76a387ac09749c8b65668083699abe20c887fb9efca07c5b1a990702ec7b31a5e8e3935cd9a77649f8c25a84131229e24ab61aec6093") + if err := tx3.UnmarshalBinary(tx3payload); err != nil { + t.Fatal(err) + } + gen.AddTx(&tx3) + } else { + var tx types.Transaction + txpayload := common.Hex2Bytes("01f8d683010f2c028443ad7d0e830186a08080b880b00e7fa3c849dce891cce5fae8a4c46cbb313d6aec0c0ffe7863e05fb7b22d4807674c6055527ffbfcb0938f3e18f7937aa8fa95d880afebd5c4cec0d85186095832d03c85cf8a60755260ab60955360cf6096536066609753606e60985360fa609953609e609a53608e609b536024609c5360f6609d536072609e5360a4609fc080a08fc6f7101f292ff1fb0de8ac69c2d320fbb23bfe61cf327173786ea5daee6e37a044c42d91838ef06646294bf4f9835588aee66243b16a66a2da37641fae4c045f") + if err := tx.UnmarshalBinary(txpayload); err != nil { + t.Fatal(err) + } + gen.AddTx(&tx) } - gen.AddTx(&tx) }) - // Check that no account has a value above 4 + // Check that values 0x29 and 0x05 are found in the storage (and that they lead + // to no update, since the contract creation code reverted) for _, stemStateDiff := range statediff[0] { + // Check that the value 0x85, which is overflowing the account header, + // is present. + if bytes.Equal(stemStateDiff.Stem[:], common.Hex2Bytes("a10042195481d30478251625e1ccef0e2174dc4e083e81d2566d880373f791")) { + for _, suffixDiff := range stemStateDiff.SuffixDiffs { + if suffixDiff.Suffix != 69 { + t.Fatalf("invalid suffix diff found for %x in block #1: %d\n", stemStateDiff.Stem, suffixDiff.Suffix) + } + } + } else if bytes.Equal(stemStateDiff.Stem[:], common.Hex2Bytes("b24fa84f214459af17d6e3f604811f252cac93146f02d67d7811bbcdfa448b")) { + for _, suffixDiff := range stemStateDiff.SuffixDiffs { + if suffixDiff.Suffix != 105 && suffixDiff.Suffix != 0 && suffixDiff.Suffix != 2 && suffixDiff.Suffix != 3 { + t.Fatalf("invalid suffix diff found for %x in block #1: %d\n", stemStateDiff.Stem, suffixDiff.Suffix) + } + } + } else { + + for _, suffixDiff := range stemStateDiff.SuffixDiffs { + if suffixDiff.Suffix > 4 { + t.Fatalf("invalid suffix diff found for %x in block #1: %d\n", stemStateDiff.Stem, suffixDiff.Suffix) + } + } + } + } + + // Check that no account has a value above 4 in the 2nd block as no storage nor + // code should make it to the witness. + for _, stemStateDiff := range statediff[1] { for _, suffixDiff := range stemStateDiff.SuffixDiffs { if suffixDiff.Suffix > 4 { - t.Fatalf("invalid suffix diff found: %d\n", suffixDiff.Suffix) + t.Fatalf("invalid suffix diff found for %x in block #2: %d\n", stemStateDiff.Stem, suffixDiff.Suffix) } } } diff --git a/trie/utils/verkle.go b/trie/utils/verkle.go index bf9872e0f347..a6917174b39b 100644 --- a/trie/utils/verkle.go +++ b/trie/utils/verkle.go @@ -186,28 +186,6 @@ func GetTreeKeyCodeChunkWithEvaluatedAddress(addressPoint *verkle.Point, chunk * return GetTreeKeyWithEvaluatedAddess(addressPoint, treeIndex, subIndex) } -func GetTreeKeyStorageSlot(address []byte, storageKey *uint256.Int) []byte { - pos := storageKey.Clone() - if storageKey.Cmp(codeStorageDelta) < 0 { - pos.Add(HeaderStorageOffset, storageKey) - } else { - pos.Add(MainStorageOffset, storageKey) - } - treeIndex := new(uint256.Int).Div(pos, VerkleNodeWidth) - - // calculate the sub_index, i.e. the index in the stem tree. - // Because the modulus is 256, it's the last byte of treeIndex - subIndexMod := new(uint256.Int).Mod(pos, VerkleNodeWidth) - var subIndex byte - if len(subIndexMod) != 0 { - // uint256 is broken into 4 little-endian quads, - // each with native endianness. Extract the least - // significant byte. - subIndex = byte(subIndexMod[0]) - } - return GetTreeKey(address, treeIndex, subIndex) -} - func PointToHash(evaluated *verkle.Point, suffix byte) []byte { // The output of Byte() is big engian for banderwagon. This // introduces an imbalance in the tree, because hashes are @@ -289,12 +267,17 @@ func GetTreeKeyStorageSlotTreeIndexes(storageKey []byte) (*uint256.Int, byte) { } // If the storage slot is in the main storage, we need to add the main storage offset. + // Get the new offset since we now know that we are above 64. + pos.Sub(&pos, codeStorageDelta) + suffix := byte(pos[0] & 0xFF) + // We first divide by VerkleNodeWidth to create room to avoid an overflow next. pos.Rsh(&pos, uint(VerkleNodeWidthLog2)) + // We add mainStorageOffset/VerkleNodeWidth which can't overflow. pos.Add(&pos, mainStorageOffsetLshVerkleNodeWidth) // The sub-index is the LSB of the original storage key, since mainStorageOffset // doesn't affect this byte, so we can avoid masks or shifts. - return &pos, storageKey[len(storageKey)-1] + return &pos, suffix } From a874d6e20d4e2d73db494c861964c598a7719002 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Thu, 18 Jan 2024 13:42:33 +0100 Subject: [PATCH 4/6] comment "fix" out --- trie/utils/verkle.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/trie/utils/verkle.go b/trie/utils/verkle.go index a6917174b39b..6462deba18bb 100644 --- a/trie/utils/verkle.go +++ b/trie/utils/verkle.go @@ -267,9 +267,15 @@ func GetTreeKeyStorageSlotTreeIndexes(storageKey []byte) (*uint256.Int, byte) { } // If the storage slot is in the main storage, we need to add the main storage offset. - // Get the new offset since we now know that we are above 64. - pos.Sub(&pos, codeStorageDelta) - suffix := byte(pos[0] & 0xFF) + // The first MAIN_STORAGE_OFFSET group will see its + // first 64 slots unreachable. This is either a typo in the + // spec or intended to conserve the 256-u256 + // aligment. If we decide to ever access these 64 + // slots, uncomment this. + // // Get the new offset since we now know that we are above 64. + // pos.Sub(&pos, codeStorageDelta) + // suffix := byte(pos[0] & 0xFF) + suffix := storageKey[len(storageKey)-1] // We first divide by VerkleNodeWidth to create room to avoid an overflow next. pos.Rsh(&pos, uint(VerkleNodeWidthLog2)) From 3833b4df8ad311904991ee76438117f88ac12d35 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Thu, 18 Jan 2024 13:42:47 +0100 Subject: [PATCH 5/6] update tests. --- core/state_processor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index cceba475f5c3..52ad5a8869d7 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -641,7 +641,7 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { // is present. if bytes.Equal(stemStateDiff.Stem[:], common.Hex2Bytes("a10042195481d30478251625e1ccef0e2174dc4e083e81d2566d880373f791")) { for _, suffixDiff := range stemStateDiff.SuffixDiffs { - if suffixDiff.Suffix != 69 { + if suffixDiff.Suffix != 133 { t.Fatalf("invalid suffix diff found for %x in block #1: %d\n", stemStateDiff.Stem, suffixDiff.Suffix) } } From 08e095e02a123c325768a7f8f487b9082ffdfeb4 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Fri, 19 Jan 2024 09:42:36 +0100 Subject: [PATCH 6/6] Add comments describing transactions, and fix linter issue --- core/state_processor_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 52ad5a8869d7..02f58dc825d9 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -591,8 +591,6 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { // Verkle trees use the snapshot, which must be enabled before the // data is saved into the tree+database. genesis := gspec.MustCommit(bcdb) - blockchain, _ := NewBlockChain(bcdb, nil, gspec, nil, beacon.New(ethash.NewFaker()), vm.Config{}, nil, nil) - defer blockchain.Stop() // Commit the genesis block to the block-generation database as it // is now independent of the blockchain database. @@ -606,12 +604,14 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { if i == 0 { var tx1, tx2, tx3 types.Transaction + // SSTORE at slot 105 and reverts tx1payload := common.Hex2Bytes("f8d48084479c2c18830186a08080b8806000602955bda3f9600060ca55600060695523b360006039551983576000601255b0620c2fde2c592ac2600060bc55e0ac6000606455a63e22600060e655eb607e605c5360a2605d5360c7605e53601d605f5360eb606053606b606153608e60625360816063536079606453601e60655360fc60665360b7606753608b60685383021e7ca0cc20c65a97d2e526b8ec0f4266e8b01bdcde43b9aeb59d8bfb44e8eb8119c109a07a8e751813ae1b2ce734960dbc39a4f954917d7822a2c5d1dca18b06c584131f") if err := tx1.UnmarshalBinary(tx1payload); err != nil { t.Fatal(err) } gen.AddTx(&tx1) + // SSTORE at slot 133 and reverts tx2payload := common.Hex2Bytes("02f8db83010f2c01843b9aca0084479c2c18830186a08080b88060006085553fad6000600a55600060565555600060b55506600060cf557f1b8b38183e7bd1bdfaa7123c5a4976e54cce0e42049d841411978fd3595e25c66019527f0538943712953cf08900aae40222a40b2d5a4ac8075ad8cf0870e2be307edbb96039527f9f3174ff85024747041ae7a611acffb987c513c088d90ab288aec080a0cd6ac65ce2cb0a912371f6b5a551ba8caffc22ec55ad4d3cb53de41d05eb77b6a02e0dfe8513dfa6ec7bfd7eda6f5c0dac21b39b982436045e128cec46cfd3f960") if err := tx2.UnmarshalBinary(tx2payload); err != nil { t.Fatal(err) @@ -626,6 +626,7 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { gen.AddTx(&tx3) } else { var tx types.Transaction + // immediately reverts txpayload := common.Hex2Bytes("01f8d683010f2c028443ad7d0e830186a08080b880b00e7fa3c849dce891cce5fae8a4c46cbb313d6aec0c0ffe7863e05fb7b22d4807674c6055527ffbfcb0938f3e18f7937aa8fa95d880afebd5c4cec0d85186095832d03c85cf8a60755260ab60955360cf6096536066609753606e60985360fa609953609e609a53608e609b536024609c5360f6609d536072609e5360a4609fc080a08fc6f7101f292ff1fb0de8ac69c2d320fbb23bfe61cf327173786ea5daee6e37a044c42d91838ef06646294bf4f9835588aee66243b16a66a2da37641fae4c045f") if err := tx.UnmarshalBinary(txpayload); err != nil { t.Fatal(err) @@ -652,7 +653,6 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) { } } } else { - for _, suffixDiff := range stemStateDiff.SuffixDiffs { if suffixDiff.Suffix > 4 { t.Fatalf("invalid suffix diff found for %x in block #1: %d\n", stemStateDiff.Stem, suffixDiff.Suffix)