Merge #19339: validation: re-delegate absurd fee checking from mempool to clients

b048b275d9711f70847afaea5450f17a0f7e673a [validation] Remove absurdfee from accepttomempool (John Newbery)
932564b9cfda8446a957649c2316a52e868ad5d4 scripted-diff: update max-fee-exceeded error message to include RPC (gzhao408)
8f1290c60159a3171c27250bc95687548c5c1b84 [rpc/node] check for high fee before ATMP in clients (gzhao408)

Pull request description:

  Picked up from #15810. Add separate fee-checking logic for clients that need to enforce max fee rates, then remove the `absurdFee` logic from ATMP.

  ATMP's `nAbsurdFee` argument is used to enforce user-specific behavior (it is not policy since it isn't applied consistently: it is only ever used in RPC and wallet, and set to 0 everywhere else internally). It should be removed from `AcceptToMemoryPool` because (1) validation results/mempool behavior should not be user-specific and (2) enforcing a max fee rate should be the responsibility of the client instead of the mempool.

  Note: this PR does not intend to _remove_ protection from high fees, just re-delegate the responsibility to clients.

ACKs for top commit:
  jnewbery:
    utACK b048b275d9711f70847afaea5450f17a0f7e673a
  LarryRuane:
    re-ACK b048b275d9711f70847afaea5450f17a0f7e673a
  MarcoFalke:
    re-ACK b048b275d9 , only change is squashing one commit 🏦
  instagibbs:
    utACK https://github.com/bitcoin/bitcoin/pull/19339/commits/b048b275d9711f70847afaea5450f17a0f7e673a

Tree-SHA512: 57c17ba16d230a4cae2896dd6a64c924f307757824e35784bf96da7b10aff2d8ea910710edf35e981035623a155f8766209a92a0fdb856549fde78bc3eaae4d2

1 files changed, 27 insertions, 11 deletions

diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 9ae4700743..b72f7b70e9 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -13,6 +13,18 @@
 
 #include <future>
 
+static TransactionError HandleATMPError(const TxValidationState& state, std::string& err_string_out) {
+    err_string_out = state.ToString();
+    if (state.IsInvalid()) {
+        if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
+            return TransactionError::MISSING_INPUTS;
+        }
+        return TransactionError::MEMPOOL_REJECTED;
+    } else {
+        return TransactionError::MEMPOOL_ERROR;
+    }
+}
+
 TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback)
 {
     // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs.
@@ -36,20 +48,24 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN;
     }
     if (!node.mempool->exists(hashTx)) {
-        // Transaction is not already in the mempool. Submit it.
+        // Transaction is not already in the mempool.
         TxValidationState state;
-        if (!AcceptToMemoryPool(*node.mempool, state, tx,
-                                nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) {
-            err_string = state.ToString();
-            if (state.IsInvalid()) {
-                if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
-                    return TransactionError::MISSING_INPUTS;
-                }
-                return TransactionError::MEMPOOL_REJECTED;
-            } else {
-                return TransactionError::MEMPOOL_ERROR;
+        CAmount fee{0};
+        if (max_tx_fee) {
+            // First, call ATMP with test_accept and check the fee. If ATMP
+            // fails here, return error immediately.
+            if (!AcceptToMemoryPool(*node.mempool, state, tx,
+                nullptr /* plTxnReplaced */, false /* bypass_limits */, /* test_accept */ true, &fee)) {
+                return HandleATMPError(state, err_string);
+            } else if (fee > max_tx_fee) {
+                return TransactionError::MAX_FEE_EXCEEDED;
             }
         }
+        // Try to submit the transaction to the mempool.
+        if (!AcceptToMemoryPool(*node.mempool, state, tx,
+                nullptr /* plTxnReplaced */, false /* bypass_limits */)) {
+            return HandleATMPError(state, err_string);
+        }
 
         // Transaction was accepted to the mempool.