From 64d60f2ef245fc7a5ea6de22c8368fa699f26f8c Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 14 Sep 2017 20:35:11 -0700 Subject: [PATCH] blockchain: Remove BFDryRun behaviour flag. This was only used to test block proposals, which has been changed to instead use CheckConnectBlockTemplate. The flag complicated the implementation of some chain processing routines and would be difficult to implement with headers-first syncing. --- blockchain/accept.go | 21 +++------------------ blockchain/chain.go | 32 +++----------------------------- blockchain/process.go | 32 ++++++++++---------------------- blockchain/validate.go | 4 ++-- blockchain/validate_test.go | 8 ++++---- 5 files changed, 22 insertions(+), 75 deletions(-) diff --git a/blockchain/accept.go b/blockchain/accept.go index 5bc517b43..a11fc94d6 100644 --- a/blockchain/accept.go +++ b/blockchain/accept.go @@ -15,17 +15,11 @@ import ( // 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 block index will not be updated 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 *btcutil.Block, flags BehaviorFlags) (bool, error) { - dryRun := flags&BFDryRun == BFDryRun - // The height of this block is one more than the referenced previous // block. blockHeight := int32(0) @@ -69,13 +63,6 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags) } b.index.AddNode(newNode) - // Undo changes to the block index when running in dry run mode. - if dryRun { - defer func() { - b.index.RemoveNode(newNode) - }() - } - // Connect the passed block to the chain while respecting proper chain // selection according to the chain with the most proof of work. This // also handles validation of the transaction scripts. @@ -87,11 +74,9 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.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, block) - b.chainLock.Lock() - } + b.chainLock.Unlock() + b.sendNotification(NTBlockAccepted, block) + b.chainLock.Lock() return isMainChain, nil } diff --git a/blockchain/chain.go b/blockchain/chain.go index 47289a526..e14056583 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -785,12 +785,8 @@ func countSpentOutputs(block *btcutil.Block) int { // 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 { // All of the blocks to detach and related spend journal entries needed // to unspend transaction outputs in the blocks being disconnected must // be loaded from the database during the reorg check phase below and @@ -883,12 +879,6 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List, flags } } - // Skip disconnecting and connecting the blocks when running with the - // dry run flag set. - if flags&BFDryRun == BFDryRun { - return nil - } - // Reset the view for the actual connection code below. This is // required because the view was previously modified when checking if // the reorg would be successful and the connection code requires the @@ -976,13 +966,10 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List, flags // 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 actually being connected and avoids any -// log messages related to modifying the state. // // This function MUST be called with the chain state lock held (for writes). func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, flags BehaviorFlags) (bool, error) { fastAdd := flags&BFFastAdd == BFFastAdd - dryRun := flags&BFDryRun == BFDryRun // We are extending the main (best) chain with a new block. This is the // most common case. @@ -1001,11 +988,6 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla } } - // 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 @@ -1037,11 +1019,6 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla // 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.bestChain.Tip().workSum) <= 0 { - // Skip Logging info when the dry run flag is set. - if dryRun { - return false, nil - } - // Log information about how the block is forking the chain. fork := b.bestChain.FindFork(node) if fork.hash.IsEqual(parentHash) { @@ -1067,11 +1044,8 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla detachNodes, attachNodes := b.getReorganizeNodes(node) // 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 fa438a8e6..6d2161bb9 100644 --- a/blockchain/process.go +++ b/blockchain/process.go @@ -29,11 +29,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 *btcutil.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) @@ -223,11 +217,8 @@ func (b *BlockChain) ProcessBlock(block *btcutil.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 } @@ -239,18 +230,15 @@ func (b *BlockChain) ProcessBlock(block *btcutil.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/blockchain/validate.go b/blockchain/validate.go index 66aafb708..ba20d7b37 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -1248,8 +1248,8 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block *btcutil.Block, vi } // CheckConnectBlockTemplate fully validates that connecting the passed block to -// the main chain does not violate any rules, aside from the proof of work -// requirement. The block must connect to the current tip of the main chain. +// the main chain does not violate any consensus rules, aside from the proof of +// work requirement. The block must connect to the current tip of the main chain. // // This function is safe for concurrent access. func (b *BlockChain) CheckConnectBlockTemplate(block *btcutil.Block) error { diff --git a/blockchain/validate_test.go b/blockchain/validate_test.go index 220160430..9bf2ff42e 100644 --- a/blockchain/validate_test.go +++ b/blockchain/validate_test.go @@ -63,8 +63,8 @@ func TestSequenceLocksActive(t *testing.T) { } } -// TestCheckConnectBlock tests the CheckConnectBlockTemplate function to ensure -// it fails. +// TestCheckConnectBlockTemplate tests the CheckConnectBlockTemplate function to +// ensure it fails. func TestCheckConnectBlockTemplate(t *testing.T) { // Create a new database and chain instance to run tests against. chain, teardownFunc, err := chainSetup("checkconnectblocktemplate", @@ -108,7 +108,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) { } } - // The block 3 should fail to connect since it's already inserted. + // Block 3 should fail to connect since it's already inserted. err = chain.CheckConnectBlockTemplate(blocks[3]) if err == nil { t.Fatal("CheckConnectBlockTemplate: Did not received expected error " + @@ -122,7 +122,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) { "block 4: %v", err) } - // The block 3a should fail to connect since does not build on chain tip. + // Block 3a should fail to connect since does not build on chain tip. err = chain.CheckConnectBlockTemplate(blocks[5]) if err == nil { t.Fatal("CheckConnectBlockTemplate: Did not received expected error " +