multi: Return fork len from ProcessBlock.

This modifies the ProcessBlock function in the blockchain package to
return the fork length for the connected block and updates all callers
and tests accordingly.  Several of the internal functions which
ProcessBlock calls are also updated in order to bubble the necessary
information back up so it can be returned.  It does not make any
behavioral changes.

This is being done to better expose information about the position of
the block within the chain to callers without them having to make
additional queries.
This commit is contained in:
Dave Collins 2018-05-27 18:46:59 -05:00
parent 7c87772d90
commit 66010e4134
No known key found for this signature in database
GPG Key ID: B8904D9D9C93D1F2
12 changed files with 94 additions and 75 deletions

View File

@ -102,22 +102,24 @@ func IsFinalizedTransaction(tx *dcrutil.Tx, blockHeight int64, blockTime time.Ti
}
// maybeAcceptBlock potentially accepts a block into the block chain and, if
// accepted, returns whether or not it is on the main chain. It performs
// accepted, returns the length of the fork the block extended. It performs
// several validation checks which depend on its position within the block chain
// before adding it. The block is expected to have already gone through
// ProcessBlock before calling this function with it.
// ProcessBlock before calling this function with it. In the case the block
// extends the best chain or is now the tip of the best chain due to causing a
// reorganize, the fork length will be 0.
//
// 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) {
func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags) (int64, error) {
// 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)
if err != nil {
log.Debugf("PrevNodeFromBlock: %v", err)
return false, err
return 0, err
}
// This function should never be called with orphan blocks or the
@ -125,7 +127,7 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
if prevNode == nil {
prevHash := &block.MsgBlock().Header.PrevBlock
str := fmt.Sprintf("previous block %s is not known", prevHash)
return false, ruleError(ErrMissingParent, str)
return 0, ruleError(ErrMissingParent, str)
}
// There is no need to validate the block if an ancestor is already
@ -134,14 +136,14 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
prevHash := &block.MsgBlock().Header.PrevBlock
str := fmt.Sprintf("previous block %s is known to be invalid",
prevHash)
return false, ruleError(ErrInvalidAncestorBlock, str)
return 0, ruleError(ErrInvalidAncestorBlock, str)
}
// The block must pass all of the validation rules which depend on the
// position of the block within the block chain.
err = b.checkBlockContext(block, prevNode, flags)
if err != nil {
return false, err
return 0, err
}
// Prune stake nodes which are no longer needed before creating a new
@ -180,7 +182,7 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
return nil
})
if err != nil {
return false, err
return 0, err
}
// Fetching a stake node could enable a new DoS vector, so restrict
@ -188,7 +190,7 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
if newNode.height < b.bestNode.height-minMemoryNodes {
newNode.stakeNode, err = b.fetchStakeNode(newNode)
if err != nil {
return false, err
return 0, err
}
newNode.stakeUndoData = newNode.stakeNode.UndoData()
}
@ -197,15 +199,15 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
// connection process.
parent, err := b.fetchBlockByHash(&newNode.parentHash)
if err != nil {
return false, err
return 0, err
}
// 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.
isMainChain, err := b.connectBestChain(newNode, block, parent, flags)
forkLen, err := b.connectBestChain(newNode, block, parent, flags)
if err != nil {
return false, err
return 0, err
}
// Notify the caller that the new block was accepted into the block
@ -213,10 +215,10 @@ func (b *BlockChain) maybeAcceptBlock(block *dcrutil.Block, flags BehaviorFlags)
// inventory to other peers.
b.chainLock.Unlock()
b.sendNotification(NTBlockAccepted, &BlockAcceptedNtfnsData{
OnMainChain: isMainChain,
Block: block,
ForkLen: forkLen,
Block: block,
})
b.chainLock.Lock()
return isMainChain, nil
return forkLen, nil
}

View File

