From 04444c1d0e4e0c1d979d4f8a5d7cf75366fc6609 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Thu, 14 Sep 2017 19:09:32 -0700
Subject: [PATCH] blockchain: Use CheckConnectBlockTemplate for RPC block
proposals.
This renames CheckConnectBlock to CheckConnectBlockTemplate and
modifies it to be easily consumable by the getblocktemplate RPC
handler. Performs full block validation now instead of partial
validation.
---
blockchain/error.go | 6 +++
blockchain/error_test.go | 1 +
blockchain/validate.go | 61 +++++++++++++++++++---------
blockchain/validate_test.go | 81 +++++++++++++++++++++++++++++++++----
mining/mining.go | 2 +-
rpcserver.go | 9 ++---
6 files changed, 125 insertions(+), 35 deletions(-)
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
}