Merge #400: 2WP: Verify number of transactions in SPV proof

85eb502 2WP: Verify number of transactions in SPV proof (Steven Roose)
621b52f Check parent RPC version on startup (Steven Roose)
7bfb100 expose CBlockIndex::nTx in getblock(header) (Gregory Sanders)

Author: instagibbs

qa/rpc-tests/pruning.py

@@ -269,10 +269,17 @@ def has_block(index):
         # should not prune because chain tip of node 3 (995) < PruneAfterHeight (1000)
         assert_raises_message(JSONRPCException, "Blockchain is too short for pruning", node.pruneblockchain, height(500))
 
+        # Save block transaction count before pruning, assert value
+        block1_details = node.getblock(node.getblockhash(1))
+        assert_equal(block1_details["nTx"], len(block1_details["tx"]))
+
         # mine 6 blocks so we are at height 1001 (i.e., above PruneAfterHeight)
         node.generate(6)
         assert_equal(node.getblockchaininfo()["blocks"], 1001)
 
+        # Pruned block should still know the number of transactions
+        assert_equal(node.getblockheader(node.getblockhash(1))["nTx"], block1_details["nTx"])
+
         # negative heights should raise an exception
         assert_raises_message(JSONRPCException, "Negative", node.pruneblockchain, -10)
 

src/callrpc.cpp

@@ -178,7 +178,7 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params, bool conn
     return reply;
 }
 
-bool IsConfirmedBitcoinBlock(const uint256& hash, int nMinConfirmationDepth)
+bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDepth, const int nbTxs)
 {
     try {
         UniValue params(UniValue::VARR);
@@ -189,8 +189,21 @@ bool IsConfirmedBitcoinBlock(const uint256& hash, int nMinConfirmationDepth)
         UniValue result = find_value(reply, "result");
         if (!result.isObject())
             return false;
-        result = find_value(result.get_obj(), "confirmations");
-        return result.isNum() && result.get_int64() >= nMinConfirmationDepth;
+
+        UniValue confirmations = find_value(result.get_obj(), "confirmations");
+        if (!confirmations.isNum() || confirmations.get_int64() < nMinConfirmationDepth) {
+            return false;
+        }
+
+        // Only perform extra test if nbTxs has been provided (non-zero).
+        if (nbTxs != 0) {
+            UniValue nTx = find_value(result.get_obj(), "nTx");
+            if (!nTx.isNum() || nTx.get_int64() != nbTxs) {
+                LogPrintf("ERROR: Invalid number of transactions in merkle block for %s", 
+                        hash.GetHex());
+                return false;
+            }
+        }
     } catch (CConnectionFailed& e) {
         LogPrintf("ERROR: Lost connection to bitcoind RPC, you will want to restart after fixing this!\n");
         return false;

src/callrpc.h

@@ -35,6 +35,11 @@ class CConnectionFailed : public std::runtime_error
 };
 
 UniValue CallRPC(const std::string& strMethod, const UniValue& params, bool connectToMainchain=false);
-bool IsConfirmedBitcoinBlock(const uint256& hash, int nMinConfirmationDepth);
+
+// Verify if the block with given hash has at least the specified minimum number
+// of confirmations.
+// For validating merkle blocks, you can provide the nbTxs parameter to verify if
+// it equals the number of transactions in the block.
+bool IsConfirmedBitcoinBlock(const uint256& hash, int nMinConfirmationDepth, int nbTxs);
 
 #endif // BITCOIN_CALLRPC_H

src/merkleblock.h

@@ -115,6 +115,11 @@ class CPartialMerkleTree
      * returns the merkle root, or 0 in case of failure
      */
     uint256 ExtractMatches(std::vector<uint256> &vMatch, std::vector<unsigned int> &vnIndex);
+
+    /** Get number of transactions the merkle proof is indicating for cross-reference with
+     * local blockchain knowledge.
+     */
+    unsigned int GetNumTransactions() const { return nTransactions; };
 };
 
 

src/primitives/bitcoin/merkleblock.h

@@ -120,6 +120,11 @@ class CPartialMerkleTree
      * returns the merkle root, or 0 in case of failure
      */
     uint256 ExtractMatches(std::vector<uint256> &vMatch, std::vector<unsigned int> &vnIndex);
