diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 400329966..461e8025d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -38,11 +39,11 @@ #include -/** High fee for sendrawtransaction and testmempoolaccept. - * By default, transaction with a fee higher than this will be rejected by the - * RPCs. This can be overridden with the maxfeerate argument. +/** High fee rate for sendrawtransaction and testmempoolaccept. + * By default, transaction with a fee rate higher than this will be rejected by + * the RPCs. This can be overridden with the maxfeerate argument. */ -constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10}; +static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) { @@ -775,7 +776,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB.\nSet to 0 to accept any fee rate.\n"}, }, @@ -805,19 +806,17 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + std::string err_string; AssertLockNotHeld(cs_main); const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); @@ -841,7 +840,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, }, RPCResult{ "[ (array) The result of the mempool acceptance test for each raw transaction in the input array.\n" @@ -881,19 +880,17 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) CTransactionRef tx(MakeTransactionRef(std::move(mtx))); const uint256& tx_hash = tx->GetHash(); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + UniValue result(UniValue::VARR); UniValue result_0(UniValue::VOBJ); result_0.pushKV("txid", tx_hash.GetHex()); diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index b9db61857..d2826dd1b 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -257,7 +257,7 @@ class SegWitTest(BitcoinTestFramework): tx.vin.append(CTxIn(COutPoint(int(txid2, 16), 0), b"")) tx.vout.append(CTxOut(int(49.95 * COIN), CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))) # Huge fee tx.calc_sha256() - txid3 = self.nodes[0].sendrawtransaction(ToHex(tx)) + txid3 = self.nodes[0].sendrawtransaction(ToHex(tx), 0) assert tx.wit.is_null() assert txid3 in self.nodes[0].getrawmempool() diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index e19e249f2..dee7a0451 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -183,6 +183,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': True}], rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) self.log.info('A transaction with no outputs') diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 433867527..d1ce97eff 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -432,17 +432,18 @@ class RawTransactionsTest(BitcoinTestFramework): self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + # Test a transaction with small fee txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) rawTx = self.nodes[0].getrawtransaction(txId, True) vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() inputs = [{ "txid" : txId, "vout" : vout['n'] }] - outputs = { self.nodes[0].getnewaddress() : Decimal("0.99999000") } # 1000 sat fee + outputs = { self.nodes[0].getnewaddress() : Decimal("0.999990000") } # 10000 sat fee rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) - # 1000 sat fee, ~100 b transaction, fee rate should land around 10 sat/b = 0.00010000 BTC/kB + # 10000 sat fee, ~100 b transaction, fee rate should land around 100 sat/b = 0.00100000 BTC/kB # Thus, testmempoolaccept should reject testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0] assert_equal(testres['allowed'], False) @@ -450,9 +451,32 @@ class RawTransactionsTest(BitcoinTestFramework): # and sendrawtransaction should throw assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) # And below calls should both succeed - testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.00070000')[0] + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0] assert_equal(testres['allowed'], True) - self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.00070000') + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex']) + + # Test a transaction with large fee + txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) + rawTx = self.nodes[0].getrawtransaction(txId, True) + vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) + + self.sync_all() + inputs = [{ "txid" : txId, "vout" : vout['n'] }] + outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") } # 2000000 sat fee + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) + assert_equal(rawTxSigned['complete'], True) + # 2000000 sat fee, ~100 b transaction, fee rate should land around 20000 sat/b = 0.20000000 BTC/kB + # Thus, testmempoolaccept should reject + testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0] + assert_equal(testres['allowed'], False) + assert_equal(testres['reject-reason'], '256: absurdly-high-fee') + # and sendrawtransaction should throw + assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex']) + # And below calls should both succeed + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0] + assert_equal(testres['allowed'], True) + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000') if __name__ == '__main__': diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ce0411024..2283bff2e 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -433,7 +433,7 @@ class WalletTest(BitcoinTestFramework): # Split into two chains rawtx = self.nodes[0].createrawtransaction([{"txid": singletxid, "vout": 0}], {chain_addrs[0]: node0_balance / 2 - Decimal('0.01'), chain_addrs[1]: node0_balance / 2 - Decimal('0.01')}) signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) - singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"]) + singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"], 0) self.nodes[0].generate(1) # Make a long chain of unconfirmed payments without hitting mempool limit