From 46b081c96d3310b4b8e42f7e63c6b7f06b6bacd0 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Sat, 21 Jul 2018 03:39:58 -0500 Subject: [PATCH] blockchain: Optimize reorg to use known status. This optimizes the chain reorganization logic by making use of the block index status flags for known valid and invalid blocks. In particular, validation is now skipped for blocks that are either already known valid or invalid. When validating blocks, the result is stored into the block index for the block accordingly, and in the case of blocks that fail validation, all of the descendants of the invalid block are marked as having an invalid ancestor. It also introduces a new error named ErrKnownInvalidBlock which is returned in the case a forced reorg is attempted to an invalid block and adds a test to ensure it works as intended. --- blockchain/chain.go | 160 ++++++++++++++++++++++++++------------- blockchain/chain_test.go | 13 ++++ blockchain/error.go | 5 ++ blockchain/error_test.go | 1 + 4 files changed, 128 insertions(+), 51 deletions(-) diff --git a/blockchain/chain.go b/blockchain/chain.go index 798ead83a3..851ce42b29 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -640,7 +640,8 @@ func (b *BlockChain) isMajorityVersion(minVer int32, startNode *blockNode, numRe // the fork point (which will be the end of the main chain after detaching the // returned list of block nodes) in order to reorganize the chain such that the // passed node is the new end of the main chain. The lists will be empty if the -// passed node is not on a side chain. +// passed node is not on a side chain or if the reorganize would involve +// reorganizing to a known invalid chain. // // This function MUST be called with the chain state lock held (for reads). func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List) { @@ -651,7 +652,11 @@ func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List return detachNodes, attachNodes } - // Don't allow a reorganize to a descendant of a known invalid block. + // Do not allow a reorganize to a known invalid chain. Note that all + // intermediate ancestors other than the direct parent are also checked + // below, however, this check allows extra to work to be avoided in the + // majority of cases since reorgs across multiple unvalidated blocks are + // not very common. if b.index.NodeStatus(node.parent).KnownInvalid() { b.index.SetStatusFlags(node, statusInvalidAncestor) return detachNodes, attachNodes @@ -661,8 +666,22 @@ func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List // to attach to the main tree. Push them onto the list in reverse order // so they are attached in the appropriate order when iterating the list // later. + // + // In the case a known invalid block is detected while constructing this + // list, mark all of its descendants as having an invalid ancestor and + // prevent the reorganize by not returning any nodes. forkNode := b.bestChain.FindFork(node) for n := node; n != nil && n != forkNode; n = n.parent { + if b.index.NodeStatus(n).KnownInvalid() { + for e := attachNodes.Front(); e != nil; e = e.Next() { + dn := e.Value.(*blockNode) + b.index.SetStatusFlags(dn, statusInvalidAncestor) + } + + attachNodes.Init() + return detachNodes, attachNodes + } + attachNodes.PushFront(n) } @@ -1201,14 +1220,38 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error // Store the loaded block for later. attachBlocks = append(attachBlocks, block) + // Skip validation if the block is already known to be valid. + // However, the UTXO view still needs to be updated. + if b.index.NodeStatus(n).KnownValid() { + err = b.connectTransactions(view, block, parent, nil) + if err != nil { + return err + } + + newBest = n + continue + } + // Notice the spent txout details are not requested here and // thus will not be generated. This is done because the state // is not being immediately written to the database, so it is // not needed. + // + // In the case the block is determined to be invalid due to a + // rule violation, mark it as invalid and mark all of its + // descendants as having an invalid ancestor. err = b.checkConnectBlock(n, block, parent, view, nil) if err != nil { + if _, ok := err.(RuleError); ok { + b.index.SetStatusFlags(n, statusValidateFailed) + for de := e.Next(); de != nil; de = de.Next() { + dn := de.Value.(*blockNode) + b.index.SetStatusFlags(dn, statusInvalidAncestor) + } + } return err } + b.index.SetStatusFlags(n, statusValid) newBest = n } @@ -1349,63 +1392,76 @@ func (b *BlockChain) forceHeadReorganization(formerBest chainhash.Hash, newBest "common parent for forced reorg") } - newBestBlock, err := b.fetchBlockByNode(newBestNode) - if err != nil { - return err + // Don't allow a reorganize to a known invalid chain. + newBestNodeStatus := b.index.NodeStatus(newBestNode) + if newBestNodeStatus.KnownInvalid() { + return ruleError(ErrKnownInvalidBlock, "block is known to be invalid") } - // Check to make sure our forced-in node validates correctly. - view := NewUtxoViewpoint() - view.SetBestHash(&formerBestNode.parent.hash) - view.SetStakeViewpoint(ViewpointPrevValidInitial) + // Only validate the block if it is not already known valid. + if !newBestNodeStatus.KnownValid() { + newBestBlock, err := b.fetchBlockByNode(newBestNode) + if err != nil { + return err + } - formerBestBlock, err := b.fetchBlockByNode(formerBestNode) - if err != nil { - return err - } - commonParentBlock, err := b.fetchMainChainBlockByNode(formerBestNode.parent) - if err != nil { - return err - } - var stxos []spentTxOut - err = b.db.View(func(dbTx database.Tx) error { - stxos, err = dbFetchSpendJournalEntry(dbTx, formerBestBlock, - commonParentBlock) - return err - }) - if err != nil { - return err - } + // Check to make sure our forced-in node validates correctly. + view := NewUtxoViewpoint() + view.SetBestHash(&formerBestNode.parent.hash) + view.SetStakeViewpoint(ViewpointPrevValidInitial) - // Quick sanity test. - if len(stxos) != countSpentOutputs(formerBestBlock, commonParentBlock) { - panicf("retrieved %v stxos when trying to disconnect block %v (height "+ - "%v), yet counted %v many spent utxos when trying to force head "+ - "reorg", len(stxos), formerBestBlock.Hash(), - formerBestBlock.Height(), - countSpentOutputs(formerBestBlock, commonParentBlock)) - } + formerBestBlock, err := b.fetchBlockByNode(formerBestNode) + if err != nil { + return err + } + commonParentBlock, err := b.fetchMainChainBlockByNode(formerBestNode.parent) + if err != nil { + return err + } + var stxos []spentTxOut + err = b.db.View(func(dbTx database.Tx) error { + stxos, err = dbFetchSpendJournalEntry(dbTx, formerBestBlock, + commonParentBlock) + return err + }) + if err != nil { + return err + } - err = b.disconnectTransactions(view, formerBestBlock, commonParentBlock, - stxos) - if err != nil { - return err - } + // Quick sanity test. + if len(stxos) != countSpentOutputs(formerBestBlock, commonParentBlock) { + panicf("retrieved %v stxos when trying to disconnect block %v "+ + "(height %v), yet counted %v many spent utxos when trying to "+ + "force head reorg", len(stxos), formerBestBlock.Hash(), + formerBestBlock.Height(), + countSpentOutputs(formerBestBlock, commonParentBlock)) + } - err = checkBlockSanity(newBestBlock, b.timeSource, BFNone, b.chainParams) - if err != nil { - return err - } + err = b.disconnectTransactions(view, formerBestBlock, commonParentBlock, + stxos) + if err != nil { + return err + } - err = b.checkBlockContext(newBestBlock, newBestNode.parent, BFNone) - if err != nil { - return err - } + err = checkBlockSanity(newBestBlock, b.timeSource, BFNone, b.chainParams) + if err != nil { + return err + } - err = b.checkConnectBlock(newBestNode, newBestBlock, commonParentBlock, - view, nil) - if err != nil { - return err + err = b.checkBlockContext(newBestBlock, newBestNode.parent, BFNone) + if err != nil { + return err + } + + err = b.checkConnectBlock(newBestNode, newBestBlock, commonParentBlock, + view, nil) + if err != nil { + if _, ok := err.(RuleError); ok { + b.index.SetStatusFlags(newBestNode, statusValidateFailed) + } + return err + } + b.index.SetStatusFlags(newBestNode, statusValid) } attach, detach := b.getReorganizeNodes(newBestNode) @@ -1462,6 +1518,8 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl view.SetStakeViewpoint(ViewpointPrevValidInitial) var stxos []spentTxOut if !fastAdd { + // Validate the block and set the applicable status + // result in the block index. err := b.checkConnectBlock(node, block, parent, view, &stxos) if err != nil { diff --git a/blockchain/chain_test.go b/blockchain/chain_test.go index dd0b909021..d45ce94d31 100644 --- a/blockchain/chain_test.go +++ b/blockchain/chain_test.go @@ -586,6 +586,19 @@ func TestForceHeadReorg(t *testing.T) { rejectForceTipReorg("b3", "b2bad0", ErrForceReorgMissingChild) expectTip("b3") + // Attempt to force tip reorganization to an invalid block that has an + // entry in the block index and is already known to be invalid. + // + // ... -> b1(0) -> b3(1) + // \-> b2(1) + // \-> b4(1) + // \-> b5(1) + // \-> b2bad0(1) + // \-> b2bad1(1) + // \-> b2bad2(1) + rejectForceTipReorg("b3", "b2bad1", ErrKnownInvalidBlock) + expectTip("b3") + // Attempt to force tip reorganization to an invalid block that has an // entry in the block index, but is not already known to be invalid. // diff --git a/blockchain/error.go b/blockchain/error.go index ff7d050697..a7b013b3cd 100644 --- a/blockchain/error.go +++ b/blockchain/error.go @@ -449,6 +449,10 @@ const ( // height had a non-zero final state. ErrInvalidEarlyFinalState + // ErrKnownInvalidBlock indicates that this block has previously failed + // validation. + ErrKnownInvalidBlock + // ErrInvalidAncestorBlock indicates that an ancestor of this block has // failed validation. ErrInvalidAncestorBlock @@ -558,6 +562,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrZeroValueOutputSpend: "ErrZeroValueOutputSpend", ErrInvalidEarlyVoteBits: "ErrInvalidEarlyVoteBits", ErrInvalidEarlyFinalState: "ErrInvalidEarlyFinalState", + ErrKnownInvalidBlock: "ErrKnownInvalidBlock", ErrInvalidAncestorBlock: "ErrInvalidAncestorBlock", ErrInvalidTemplateParent: "ErrInvalidTemplateParent", } diff --git a/blockchain/error_test.go b/blockchain/error_test.go index 266e52f29b..d41d1eefa7 100644 --- a/blockchain/error_test.go +++ b/blockchain/error_test.go @@ -110,6 +110,7 @@ func TestErrorCodeStringer(t *testing.T) { {ErrZeroValueOutputSpend, "ErrZeroValueOutputSpend"}, {ErrInvalidEarlyVoteBits, "ErrInvalidEarlyVoteBits"}, {ErrInvalidEarlyFinalState, "ErrInvalidEarlyFinalState"}, + {ErrKnownInvalidBlock, "ErrKnownInvalidBlock"}, {ErrInvalidAncestorBlock, "ErrInvalidAncestorBlock"}, {ErrInvalidTemplateParent, "ErrInvalidTemplateParent"}, {0xffff, "Unknown ErrorCode (65535)"},