[rpc/node] check for high fee before ATMP in clients

Check absurd fee in BroadcastTransaction and RPC,
return TransactionError::MAX_FEE_EXCEEDED instead
of TxValidationResult::TX_NOT_STANDARD because this
is client preference, not a node-wide policy.

Author: glozow

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 */, /* absurdfee*/ 0, /* 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 */, max_tx_fee)) {
+            return HandleATMPError(state, err_string);
+        }
 
         // Transaction was accepted to the mempool.
 

src/rpc/rawtransaction.cpp

@@ -947,12 +947,20 @@ static RPCHelpMan testmempoolaccept()
 
     TxValidationState state;
     bool test_accept_res;
-    CAmount fee;
+    CAmount fee{0};
     {
         LOCK(cs_main);
         test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx),
             nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true, &fee);
     }
+
+    // Check that fee does not exceed maximum fee
+    if (test_accept_res && max_raw_tx_fee && fee > max_raw_tx_fee) {
+        result_0.pushKV("allowed", false);
+        result_0.pushKV("reject-reason", "max-fee-exceeded");
+        result.push_back(std::move(result_0));
+        return result;
+    }
     result_0.pushKV("allowed", test_accept_res);
 
     // Only return the fee and vsize if the transaction would pass ATMP.

src/validation.cpp

@@ -730,8 +730,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false;
 
     if (nAbsurdFee && nFees > nAbsurdFee)
-        return state.Invalid(TxValidationResult::TX_NOT_STANDARD,
-                "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee));
+        LogPrintf("Ignoring Absurdfee\n");
 
     const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts);
     // Calculate in-mempool ancestors, up to a limit.

test/functional/rpc_rawtransaction.py

@@ -456,9 +456,9 @@ def run_test(self):
         # Thus, testmempoolaccept should reject
         testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0]
         assert_equal(testres['allowed'], False)
-        assert_equal(testres['reject-reason'], 'absurdly-high-fee')
+        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
         # and sendrawtransaction should throw
-        assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
+        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by -maxtxfee', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
         # and the following calls should both succeed
         testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0]
         assert_equal(testres['allowed'], True)
@@ -480,9 +480,9 @@ def run_test(self):
         # Thus, testmempoolaccept should reject
         testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0]
         assert_equal(testres['allowed'], False)
-        assert_equal(testres['reject-reason'], 'absurdly-high-fee')
+        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
         # and sendrawtransaction should throw
-        assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
+        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by -maxtxfee', self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
         # and the following calls should both succeed
         testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0]
         assert_equal(testres['allowed'], True)