Merge #20573: wallet, bugfix: allow send with string fee_rate amounts

6fa72ceb8021c3b5aea62f6cfe92665c29212923 test: add coverage for passing fee rate as a string (Jon Atack)
ce207d6b93d35bc02fcd2dd28f1fd95869261d43 wallet, bugfix: allow send to take string fee rate values (Jon Atack)

Pull request description:

  RPC send currently only accepts fee rates as numbers, which is a user-facing bug. It should accept fee rates as an amount, e.g. a string or a number, as documented in its help and like sendtoaddress, sendmany, fundrawtransaction, walletcreatefundedpsbt, and bumpfee. Provide a fix and regression test coverage.

ACKs for top commit:
  MarcoFalke:
    review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
  achow101:
    Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
  promag:
    Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923.

Tree-SHA512: 735f9269cb1b81953764b5283449c0b154bd62de034225be5bcedc515c84faf767fe8fe0741008679fe412922c847b00d116cb11aab775236b779c847ba87167
This commit is contained in:
MarcoFalke 2020-12-10 08:45:52 +01:00 committed by Konstantin Akimov
parent db4a2169bb
commit 5ad8a489a5
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
5 changed files with 34 additions and 9 deletions

View File

@ -4253,7 +4253,7 @@ static RPCHelpMan send()
UniValueType(), // outputs (ARR or OBJ, checked later) UniValueType(), // outputs (ARR or OBJ, checked later)
UniValue::VNUM, // conf_target UniValue::VNUM, // conf_target
UniValue::VSTR, // estimate_mode UniValue::VSTR, // estimate_mode
UniValue::VNUM, // fee_rate UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode()
UniValue::VOBJ, // options UniValue::VOBJ, // options
}, true }, true
); );

View File

@ -713,10 +713,10 @@ class RawTransactionsTest(BitcoinTestFramework):
result = node.fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee) result = node.fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee)
btc_kvb_to_sat_vb = 100000 # (1e5) btc_kvb_to_sat_vb = 100000 # (1e5)
result1 = node.fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee}) result1 = node.fundrawtransaction(rawtx, {"fee_rate": str(2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee)})
result2 = node.fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}) result2 = node.fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee})
result3 = node.fundrawtransaction(rawtx, {"fee_rate": 10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee}) result3 = node.fundrawtransaction(rawtx, {"fee_rate": 10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee})
result4 = node.fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee}) result4 = node.fundrawtransaction(rawtx, {"feeRate": str(10 * self.min_relay_tx_fee)})
# Test that funding non-standard "zero-fee" transactions is valid. # Test that funding non-standard "zero-fee" transactions is valid.
result5 = self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 0}) result5 = self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 0})
result6 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 0}) result6 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 0})

View File

@ -111,11 +111,11 @@ class PSBTTest(BitcoinTestFramework):
self.log.info("Test walletcreatefundedpsbt fee rate of 10000 sat/vB and 0.1 BTC/kvB produces a total fee at or slightly below -maxtxfee (~0.05290000)") self.log.info("Test walletcreatefundedpsbt fee rate of 10000 sat/vB and 0.1 BTC/kvB produces a total fee at or slightly below -maxtxfee (~0.05290000)")
res1 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 10000, "add_inputs": True}) res1 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 10000, "add_inputs": True})
assert_approx(res1["fee"], 0.04, 0.005) assert_approx(res1["fee"], 0.04, 0.005)
res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True}) res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": "0.1", "add_inputs": True})
assert_approx(res2["fee"], 0.04, 0.005) assert_approx(res2["fee"], 0.04, 0.005)
self.log.info("Test min fee rate checks with walletcreatefundedpsbt are bypassed, e.g. a fee_rate under 1 sat/vB is allowed") self.log.info("Test min fee rate checks with walletcreatefundedpsbt are bypassed, e.g. a fee_rate under 1 sat/vB is allowed")
res3 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 0.99999999, "add_inputs": True}) res3 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": "0.99999999", "add_inputs": True})
assert_approx(res3["fee"], 0.00000224, 0.0000001) assert_approx(res3["fee"], 0.00000224, 0.0000001)
res4 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.00000999, "add_inputs": True}) res4 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.00000999, "add_inputs": True})
assert_approx(res4["fee"], 0.00000224, 0.0000001) assert_approx(res4["fee"], 0.00000224, 0.0000001)

View File

@ -250,7 +250,8 @@ class WalletTest(BitcoinTestFramework):
fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000 explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000
txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=fee_rate_sat_vb) # Test passing fee_rate as a string
txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=str(fee_rate_sat_vb))
self.nodes[2].generate(1) self.nodes[2].generate(1)
self.sync_all(self.nodes[0:3]) self.sync_all(self.nodes[0:3])
balance = self.nodes[2].getbalance() balance = self.nodes[2].getbalance()
@ -259,6 +260,17 @@ class WalletTest(BitcoinTestFramework):
node_0_bal += Decimal('10') node_0_bal += Decimal('10')
assert_equal(self.nodes[0].getbalance(), node_0_bal) assert_equal(self.nodes[0].getbalance(), node_0_bal)
# Test passing fee_rate as an integer
amount = Decimal("0.0001")
txid = self.nodes[2].sendmany(amounts={address: amount}, fee_rate=fee_rate_sat_vb)
self.nodes[2].generate(1)
self.sync_all(self.nodes[0:3])
balance = self.nodes[2].getbalance()
node_2_bal = self.check_fee_amount(balance, node_2_bal - amount, explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))
assert_equal(balance, node_2_bal)
node_0_bal += amount
assert_equal(self.nodes[0].getbalance(), node_0_bal)
for key in ["totalFee", "feeRate"]: for key in ["totalFee", "feeRate"]:
assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1) assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1)
@ -420,7 +432,7 @@ class WalletTest(BitcoinTestFramework):
amount = 3 amount = 3
fee_rate_sat_vb = 2 fee_rate_sat_vb = 2
fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
# Test passing fee_rate as an integer
txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb) txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb)
tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
self.nodes[0].generate(1) self.nodes[0].generate(1)
@ -429,6 +441,19 @@ class WalletTest(BitcoinTestFramework):
fee = prebalance - postbalance - Decimal(amount) fee = prebalance - postbalance - Decimal(amount)
assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb)) assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb))
prebalance = self.nodes[2].getbalance()
amount = Decimal("0.001")
fee_rate_sat_vb = 1.23
fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
# Test passing fee_rate as a string
txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=str(fee_rate_sat_vb))
tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
self.nodes[0].generate(1)
self.sync_all(self.nodes[0:3])
postbalance = self.nodes[2].getbalance()
fee = prebalance - postbalance - amount
assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb))
for key in ["totalFee", "feeRate"]: for key in ["totalFee", "feeRate"]:
assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1) assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1)

View File

@ -307,8 +307,8 @@ class WalletSendTest(BitcoinTestFramework):
assert res["complete"] assert res["complete"]
self.log.info("Test setting explicit fee rate") self.log.info("Test setting explicit fee rate")
res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, add_to_wallet=False) res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate="1", add_to_wallet=False)
res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=1, add_to_wallet=False) res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate="1", add_to_wallet=False)
assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"]) assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"])
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False) res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False)