diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 9da8a56797..6e1633073e 100644 --- a/src/consensus/tx_verify.cpp +++ b/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 pegin = std::make_pair(uint256(tx.vin[i].m_pegin_witness.stack[2]), prevout); diff --git a/src/pegins.cpp b/src/pegins.cpp index 8561a94eb1..b7a457e33a 100644 --- a/src/pegins.cpp +++ b/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 >& 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; } } diff --git a/src/pegins.h b/src/pegins.h index 2a860a77ae..aa3f4fbe43 100644 --- a/src/pegins.h +++ b/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); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index baf2f7b720..d85ed13956 100644 --- a/src/rpc/rawtransaction.cpp +++ b/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))) { - TxInErrorToJSON(txin, vErrors, "Peg-in input has invalid proof."); + } else if (txin.m_is_pegin && (!IsValidPeginWitness(txConst.vin[i].m_pegin_witness, txin.prevout, err, false))) { + 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; } diff --git a/src/test/pegin_witness_tests.cpp b/src/test/pegin_witness_tests.cpp index 27f7dbbfa2..41fd59f103 100644 --- a/src/test/pegin_witness_tests.cpp +++ b/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 > setPeginsSpent; CValidationState state; diff --git a/src/validation.cpp b/src/validation.cpp index 52476a2808..3b27f6341f 100644 --- a/src/validation.cpp +++ b/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 outpoint = std::make_pair(uint256(pegin_witness.stack[2]), txin.prevout); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9554bc3d1a..109307e7cb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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."); }