Merge bitcoin#614: Improve double pegin claim behavior

52ef283 Improve the fedpeg test with extra double claim tests (Steven Roose)
48a82fa Make claimpegin fail in the case of a double claim (Steven Roose)
af22646 Prevent pegin double spends from entering the mempool (Steven Roose)
50c5570 Report IsValidPeginWitness error message (Steven Roose)

Pull request description:

  Fixes bitcoin#612 and bitcoin#613.

  - prevents a double claim from entering mempool
  - makes `claimpegin` throw an exception when trying to double claim a pegin
  - check the last commit message on all the extra cases that are tested.

Tree-SHA512: dd9602cd4bc78a3e8f7a6b566f881ede09dba54900425348c1eb528268a3e869416e3c9ca97c1a8da4d120875ef59c4e21a857256312ce77e06b7b6ae04abf92

src/validation.cpp

@@ -681,10 +681,27 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
         }
 
         // do all inputs exist?
-        for (const CTxIn& txin : tx.vin) {
+        for (unsigned int i = 0; i < tx.vin.size(); i++) {
+            const CTxIn& txin = tx.vin[i];
+
             // ELEMENTS:
-            // Don't look for coins that only exist in parent chain
+            // For pegin inputs check whether the pegins have already been claimed before.
+            // This only checks the UTXO set for already claimed pegins. For mempool conflicts,
+            // we rely on the GetConflictTx check done above.
             if (txin.m_is_pegin) {
+                // Quick sanity check on witness first.
+                if (tx.witness.vtxinwit.size() <= i ||
+                        tx.witness.vtxinwit[i].m_pegin_witness.stack.size() < 6 ||
+                        uint256(tx.witness.vtxinwit[i].m_pegin_witness.stack[2]).IsNull() ||
+                        tx.vin[i].prevout.hash.IsNull()) {
+                    return state.Invalid(false, REJECT_INVALID, "pegin-no-witness");
+                }
+
+                std::pair<uint256, COutPoint> pegin = std::make_pair(uint256(tx.witness.vtxinwit[i].m_pegin_witness.stack[2]), tx.vin[i].prevout);
+                // This assumes non-null prevout and genesis block hash
+                if (view.IsPeginSpent(pegin)) {
+                    return state.Invalid(false, REJECT_INVALID, "pegin-already-claimed");
+                }
                 continue;
             }
 

src/wallet/rpcwallet.cpp

@@ -5209,7 +5209,7 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef
     // We re-check depth before returning with more descriptive result
     std::string err;
     if (!IsValidPeginWitness(pegin_witness, mtx.vin[0].prevout, err, false)) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Constructed peg-in witness is invalid.");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Constructed peg-in witness is invalid: %s", err));
     }
 
     // Put input witness in transaction
@@ -5317,6 +5317,15 @@ UniValue claimpegin(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
     }
 
+    // To check if it's not double spending an existing pegin UTXO, we check mempool acceptance.
+    CValidationState acceptState;
+    bool accepted = ::AcceptToMemoryPool(mempool, acceptState, MakeTransactionRef(mtx), nullptr /* pfMissingInputs */,
+                            nullptr /* plTxnReplaced */, false /* bypass_limits */, maxTxFee, true /* test_accept */);
+    if (!accepted) {
+        std::string strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(acceptState));
+        throw JSONRPCError(RPC_WALLET_ERROR, strError);
+    }
+
     // Send it
     CValidationState state;
     mapValue_t mapValue;

test/functional/feature_fedpeg.py

@@ -12,12 +12,18 @@
     p2p_port,
     assert_raises_rpc_error,
     assert_equal,
+    bytes_to_hex_str,
 )
+from test_framework import util
 from test_framework.messages import (
+    CBlock,
     CTransaction,
     CTxInWitness,
     FromHex,
 )
+from test_framework.blocktools import (
+    add_witness_commitment,
+)
 from decimal import Decimal
 
 def get_new_unconfidential_address(node, addr_type="p2sh-segwit"):
@@ -167,6 +173,7 @@ def run_test(self):
         #parent2 = self.nodes[1]
         sidechain = self.nodes[2]
         sidechain2 = self.nodes[3]
+        util.node_fastmerkle = sidechain
 
         parent.generate(101)
         sidechain.generate(101)
@@ -214,6 +221,22 @@ def run_test(self):
 
         # 12 confirms allows in mempool
         parent.generate(1)
