Skip to content

Commit

Permalink
Merge bitcoin#16521: rpc: Use the default maxfeerate value as BTC/kB
Browse files Browse the repository at this point in the history 
2dfd683 test: Add test for default maxfeerate in sendrawtransaction (Joonmo Yang)
261843e wallet/rpc: Use the default maxfeerate value as BTC/kB (Joonmo Yang)

Pull request description:

  Fixes bitcoin#16382

  This patch tries to treat `maxfeerate` in sendrawtransaction/testmempoolaccept RPC as a rate(BTC/kB) instead of an absolute value(BTC).
  The included test case checks if the new behavior works correctly, by using the transaction with an absolute fee of ~0.02BTC, where the fee rate is ~0.2BTC/kB.
  This test should be failing if the default `maxfeerate` is 0.1BTC, but pass if the default value is 0.1BTC/kB

ACKs for top commit:
  laanwj:
    ACK 2dfd683 (ACKs by Sjors and MarcoFalke above for trivially different code)

Tree-SHA512: a1795bffe8a182acef8844797955db1f60bb0c0ded97148f3572dc265234d5219271a3a7aa0b6418a43f73b2b2720ef7412ba169c99bb1cdcac52051f537d6af
  • Loading branch information
@laanwj @vijaydasmp
laanwj authored and vijaydasmp committed Nov 13, 2022
1 parent 0db359a commit 1f379df
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 17 deletions.
29 changes: 17 additions & 12 deletions src/rpc/rawtransaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@

#include <univalue.h>

/** 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};

void TxToJSON(const CTransaction& tx, const uint256 hashBlock, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman, UniValue& entry)
{
Expand Down Expand Up @@ -798,7 +798,7 @@ 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), "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"},
{"instantsend", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Deprecated and ignored"},
{"bypasslimits", RPCArg::Type::BOOL, /* default_val */ "false", "Bypass transaction policy limits"},
},
Expand Down Expand Up @@ -828,16 +828,18 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
if (!DecodeHexTx(mtx, request.params[0].get_str()))
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()) {
CFeeRate fr(AmountFromValue(request.params[1]));
max_raw_tx_fee = fr.GetFee(GetVirtualTransactionSize(*tx));
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);

bool bypass_limits = false;
if (!request.params[3].isNull()) bypass_limits = request.params[3].get_bool();
std::string err_string;
Expand All @@ -863,7 +865,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{
RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n"
Expand Down Expand Up @@ -905,15 +907,18 @@ 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()) {
CFeeRate fr(AmountFromValue(request.params[1]));
max_raw_tx_fee = fr.GetFee(GetVirtualTransactionSize(*tx));
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);


CTxMemPool& mempool = EnsureMemPool(request.context);

UniValue result(UniValue::VARR);
Expand Down
1 change: 1 addition & 0 deletions test/functional/mempool_accept.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ def run_test(self):
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')
Expand Down
31 changes: 27 additions & 4 deletions test/functional/rpc_rawtransaction.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,28 +370,51 @@ def run_test(self):

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, ~200 b transaction, fee rate should land around 5 sat/b = 0.00005000 BTC/kB
# 10000 sat fee, ~200 b transaction, fee rate should land around 50 sat/b = 0.0005000 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]
testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0]
assert_equal(testres['allowed'], True)
self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.00007000')
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__':
RawTransactionsTest().main()
2 changes: 1 addition & 1 deletion test/functional/wallet_basic.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ def run_test(self):
# 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
Expand Down

0 comments on commit 1f379df

Please sign in to comment.