MERGE FIX: Augment IsValidPeginWitness with err string return

src/consensus/tx_verify.cpp

@@ -159,7 +159,8 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
 
     for (unsigned int i = 0; i < tx.vin.size(); i++)
     {
-        if (tx.vin[i].m_is_pegin && !IsValidPeginWitness(tx.vin[i].m_pegin_witness, tx.vin[i].prevout)) {
+        std::string err;
+        if (tx.vin[i].m_is_pegin && !IsValidPeginWitness(tx.vin[i].m_pegin_witness, tx.vin[i].prevout, err, true)) {
             continue;
         }
 
@@ -239,7 +240,8 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
         const COutPoint &prevout = tx.vin[i].prevout;
         if (tx.vin[i].m_is_pegin) {
             // Check existence and validity of pegin witness
-            if (!IsValidPeginWitness(tx.vin[i].m_pegin_witness, prevout)) {
+            std::string err;
+            if (!IsValidPeginWitness(tx.vin[i].m_pegin_witness, prevout, err, true)) {
                 return state.DoS(0, false, REJECT_PEGIN, "bad-pegin-witness");
             }
             std::pair<uint256, COutPoint> pegin = std::make_pair(uint256(tx.vin[i].m_pegin_witness.stack[2]), prevout);

src/pegins.cpp

@@ -228,9 +228,10 @@ bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::P
     return true;
 }
 
-bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, bool check_depth) {
+bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, std::string& err_msg, bool check_depth) {
     // 0) Return false if !consensus.has_parent_chain
     if (!Params().GetConsensus().has_parent_chain) {
+        err_msg = "Parent chain is not enabled on this network.";
         return false;
     }
 
@@ -249,6 +250,7 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     const std::vector<std::vector<unsigned char> >& stack = pegin_witness.stack;
     // Must include all elements
     if (stack.size() != 6) {
+        err_msg = "Not enough stack items.";
         return false;
     }
 
@@ -257,15 +259,18 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     try {
         stream >> value;
     } catch (...) {
+        err_msg = "Could not deserialize value.";
         return false;
     }
 
     if (!MoneyRange(value)) {
+        err_msg = "Value was not in valid value range.";
         return false;
     }
 
     // Get asset type
     if (stack[1].size() != 32) {
+        err_msg = "Asset type was not 32 bytes.";
         return false;
     }
     //TODO(rebase) CA
@@ -273,6 +278,7 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
 
     // Get genesis blockhash
     if (stack[2].size() != 32) {
+        err_msg = "Parent genesis blockchaash was not 32 bytes.";
         return false;
     }
     uint256 gen_hash(stack[2]);
@@ -280,6 +286,7 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     // Get claim_script, sanity check size
     CScript claim_script(stack[3].begin(), stack[3].end());
     if (claim_script.size() > 100) {
+        err_msg = "Claim script is too large.";
         return false;
     }
 
@@ -290,14 +297,17 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     if (Params().GetConsensus().ParentChainHasPow()) {
         Sidechain::Bitcoin::CMerkleBlock merkle_block_pow;
         if (!GetBlockAndTxFromMerkleBlock(block_hash, tx_hash, merkle_block_pow, stack[5])) {
+            err_msg = "Could not extract block and tx from merkleblock.";
             return false;
         }
         if (!CheckParentProofOfWork(block_hash, merkle_block_pow.header.nBits, Params().GetConsensus())) {
+            err_msg = "Parent proof of work is invalid or insufficient.";
             return false;
         }
 
         Sidechain::Bitcoin::CTransactionRef pegtx;
         if (!CheckPeginTx(stack[4], pegtx, prevout, value, claim_script)) {
+            err_msg = "Peg-in tx is invalid.";
             return false;
         }
 
@@ -305,15 +315,18 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     } else {
         CMerkleBlock merkle_block;
         if (!GetBlockAndTxFromMerkleBlock(block_hash, tx_hash, merkle_block, stack[5])) {
+            err_msg = "Could not extract block and tx from merkleblock.";
             return false;
         }
 
         if (!CheckProofSignedParent(merkle_block.header, Params().GetConsensus())) {
+            err_msg = "Parent signed block is invalid.";
             return false;
         }
 
         CTransactionRef pegtx;
         if (!CheckPeginTx(stack[4], pegtx, prevout, value, claim_script)) {
+            err_msg = "Peg-in tx is invalid.";
             return false;
         }
 
@@ -322,11 +335,13 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
 
     // Check that the merkle proof corresponds to the txid
     if (prevout.hash != tx_hash) {
+        err_msg = "Merkle proof and txid mismatch.";
         return false;
     }
 
     // Check the genesis block corresponds to a valid peg (only one for now)
     if (gen_hash != Params().ParentGenesisBlockHash()) {
+        err_msg = "Parent genesis block mismatch.";
         return false;
     }
 
@@ -339,6 +354,7 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& p
     // Finally, validate peg-in via rpc call
     if (check_depth && gArgs.GetBoolArg("-validatepegin", DEFAULT_VALIDATE_PEGIN)) {
         if (!IsConfirmedBitcoinBlock(block_hash, Params().GetConsensus().pegin_min_depth, num_txs)) {
+            err_msg = "Needs more confirmations.";
             return false;
         }
     }

src/pegins.h

@@ -18,7 +18,7 @@ bool GetAmountFromParentChainPegin(CAmount& amount, const CTransaction& txBTC, u
 /** Check whether a parent chain block hash satisfies the proof-of-work requirement specified by nBits */
 bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params&);
 /** Checks pegin witness for validity */
-bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, bool check_depth = true);
+bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const COutPoint& prevout, std::string& err_msg, bool check_depth);
 // Constructs unblinded output to be used in amount and scriptpubkey checks during pegin
 CTxOut GetPeginOutputFromWitness(const CScriptWitness& pegin_witness);
 

src/rpc/rawtransaction.cpp

@@ -878,15 +878,17 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         CTxIn& txin = mtx.vin[i];
         const Coin& coin = view.AccessCoin(txin.prevout);
 
+        std::string err;
         if (!txin.m_is_pegin && coin.IsSpent()) {
             TxInErrorToJSON(txin, vErrors, "Input not found or already spent");
             continue;
-        } else if (txin.m_is_pegin && (!IsValidPeginWitness(txConst.vin[i].m_pegin_witness, txin.prevout, false))) {
+        } else if (txin.m_is_pegin && (!IsValidPeginWitness(txConst.vin[i].m_pegin_witness, txin.prevout, err, false))) {
-            TxInErrorToJSON(txin, vErrors, "Peg-in input has invalid proof.");
+            TxInErrorToJSON(txin, vErrors, "Peg-in witness error: " + err);
             continue;
         }
         // Report warning about immature peg-in though
-        if(txin.m_is_pegin && !IsValidPeginWitness(txConst.vin[i].m_pegin_witness, txin.prevout, true)) {
+        if(txin.m_is_pegin && !IsValidPeginWitness(txConst.vin[i].m_pegin_witness, txin.prevout, err, true)) {
+            assert(err == "Needs more confirmations.");
             immature_pegin = true;
         }
 

src/test/pegin_witness_tests.cpp

@@ -49,7 +49,10 @@ BOOST_AUTO_TEST_CASE(witness_valid)
     CScriptWitness witness;
     witness.stack = witness_stack;
 
-    BOOST_CHECK(IsValidPeginWitness(witness, prevout));
+    std::string err;
+
+    bool valid = IsValidPeginWitness(witness, prevout, err, false);
+    BOOST_CHECK(err == "");
 
     // Missing byte on each field to make claim ill-formatted
     // This will break deserialization and other data-matching checks
@@ -59,32 +62,32 @@ BOOST_AUTO_TEST_CASE(witness_valid)
             continue;
         }
         witness.stack[i].pop_back();
-        BOOST_CHECK(!IsValidPeginWitness(witness, prevout));
+        BOOST_CHECK(!IsValidPeginWitness(witness, prevout, err, false));
         witness.stack = witness_stack;
-        BOOST_CHECK(IsValidPeginWitness(witness, prevout));
+        BOOST_CHECK(IsValidPeginWitness(witness, prevout, err, false));
     }
 
     // Test mismatched but valid nOut to proof
     COutPoint fake_prevout = prevout;
     fake_prevout.n = 0;
-    BOOST_CHECK(!IsValidPeginWitness(witness, fake_prevout));
+    BOOST_CHECK(!IsValidPeginWitness(witness, fake_prevout, err, false));
 
     // Test mistmatched but valid txid
     fake_prevout = prevout;
     fake_prevout.hash = uint256S("2f103ee04a5649eecb932b4da4ca9977f53a12bbe04d9d1eb5ccc0f4a06334");
-    BOOST_CHECK(!IsValidPeginWitness(witness, fake_prevout));
+    BOOST_CHECK(!IsValidPeginWitness(witness, fake_prevout, err, false));
 
     // Ensure that all witness stack sizes are handled
-    BOOST_CHECK(IsValidPeginWitness(witness, prevout));
+    BOOST_CHECK(IsValidPeginWitness(witness, prevout, err, false));
     for (unsigned int i = 0; i < witness.stack.size(); i++) {
         witness.stack.pop_back();
-        BOOST_CHECK(!IsValidPeginWitness(witness, prevout));
+        BOOST_CHECK(!IsValidPeginWitness(witness, prevout, err, false));
     }
     witness.stack = witness_stack;
 
     // Extra element causes failure
     witness.stack.push_back(witness.stack.back());
-    BOOST_CHECK(!IsValidPeginWitness(witness, prevout));
+    BOOST_CHECK(!IsValidPeginWitness(witness, prevout, err, false));
     witness.stack = witness_stack;
 
     // Check validation of peg-in transaction's inputs and balance
@@ -99,7 +102,7 @@ BOOST_AUTO_TEST_CASE(witness_valid)
     // Check that serialization doesn't cause issuance to become non-null
     //TODO(rebase) CA
     //BOOST_CHECK(tx.vin[0].assetIssuance.IsNull());
-    BOOST_CHECK(IsValidPeginWitness(tx.vin[0].m_pegin_witness, prevout));
+    BOOST_CHECK(IsValidPeginWitness(tx.vin[0].m_pegin_witness, prevout, err, false));
 
     std::set<std::pair<uint256, COutPoint> > setPeginsSpent;
     CValidationState state;

src/validation.cpp

@@ -1615,7 +1615,8 @@ int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out, cons
         // it is an overwrite.
         view.AddCoin(out, std::move(undo), !fClean);
     } else {
-        if (!IsValidPeginWitness(pegin_witness, txin.prevout)) {
+        std::string err;
+        if (!IsValidPeginWitness(pegin_witness, txin.prevout, err, false)) {
             fClean = fClean && error("%s: peg-in occurred without proof", __func__);
         } else {
             std::pair<uint256, COutPoint> outpoint = std::make_pair(uint256(pegin_witness.stack[2]), txin.prevout);

src/wallet/rpcwallet.cpp

@@ -4900,7 +4900,8 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef
 
     // Peg-in witness isn't valid, even though the block header is(without depth check)
     // We re-check depth before returning with more descriptive result
-    if (!IsValidPeginWitness(pegin_witness, mtx.vin[0].prevout, false)) {
+    std::string err;
+    if (!IsValidPeginWitness(pegin_witness, mtx.vin[0].prevout, err, false)) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Constructed peg-in witness is invalid.");
     }