Merge a3a4ade82c into a46065e36c

doc/release-notes-28121.md

@@ -0,0 +1,5 @@
+RPC
+---
+
+`testmempoolaccept` response will now include verbose field "debug-message"
+similar to the complete error messages returned by `sendrawtransaction` (#28121)

src/policy/rbf.cpp

@@ -71,7 +71,7 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
         // descendants (i.e. if multiple conflicts share a descendant, it will be counted multiple
         // times), but we just want to be conservative to avoid doing too much work.
         if (nConflictingCount > MAX_REPLACEMENT_CANDIDATES) {
-            return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
+            return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)",
                              txid.ToString(),
                              nConflictingCount,
                              MAX_REPLACEMENT_CANDIDATES);

src/rpc/mempool.cpp

@@ -141,6 +141,7 @@ static RPCHelpMan testmempoolaccept()
                         }},
                     }},
                     {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"},
+                    {RPCResult::Type::STR, "debug-message", /*optional=*/true, "Rejection details (only present when 'allowed' is false and a message is provided)"},
                 }},
             }
         },
@@ -239,6 +240,7 @@ static RPCHelpMan testmempoolaccept()
                         result_inner.pushKV("reject-reason", "missing-inputs");
                     } else {
                         result_inner.pushKV("reject-reason", state.GetRejectReason());
+                        if (!state.GetDebugMessage().empty()) result_inner.pushKV("debug-message", state.GetDebugMessage());
                     }
                 }
                 rpc_result.push_back(result_inner);

test/functional/feature_rbf.py

@@ -111,14 +111,22 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         """Simple doublespend"""
         # we use MiniWallet to create a transaction template with inputs correctly set,
         # and modify the output (amount, scriptPubKey) according to our needs
-        tx = self.wallet.create_self_transfer()["tx"]
+        tx = self.wallet.create_self_transfer(fee_rate=Decimal("0.003"))["tx"]
         tx1a_txid = self.nodes[0].sendrawtransaction(tx.serialize().hex())
 
         # Should fail because we haven't changed the fee
         tx.vout[0].scriptPubKey[-1] ^= 1
+        tx.rehash()
+        tx_hex = tx.serialize().hex()
 
         # This will raise an exception due to insufficient fee
-        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex(), 0)
+        reject_reason = "insufficient fee"
+        debug_message = f"rejecting replacement {tx.hash}; new feerate 0.00300000 BTC/kvB <= old feerate 0.00300000 BTC/kvB"
+        assert_raises_rpc_error(-26, f"{reject_reason}, {debug_message}", self.nodes[0].sendrawtransaction, tx_hex, 0)
+
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx_hex])[0]
+        assert res["reject-reason"] == reject_reason
+        assert res["debug-message"] == debug_message
 
         # Extra 0.1 BTC fee
         tx.vout[0].nValue -= int(0.1 * COIN)
@@ -162,7 +170,14 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         dbl_tx_hex = dbl_tx.serialize().hex()
 
         # This will raise an exception due to insufficient fee
-        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0)
+        reject_reason = "insufficient fee"
+        debug_message = f"rejecting replacement {dbl_tx.hash}, less fees than conflicting txs; 3.00 < 4.00"
+        assert_raises_rpc_error(-26, f"{reject_reason}, {debug_message}", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0)
+
+        res = self.nodes[0].testmempoolaccept(rawtxs=[dbl_tx_hex])[0]
+        assert res["reject-reason"] == reject_reason
+        assert res["debug-message"] == debug_message
+
 
         # Accepted with sufficient fee
         dbl_tx.vout[0].nValue = int(0.1 * COIN)
@@ -281,22 +296,30 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         utxo1 = self.make_utxo(self.nodes[0], int(1.2 * COIN))
         utxo2 = self.make_utxo(self.nodes[0], 3 * COIN)
 
-        tx1a_utxo = self.wallet.send_self_transfer(
+        tx1a = self.wallet.send_self_transfer(
             from_node=self.nodes[0],
             utxo_to_spend=utxo1,
             sequence=0,
             fee=Decimal("0.1"),
-        )["new_utxo"]
+        )
+        tx1a_utxo = tx1a["new_utxo"]
 
         # Direct spend an output of the transaction we're replacing.
-        tx2_hex = self.wallet.create_self_transfer_multi(
+        tx2 = self.wallet.create_self_transfer_multi(
             utxos_to_spend=[utxo1, utxo2, tx1a_utxo],
             sequence=0,
             amount_per_output=int(COIN * tx1a_utxo["value"]),
-        )["hex"]
+        )["tx"]
+        tx2_hex = tx2.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+        reject_reason = "bad-txns-spends-conflicting-tx"
+        debug_message = f"{tx2.hash} spends conflicting transaction {tx1a['tx'].hash}"
+        assert_raises_rpc_error(-26, f"{reject_reason}, {debug_message}", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx2_hex])[0]
+        assert res["reject-reason"] == reject_reason
+        assert res["debug-message"] == debug_message
 
         # Spend tx1a's output to test the indirect case.
         tx1b_utxo = self.wallet.send_self_transfer(
@@ -327,14 +350,22 @@ class ReplaceByFeeTest(BitcoinTestFramework):
             fee=Decimal("0.1"),
         )
 
-        tx2_hex = self.wallet.create_self_transfer_multi(
+        tx2 = self.wallet.create_self_transfer_multi(
             utxos_to_spend=[confirmed_utxo, unconfirmed_utxo],
             sequence=0,
             amount_per_output=1 * COIN,
-        )["hex"]
+        )["tx"]
+        tx2_hex = tx2.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "replacement-adds-unconfirmed", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+        reject_reason = "replacement-adds-unconfirmed"
+        debug_message = f"replacement {tx2.hash} adds unconfirmed input, idx 1"
+        assert_raises_rpc_error(-26, f"{reject_reason}, {debug_message}", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+
+
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx2_hex])[0]
+        assert res["reject-reason"] == reject_reason
+        assert res["debug-message"] == debug_message
 
     def test_too_many_replacements(self):
         """Replacements that evict too many transactions are rejected"""
@@ -376,7 +407,13 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         double_tx_hex = double_tx.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "too many potential replacements", self.nodes[0].sendrawtransaction, double_tx_hex, 0)
+        reject_reason = "too many potential replacements"
+        debug_message = f"rejecting replacement {double_tx.hash}; too many potential replacements ({MAX_REPLACEMENT_LIMIT + 1} > {MAX_REPLACEMENT_LIMIT})"
+        assert_raises_rpc_error(-26, f"{reject_reason}, {debug_message}", self.nodes[0].sendrawtransaction, double_tx_hex, 0)
+
+        res = self.nodes[0].testmempoolaccept(rawtxs=[double_tx_hex])[0]
+        assert res["reject-reason"] == reject_reason
+        assert res["debug-message"] == debug_message
 
         # If we remove an input, it should pass
         double_tx.vin.pop()

test/functional/feature_reindex_readonly.py

@@ -75,7 +75,7 @@ class BlockstoreReindexTest(BitcoinTestFramework):
 
         if undo_immutable:
             self.log.debug("Attempt to restart and reindex the node with the unwritable block file")
-            with self.nodes[0].wait_for_debug_log([b"Reindexing finished"]):
+            with self.nodes[0].assert_debug_log(["Reindexing finished"]):
                 self.start_node(0, extra_args=['-reindex', '-fastprune'])
             assert block_count == self.nodes[0].getblockcount()
             undo_immutable()