[mempool] Restrain carve-out acceptance to only-spending tagged output

Offchain multiparty transaction may allow one of the party to
spend exclusively one of the output. A party with the ability
to spend two or more of this kind of output can use one to
hit the descendant limit and the other one to carve-out the
transaction with a CPFP good enough to get into the mempool but
not get confirmed and so pin the parent transaction. It lets
other parties without remedy to bump parent transaction feerate
and may incumber a loss of funds on cascade of time-constrained
offchain transactions, e.g LN-channels.

This change restrain carve-out to enable it to play its end-goal
design. A CPFP to get benefits of carve-out relaxation rules
must spend first output of parent transaction.

Offchain protocols while building pre-signed transaction MUST
verify than the first output is a anyone-can-spend or than any
party is able to redeem it. This output is "tagged" by being
inserted as first one at transaction construction.

Explicitly test a non-first-output carve-out spend in
mempool_package_onemore which should fail

src/validation.cpp

@@ -760,8 +760,21 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         // to be secure by simply only having two immediately-spendable
         // outputs - one for each counterparty. For more info on the uses for
         // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
-        if (nSize >  EXTRA_DESCENDANT_TX_SIZE_LIMIT ||
-                !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) {
+
+        // Verify CPFP transaction is spending carve-out output tagged by contracting/payment channels parties to
+        // avoid a single-party spend on a N-party pre-signed transaction being used as a malicious carve-out
+        // output to stuck the parent transaction with a not-good-enough feerate CPFP.
+        // See, https://github.com/lightningnetwork/lightning-rfc/pull/688#issuecomment-549480380
+        bool fSpendsTaggedOutput = true;
+        for (const CTxIn& txin : tx.vin) {
+            if (txin.prevout.n != 0) {
+                fSpendsTaggedOutput = false;
+                break;
+            }
+        }
+
+        if (!fSpendsTaggedOutput || nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT ||
+            !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) {
             return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString);
         }
     }

test/functional/mempool_package_onemore.py

@@ -52,8 +52,8 @@ def run_test(self):
         # MAX_ANCESTORS transactions off a confirmed tx should be fine
         chain = []
         for _ in range(4):
-            (txid, sent_value) = self.chain_transaction(self.nodes[0], [txid], [vout], value, fee, 2)
-            vout = 0
+            (txid, sent_value) = self.chain_transaction(self.nodes[0], [txid], [vout], value, fee, 3)
+            vout = 1
             value = sent_value
             chain.append([txid, value])
         for _ in range(MAX_ANCESTORS - 4):
@@ -68,20 +68,22 @@ def run_test(self):
         # Adding one more transaction on to the chain should fail.
         assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_transaction, self.nodes[0], [txid], [0], value, fee, 1)
         # ...even if it chains on from some point in the middle of the chain.
-        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[2][0]], [1], chain[2][1], fee, 1)
-        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[1][0]], [1], chain[1][1], fee, 1)
+        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[2][0]], [0], chain[2][1], fee, 1)
+        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[1][0]], [0], chain[1][1], fee, 1)
         # ...even if it chains on to two parent transactions with one in the chain.
-        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0], second_chain], [1, 0], chain[0][1] + second_chain_value, fee, 1)
-        # ...especially if its > 40k weight
-        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 350)
+        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0], second_chain], [0, 0], chain[0][1] + second_chain_value, fee, 1)
+        # ...especially if it chains directly off the first transaction but > 40k weight
+        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0]], [0], chain[0][1], fee, 350)
+        # ... especially if it chains directly off the first transaction but not on the first output
+        assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0]], [2], chain[0][1], fee, 1)
         # But not if it chains directly off the first transaction
-        (replacable_txid, replacable_orig_value) = self.chain_transaction(self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 1)
+        (replacable_txid, replacable_orig_value) = self.chain_transaction(self.nodes[0], [chain[0][0]], [0], chain[0][1], fee, 1)
         # and the second chain should work just fine
         self.chain_transaction(self.nodes[0], [second_chain], [0], second_chain_value, fee, 1)
 
         # Make sure we can RBF the chain which used our carve-out rule
         second_tx_outputs = {self.nodes[0].getrawtransaction(replacable_txid, True)["vout"][0]['scriptPubKey']['addresses'][0]: replacable_orig_value - (Decimal(1) / Decimal(100))}
-        second_tx = self.nodes[0].createrawtransaction([{'txid': chain[0][0], 'vout': 1}], second_tx_outputs)
+        second_tx = self.nodes[0].createrawtransaction([{'txid': chain[0][0], 'vout': 0}], second_tx_outputs)
         signed_second_tx = self.nodes[0].signrawtransactionwithwallet(second_tx)
         self.nodes[0].sendrawtransaction(signed_second_tx['hex'])