From ea1d3750ba6b7307793335846bbfe5264dd190d3 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Fri, 23 Feb 2018 18:53:41 -0600 Subject: [PATCH] blockchain: Remove dry run flag. This switches block template proposal checks over to use CheckConnectBlock now that all of the core checks needed for block templates is included there, and, more importantly, removes the the BFDryRun flag since it was only used to test block template proposals and it is much simpler to allow the main chain processing paths to run without the complication of selectively processing code for dry run behavior. --- blockchain/accept.go | 40 ++++++++++++---------------------------- blockchain/chain.go | 35 ++++------------------------------- blockchain/process.go | 33 +++++++++++---------------------- rpcserver.go | 7 ++----- 4 files changed, 29 insertions(+), 86 deletions(-) diff --git a/blockchain/accept.go b/blockchain/accept.go index a569e3be..1fb1c107 100644 --- a/blockchain/accept.go +++ b/blockchain/accept.go @@ -102,14 +102,11 @@ func IsFinalizedTransaction(tx *dcrutil.Tx, blockHeight int64, blockTime time.Ti // before adding it. The block is expected to have already gone through // ProcessBlock before calling this function with it. // -// The flags modify the behavior of this function as follows: -// - BFDryRun: The memory chain index will not be pruned and no accept -// notification will be sent since the block is not being accepted. +// The flags are also passed to checkBlockContext and connectBestChain. See +// their documentation for how the flags modify their behavior. // // This function MUST be called with the chain state lock held (for writes). func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) (bool, error) { - dryRun := flags&BFDryRun == BFDryRun - // Get a block node for the block previous to this one. Will be nil // if this is the genesis block. prevNode, err := b.index.PrevNodeFromBlock(block) @@ -144,9 +141,7 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) // Prune stake nodes which are no longer needed before creating a new // node. - if !dryRun { - b.pruner.pruneChainIfNeeded() - } + b.pruner.pruneChainIfNeeded() // Create a new block node for the block and add it to the block index. // The block could either be on a side chain or the main chain, but it @@ -167,17 +162,14 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) // such as making blocks that never become part of the main chain or // blocks that fail to connect available for further analysis. // - // Also, store the associated block index entry when not running in dry - // run mode. + // Also, store the associated block index entry. err = b.db.Update(func(dbTx database.Tx) error { if err := dbMaybeStoreBlock(dbTx, block); err != nil { return err } - if !dryRun { - if err := dbPutBlockNode(dbTx, newNode); err != nil { - return err - } + if err := dbPutBlockNode(dbTx, newNode); err != nil { + return err } return nil @@ -186,14 +178,6 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) return false, err } - // Remove the node from the block index and disconnect it from the - // parent node when running in dry run mode. - if dryRun { - defer func() { - b.index.RemoveNode(newNode) - }() - } - // Fetching a stake node could enable a new DoS vector, so restrict // this only to blocks that are recent in history. if newNode.height < b.bestNode.height-minMemoryNodes { @@ -222,12 +206,12 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) // Notify the caller that the new block was accepted into the block // chain. The caller would typically want to react by relaying the // inventory to other peers. - if !dryRun { - b.chainLock.Unlock() - b.sendNotification(NTBlockAccepted, - &BlockAcceptedNtfnsData{isMainChain, block}) - b.chainLock.Lock() - } + b.chainLock.Unlock() + b.sendNotification(NTBlockAccepted, &BlockAcceptedNtfnsData{ + OnMainChain: isMainChain, + Block: block, + }) + b.chainLock.Lock() return isMainChain, nil } diff --git a/blockchain/chain.go b/blockchain/chain.go index 1cec1c99..032a0ace 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -1189,12 +1189,8 @@ func countNumberOfTransactions(block, parent *dcrutil.Block) uint64 { // the chain) and nodes the are being attached must be in forwards order // (think pushing them onto the end of the chain). // -// The flags modify the behavior of this function as follows: -// - BFDryRun: Only the checks which ensure the reorganize can be completed -// successfully are performed. The chain is not reorganized. -// // This function MUST be called with the chain state lock held (for writes). -func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List, flags BehaviorFlags) error { +func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error { // Nothing to do if no reorganize nodes were provided. if detachNodes.Len() == 0 && attachNodes.Len() == 0 { return nil @@ -1369,12 +1365,6 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List, flags log.Debugf("New best chain validation completed successfully, " + "commencing with the reorganization.") - // Skip disconnecting and connecting the blocks when running with the - // dry run flag set. - if flags&BFDryRun == BFDryRun { - return nil - } - // Send a notification that a blockchain reorganization is in progress. reorgData := &ReorganizationNtfnsData{ oldBest.hash, @@ -1572,7 +1562,7 @@ func (b *BlockChain) forceHeadReorganization(formerBest chainhash.Hash, newBest return err } - return b.reorganizeChain(attach, detach, BFNone) + return b.reorganizeChain(attach, detach) } // ForceHeadReorganization is the exported version of forceHeadReorganization. @@ -1594,14 +1584,10 @@ func (b *BlockChain) ForceHeadReorganization(formerBest chainhash.Hash, newBest // The flags modify the behavior of this function as follows: // - BFFastAdd: Avoids several expensive transaction validation operations. // This is useful when using checkpoints. -// - BFDryRun: Prevents the block from being connected and avoids modifying the -// state of the memory chain index. Also, any log messages related to -// modifying the state are avoided. // // This function MUST be called with the chain state lock held (for writes). func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Block, flags BehaviorFlags) (bool, error) { fastAdd := flags&BFFastAdd == BFFastAdd - dryRun := flags&BFDryRun == BFDryRun // Ensure the passed parent is actually the parent of the block. if *parent.Hash() != node.parentHash { @@ -1635,11 +1621,6 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl b.index.SetStatusFlags(node, statusValid) } - // Don't connect the block if performing a dry run. - if dryRun { - return true, nil - } - // In the fast add case the code to check the block connection // was skipped, so the utxo view needs to load the referenced // utxos, spend them, and add the new utxos being created by @@ -1684,11 +1665,6 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl // We're extending (or creating) a side chain, but the cumulative // work for this new side chain is not enough to make it the new chain. if node.workSum.Cmp(b.bestNode.workSum) <= 0 { - // Skip Logging info when the dry run flag is set. - if dryRun { - return false, nil - } - // Find the fork point. fork := node for fork.parent != nil { @@ -1736,11 +1712,8 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl } // Reorganize the chain. - if !dryRun { - log.Infof("REORGANIZE: Block %v is causing a reorganize.", - node.hash) - } - err = b.reorganizeChain(detachNodes, attachNodes, flags) + log.Infof("REORGANIZE: Block %v is causing a reorganize.", node.hash) + err = b.reorganizeChain(detachNodes, attachNodes) if err != nil { return false, err } diff --git a/blockchain/process.go b/blockchain/process.go index 6f35a7c2..2b9286fd 100644 --- a/blockchain/process.go +++ b/blockchain/process.go @@ -30,11 +30,6 @@ const ( // not be performed. BFNoPoWCheck - // BFDryRun may be set to indicate the block should not modify the chain - // or memory chain index. This is useful to test that a block is valid - // without modifying the current state. - BFDryRun - // BFNone is a convenience value to specifically indicate no flags. BFNone BehaviorFlags = 0 ) @@ -149,7 +144,6 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo defer b.chainLock.Unlock() fastAdd := flags&BFFastAdd == BFFastAdd - dryRun := flags&BFDryRun == BFDryRun blockHash := block.Hash() log.Tracef("Processing block %v", blockHash) @@ -231,11 +225,9 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo return false, false, err } if !prevHashExists { - if !dryRun { - log.Infof("Adding orphan block %v with parent %v", - blockHash, prevHash) - b.addOrphanBlock(block) - } + log.Infof("Adding orphan block %v with parent %v", blockHash, + prevHash) + b.addOrphanBlock(block) return false, true, nil } @@ -247,18 +239,15 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo return false, false, err } - // Don't process any orphans or log when the dry run flag is set. - if !dryRun { - // Accept any orphan blocks that depend on this block (they are - // no longer orphans) and repeat for those accepted blocks until - // there are no more. - err := b.processOrphans(blockHash, flags) - if err != nil { - return false, false, err - } - - log.Debugf("Accepted block %v", blockHash) + // Accept any orphan blocks that depend on this block (they are no + // longer orphans) and repeat for those accepted blocks until there are + // no more. + err = b.processOrphans(blockHash, flags) + if err != nil { + return false, false, err } + log.Debugf("Accepted block %v", blockHash) + return isMainChain, false, nil } diff --git a/rpcserver.go b/rpcserver.go index e22909f4..56e5f3aa 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2967,8 +2967,8 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *dcrjson.TemplateReque return "bad-prevblk", nil } - flags := blockchain.BFDryRun | blockchain.BFNoPoWCheck - isOrphan, err := s.server.blockManager.ProcessBlock(block, flags) + flags := blockchain.BFNoPoWCheck + err = s.server.blockManager.chain.CheckConnectBlock(block, flags) if err != nil { if _, ok := err.(blockchain.RuleError); !ok { errStr := fmt.Sprintf("Failed to process block "+ @@ -2981,9 +2981,6 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *dcrjson.TemplateReque rpcsLog.Infof("Rejected block proposal: %v", err) return chainErrToGBTErrString(err), nil } - if isOrphan { - return "orphan", nil - } return nil, nil }