diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -88,8 +88,10 @@ {"signrawtransactionwithkey", 2, "prevtxs"}, {"signrawtransactionwithwallet", 1, "prevtxs"}, {"sendrawtransaction", 1, "allowhighfees"}, + {"sendrawtransaction", 1, "maxfeerate"}, {"testmempoolaccept", 0, "rawtxs"}, {"testmempoolaccept", 1, "allowhighfees"}, + {"testmempoolaccept", 1, "maxfeerate"}, {"combinerawtransaction", 0, "txs"}, {"fundrawtransaction", 1, "options"}, {"walletcreatefundedpsbt", 0, "inputs"}, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -907,8 +908,11 @@ { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"allowhighfees", RPCArg::Type::BOOL, /* default */ "false", - "Allow high fees"}, + {"maxfeerate", RPCArg::Type::AMOUNT, + /* default */ FormatMoney(maxTxFee), + "Reject transactions whose fee rate is higher than the " + "specified value, expressed in " + + CURRENCY_UNIT + "/kB\n"}, }, RPCResult{ "\"hex\" (string) The transaction hash in hex\n"}, @@ -928,7 +932,11 @@ .ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL}); + RPCTypeCheck(request.params, { + UniValue::VSTR, + // NUM or BOOL, checked later + UniValueType(), + }); // parse hex string from parameter CMutableTransaction mtx; @@ -938,15 +946,26 @@ CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - bool allowhighfees = false; - if (!request.params[1].isNull()) { - allowhighfees = request.params[1].get_bool(); + Amount max_raw_tx_fee = maxTxFee; + // TODO: temporary migration code for old clients. Remove in v0.22 + 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].isNum()) { + size_t sz = tx->GetTotalSize(); + CFeeRate fr(AmountFromValue(request.params[1])); + max_raw_tx_fee = fr.GetFee(sz); + } else if (!request.params[1].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "second argument (maxfeerate) must be numeric"); } - const Amount highfee{allowhighfees ? Amount::zero() : ::maxTxFee}; + TxId txid; std::string err_string; const TransactionError err = - BroadcastTransaction(config, tx, txid, err_string, highfee); + BroadcastTransaction(config, tx, txid, err_string, max_raw_tx_fee); if (err != TransactionError::OK) { throw JSONRPCTransactionError(err, err_string); } @@ -978,8 +997,11 @@ RPCArg::Optional::OMITTED, ""}, }, }, - {"allowhighfees", RPCArg::Type::BOOL, /* default */ "false", - "Allow high fees"}, + {"maxfeerate", RPCArg::Type::AMOUNT, + /* default */ FormatMoney(maxTxFee), + "Reject transactions whose fee rate is higher than the " + "specified value, expressed in " + + CURRENCY_UNIT + "/kB\n"}, }, RPCResult{ "[ (array) The result of the mempool " @@ -1009,7 +1031,12 @@ .ToString()); } - RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VBOOL}); + RPCTypeCheck(request.params, { + UniValue::VARR, + // NUM or BOOL, checked later + UniValueType(), + }); + if (request.params[0].get_array().size() != 1) { throw JSONRPCError( RPC_INVALID_PARAMETER, @@ -1021,11 +1048,22 @@ throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); } CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - const uint256 &txid = tx->GetId(); + const TxId &txid = tx->GetId(); Amount max_raw_tx_fee = maxTxFee; - if (!request.params[1].isNull() && request.params[1].get_bool()) { - max_raw_tx_fee = Amount::zero(); + // 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].isNum()) { + size_t sz = tx->GetTotalSize(); + CFeeRate fr(AmountFromValue(request.params[1])); + max_raw_tx_fee = fr.GetFee(sz); + } else if (!request.params[1].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "second argument (maxfeerate) must be numeric"); } UniValue result(UniValue::VARR); @@ -1926,10 +1964,10 @@ { "rawtransactions", "createrawtransaction", createrawtransaction, {"inputs","outputs","locktime"} }, { "rawtransactions", "decoderawtransaction", decoderawtransaction, {"hexstring"} }, { "rawtransactions", "decodescript", decodescript, {"hexstring"} }, - { "rawtransactions", "sendrawtransaction", sendrawtransaction, {"hexstring","allowhighfees"} }, + { "rawtransactions", "sendrawtransaction", sendrawtransaction, {"hexstring","allowhighfees|maxfeerate"} }, { "rawtransactions", "combinerawtransaction", combinerawtransaction, {"txs"} }, { "rawtransactions", "signrawtransactionwithkey", signrawtransactionwithkey, {"hexstring","privkeys","prevtxs","sighashtype"} }, - { "rawtransactions", "testmempoolaccept", testmempoolaccept, {"rawtxs","allowhighfees"} }, + { "rawtransactions", "testmempoolaccept", testmempoolaccept, {"rawtxs","allowhighfees|maxfeerate"} }, { "rawtransactions", "decodepsbt", decodepsbt, {"psbt"} }, { "rawtransactions", "combinepsbt", combinepsbt, {"txs"} }, { "rawtransactions", "finalizepsbt", finalizepsbt, {"psbt", "extract"} }, diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -168,7 +168,7 @@ [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Negative locktime)'}], self.nodes[0].testmempoolaccept( - rawtxs=[spendtx.serialize().hex()], allowhighfees=True) + rawtxs=[spendtx.serialize().hex()], maxfeerate=0) ) rejectedtx_signed = self.nodes[0].signrawtransactionwithwallet( diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -8,7 +8,7 @@ """ from test_framework.blocktools import create_block, create_coinbase, create_transaction -from test_framework.messages import msg_block, ToHex +from test_framework.messages import msg_block from test_framework.mininode import ( mininode_lock, P2PInterface, @@ -90,7 +90,7 @@ [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '16: mandatory-script-verify-flag-failed (Non-canonical DER signature)'}], self.nodes[0].testmempoolaccept( - rawtxs=[ToHex(spendtx)], allowhighfees=True) + rawtxs=[spendtx.serialize().hex()], maxfeerate=0) ) # Now we verify that a block with this transaction is also invalid. diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -71,7 +71,7 @@ outputs=[{node.getnewaddress(): 0.3}, {node.getnewaddress(): 49}], ))['hex'] txid_in_block = node.sendrawtransaction( - hexstring=raw_tx_in_block, allowhighfees=True) + hexstring=raw_tx_in_block, maxfeerate=0) node.generate(1) self.mempool_size = 0 self.check_mempool_result( @@ -106,10 +106,10 @@ tx = FromHex(CTransaction(), raw_tx_final) self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': True}], - rawtxs=[ToHex(tx)], - allowhighfees=True, + rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) - node.sendrawtransaction(hexstring=raw_tx_final, allowhighfees=True) + node.sendrawtransaction(hexstring=raw_tx_final, maxfeerate=0) self.mempool_size += 1 self.log.info('A transaction in the mempool') @@ -127,17 +127,17 @@ self.log.info('A transaction that conflicts with an unconfirmed tx') # Send the transaction that conflicts with the mempool transaction - node.sendrawtransaction( - hexstring=ToHex(tx), allowhighfees=True) + node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0) # take original raw_tx_0 tx = FromHex(CTransaction(), raw_tx_0) tx.vout[0].nValue -= int(4 * fee * COIN) # Set more fee # skip re-signing the tx self.check_mempool_result( - result_expected=[{'txid': tx.rehash( - ), 'allowed': False, 'reject-reason': '18: txn-mempool-conflict'}], - rawtxs=[ToHex(tx)], - allowhighfees=True, + result_expected=[{'txid': tx.rehash(), + 'allowed': False, + 'reject-reason': '18: txn-mempool-conflict'}], + rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) self.log.info('A transaction with missing inputs, that never existed') @@ -158,8 +158,7 @@ tx.vin[0].prevout.n = 1 raw_tx_1 = node.signrawtransactionwithwallet( ToHex(tx))['hex'] - txid_1 = node.sendrawtransaction( - hexstring=raw_tx_1, allowhighfees=True) + txid_1 = node.sendrawtransaction(hexstring=raw_tx_1, maxfeerate=0) # Now spend both to "clearly hide" the outputs, ie. remove the coins # from the utxo set by spending them raw_tx_spend_both = node.signrawtransactionwithwallet(node.createrawtransaction( @@ -170,7 +169,7 @@ outputs=[{node.getnewaddress(): 0.1}] ))['hex'] txid_spend_both = node.sendrawtransaction( - hexstring=raw_tx_spend_both, allowhighfees=True) + hexstring=raw_tx_spend_both, maxfeerate=0) node.generate(1) self.mempool_size = 0 # Now see if we can add the coins back to the utxo set by sending the @@ -338,10 +337,11 @@ tx.vin[0].nSequence = 2 # Can skip re-signing the tx because of early rejection self.check_mempool_result( - result_expected=[ - {'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: non-BIP68-final'}], - rawtxs=[ToHex(tx)], - allowhighfees=True, + result_expected=[{'txid': tx.rehash(), + 'allowed': False, + 'reject-reason': '64: non-BIP68-final'}], + rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -93,7 +93,7 @@ rawtx2["hex"], [self.priv[-1]], prevtxs) self.moved += outval - tx = node0.sendrawtransaction(rawtx3["hex"], True) + tx = node0.sendrawtransaction(rawtx3["hex"], 0) blk = node0.generate(1)[0] assert tx in node0.getblock(blk)["tx"] diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -335,11 +335,8 @@ txDetails = self.nodes[0].gettransaction(txId, True) rawTx = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = False - for outpoint in rawTx['vout']: - if outpoint['value'] == Decimal('2.20000000'): - vout = outpoint - break + vout = next(o for o in rawTx['vout'] + if o['value'] == Decimal('2.20000000')) bal = self.nodes[0].getbalance() inputs = [{ @@ -394,11 +391,8 @@ txDetails = self.nodes[0].gettransaction(txId, True) rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = False - for outpoint in rawTx2['vout']: - if outpoint['value'] == Decimal('2.20000000'): - vout = outpoint - break + vout = next(o for o in rawTx2['vout'] + if o['value'] == Decimal('2.20000000')) bal = self.nodes[0].getbalance() inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey'] @@ -537,6 +531,40 @@ decrawtx = self.nodes[0].decoderawtransaction(rawtx) assert_equal(decrawtx['version'], 0x7fffffff) + self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + + 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']}] + # 1000 sat fee + outputs = {self.nodes[0].getnewaddress(): Decimal("0.99999000")} + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) + assert_equal(rawTxSigned['complete'], True) + # 1000 sat fee, ~200 b transaction, fee rate should land around 5 sat/b = 0.00005000 BTC/kB + # Thus, testmempoolaccept should reject + testres = self.nodes[2].testmempoolaccept( + [rawTxSigned['hex']], 0.00001000)[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'], + 0.00001000) + # And below calls should both succeed + testres = self.nodes[2].testmempoolaccept( + rawtxs=[rawTxSigned['hex']], maxfeerate=0.00007000)[0] + assert_equal(testres['allowed'], True) + self.nodes[2].sendrawtransaction( + hexstring=rawTxSigned['hex'], + maxfeerate=0.00007000) + ########################################## # Decoding weird scripts in transactions # ########################################## diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -226,6 +226,6 @@ ctx.vout[0].nValue -= int(fee_multiplier * node.calculate_fee(ctx)) signresult = node.signrawtransactionwithwallet( ToHex(ctx), None, "NONE|FORKID") - txid = node.sendrawtransaction(signresult["hex"], True) + txid = node.sendrawtransaction(signresult["hex"], 0) txids.append(txid) return txids diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -538,7 +538,7 @@ rawtx = from_node.createrawtransaction(inputs, outputs) signresult = from_node.signrawtransactionwithwallet(rawtx) - txid = from_node.sendrawtransaction(signresult["hex"], True) + txid = from_node.sendrawtransaction(signresult["hex"], 0) return (txid, signresult["hex"], fee) @@ -584,7 +584,7 @@ newtx = newtx + rawtx[94:] signresult = node.signrawtransactionwithwallet( newtx, None, "NONE|FORKID") - txid = node.sendrawtransaction(signresult["hex"], True) + txid = node.sendrawtransaction(signresult["hex"], 0) txids.append(txid) return txids diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -167,8 +167,8 @@ self.nodes[0].signrawtransactionwithwallet(raw_tx)) # Have node 1 (miner) send the transactions - self.nodes[1].sendrawtransaction(txns_to_send[0]["hex"], True) - self.nodes[1].sendrawtransaction(txns_to_send[1]["hex"], True) + self.nodes[1].sendrawtransaction(txns_to_send[0]["hex"], 0) + self.nodes[1].sendrawtransaction(txns_to_send[1]["hex"], 0) # Have node1 mine a block to confirm transactions: self.nodes[1].generate(1)