From 849dd2c504c7e454bc6b333251eb2a79ab1006f2 Mon Sep 17 00:00:00 2001 From: NganSM Date: Tue, 9 Jan 2024 17:50:36 +0700 Subject: [PATCH 1/4] blockchain: fix inconsistent reorg behavior If the chain is reorganized because the newly submit block has higher justified block number, it can be reverted back to the old chain prior to reorg because the old blocks cause ErrKnownBlock during insertion via the insertChain method, which invokes writeKnownBlock to set the old chain as the canonical chain again. --- core/blockchain.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 5335158c59..b51cef2489 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1665,7 +1665,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er ) for block != nil && bc.skipBlock(err, it) { externTd = new(big.Int).Add(externTd, block.Difficulty()) - if localTd.Cmp(externTd) < 0 { + if bc.reorgNeeded(current, localTd, block, externTd) { break } log.Debug("Ignoring already known block", "number", block.Number(), "hash", block.Hash()) @@ -1734,7 +1734,19 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er } }() - for ; block != nil && err == nil || errors.Is(err, ErrKnownBlock); block, err = it.next() { + var ( + current = bc.CurrentBlock() + localTd = bc.GetTd(current.Hash(), current.NumberU64()) + externTd = common.Big0 + ) + + if block != nil { + externTd = bc.GetTd(block.ParentHash(), block.NumberU64()-1) + } + + for ; (block != nil && err == nil) || errors.Is(err, ErrKnownBlock); block, err = it.next() { + // err == ErrknownBlock means block != nil + externTd = new(big.Int).Add(externTd, block.Difficulty()) // If the chain is terminating, stop processing blocks if bc.insertStopped() { log.Debug("Abort during block processing") @@ -1751,7 +1763,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er // just skip the block (we already validated it once fully (and crashed), since // its header and body was already in the database). But if the corresponding // snapshot layer is missing, forcibly rerun the execution to build it. - if bc.skipBlock(err, it) { + if bc.skipBlock(err, it) && bc.reorgNeeded(current, localTd, block, externTd) { logger := log.Debug if bc.chainConfig.Clique == nil { logger = log.Warn From 0a839478af1fabc7d1fd76c5a6c3567a81e21fe3 Mon Sep 17 00:00:00 2001 From: Bui Quang Minh Date: Wed, 10 Jan 2024 13:36:20 +0700 Subject: [PATCH 2/4] consortium-v2: make the engine fully implement FastFinalityPoSA interface When testing, we may need to create the standalone consortium-v2 engine so we need to make it fully implement FastFinalityPoSA interface. --- consensus/consortium/v2/consortium.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/consensus/consortium/v2/consortium.go b/consensus/consortium/v2/consortium.go index 8f7df5a0e5..9106d81709 100644 --- a/consensus/consortium/v2/consortium.go +++ b/consensus/consortium/v2/consortium.go @@ -170,6 +170,16 @@ func (c *Consortium) IsSystemMessage(msg core.Message, header *types.Header) boo return false } +// In normal case, IsSystemTransaction in consortium/main.go is used instead of this function. This function +// is only used in testing when we create standalone consortium v2 engine without the v1 +func (c *Consortium) IsSystemTransaction(tx *types.Transaction, header *types.Header) (bool, error) { + msg, err := tx.AsMessage(types.MakeSigner(c.chainConfig, header.Number), header.BaseFee) + if err != nil { + return false, err + } + return c.IsSystemMessage(msg, header), nil +} + // IsSystemContract implements consensus.PoSA, checking whether a contract is a system // contract or not // A system contract is a contract is defined in params.ConsortiumV2Contracts @@ -191,11 +201,23 @@ func (c *Consortium) VerifyHeader(chain consensus.ChainHeaderReader, header *typ } // VerifyHeaders implements consensus.Engine, always returning an empty abort and results channels. -// This method will be handled consortium/main.go instead +// In normal case, VerifyHeaders in consortium/main.go is used instead of this function. This function +// is only used in testing when we create standalone consortium v2 engine without the v1 func (c *Consortium) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) { abort := make(chan struct{}) results := make(chan error, len(headers)) + go func() { + for i, header := range headers { + err := c.VerifyHeaderAndParents(chain, header, headers[:i]) + select { + case <-abort: + return + case results <- err: + } + } + }() + return abort, results } From c6d3818361df49acc12ed1ed023935ef03aaac05 Mon Sep 17 00:00:00 2001 From: Bui Quang Minh Date: Wed, 10 Jan 2024 13:17:44 +0700 Subject: [PATCH 3/4] chain_makers: add GenerateConsortiumChain to genernate Consortium blocks When generating Consortium blocks, we need to modify the block after FinalizeAndAssemble to change the block signature in header's extra data. New function GenerateConsortiumChain adds a new parameter so the caller can pass a function to modify block after FinalizeAndAssemble. Besides, the fake chain reader also needs to return correct block header to be used by consensus engine in FinalizeAndAssemble. --- core/chain_makers.go | 82 +++++++++++++++++++++++++++++++----- core/state_processor_test.go | 2 +- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/core/chain_makers.go b/core/chain_makers.go index 1eb008d9f9..8341296ec2 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -185,6 +185,23 @@ func (b *BlockGen) OffsetTime(seconds int64) { b.header.Difficulty = b.engine.CalcDifficulty(chainreader, b.header.Time, b.parent.Header()) } +func (b *BlockGen) Header() *types.Header { + return types.CopyHeader(b.header) +} + +func GenerateConsortiumChain( + config *params.ChainConfig, + parent *types.Block, + engine consensus.Engine, + db ethdb.Database, + n int, + gen func(int, *BlockGen), + flushDisk bool, + postFinalize func(int, *BlockGen), +) ([]*types.Block, []types.Receipts) { + return generateChain(config, parent, engine, db, n, gen, flushDisk, postFinalize) +} + // GenerateChain creates a chain of n blocks. The first block's // parent will be the provided parent. db is used to store // intermediate states and should contain the parent's state trie. @@ -205,12 +222,25 @@ func GenerateChain( n int, gen func(int, *BlockGen), flushDisk bool, +) ([]*types.Block, []types.Receipts) { + return generateChain(config, parent, engine, db, n, gen, flushDisk, nil) +} + +func generateChain( + config *params.ChainConfig, + parent *types.Block, + engine consensus.Engine, + db ethdb.Database, + n int, + gen func(int, *BlockGen), + flushDisk bool, + postFinalize func(int, *BlockGen), ) ([]*types.Block, []types.Receipts) { if config == nil { config = params.TestChainConfig } blocks, receipts := make(types.Blocks, n), make([]types.Receipts, n) - chainreader := &fakeChainReader{config: config} + chainreader := newFakeChainReader(config, db) genblock := func(i int, parent *types.Block, statedb *state.StateDB) (*types.Block, types.Receipts) { b := &BlockGen{i: i, chain: blocks, parent: parent, statedb: statedb, config: config, engine: engine} b.header = makeHeader(chainreader, parent, statedb, b.engine) @@ -256,6 +286,14 @@ func GenerateChain( panic(err) } + // Execute any user modifications to the block + if config.Consortium != nil && postFinalize != nil { + b := &BlockGen{i: i, chain: blocks, parent: parent, statedb: statedb, config: config, engine: engine} + b.header = block.Header() + postFinalize(i, b) + block = types.NewBlockWithHeader(b.header) + } + // Write state changes to db root, err := statedb.Commit(config.IsEIP158(b.header.Number)) if err != nil { @@ -281,6 +319,7 @@ func GenerateChain( blocks[i] = block receipts[i] = receipt parent = block + chainreader.addHeader(block.Hash(), block.Header()) } return blocks, receipts } @@ -348,7 +387,21 @@ func makeBlockChain(parent *types.Block, n int, engine consensus.Engine, db ethd } type fakeChainReader struct { - config *params.ChainConfig + config *params.ChainConfig + db ethdb.Database + headers map[common.Hash]*types.Header +} + +func newFakeChainReader(config *params.ChainConfig, db ethdb.Database) *fakeChainReader { + return &fakeChainReader{ + config: config, + db: db, + headers: make(map[common.Hash]*types.Header), + } +} + +func (cr *fakeChainReader) addHeader(hash common.Hash, header *types.Header) { + cr.headers[hash] = header } // Config returns the chain configuration. @@ -356,11 +409,20 @@ func (cr *fakeChainReader) Config() *params.ChainConfig { return cr.config } -func (cr *fakeChainReader) CurrentHeader() *types.Header { return nil } -func (cr *fakeChainReader) GetHeaderByNumber(number uint64) *types.Header { return nil } -func (cr *fakeChainReader) GetHeaderByHash(hash common.Hash) *types.Header { return nil } -func (cr *fakeChainReader) GetHeader(hash common.Hash, number uint64) *types.Header { return nil } -func (cr *fakeChainReader) GetBlock(hash common.Hash, number uint64) *types.Block { return nil } -func (cr *fakeChainReader) DB() ethdb.Database { return nil } -func (cr *fakeChainReader) StateCache() state.Database { return nil } -func (cr *fakeChainReader) OpEvents() []*vm.PublishEvent { return nil } +func (cr *fakeChainReader) CurrentHeader() *types.Header { return nil } +func (cr *fakeChainReader) GetHeaderByNumber(number uint64) *types.Header { return nil } +func (cr *fakeChainReader) GetHeaderByHash(hash common.Hash) *types.Header { return nil } +func (cr *fakeChainReader) GetHeader(hash common.Hash, number uint64) *types.Header { + if cr.db != nil { + if header, ok := cr.headers[hash]; ok { + return header + } + + return rawdb.ReadHeader(cr.db, hash, number) + } + return nil +} +func (cr *fakeChainReader) GetBlock(hash common.Hash, number uint64) *types.Block { return nil } +func (cr *fakeChainReader) DB() ethdb.Database { return nil } +func (cr *fakeChainReader) StateCache() state.Database { return nil } +func (cr *fakeChainReader) OpEvents() []*vm.PublishEvent { return nil } diff --git a/core/state_processor_test.go b/core/state_processor_test.go index aa8e4bebf9..f99d4231f9 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -307,7 +307,7 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr header := &types.Header{ ParentHash: parent.Hash(), Coinbase: parent.Coinbase(), - Difficulty: engine.CalcDifficulty(&fakeChainReader{config}, parent.Time()+10, &types.Header{ + Difficulty: engine.CalcDifficulty(&fakeChainReader{config: config}, parent.Time()+10, &types.Header{ Number: parent.Number(), Time: parent.Time(), Difficulty: parent.Difficulty(), From cd2adc83b609cb65d02f7779b36e90978efcf484 Mon Sep 17 00:00:00 2001 From: Bui Quang Minh Date: Wed, 17 Jan 2024 11:46:35 +0700 Subject: [PATCH 4/4] consortium-v2: add unit test for known block reorg case --- consensus/consortium/v2/consortium.go | 2 +- consensus/consortium/v2/consortium_test.go | 265 +++++++++++++++++++++ 2 files changed, 266 insertions(+), 1 deletion(-) diff --git a/consensus/consortium/v2/consortium.go b/consensus/consortium/v2/consortium.go index 9106d81709..d898cc6ffe 100644 --- a/consensus/consortium/v2/consortium.go +++ b/consensus/consortium/v2/consortium.go @@ -764,7 +764,7 @@ func (c *Consortium) processSystemTransactions(chain consensus.ChainHeaderReader // If the parent's block includes the finality votes, distribute reward for the voters if c.chainConfig.IsShillin(new(big.Int).Sub(header.Number, common.Big1)) { - parentHeader := chain.GetHeaderByHash(header.ParentHash) + parentHeader := chain.GetHeader(header.ParentHash, header.Number.Uint64()-1) extraData, err := finality.DecodeExtra(parentHeader.Extra, true) if err != nil { return err diff --git a/consensus/consortium/v2/consortium_test.go b/consensus/consortium/v2/consortium_test.go index 653eca4854..f2a6612d14 100644 --- a/consensus/consortium/v2/consortium_test.go +++ b/consensus/consortium/v2/consortium_test.go @@ -2,6 +2,7 @@ package v2 import ( "bytes" + "crypto/ecdsa" "encoding/binary" "errors" "math/big" @@ -17,6 +18,7 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/bls/blst" blsCommon "github.com/ethereum/go-ethereum/crypto/bls/common" "github.com/ethereum/go-ethereum/params" @@ -972,3 +974,266 @@ func TestVerifyVote(t *testing.T) { t.Errorf("Expect sucessful verification have %s", err) } } + +func TestKnownBlockReorg(t *testing.T) { + db := rawdb.NewMemoryDatabase() + + blsKeys := make([]blsCommon.SecretKey, 3) + ecdsaKeys := make([]*ecdsa.PrivateKey, 3) + validatorAddrs := make([]common.Address, 3) + + for i := range blsKeys { + blsKey, err := blst.RandKey() + if err != nil { + t.Fatal(err) + } + blsKeys[i] = blsKey + + secretKey, err := crypto.GenerateKey() + if err != nil { + t.Fatal(err) + } + ecdsaKeys[i] = secretKey + validatorAddrs[i] = crypto.PubkeyToAddress(secretKey.PublicKey) + } + + for i := 0; i < len(blsKeys)-1; i++ { + for j := i; j < len(blsKeys); j++ { + if bytes.Compare(validatorAddrs[i][:], validatorAddrs[j][:]) > 0 { + validatorAddrs[i], validatorAddrs[j] = validatorAddrs[j], validatorAddrs[i] + blsKeys[i], blsKeys[j] = blsKeys[j], blsKeys[i] + ecdsaKeys[i], ecdsaKeys[j] = ecdsaKeys[j], ecdsaKeys[i] + } + } + } + + chainConfig := params.ChainConfig{ + ChainID: big.NewInt(2021), + HomesteadBlock: common.Big0, + EIP150Block: common.Big0, + EIP155Block: common.Big0, + EIP158Block: common.Big0, + ConsortiumV2Block: common.Big0, + ShillinBlock: big.NewInt(10), + Consortium: ¶ms.ConsortiumConfig{ + EpochV2: 10, + }, + } + + genesis := (&core.Genesis{ + Config: &chainConfig, + }).MustCommit(db) + + mock := &mockContract{ + validators: make(map[common.Address]blsCommon.PublicKey), + } + mock.validators[validatorAddrs[0]] = blsKeys[0].PublicKey() + recents, _ := lru.NewARC(inmemorySnapshots) + signatures, _ := lru.NewARC(inmemorySignatures) + + v2 := Consortium{ + chainConfig: &chainConfig, + contract: mock, + recents: recents, + signatures: signatures, + config: chainConfig.Consortium, + db: db, + } + + chain, _ := core.NewBlockChain(db, nil, &chainConfig, &v2, vm.Config{}, nil, nil) + extraData := [consortiumCommon.ExtraVanity + consortiumCommon.ExtraSeal]byte{} + + blocks, _ := core.GenerateConsortiumChain( + &chainConfig, + genesis, + &v2, + db, + 9, + func(i int, bg *core.BlockGen) { + bg.SetCoinbase(validatorAddrs[0]) + bg.SetExtra(extraData[:]) + bg.SetDifficulty(big.NewInt(7)) + }, + true, + func(i int, bg *core.BlockGen) { + header := bg.Header() + hash := calculateSealHash(header, big.NewInt(2021)) + sig, err := crypto.Sign(hash[:], ecdsaKeys[0]) + if err != nil { + t.Fatalf("Failed to sign block, err %s", err) + } + copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig) + bg.SetExtra(header.Extra) + }, + ) + + _, err := chain.InsertChain(blocks) + if err != nil { + t.Fatalf("Failed to insert block, err %s", err) + } + + for i := range validatorAddrs { + mock.validators[validatorAddrs[i]] = blsKeys[i].PublicKey() + } + + var checkpointValidators []finality.ValidatorWithBlsPub + for i := range validatorAddrs { + checkpointValidators = append(checkpointValidators, finality.ValidatorWithBlsPub{ + Address: validatorAddrs[i], + BlsPublicKey: blsKeys[i].PublicKey(), + }) + } + + // Prepare checkpoint block + blocks, _ = core.GenerateConsortiumChain( + &chainConfig, + blocks[len(blocks)-1], + &v2, + db, + 1, + func(i int, bg *core.BlockGen) { + var extra finality.HeaderExtraData + + bg.SetCoinbase(validatorAddrs[0]) + bg.SetDifficulty(big.NewInt(7)) + extra.CheckpointValidators = checkpointValidators + bg.SetExtra(extra.Encode(true)) + }, + true, + func(i int, bg *core.BlockGen) { + header := bg.Header() + hash := calculateSealHash(header, big.NewInt(2021)) + sig, err := crypto.Sign(hash[:], ecdsaKeys[0]) + if err != nil { + t.Fatalf("Failed to sign block, err %s", err) + } + copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig) + bg.SetExtra(header.Extra) + }, + ) + + _, err = chain.InsertChain(blocks) + if err != nil { + t.Fatalf("Failed to insert block, err %s", err) + } + + extraDataShillin := [consortiumCommon.ExtraVanity + 1 + consortiumCommon.ExtraSeal]byte{} + knownBlocks, _ := core.GenerateConsortiumChain( + &chainConfig, + blocks[len(blocks)-1], + &v2, + db, + 1, + func(i int, bg *core.BlockGen) { + bg.SetCoinbase(validatorAddrs[2]) + bg.SetExtra(extraDataShillin[:]) + bg.SetDifficulty(big.NewInt(7)) + }, + true, + func(i int, bg *core.BlockGen) { + header := bg.Header() + hash := calculateSealHash(header, big.NewInt(2021)) + sig, err := crypto.Sign(hash[:], ecdsaKeys[2]) + if err != nil { + t.Fatalf("Failed to sign block, err %s", err) + } + copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig) + bg.SetExtra(header.Extra) + }, + ) + + _, err = chain.InsertChain(knownBlocks) + if err != nil { + t.Fatalf("Failed to insert block, err %s", err) + } + + header := chain.CurrentHeader() + if header.Number.Uint64() != 11 { + t.Fatalf("Expect head header to be %d, got %d", 11, header.Number.Uint64()) + } + if header.Difficulty.Cmp(big.NewInt(7)) != 0 { + t.Fatalf("Expect header header to have difficulty %d, got %d", 7, header.Difficulty.Uint64()) + } + + justifiedBlocks, _ := core.GenerateConsortiumChain( + &chainConfig, + blocks[len(blocks)-1], + &v2, + db, + 2, + func(i int, bg *core.BlockGen) { + if bg.Number().Uint64() == 11 { + bg.SetCoinbase(validatorAddrs[1]) + bg.SetExtra(extraDataShillin[:]) + } else { + bg.SetCoinbase(validatorAddrs[2]) + + var ( + extra finality.HeaderExtraData + voteBitset finality.FinalityVoteBitSet + signatures []blsCommon.Signature + ) + voteBitset.SetBit(0) + voteBitset.SetBit(1) + voteBitset.SetBit(2) + extra.HasFinalityVote = 1 + extra.FinalityVotedValidators = voteBitset + + block := bg.PrevBlock(-1) + voteData := types.VoteData{ + TargetNumber: block.NumberU64(), + TargetHash: block.Hash(), + } + for i := range blsKeys { + signatures = append(signatures, blsKeys[i].Sign(voteData.Hash().Bytes())) + } + + extra.AggregatedFinalityVotes = blst.AggregateSignatures(signatures) + bg.SetExtra(extra.Encode(true)) + } + + bg.SetDifficulty(big.NewInt(3)) + }, + true, + func(i int, bg *core.BlockGen) { + header := bg.Header() + hash := calculateSealHash(header, big.NewInt(2021)) + + var ecdsaKey *ecdsa.PrivateKey + if bg.Number().Uint64() == 11 { + ecdsaKey = ecdsaKeys[1] + } else { + ecdsaKey = ecdsaKeys[2] + } + sig, err := crypto.Sign(hash[:], ecdsaKey) + if err != nil { + t.Fatalf("Failed to sign block, err %s", err) + } + copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig) + bg.SetExtra(header.Extra) + }, + ) + + _, err = chain.InsertChain(justifiedBlocks) + if err != nil { + t.Fatalf("Failed to insert block, err %s", err) + } + + header = chain.CurrentHeader() + if header.Number.Uint64() != 12 { + t.Fatalf("Expect head header to be %d, got %d", 12, header.Number.Uint64()) + } + + _, err = chain.InsertChain(knownBlocks) + if err != nil { + t.Fatalf("Failed to insert block, err %s", err) + } + header = chain.CurrentHeader() + if header.Number.Uint64() != 12 { + t.Fatalf("Expect head header to be %d, got %d", 12, header.Number.Uint64()) + } + header = chain.GetHeaderByNumber(11) + if header.Difficulty.Uint64() != 3 { + t.Fatalf("Expect head header to have difficulty %d, got %d", 3, header.Difficulty.Uint64()) + } +}