@ -1550,21 +1550,23 @@ func (b *BlockChain) ForceHeadReorganization(formerBest chainhash.Hash, newBest
// proof of work. In the typical case, the new block simply extends the main
// chain. However, it may also be extending (or creating) a side chain (fork)
// which may or may not end up becoming the main chain depending on which fork
// cumulatively has the most proof of work. It returns whether or not the block
// ended up on the main chain (either due to extending the main chain or causing
// a reorganization to become the main chain).
// cumulatively has the most proof of work. It returns the resulting fork
// length, that is to say the number of blocks to the fork point from the main
// chain, which will be zero if the block ends up on the main chain (either
// due to extending the main chain or causing a reorganization to become the
// main chain).
//
// The flags modify the behavior of this function as follows:
// - BFFastAdd: Avoids several expensive transaction validation operations.
// This is useful when using checkpoints.
//
// 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) {
func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Block, flags BehaviorFlags) (int64, error) {
fastAdd := flags&BFFastAdd == BFFastAdd
// Ensure the passed parent is actually the parent of the block.
if *parent.Hash() != node.parentHash {
return false, AssertError("connectBlock must be called with the " +
return 0, AssertError("connectBlock must be called with the " +
"correct parent block")
}
@ -1589,7 +1591,7 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
if _, ok := err.(RuleError); ok {
b.index.SetStatusFlags(node, statusValidateFailed)
}
return false, err
return 0, err
}
b.index.SetStatusFlags(node, statusValid)
}
@ -1601,18 +1603,18 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
if fastAdd {
err := view.fetchInputUtxos(b.db, block, parent)
if err != nil {
return false, err
return 0, err
}
err = b.connectTransactions(view, block, parent, &stxos)
if err != nil {
return false, err
return 0, err
}
}
// Connect the block to the main chain.
err := b.connectBlock(node, block, parent, view, stxos)
if err != nil {
return false, err
return 0, err
}
validateStr := "validating"
@ -1624,7 +1626,9 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
"%v the previous block", node.hash, node.height,
validateStr)
return true, nil
// The fork length is zero since the block is now the tip of the
// best chain.
return 0, nil
}
if fastAdd {
log.Warnf("fastAdd set in the side chain case? %v\n",
@ -1647,7 +1651,7 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
var err error
fork, err = b.index.PrevNodeFromNode(fork)
if err != nil {
return false, err
return 0, err
}
}
@ -1655,21 +1659,15 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
if fork.hash == node.parent.hash {
log.Infof("FORK: Block %v (height %v) forks the chain at height "+
"%d/block %v, but does not cause a reorganize",
node.hash,
node.height,
fork.height,
fork.hash)
node.hash, node.height, fork.height, fork.hash)
} else {
log.Infof("EXTEND FORK: Block %v (height %v) extends a side chain "+
"which forks the chain at height "+
"%d/block %v",
node.hash,
node.height,
fork.height,
fork.hash)
"which forks the chain at height %d/block %v",
node.hash, node.height, fork.height, fork.hash)
}
return false, nil
forkLen := node.height - fork.height
return forkLen, nil
}
// We're extending (or creating) a side chain and the cumulative work
@ -1681,17 +1679,19 @@ func (b *BlockChain) connectBestChain(node *blockNode, block, parent *dcrutil.Bl
// common ancenstor (the point where the chain forked).
detachNodes, attachNodes, err := b.getReorganizeNodes(node)
if err != nil {
return false, err
return 0, err
}
// Reorganize the chain.
log.Infof("REORGANIZE: Block %v is causing a reorganize.", node.hash)
err = b.reorganizeChain(detachNodes, attachNodes)
if err != nil {
return false, err
return 0, err
}
return true, nil
// The fork length is zero since the block is now the tip of the best
// chain.
return 0, nil
}
// isCurrent returns whether or not the chain believes it is current. Several

View File

