From 62891378274a58c84b9fa35f3c9b99155644b5eb Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 11 Oct 2021 23:16:46 +0200 Subject: [PATCH] consensus/clique, core: API cleanup (#23100) This removes some code: - The clique engine calculated the snapshot twice when verifying headers/blocks. - The method GetBlockHashesFromHash in Header/Block/Lightchain was only used by tests. It is now removed from the API. - The method GetTdByHash internally looked up the number before calling GetTd(hash, num). In many cases, callers already had the number, and used this method just because it has a shorter name. I have removed the method to make the API surface smaller. --- consensus/clique/clique.go | 10 ++-------- core/blockchain.go | 12 ----------- core/blockchain_test.go | 26 ++++++++++++++---------- core/headerchain.go | 33 ------------------------------- eth/api_backend.go | 5 ++++- eth/protocols/eth/handler_test.go | 8 +++++++- eth/sync.go | 4 ++-- light/lightchain.go | 12 ----------- light/lightchain_test.go | 12 ++++++----- 9 files changed, 38 insertions(+), 84 deletions(-) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index 449095e723..a6a16c84af 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -363,7 +363,7 @@ func (c *Clique) verifyCascadingFields(chain consensus.ChainHeaderReader, header } } // All basic checks passed, verify the seal and return - return c.verifySeal(chain, header, parents) + return c.verifySeal(snap, header, parents) } // snapshot retrieves the authorization snapshot at a given point in time. @@ -460,18 +460,12 @@ func (c *Clique) VerifyUncles(chain consensus.ChainReader, block *types.Block) e // consensus protocol requirements. The method accepts an optional list of parent // headers that aren't yet part of the local blockchain to generate the snapshots // from. -func (c *Clique) verifySeal(chain consensus.ChainHeaderReader, header *types.Header, parents []*types.Header) error { +func (c *Clique) verifySeal(snap *Snapshot, header *types.Header, parents []*types.Header) error { // Verifying the genesis block is not supported number := header.Number.Uint64() if number == 0 { return errUnknownBlock } - // Retrieve the snapshot needed to verify this header and cache it - snap, err := c.snapshot(chain, number-1, header.ParentHash, parents) - if err != nil { - return err - } - // Resolve the authorization key and check against signers signer, err := ecrecover(header, c.signatures) if err != nil { diff --git a/core/blockchain.go b/core/blockchain.go index 559800a068..00f90415c1 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2411,12 +2411,6 @@ func (bc *BlockChain) GetTd(hash common.Hash, number uint64) *big.Int { return bc.hc.GetTd(hash, number) } -// GetTdByHash retrieves a block's total difficulty in the canonical chain from the -// database by hash, caching it if found. -func (bc *BlockChain) GetTdByHash(hash common.Hash) *big.Int { - return bc.hc.GetTdByHash(hash) -} - // GetHeader retrieves a block header from the database by hash and number, // caching it if found. func (bc *BlockChain) GetHeader(hash common.Hash, number uint64) *types.Header { @@ -2450,12 +2444,6 @@ func (bc *BlockChain) GetCanonicalHash(number uint64) common.Hash { return bc.hc.GetCanonicalHash(number) } -// GetBlockHashesFromHash retrieves a number of block hashes starting at a given -// hash, fetching towards the genesis block. -func (bc *BlockChain) GetBlockHashesFromHash(hash common.Hash, max uint64) []common.Hash { - return bc.hc.GetBlockHashesFromHash(hash, max) -} - // GetAncestor retrieves the Nth ancestor of a given block. It assumes that either the given block or // a close ancestor of it is canonical. maxNonCanonical points to a downwards counter limiting the // number of blocks to be individually checked before we reach the canonical chain. diff --git a/core/blockchain_test.go b/core/blockchain_test.go index fd2f214717..62036ae757 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -118,17 +118,21 @@ func testFork(t *testing.T, blockchain *BlockChain, i, n int, full bool, compara var tdPre, tdPost *big.Int if full { - tdPre = blockchain.GetTdByHash(blockchain.CurrentBlock().Hash()) + cur := blockchain.CurrentBlock() + tdPre = blockchain.GetTd(cur.Hash(), cur.NumberU64()) if err := testBlockChainImport(blockChainB, blockchain); err != nil { t.Fatalf("failed to import forked block chain: %v", err) } - tdPost = blockchain.GetTdByHash(blockChainB[len(blockChainB)-1].Hash()) + last := blockChainB[len(blockChainB)-1] + tdPost = blockchain.GetTd(last.Hash(), last.NumberU64()) } else { - tdPre = blockchain.GetTdByHash(blockchain.CurrentHeader().Hash()) + cur := blockchain.CurrentHeader() + tdPre = blockchain.GetTd(cur.Hash(), cur.Number.Uint64()) if err := testHeaderChainImport(headerChainB, blockchain); err != nil { t.Fatalf("failed to import forked header chain: %v", err) } - tdPost = blockchain.GetTdByHash(headerChainB[len(headerChainB)-1].Hash()) + last := headerChainB[len(headerChainB)-1] + tdPost = blockchain.GetTd(last.Hash(), last.Number.Uint64()) } // Compare the total difficulties of the chains comparator(tdPre, tdPost) @@ -165,7 +169,7 @@ func testBlockChainImport(chain types.Blocks, blockchain *BlockChain) error { } blockchain.chainmu.MustLock() - rawdb.WriteTd(blockchain.db, block.Hash(), block.NumberU64(), new(big.Int).Add(block.Difficulty(), blockchain.GetTdByHash(block.ParentHash()))) + rawdb.WriteTd(blockchain.db, block.Hash(), block.NumberU64(), new(big.Int).Add(block.Difficulty(), blockchain.GetTd(block.ParentHash(), block.NumberU64()-1))) rawdb.WriteBlock(blockchain.db, block) statedb.Commit(false) blockchain.chainmu.Unlock() @@ -183,7 +187,7 @@ func testHeaderChainImport(chain []*types.Header, blockchain *BlockChain) error } // Manually insert the header into the database, but don't reorganise (allows subsequent testing) blockchain.chainmu.MustLock() - rawdb.WriteTd(blockchain.db, header.Hash(), header.Number.Uint64(), new(big.Int).Add(header.Difficulty, blockchain.GetTdByHash(header.ParentHash))) + rawdb.WriteTd(blockchain.db, header.Hash(), header.Number.Uint64(), new(big.Int).Add(header.Difficulty, blockchain.GetTd(header.ParentHash, header.Number.Uint64()-1))) rawdb.WriteHeader(blockchain.db, header) blockchain.chainmu.Unlock() } @@ -436,11 +440,13 @@ func testReorg(t *testing.T, first, second []int64, td int64, full bool) { // Make sure the chain total difficulty is the correct one want := new(big.Int).Add(blockchain.genesisBlock.Difficulty(), big.NewInt(td)) if full { - if have := blockchain.GetTdByHash(blockchain.CurrentBlock().Hash()); have.Cmp(want) != 0 { + cur := blockchain.CurrentBlock() + if have := blockchain.GetTd(cur.Hash(), cur.NumberU64()); have.Cmp(want) != 0 { t.Errorf("total difficulty mismatch: have %v, want %v", have, want) } } else { - if have := blockchain.GetTdByHash(blockchain.CurrentHeader().Hash()); have.Cmp(want) != 0 { + cur := blockchain.CurrentHeader() + if have := blockchain.GetTd(cur.Hash(), cur.Number.Uint64()); have.Cmp(want) != 0 { t.Errorf("total difficulty mismatch: have %v, want %v", have, want) } } @@ -676,10 +682,10 @@ func TestFastVsFullChains(t *testing.T) { for i := 0; i < len(blocks); i++ { num, hash := blocks[i].NumberU64(), blocks[i].Hash() - if ftd, atd := fast.GetTdByHash(hash), archive.GetTdByHash(hash); ftd.Cmp(atd) != 0 { + if ftd, atd := fast.GetTd(hash, num), archive.GetTd(hash, num); ftd.Cmp(atd) != 0 { t.Errorf("block #%d [%x]: td mismatch: fastdb %v, archivedb %v", num, hash, ftd, atd) } - if antd, artd := ancient.GetTdByHash(hash), archive.GetTdByHash(hash); antd.Cmp(artd) != 0 { + if antd, artd := ancient.GetTd(hash, num), archive.GetTd(hash, num); antd.Cmp(artd) != 0 { t.Errorf("block #%d [%x]: td mismatch: ancientdb %v, archivedb %v", num, hash, antd, artd) } if fheader, aheader := fast.GetHeaderByHash(hash), archive.GetHeaderByHash(hash); fheader.Hash() != aheader.Hash() { diff --git a/core/headerchain.go b/core/headerchain.go index 7ef7dd43f7..9f2b708d0a 100644 --- a/core/headerchain.go +++ b/core/headerchain.go @@ -394,29 +394,6 @@ func (hc *HeaderChain) InsertHeaderChain(chain []*types.Header, start time.Time) return res.status, err } -// GetBlockHashesFromHash retrieves a number of block hashes starting at a given -// hash, fetching towards the genesis block. -func (hc *HeaderChain) GetBlockHashesFromHash(hash common.Hash, max uint64) []common.Hash { - // Get the origin header from which to fetch - header := hc.GetHeaderByHash(hash) - if header == nil { - return nil - } - // Iterate the headers until enough is collected or the genesis reached - chain := make([]common.Hash, 0, max) - for i := uint64(0); i < max; i++ { - next := header.ParentHash - if header = hc.GetHeader(next, header.Number.Uint64()-1); header == nil { - break - } - chain = append(chain, next) - if header.Number.Sign() == 0 { - break - } - } - return chain -} - // GetAncestor retrieves the Nth ancestor of a given block. It assumes that either the given block or // a close ancestor of it is canonical. maxNonCanonical points to a downwards counter limiting the // number of blocks to be individually checked before we reach the canonical chain. @@ -472,16 +449,6 @@ func (hc *HeaderChain) GetTd(hash common.Hash, number uint64) *big.Int { return td } -// GetTdByHash retrieves a block's total difficulty in the canonical chain from the -// database by hash, caching it if found. -func (hc *HeaderChain) GetTdByHash(hash common.Hash) *big.Int { - number := hc.GetBlockNumber(hash) - if number == nil { - return nil - } - return hc.GetTd(hash, *number) -} - // GetHeader retrieves a block header from the database by hash and number, // caching it if found. func (hc *HeaderChain) GetHeader(hash common.Hash, number uint64) *types.Header { diff --git a/eth/api_backend.go b/eth/api_backend.go index 1af33414cd..308c84b64a 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -194,7 +194,10 @@ func (b *EthAPIBackend) GetLogs(ctx context.Context, hash common.Hash) ([][]*typ } func (b *EthAPIBackend) GetTd(ctx context.Context, hash common.Hash) *big.Int { - return b.eth.blockchain.GetTdByHash(hash) + if header := b.eth.blockchain.GetHeaderByHash(hash); header != nil { + return b.eth.blockchain.GetTd(hash, header.Number.Uint64()) + } + return nil } func (b *EthAPIBackend) GetEVM(ctx context.Context, msg core.Message, state *state.StateDB, header *types.Header, vmConfig *vm.Config) (*vm.EVM, func() error, error) { diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index 809f17e36c..d88965c157 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -126,6 +126,12 @@ func testGetBlockHeaders(t *testing.T, protocol uint) { for i := range unknown { unknown[i] = byte(i) } + getHashes := func(from, limit uint64) (hashes []common.Hash) { + for i := uint64(0); i < limit; i++ { + hashes = append(hashes, backend.chain.GetCanonicalHash(from-1-i)) + } + return hashes + } // Create a batch of tests for various scenarios limit := uint64(maxHeadersServe) tests := []struct { @@ -183,7 +189,7 @@ func testGetBlockHeaders(t *testing.T, protocol uint) { // Ensure protocol limits are honored { &GetBlockHeadersPacket{Origin: HashOrNumber{Number: backend.chain.CurrentBlock().NumberU64() - 1}, Amount: limit + 10, Reverse: true}, - backend.chain.GetBlockHashesFromHash(backend.chain.CurrentBlock().Hash(), limit), + getHashes(backend.chain.CurrentBlock().NumberU64(), limit), }, // Check that requesting more than available is handled gracefully { diff --git a/eth/sync.go b/eth/sync.go index 27941158f3..d156c144ea 100644 --- a/eth/sync.go +++ b/eth/sync.go @@ -182,7 +182,7 @@ func (cs *chainSyncer) modeAndLocalHead() (downloader.SyncMode, *big.Int) { // If we're in fast sync mode, return that directly if atomic.LoadUint32(&cs.handler.fastSync) == 1 { block := cs.handler.chain.CurrentFastBlock() - td := cs.handler.chain.GetTdByHash(block.Hash()) + td := cs.handler.chain.GetTd(block.Hash(), block.NumberU64()) return downloader.FastSync, td } // We are probably in full sync, but we might have rewound to before the @@ -190,7 +190,7 @@ func (cs *chainSyncer) modeAndLocalHead() (downloader.SyncMode, *big.Int) { if pivot := rawdb.ReadLastPivotNumber(cs.handler.database); pivot != nil { if head := cs.handler.chain.CurrentBlock(); head.NumberU64() < *pivot { block := cs.handler.chain.CurrentFastBlock() - td := cs.handler.chain.GetTdByHash(block.Hash()) + td := cs.handler.chain.GetTd(block.Hash(), block.NumberU64()) return downloader.FastSync, td } } diff --git a/light/lightchain.go b/light/lightchain.go index ca6fbfac49..c481734ffe 100644 --- a/light/lightchain.go +++ b/light/lightchain.go @@ -430,12 +430,6 @@ func (lc *LightChain) GetTd(hash common.Hash, number uint64) *big.Int { return lc.hc.GetTd(hash, number) } -// GetTdByHash retrieves a block's total difficulty in the canonical chain from the -// database by hash, caching it if found. -func (lc *LightChain) GetTdByHash(hash common.Hash) *big.Int { - return lc.hc.GetTdByHash(hash) -} - // GetHeaderByNumberOdr retrieves the total difficult from the database or // network by hash and number, caching it (associated with its hash) if found. func (lc *LightChain) GetTdOdr(ctx context.Context, hash common.Hash, number uint64) *big.Int { @@ -470,12 +464,6 @@ func (bc *LightChain) GetCanonicalHash(number uint64) common.Hash { return bc.hc.GetCanonicalHash(number) } -// GetBlockHashesFromHash retrieves a number of block hashes starting at a given -// hash, fetching towards the genesis block. -func (lc *LightChain) GetBlockHashesFromHash(hash common.Hash, max uint64) []common.Hash { - return lc.hc.GetBlockHashesFromHash(hash, max) -} - // GetAncestor retrieves the Nth ancestor of a given block. It assumes that either the given block or // a close ancestor of it is canonical. maxNonCanonical points to a downwards counter limiting the // number of blocks to be individually checked before we reach the canonical chain. diff --git a/light/lightchain_test.go b/light/lightchain_test.go index af36ebd96a..8600e56345 100644 --- a/light/lightchain_test.go +++ b/light/lightchain_test.go @@ -104,12 +104,13 @@ func testFork(t *testing.T, LightChain *LightChain, i, n int, comparator func(td } // Sanity check that the forked chain can be imported into the original var tdPre, tdPost *big.Int - - tdPre = LightChain.GetTdByHash(LightChain.CurrentHeader().Hash()) + cur := LightChain.CurrentHeader() + tdPre = LightChain.GetTd(cur.Hash(), cur.Number.Uint64()) if err := testHeaderChainImport(headerChainB, LightChain); err != nil { t.Fatalf("failed to import forked header chain: %v", err) } - tdPost = LightChain.GetTdByHash(headerChainB[len(headerChainB)-1].Hash()) + last := headerChainB[len(headerChainB)-1] + tdPost = LightChain.GetTd(last.Hash(), last.Number.Uint64()) // Compare the total difficulties of the chains comparator(tdPre, tdPost) } @@ -124,7 +125,8 @@ func testHeaderChainImport(chain []*types.Header, lightchain *LightChain) error } // Manually insert the header into the database, but don't reorganize (allows subsequent testing) lightchain.chainmu.Lock() - rawdb.WriteTd(lightchain.chainDb, header.Hash(), header.Number.Uint64(), new(big.Int).Add(header.Difficulty, lightchain.GetTdByHash(header.ParentHash))) + rawdb.WriteTd(lightchain.chainDb, header.Hash(), header.Number.Uint64(), + new(big.Int).Add(header.Difficulty, lightchain.GetTd(header.ParentHash, header.Number.Uint64()-1))) rawdb.WriteHeader(lightchain.chainDb, header) lightchain.chainmu.Unlock() } @@ -309,7 +311,7 @@ func testReorg(t *testing.T, first, second []int, td int64) { } // Make sure the chain total difficulty is the correct one want := new(big.Int).Add(bc.genesisBlock.Difficulty(), big.NewInt(td)) - if have := bc.GetTdByHash(bc.CurrentHeader().Hash()); have.Cmp(want) != 0 { + if have := bc.GetTd(bc.CurrentHeader().Hash(), bc.CurrentHeader().Number.Uint64()); have.Cmp(want) != 0 { t.Errorf("total difficulty mismatch: have %v, want %v", have, want) } }