+
+        # Make sure that a tx with a duplicate pegin claim input gets rejected.
+        raw_pegin = sidechain.createrawpegin(raw, proof)["hex"]
+        raw_pegin = FromHex(CTransaction(), raw_pegin)
+        raw_pegin.vin.append(raw_pegin.vin[0]) # duplicate the pegin input
+        raw_pegin = sidechain.signrawtransactionwithwallet(raw_pegin.serialize().hex())["hex"]
+        assert_raises_rpc_error(-26, "bad-txns-inputs-duplicate", sidechain.sendrawtransaction, raw_pegin)
+        # Also try including this tx in a block manually and submitting it.
+        doublespendblock = FromHex(CBlock(), sidechain.getnewblockhex())
+        doublespendblock.vtx.append(FromHex(CTransaction(), raw_pegin))
+        doublespendblock.hashMerkleRoot = doublespendblock.calc_merkle_root()
+        add_witness_commitment(doublespendblock)
+        doublespendblock.solve()
+        block_hex = bytes_to_hex_str(doublespendblock.serialize(True))
+        assert_raises_rpc_error(-25, "bad-txns-inputs-duplicate", sidechain.testproposedblock, block_hex, True)
+
         # Should succeed via wallet lookup for address match, and when given
         raw_pegin = sidechain.createrawpegin(raw, proof)['hex']
         signed_pegin = sidechain.signrawtransactionwithwallet(raw_pegin)
@@ -225,6 +248,9 @@ def run_test(self):
         sample_pegin_witness = sample_pegin_struct.wit.vtxinwit[0].peginWitness
 
         pegtxid1 = sidechain.claimpegin(raw, proof)
+        # Make sure a second pegin claim does not get accepted in the mempool when
+        # another mempool tx already claims that pegin.
+        assert_raises_rpc_error(-4, "txn-mempool-conflict", sidechain.claimpegin, raw, proof)
 
         # Will invalidate the block that confirms this transaction later
         self.sync_all(self.node_groups)
@@ -254,6 +280,21 @@ def run_test(self):
         sidechain.invalidateblock(blockhash[0])
         if sidechain.gettransaction(pegtxid1)["confirmations"] != 0:
             raise Exception("Peg-in didn't unconfirm after invalidateblock call.")
+
+        # Create duplicate claim, put it in block along with current one in mempool
+        # to test duplicate-in-block claims between two txs that are in the same block.
+        raw_pegin = sidechain.createrawpegin(raw, proof)["hex"]
+        raw_pegin = sidechain.signrawtransactionwithwallet(raw_pegin)["hex"]
+        raw_pegin = FromHex(CTransaction(), raw_pegin)
+        doublespendblock = FromHex(CBlock(), sidechain.getnewblockhex())
+        assert(len(doublespendblock.vtx) == 2) # coinbase and pegin
+        doublespendblock.vtx.append(raw_pegin)
+        doublespendblock.hashMerkleRoot = doublespendblock.calc_merkle_root()
+        add_witness_commitment(doublespendblock)
+        doublespendblock.solve()
+        block_hex = bytes_to_hex_str(doublespendblock.serialize(True))
+        assert_raises_rpc_error(-25, "bad-txns-double-pegin", sidechain.testproposedblock, block_hex, True)
+
         # Re-enters block
         sidechain.generate(1)
         if sidechain.gettransaction(pegtxid1)["confirmations"] != 1:
@@ -262,6 +303,19 @@ def run_test(self):
         if sidechain.gettransaction(pegtxid1)["confirmations"] != 6:
             raise Exception("Peg-in should be back to 6 confirms.")
 
+        # Now the pegin is already claimed in a confirmed tx.
+        # In that case, a duplicate claim should (1) not be accepted in the mempool
+        # and (2) not be accepted in a block.
+        assert_raises_rpc_error(-4, "pegin-already-claimed", sidechain.claimpegin, raw, proof)
+        # For case (2), manually craft a block and include the tx.
+        doublespendblock = FromHex(CBlock(), sidechain.getnewblockhex())
+        doublespendblock.vtx.append(raw_pegin)
+        doublespendblock.hashMerkleRoot = doublespendblock.calc_merkle_root()
+        add_witness_commitment(doublespendblock)
+        doublespendblock.solve()
+        block_hex = bytes_to_hex_str(doublespendblock.serialize(True))
+        assert_raises_rpc_error(-25, "bad-txns-double-pegin", sidechain.testproposedblock, block_hex, True)
+
         # Do multiple claims in mempool
         n_claims = 6