From ad742cbfa4d7de963bec3775b3125c5609bef464 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 20 Feb 2023 14:38:09 -0500 Subject: [PATCH 1/7] add start of withdrawal gen repro --- core/chain_makers_test.go | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index 166ac3f227fc..b4ae3c52a9d4 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -20,6 +20,7 @@ import ( "fmt" "math/big" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" @@ -28,6 +29,83 @@ import ( "github.com/ethereum/go-ethereum/params" ) +func TestGenerateWithdrawalChain() { + var ( + keyHex = "9c647b8b7c4e7c3490668fb6c11473619db80c93704c70893d3813af4090c39c" + key, _ = crypto.HexToECDSA(keyHex) + address = crypto.PubkeyToAddress(key.PublicKey) // 658bdf435d810c91414ec09147daa6db62406379 + aa = common.Address{0xaa} + bb = common.Address{0xbb} + funds = big.NewInt(0).Mul(big.NewInt(1337), big.NewInt(params.Ether)) + gspec = &Genesis{ + Config: params.AllEthashProtocolChanges, + Alloc: GenesisAlloc{address: {Balance: funds}}, + BaseFee: big.NewInt(params.InitialBaseFee), + Difficulty: common.Big1, + GasLimit: 5_000_000, + } + gendb = rawdb.NewMemoryDatabase() + signer = types.LatestSigner(gspec.Config) + db = rawdb.NewMemoryDatabase() + ) + gspec.Config.TerminalTotalDifficultyPassed = true + gspec.Config.TerminalTotalDifficulty = common.Big0 + gspec.Config.ShanghaiTime = u64(0) + + // init 0xaa with some storage elements + storage := make(map[common.Hash]common.Hash) + storage[common.Hash{0x00}] = common.Hash{0x00} + storage[common.Hash{0x01}] = common.Hash{0x01} + storage[common.Hash{0x02}] = common.Hash{0x02} + storage[common.Hash{0x03}] = common.HexToHash("0303") + gspec.Alloc[aa] = GenesisAccount{ + Balance: common.Big1, + Nonce: 1, + Storage: storage, + Code: common.Hex2Bytes("6042"), + } + gspec.Alloc[bb] = GenesisAccount{ + Balance: common.Big2, + Nonce: 1, + Storage: storage, + Code: common.Hex2Bytes("600154600354"), + } + + genesis := gspec.MustCommit(gendb) + + // sealingEngine := sealingEngine{engine} + chain, _ := GenerateChain(gspec.Config, genesis, ethash.NewFaker(), gendb, 4, func(i int, gen *BlockGen) { + tx, _ := types.SignTx(types.NewTransaction(gen.TxNonce(address), address, big.NewInt(1000), params.TxGas, new(big.Int).Add(gen.BaseFee(), common.Big1), nil), signer, key) + gen.AddTx(tx) + if i == 1 { + // first index for the withdrawal block + idxOne := uint64(123) + + gen.AddWithdrawal(&types.Withdrawal{ + Index: idxOne, + Validator: 42, + Address: common.Address{0xee}, + Amount: 1337, + }) + gen.AddWithdrawal(&types.Withdrawal{ + Index: idxOne + 1, + Validator: 13, + Address: common.Address{0xee}, + Amount: 1, + }) + } + }) + + // Import the chain. This runs all block validation rules. + blockchain, _ := NewBlockChain(db, nil, gspec, nil, ethash.NewFaker(), vm.Config{}, nil, nil) + defer blockchain.Stop() + + if i, err := blockchain.InsertChain(chain); err != nil { + fmt.Printf("insert error (block %d): %v\n", chain[i].NumberU64(), err) + return + } +} + func ExampleGenerateChain() { var ( key1, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") From 398136577a0096f90e5d3daa2060165df0b003ec Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 20 Feb 2023 16:58:32 -0500 Subject: [PATCH 2/7] core: add chain maker withdrawal test --- core/chain_makers_test.go | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index b4ae3c52a9d4..f573e59a490e 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -19,8 +19,10 @@ package core import ( "fmt" "math/big" + "testing" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/consensus/beacon" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" @@ -29,7 +31,7 @@ import ( "github.com/ethereum/go-ethereum/params" ) -func TestGenerateWithdrawalChain() { +func TestGenerateWithdrawalChain(t *testing.T) { var ( keyHex = "9c647b8b7c4e7c3490668fb6c11473619db80c93704c70893d3813af4090c39c" key, _ = crypto.HexToECDSA(keyHex) @@ -74,12 +76,12 @@ func TestGenerateWithdrawalChain() { genesis := gspec.MustCommit(gendb) // sealingEngine := sealingEngine{engine} - chain, _ := GenerateChain(gspec.Config, genesis, ethash.NewFaker(), gendb, 4, func(i int, gen *BlockGen) { + chain, _ := GenerateChain(gspec.Config, genesis, beacon.NewFaker(), gendb, 4, func(i int, gen *BlockGen) { tx, _ := types.SignTx(types.NewTransaction(gen.TxNonce(address), address, big.NewInt(1000), params.TxGas, new(big.Int).Add(gen.BaseFee(), common.Big1), nil), signer, key) gen.AddTx(tx) if i == 1 { // first index for the withdrawal block - idxOne := uint64(123) + idxOne := uint64(0) gen.AddWithdrawal(&types.Withdrawal{ Index: idxOne, @@ -97,13 +99,32 @@ func TestGenerateWithdrawalChain() { }) // Import the chain. This runs all block validation rules. - blockchain, _ := NewBlockChain(db, nil, gspec, nil, ethash.NewFaker(), vm.Config{}, nil, nil) + blockchain, _ := NewBlockChain(db, nil, gspec, nil, beacon.NewFaker(), vm.Config{}, nil, nil) defer blockchain.Stop() if i, err := blockchain.InsertChain(chain); err != nil { fmt.Printf("insert error (block %d): %v\n", chain[i].NumberU64(), err) return } + + // enforce that withdrawal indexes are monotonically increasing from 0 + head := blockchain.CurrentBlock().NumberU64() + for i := 0; i < int(head); i++ { + block := blockchain.GetBlockByNumber(uint64(i)) + if block == nil { + t.Fatalf("block %d not found", i) + } + + if block.Withdrawals() == nil { + continue + } + + for j := 0; j < len(block.Withdrawals()); j++ { + if block.Withdrawals()[j].Index != uint64(j) { + t.Fatalf("withdrawal index %d does not equal expected index %d", block.Withdrawals()[j].Index, j) + } + } + } } func ExampleGenerateChain() { From 1943b610bb48be243e430884e8cd6ad6059d6994 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Tue, 21 Feb 2023 20:56:14 -0500 Subject: [PATCH 3/7] make same-block withdrawals increasing --- core/chain_makers.go | 10 ++++++++++ core/chain_makers_test.go | 5 ----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/core/chain_makers.go b/core/chain_makers.go index 3518929f8e71..35e82052d6bd 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -210,6 +210,16 @@ func (b *BlockGen) AddUncle(h *types.Header) { func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) { // The withdrawal will be assigned the next valid index. var idx uint64 + + // set the initial index to the last withdrawal index in the block + 1, if + // it exists. otherwise, determine the proper index from parent blocks + if len(b.withdrawals) != 0 { + idx = b.withdrawals[len(b.withdrawals)-1].Index + 1 + w.Index = idx + b.withdrawals = append(b.withdrawals, w) + return + } + for i := b.i - 1; i >= 0; i-- { if wd := b.chain[i].Withdrawals(); len(wd) != 0 { idx = wd[len(wd)-1].Index + 1 diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index f573e59a490e..fa9b7de6e5a4 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -80,17 +80,12 @@ func TestGenerateWithdrawalChain(t *testing.T) { tx, _ := types.SignTx(types.NewTransaction(gen.TxNonce(address), address, big.NewInt(1000), params.TxGas, new(big.Int).Add(gen.BaseFee(), common.Big1), nil), signer, key) gen.AddTx(tx) if i == 1 { - // first index for the withdrawal block - idxOne := uint64(0) - gen.AddWithdrawal(&types.Withdrawal{ - Index: idxOne, Validator: 42, Address: common.Address{0xee}, Amount: 1337, }) gen.AddWithdrawal(&types.Withdrawal{ - Index: idxOne + 1, Validator: 13, Address: common.Address{0xee}, Amount: 1, From 2ea06ba7c9422dfb1b81b2fd06dfdcc6d728eab5 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 23 Feb 2023 10:41:16 +0800 Subject: [PATCH 4/7] core: improve unit test --- core/chain_makers.go | 3 +-- core/chain_makers_test.go | 27 ++++++++++++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/core/chain_makers.go b/core/chain_makers.go index 35e82052d6bd..4921851c6efe 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -211,7 +211,7 @@ func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) { // The withdrawal will be assigned the next valid index. var idx uint64 - // set the initial index to the last withdrawal index in the block + 1, if + // Set the initial index to the last withdrawal index in the block + 1, if // it exists. otherwise, determine the proper index from parent blocks if len(b.withdrawals) != 0 { idx = b.withdrawals[len(b.withdrawals)-1].Index + 1 @@ -219,7 +219,6 @@ func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) { b.withdrawals = append(b.withdrawals, w) return } - for i := b.i - 1; i >= 0; i-- { if wd := b.chain[i].Withdrawals(); len(wd) != 0 { idx = wd[len(wd)-1].Index + 1 diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index fa9b7de6e5a4..7f69f9676563 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -75,7 +75,6 @@ func TestGenerateWithdrawalChain(t *testing.T) { genesis := gspec.MustCommit(gendb) - // sealingEngine := sealingEngine{engine} chain, _ := GenerateChain(gspec.Config, genesis, beacon.NewFaker(), gendb, 4, func(i int, gen *BlockGen) { tx, _ := types.SignTx(types.NewTransaction(gen.TxNonce(address), address, big.NewInt(1000), params.TxGas, new(big.Int).Add(gen.BaseFee(), common.Big1), nil), signer, key) gen.AddTx(tx) @@ -91,6 +90,18 @@ func TestGenerateWithdrawalChain(t *testing.T) { Amount: 1, }) } + if i == 3 { + gen.AddWithdrawal(&types.Withdrawal{ + Validator: 42, + Address: common.Address{0xee}, + Amount: 1337, + }) + gen.AddWithdrawal(&types.Withdrawal{ + Validator: 13, + Address: common.Address{0xee}, + Amount: 1, + }) + } }) // Import the chain. This runs all block validation rules. @@ -103,21 +114,23 @@ func TestGenerateWithdrawalChain(t *testing.T) { } // enforce that withdrawal indexes are monotonically increasing from 0 - head := blockchain.CurrentBlock().NumberU64() + var ( + withdrawalIndex uint64 + head = blockchain.CurrentBlock().NumberU64() + ) for i := 0; i < int(head); i++ { block := blockchain.GetBlockByNumber(uint64(i)) if block == nil { t.Fatalf("block %d not found", i) } - - if block.Withdrawals() == nil { + if len(block.Withdrawals()) == 0 { continue } - for j := 0; j < len(block.Withdrawals()); j++ { - if block.Withdrawals()[j].Index != uint64(j) { - t.Fatalf("withdrawal index %d does not equal expected index %d", block.Withdrawals()[j].Index, j) + if block.Withdrawals()[j].Index != withdrawalIndex { + t.Fatalf("withdrawal index %d does not equal expected index %d", block.Withdrawals()[j].Index, withdrawalIndex) } + withdrawalIndex += 1 } } } From 39d9834047f2ca20e0f6dd7b6bb34cee97a880c0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 23 Feb 2023 10:49:42 +0100 Subject: [PATCH 5/7] Update chain_makers.go --- core/chain_makers.go | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/core/chain_makers.go b/core/chain_makers.go index 4921851c6efe..2f7ddf18d647 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -208,31 +208,27 @@ func (b *BlockGen) AddUncle(h *types.Header) { // AddWithdrawal adds a withdrawal to the generated block. func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) { - // The withdrawal will be assigned the next valid index. - var idx uint64 + w.Index = b.nextWithdrawalIndex() + b.withdrawals = append(b.withdrawals, w) +} - // Set the initial index to the last withdrawal index in the block + 1, if - // it exists. otherwise, determine the proper index from parent blocks +// nextWithdrawalIndex computes the index of the next withdrawal. +func (b *BlockGen) nextWithdrawalIndex() uint64 { if len(b.withdrawals) != 0 { - idx = b.withdrawals[len(b.withdrawals)-1].Index + 1 - w.Index = idx - b.withdrawals = append(b.withdrawals, w) - return + return b.withdrawals[len(b.withdrawals)-1].Index + 1 } for i := b.i - 1; i >= 0; i-- { if wd := b.chain[i].Withdrawals(); len(wd) != 0 { - idx = wd[len(wd)-1].Index + 1 - break + return wd[len(wd)-1].Index + 1 } if i == 0 { - // Correctly set the index if no parent had withdrawals + // Correctly set the index if no parent had withdrawals. if wd := b.parent.Withdrawals(); len(wd) != 0 { - idx = wd[len(wd)-1].Index + 1 + return wd[len(wd)-1].Index + 1 } } } - w.Index = idx - b.withdrawals = append(b.withdrawals, w) + return 0 } // PrevBlock returns a previously generated block by number. It panics if From bf6c763d7142f988f64569fb71067f07b486d137 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 27 Feb 2023 11:33:03 +0100 Subject: [PATCH 6/7] core: avoid modifying chain config in test --- core/chain_makers_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index 7f69f9676563..7e895a8d2057 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -39,8 +39,9 @@ func TestGenerateWithdrawalChain(t *testing.T) { aa = common.Address{0xaa} bb = common.Address{0xbb} funds = big.NewInt(0).Mul(big.NewInt(1337), big.NewInt(params.Ether)) + config = *params.AllEthashProtocolChanges gspec = &Genesis{ - Config: params.AllEthashProtocolChanges, + Config: &config, Alloc: GenesisAlloc{address: {Balance: funds}}, BaseFee: big.NewInt(params.InitialBaseFee), Difficulty: common.Big1, @@ -50,9 +51,10 @@ func TestGenerateWithdrawalChain(t *testing.T) { signer = types.LatestSigner(gspec.Config) db = rawdb.NewMemoryDatabase() ) - gspec.Config.TerminalTotalDifficultyPassed = true - gspec.Config.TerminalTotalDifficulty = common.Big0 - gspec.Config.ShanghaiTime = u64(0) + + config.TerminalTotalDifficultyPassed = true + config.TerminalTotalDifficulty = common.Big0 + config.ShanghaiTime = u64(0) // init 0xaa with some storage elements storage := make(map[common.Hash]common.Hash) From a3472caf193912efa6fd2605554116267f4c7b9c Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 27 Feb 2023 15:35:49 +0100 Subject: [PATCH 7/7] core: avoid modifying withdrawal --- core/chain_makers.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/chain_makers.go b/core/chain_makers.go index 2f7ddf18d647..052d6efae2f7 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -207,9 +207,12 @@ func (b *BlockGen) AddUncle(h *types.Header) { } // AddWithdrawal adds a withdrawal to the generated block. -func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) { - w.Index = b.nextWithdrawalIndex() - b.withdrawals = append(b.withdrawals, w) +// It returns the withdrawal index. +func (b *BlockGen) AddWithdrawal(w *types.Withdrawal) uint64 { + cpy := *w + cpy.Index = b.nextWithdrawalIndex() + b.withdrawals = append(b.withdrawals, &cpy) + return cpy.Index } // nextWithdrawalIndex computes the index of the next withdrawal.