+
+    /** Get number of transactions the merkle proof is indicating for cross-reference with
+     * local blockchain knowledge.
+     */
+    unsigned int GetNumTransactions() const { return nTransactions; };
 };
 
 

src/rpc/blockchain.cpp

@@ -62,6 +62,7 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)
     result.push_back(Pair("merkleroot", blockindex->hashMerkleRoot.GetHex()));
     result.push_back(Pair("time", (int64_t)blockindex->nTime));
     result.push_back(Pair("mediantime", (int64_t)blockindex->GetMedianTimePast()));
+    result.push_back(Pair("nTx", (uint64_t)blockindex->nTx));
     result.push_back(Pair("signblock_witness_asm", ScriptToAsmStr(blockindex->proof.solution)));
     result.push_back(Pair("signblock_witness_hex", HexStr(blockindex->proof.solution)));
 
@@ -104,6 +105,7 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx
     result.push_back(Pair("tx", txs));
     result.push_back(Pair("time", block.GetBlockTime()));
     result.push_back(Pair("mediantime", (int64_t)blockindex->GetMedianTimePast()));
+    result.push_back(Pair("nTx", (int64_t)blockindex->nTx));
     result.push_back(Pair("signblock_witness_asm", ScriptToAsmStr(blockindex->proof.solution)));
     result.push_back(Pair("signblock_witness_hex", HexStr(blockindex->proof.solution)));
 
@@ -605,8 +607,7 @@ UniValue getblockheader(const JSONRPCRequest& request)
             "  \"merkleroot\" : \"xxxx\", (string) The merkle root\n"
             "  \"time\" : ttt,          (numeric) The block time in seconds since epoch (Jan 1 1970 GMT)\n"
             "  \"mediantime\" : ttt,    (numeric) The median block time in seconds since epoch (Jan 1 1970 GMT)\n"
-            "  \"signblock_witness_asm\":\"asm\", (string) scriptSig for block signing (asm)'\n"
-            "  \"signblock_witness_hex\":\"hex\", (string) scriptSig for block signing (hex)'\n"
+            "  \"nTx\" : n,             (numeric) The number of transactions in the block.\n"
             "  \"previousblockhash\" : \"hash\",  (string) The hash of the previous block\n"
             "  \"nextblockhash\" : \"hash\",      (string) The hash of the next block\n"
             "}\n"
@@ -691,8 +692,7 @@ static UniValue getblock(const JSONRPCRequest& request)
             "  ],\n"
             "  \"time\" : ttt,          (numeric) The block time in seconds since epoch (Jan 1 1970 GMT)\n"
             "  \"mediantime\" : ttt,    (numeric) The median block time in seconds since epoch (Jan 1 1970 GMT)\n"
-            "  \"signblock_witness_asm\":\"asm\", (string) scriptSig for block signing (asm)'\n"
-            "  \"signblock_witness_hex\":\"hex\", (string) scriptSig for block signing (hex)'\n"
+            "  \"nTx\" : n,             (numeric) The number of transactions in the block.\n"
             "  \"previousblockhash\" : \"hash\",  (string) The hash of the previous block\n"
             "  \"nextblockhash\" : \"hash\"       (string) The hash of the next block\n"
             "}\n"

src/validation.cpp

@@ -2229,16 +2229,18 @@ bool BitcoindRPCCheck(const bool init)
     vblocksToReconsiderAgain.clear();
     pblocktree->WriteInvalidBlockQueue(vblocksToReconsider);
 