@ -158,7 +158,7 @@ func TestForceHeadReorg(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -167,6 +167,7 @@ func TestForceHeadReorg(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if !isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want true", g.TipName(),
@ -197,7 +198,7 @@ func TestForceHeadReorg(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -206,6 +207,7 @@ func TestForceHeadReorg(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want false", g.TipName(),

View File

@ -60,12 +60,13 @@ func ExampleBlockChain_ProcessBlock() {
// cause an error by trying to process the genesis block which already
// exists.
genesisBlock := dcrutil.NewBlock(chaincfg.MainNetParams.GenesisBlock)
isMainChain, isOrphan, err := chain.ProcessBlock(genesisBlock,
forkLen, isOrphan, err := chain.ProcessBlock(genesisBlock,
blockchain.BFNone)
if err != nil {
fmt.Printf("Failed to create chain instance: %v\n", err)
return
}
isMainChain := !isOrphan && forkLen == 0
fmt.Printf("Block accepted. Is it on the main chain?: %v", isMainChain)
fmt.Printf("Block accepted. Is it an orphan?: %v", isOrphan)

View File

@ -154,7 +154,7 @@ func TestFullBlocks(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
item.Name, block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block,
forkLen, isOrphan, err := chain.ProcessBlock(block,
blockchain.BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
@ -164,6 +164,7 @@ func TestFullBlocks(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if isMainChain != item.IsMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want %v", item.Name,

View File

@ -46,7 +46,7 @@ func TestStakeVersion(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -55,6 +55,7 @@ func TestStakeVersion(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if !isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want true", g.TipName(),

View File

@ -69,8 +69,8 @@ func (n NotificationType) String() string {
// BlockAcceptedNtfnsData is the structure for data indicating information
// about a block being accepted.
type BlockAcceptedNtfnsData struct {
OnMainChain bool
Block *dcrutil.Block
ForkLen int64
Block *dcrutil.Block
}
// ReorganizationNtfnsData is the structure for data indicating information

View File

@ -135,11 +135,15 @@ func (b *BlockChain) processOrphans(hash *chainhash.Hash, flags BehaviorFlags) e
// the block chain along with best chain selection and reorganization.
//
// When no errors occurred during processing, the first return value indicates
// whether or not the block is on the main chain and the second indicates
// whether or not the block is an orphan.
// the length of the fork the block extended. In the case it either exteneded
// the best chain or is now the tip of the best chain due to causing a
// reorganize, the fork length will be 0. The second return value indicates
// whether or not the block is an orphan, in which case the fork length will
// also be zero as expected, because it, by definition, does not connect ot the
// best chain.
//
// This function is safe for concurrent access.
func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bool, bool, error) {
func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (int64, bool, error) {
b.chainLock.Lock()
defer b.chainLock.Unlock()
@ -157,23 +161,23 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
// The block must not already exist in the main chain or side chains.
exists, err := b.blockExists(blockHash)
if err != nil {
return false, false, err
return 0, false, err
}
if exists {
str := fmt.Sprintf("already have block %v", blockHash)
return false, false, ruleError(ErrDuplicateBlock, str)
return 0, false, ruleError(ErrDuplicateBlock, str)
}
// The block must not already exist as an orphan.
if _, exists := b.orphans[*blockHash]; exists {
str := fmt.Sprintf("already have block (orphan) %v", blockHash)
return false, false, ruleError(ErrDuplicateBlock, str)
return 0, false, ruleError(ErrDuplicateBlock, str)
}
// Perform preliminary sanity checks on the block and its transactions.
err = checkBlockSanity(block, b.timeSource, flags, b.chainParams)
if err != nil {
return false, false, err
return 0, false, err
}
// Find the previous checkpoint and perform some additional checks based
@ -185,7 +189,7 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
blockHeader := &block.MsgBlock().Header
checkpointBlock, err := b.findPreviousCheckpoint()
if err != nil {
return false, false, err
return 0, false, err
}
if checkpointBlock != nil {
// Ensure the block timestamp is after the checkpoint timestamp.
@ -195,7 +199,7 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
str := fmt.Sprintf("block %v has timestamp %v before "+
"last checkpoint timestamp %v", blockHash,
blockHeader.Timestamp, checkpointTime)
return false, false, ruleError(ErrCheckpointTimeTooOld, str)
return 0, false, ruleError(ErrCheckpointTimeTooOld, str)
}
if !fastAdd {
@ -213,7 +217,7 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
str := fmt.Sprintf("block target difficulty of %064x "+
"is too low when compared to the previous "+
"checkpoint", currentTarget)
return false, false, ruleError(ErrDifficultyTooLow, str)
return 0, false, ruleError(ErrDifficultyTooLow, str)
}
}
}
@ -222,21 +226,23 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
prevHash := &blockHeader.PrevBlock
prevHashExists, err := b.blockExists(prevHash)
if err != nil {
return false, false, err
return 0, false, err
}
if !prevHashExists {
log.Infof("Adding orphan block %v with parent %v", blockHash,
prevHash)
b.addOrphanBlock(block)
return false, true, nil
// The fork length of orphans is unknown since they, by definition, do
// not connect to the best chain.
return 0, true, nil
}
// The block has passed all context independent checks and appears sane
// enough to potentially accept it into the block chain.
isMainChain, err := b.maybeAcceptBlock(block, flags)
forkLen, err := b.maybeAcceptBlock(block, flags)
if err != nil {
return false, false, err
return 0, false, err
}
// Accept any orphan blocks that depend on this block (they are no
@ -244,10 +250,10 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (bo
// no more.
err = b.processOrphans(blockHash, flags)
if err != nil {
return false, false, err
return 0, false, err
}
log.Debugf("Accepted block %v", blockHash)
return isMainChain, false, nil
return forkLen, false, nil
}

View File

@ -162,7 +162,7 @@ func TestThresholdState(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -171,6 +171,7 @@ func TestThresholdState(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if !isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want true", g.TipName(),

View File

@ -372,7 +372,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -381,6 +381,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if !isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want true", g.TipName(),
@ -441,7 +442,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) {
t.Logf("Testing block %s (hash %s, height %d)",
g.TipName(), block.Hash(), blockHeight)
isMainChain, isOrphan, err := chain.ProcessBlock(block, BFNone)
forkLen, isOrphan, err := chain.ProcessBlock(block, BFNone)
if err != nil {
t.Fatalf("block %q (hash %s, height %d) should "+
"have been accepted: %v", g.TipName(),
@ -450,6 +451,7 @@ func TestCheckConnectBlockTemplate(t *testing.T) {
// Ensure the main chain and orphan flags match the values
// specified in the test.
isMainChain := !isOrphan && forkLen == 0
if isMainChain {
t.Fatalf("block %q (hash %s, height %d) unexpected main "+
"chain flag -- got %v, want false", g.TipName(),

View File

@ -195,9 +195,9 @@ type forceReorganizationMsg struct {
// processBlockResponse is a response sent to the reply channel of a
// processBlockMsg.
type processBlockResponse struct {
onMainChain bool
isOrphan bool
err error
forkLen int64
isOrphan bool
err error
}
// processBlockMsg is a message type to be sent across the message channel
@ -1000,7 +1000,7 @@ func (b *blockManager) handleBlockMsg(bmsg *blockMsg) {
// Process the block to include validation, best chain selection, orphan
// handling, etc.
onMainChain, isOrphan, err := b.chain.ProcessBlock(bmsg.block,
forkLen, isOrphan, err := b.chain.ProcessBlock(bmsg.block,
behaviorFlags)
if err != nil {
// When the error is a rule error, it means the block was simply
@ -1111,6 +1111,7 @@ func (b *blockManager) handleBlockMsg(bmsg *blockMsg) {
}
}
onMainChain := !isOrphan && forkLen == 0
if onMainChain {
// A new block is connected, however, this new block may have
// votes in it that were hidden from the network and which
@ -1753,13 +1754,13 @@ out:
}
case processBlockMsg:
onMainChain, isOrphan, err := b.chain.ProcessBlock(
forkLen, isOrphan, err := b.chain.ProcessBlock(
msg.block, msg.flags)
if err != nil {
msg.reply <- processBlockResponse{
onMainChain: onMainChain,
isOrphan: isOrphan,
err: err,
forkLen: forkLen,
isOrphan: isOrphan,
err: err,
}
continue
}
@ -1812,6 +1813,7 @@ out:
// If the block added to the main chain, then we need to
// update the tip locally on block manager.
onMainChain := !isOrphan && forkLen == 0
if onMainChain {
// Query the chain for the latest best block
// since the block that was processed could be

View File

@ -132,11 +132,12 @@ func (bi *blockImporter) processBlock(serializedBlock []byte) (bool, error) {
// Ensure the blocks follows all of the chain rules and match up to the
// known checkpoints.
isMainChain, isOrphan, err := bi.chain.ProcessBlock(block,
forkLen, isOrphan, err := bi.chain.ProcessBlock(block,
blockchain.BFFastAdd)
if err != nil {
return false, err
}
isMainChain := !isOrphan && forkLen == 0
if !isMainChain {
return false, fmt.Errorf("import file contains an block that "+
"does not extend the main chain: %v", blockHash)