diff --git a/blockchain/error.go b/blockchain/error.go index eb30d8ae..0f3f2b59 100644 --- a/blockchain/error.go +++ b/blockchain/error.go @@ -212,6 +212,11 @@ const ( // included in the block's coinbase transaction doesn't match the // manually computed witness commitment. ErrWitnessCommitmentMismatch + + // ErrPrevBlockNotBest indicates that the block's previous block is not the + // current chain tip. This is not a block validation rule, but is required + // for block proposals submitted via getblocktemplate RPC. + ErrPrevBlockNotBest ) // Map of ErrorCode values back to their constant names for pretty printing. @@ -257,6 +262,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrUnexpectedWitness: "ErrUnexpectedWitness", ErrInvalidWitnessCommitment: "ErrInvalidWitnessCommitment", ErrWitnessCommitmentMismatch: "ErrWitnessCommitmentMismatch", + ErrPrevBlockNotBest: "ErrPrevBlockNotBest", } // String returns the ErrorCode as a human-readable name. diff --git a/blockchain/error_test.go b/blockchain/error_test.go index cf740de9..dda8ad26 100644 --- a/blockchain/error_test.go +++ b/blockchain/error_test.go @@ -52,6 +52,7 @@ func TestErrorCodeStringer(t *testing.T) { {ErrBadCoinbaseHeight, "ErrBadCoinbaseHeight"}, {ErrScriptMalformed, "ErrScriptMalformed"}, {ErrScriptValidation, "ErrScriptValidation"}, + {ErrPrevBlockNotBest, "ErrPrevBlockNotBest"}, {0xffff, "Unknown ErrorCode (65535)"}, } diff --git a/blockchain/validate.go b/blockchain/validate.go index 3eef85cc..66aafb70 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -981,14 +981,18 @@ func CheckTransactionInputs(tx *btcutil.Tx, txHeight int32, utxoView *UtxoViewpo // represent the state of the chain as if the block were actually connected and // consequently the best hash for the view is also updated to passed block. // -// The CheckConnectBlock function makes use of this function to perform the -// bulk of its work. The only difference is this function accepts a node which -// may or may not require reorganization to connect it to the main chain whereas -// CheckConnectBlock creates a new node which specifically connects to the end -// of the current main chain and then calls this function with that node. +// An example of some of the checks performed are ensuring connecting the block +// would not cause any duplicate transaction hashes for old transactions that +// aren't already fully spent, double spends, exceeding the maximum allowed +// signature operations per block, invalid values in relation to the expected +// block subsidy, or fail transaction script validation. // -// See the comments for CheckConnectBlock for some examples of the type of -// checks performed by this function. +// The CheckConnectBlockTemplate function makes use of this function to perform +// the bulk of its work. The only difference is this function accepts a node +// which may or may not require reorganization to connect it to the main chain +// whereas CheckConnectBlockTemplate creates a new node which specifically +// connects to the end of the current main chain and then calls this function +// with that node. // // This function MUST be called with the chain state lock held (for writes). func (b *BlockChain) checkConnectBlock(node *blockNode, block *btcutil.Block, view *UtxoViewpoint, stxos *[]spentTxOut) error { @@ -1243,27 +1247,44 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block *btcutil.Block, vi return nil } -// CheckConnectBlock performs several checks to confirm connecting the passed -// block to the main chain does not violate any rules. An example of some of -// the checks performed are ensuring connecting the block would not cause any -// duplicate transaction hashes for old transactions that aren't already fully -// spent, double spends, exceeding the maximum allowed signature operations -// per block, invalid values in relation to the expected block subsidy, or fail -// transaction script validation. +// 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. // // This function is safe for concurrent access. -func (b *BlockChain) CheckConnectBlock(block *btcutil.Block) error { +func (b *BlockChain) CheckConnectBlockTemplate(block *btcutil.Block) error { b.chainLock.Lock() defer b.chainLock.Unlock() - prevNode := b.bestChain.Tip() - newNode := newBlockNode(&block.MsgBlock().Header, prevNode.height+1) - newNode.parent = prevNode - newNode.workSum.Add(prevNode.workSum, newNode.workSum) + // Skip the proof of work check as this is just a block template. + flags := BFNoPoWCheck + + // This only checks whether the block can be connected to the tip of the + // current chain. + tip := b.bestChain.Tip() + header := block.MsgBlock().Header + if tip.hash != header.PrevBlock { + str := fmt.Sprintf("previous block must be the current chain tip %v, "+ + "instead got %v", tip.hash, header.PrevBlock) + return ruleError(ErrPrevBlockNotBest, str) + } + + err := checkBlockSanity(block, b.chainParams.PowLimit, b.timeSource, flags) + if err != nil { + return err + } + + err = b.checkBlockContext(block, tip, flags) + if err != nil { + return err + } // Leave the spent txouts entry nil in the state since the information // is not needed and thus extra work can be avoided. view := NewUtxoViewpoint() - view.SetBestHash(&prevNode.hash) + view.SetBestHash(&tip.hash) + newNode := newBlockNode(&header, tip.height+1) + newNode.parent = tip + newNode.workSum = newNode.workSum.Add(tip.workSum, newNode.workSum) return b.checkConnectBlock(newNode, block, view, nil) } diff --git a/blockchain/validate_test.go b/blockchain/validate_test.go index ede41514..22016043 100644 --- a/blockchain/validate_test.go +++ b/blockchain/validate_test.go @@ -63,11 +63,11 @@ func TestSequenceLocksActive(t *testing.T) { } } -// TestCheckConnectBlock tests the CheckConnectBlock function to ensure it -// fails. -func TestCheckConnectBlock(t *testing.T) { +// TestCheckConnectBlock 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("checkconnectblock", + chain, teardownFunc, err := chainSetup("checkconnectblocktemplate", &chaincfg.MainNetParams) if err != nil { t.Errorf("Failed to setup chain instance: %v", err) @@ -75,11 +75,76 @@ func TestCheckConnectBlock(t *testing.T) { } defer teardownFunc() - // The genesis block should fail to connect since it's already inserted. - genesisBlock := chaincfg.MainNetParams.GenesisBlock - err = chain.CheckConnectBlock(btcutil.NewBlock(genesisBlock)) + // Since we're not dealing with the real block chain, set the coinbase + // maturity to 1. + chain.TstSetCoinbaseMaturity(1) + + // Load up blocks such that there is a side chain. + // (genesis block) -> 1 -> 2 -> 3 -> 4 + // \-> 3a + testFiles := []string{ + "blk_0_to_4.dat.bz2", + "blk_3A.dat.bz2", + } + + var blocks []*btcutil.Block + for _, file := range testFiles { + blockTmp, err := loadBlocks(file) + if err != nil { + t.Fatalf("Error loading file: %v\n", err) + } + blocks = append(blocks, blockTmp...) + } + + for i := 1; i <= 3; i++ { + isMainChain, _, err := chain.ProcessBlock(blocks[i], BFNone) + if err != nil { + t.Fatalf("CheckConnectBlockTemplate: Received unexpected error "+ + "processing block %d: %v", i, err) + } + if !isMainChain { + t.Fatalf("CheckConnectBlockTemplate: Expected block %d to connect "+ + "to main chain", i) + } + } + + // The block 3 should fail to connect since it's already inserted. + err = chain.CheckConnectBlockTemplate(blocks[3]) if err == nil { - t.Errorf("CheckConnectBlock: Did not received expected error") + t.Fatal("CheckConnectBlockTemplate: Did not received expected error " + + "on block 3") + } + + // Block 4 should connect successfully to tip of chain. + err = chain.CheckConnectBlockTemplate(blocks[4]) + if err != nil { + t.Fatalf("CheckConnectBlockTemplate: Received unexpected error on "+ + "block 4: %v", err) + } + + // The 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 " + + "on block 3a") + } + + // Block 4 should connect even if proof of work is invalid. + invalidPowBlock := *blocks[4].MsgBlock() + invalidPowBlock.Header.Nonce++ + err = chain.CheckConnectBlockTemplate(btcutil.NewBlock(&invalidPowBlock)) + if err != nil { + t.Fatalf("CheckConnectBlockTemplate: Received unexpected error on "+ + "block 4 with bad nonce: %v", err) + } + + // Invalid block building on chain tip should fail to connect. + invalidBlock := *blocks[4].MsgBlock() + invalidBlock.Header.Bits-- + err = chain.CheckConnectBlockTemplate(btcutil.NewBlock(&invalidBlock)) + if err == nil { + t.Fatal("CheckConnectBlockTemplate: Did not received expected error " + + "on block 4 with invalid difficulty bits") } } diff --git a/mining/mining.go b/mining/mining.go index 37158940..09aa391f 100644 --- a/mining/mining.go +++ b/mining/mining.go @@ -879,7 +879,7 @@ mempoolLoop: // chain with no issues. block := btcutil.NewBlock(&msgBlock) block.SetHeight(nextBlockHeight) - if err := g.chain.CheckConnectBlock(block); err != nil { + if err := g.chain.CheckConnectBlockTemplate(block); err != nil { return nil, err } diff --git a/rpcserver.go b/rpcserver.go index db4dece8..1033b20f 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2040,6 +2040,8 @@ func chainErrToGBTErrString(err error) string { return "bad-script-malformed" case blockchain.ErrScriptValidation: return "bad-script-validate" + case blockchain.ErrPrevBlockNotBest: + return "inconclusive-not-best-prvblk" } return "rejected: " + err.Error() @@ -2088,9 +2090,7 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *btcjson.TemplateReque return "bad-prevblk", nil } - flags := blockchain.BFDryRun | blockchain.BFNoPoWCheck - isOrphan, err := s.cfg.SyncMgr.SubmitBlock(block, flags) - if err != nil { + if err := s.cfg.Chain.CheckConnectBlockTemplate(block); err != nil { if _, ok := err.(blockchain.RuleError); !ok { errStr := fmt.Sprintf("Failed to process block proposal: %v", err) rpcsLog.Error(errStr) @@ -2103,9 +2103,6 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *btcjson.TemplateReque rpcsLog.Infof("Rejected block proposal: %v", err) return chainErrToGBTErrString(err), nil } - if isOrphan { - return "orphan", nil - } return nil, nil }