-    //Next, check for working rpc
+    // Next, check for working and valid rpc
     if (GetBoolArg("-validatepegin", DEFAULT_VALIDATE_PEGIN)) {
         // During init try until a non-RPC_IN_WARMUP result
         while (true) {
             try {
+                // The first thing we have to check is the version of the node.
                 UniValue params(UniValue::VARR);
-                params.push_back(UniValue(0));
-                UniValue reply = CallRPC("getblockhash", params, true);
-                UniValue error = find_value(reply, "error");
+                UniValue reply = CallRPC("getnetworkinfo", params, true);
+                UniValue error = reply["error"];
                 if (!error.isNull()) {
+                    // On the first call, it's possible to node is still in 
+                    // warmup; in that case, just wait and retry.
                     if (error["code"].get_int() == RPC_IN_WARMUP) {
                         MilliSleep(1000);
                         continue;
@@ -2249,6 +2251,22 @@ bool BitcoindRPCCheck(const bool init)
                     }
                 }
                 UniValue result = reply["result"];
+                if (!result.isObject() || !result.get_obj()["version"].isNum() ||
+                        result.get_obj()["version"].get_int() < MIN_PARENT_NODE_VERSION) {
+                    LogPrintf("ERROR: Parent chain daemon too old; "
+                            "need Bitcoin Core version 0.16.2 or newer.\n");
+                    return false;
+                }
+
+                // Then check the genesis block to correspond to parent chain.
+                params.push_back(UniValue(0));
+                reply = CallRPC("getblockhash", params, true);
+                error = reply["error"];
+                if (!error.isNull()) {
+                    LogPrintf("ERROR: Bitcoind RPC check returned 'error' response.\n");
+                    return false;
+                }
+                result = reply["result"];
                 if (!result.isStr() || result.get_str() != Params().ParentGenesisBlockHash().GetHex()) {
                     LogPrintf("ERROR: Invalid parent genesis block hash response via RPC. Contacting wrong parent daemon?\n");
                     return false;
@@ -2304,7 +2322,7 @@ bool BitcoindRPCCheck(const bool init)
 
         //Write back remaining blocks
         pblocktree->WriteInvalidBlockQueue(vblocksToReconsiderAgain);
-        }
+    }
     return true;
 }
 
@@ -2513,6 +2531,7 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
 
     uint256 block_hash;
     uint256 tx_hash;
+    int num_txs;
     // Get txout proof
     if (Params().GetConsensus().ParentChainHasPow()) {
 
@@ -2528,6 +2547,8 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
         if (!CheckPeginTx(stack[4], pegtx, prevout, value, claim_script)) {
             return false;
         }
+
+        num_txs = merkle_block_pow.txn.GetNumTransactions();
     } else {
 
         CMerkleBlock merkle_block;
@@ -2543,6 +2564,8 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
         if (!CheckPeginTx(stack[4], pegtx, prevout, value, claim_script)) {
             return false;
         }
+
+        num_txs = merkle_block.txn.GetNumTransactions();
     }
 
     // Check that the merkle proof corresponds to the txid
@@ -2562,7 +2585,9 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
 
     // Finally, validate peg-in via rpc call
     if (check_depth && GetBoolArg("-validatepegin", DEFAULT_VALIDATE_PEGIN)) {
-        return IsConfirmedBitcoinBlock(block_hash, Params().GetConsensus().pegin_min_depth);
+        if (!IsConfirmedBitcoinBlock(block_hash, Params().GetConsensus().pegin_min_depth, num_txs)) {
+            return false;
+        }
     }
     return true;
 }

src/validation.h

@@ -149,6 +149,10 @@ static const int MAX_UNCONNECTING_HEADERS = 10;
 
 static const bool DEFAULT_PEERBLOOMFILTERS = false;
 
+/** The minimum version for the parent chain node.
+ *  We need v0.16.2 to get the nTx field in getblockheader. */
+static const int MIN_PARENT_NODE_VERSION = 160200; // 0.16.2
+
 struct BlockHasher
 {
     size_t operator()(const uint256& hash) const { return hash.GetCheapHash(); }

src/wallet/rpcwallet.cpp

@@ -3702,7 +3702,8 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef
 
     // Additional block lee-way to avoid bitcoin block races
     if (GetBoolArg("-validatepegin", DEFAULT_VALIDATE_PEGIN)) {
-        ret.push_back(Pair("mature", IsConfirmedBitcoinBlock(merkleBlock.header.GetHash(), Params().GetConsensus().pegin_min_depth+2)));
+        ret.push_back(Pair("mature", IsConfirmedBitcoinBlock(merkleBlock.header.GetHash(), 
+			Params().GetConsensus().pegin_min_depth+2, merkleBlock.txn.GetNumTransactions())));
     }
 
     return ret;