foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge #19743: -maxapsfee follow-up

Browse source 
7e31ea9fa0 -maxapsfee: follow-up fixes (Karl-Johan Alm)
9f77b82176 doc: release notes for -maxapsfee (Karl-Johan Alm)

Pull request description:

  Addresses feedback from jonatack and meshcollider in #14582.

ACKs for top commit:
  jonatack:
    ACK 7e31ea9fa0
  meshcollider:
    re-ACK 7e31ea9fa0

Tree-SHA512: 5d159d78e917e41140e1394bef05e821430dbeac585e9bd708f897041dd7104c2a6b563bfab8b2c85e6d923441160a3880d7864d768aa8e0e66860e0a2c4f56b
This commit is contained in:
Samuel Dobson 2020-08-18 22:42:52 +12:00
commit a2a250c7d0
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
3 changed files with 61 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
doc/release-notes-14582.md Normal file
View file

@ -0,0 +1,14 @@
Configuration
-------------
A new configuration flag `-maxapsfee` has been added, which sets the max allowed
avoid partial spends (APS) fee. It defaults to 0 (i.e. fee is the same with
and without APS). Setting it to -1 will disable APS, unless `-avoidpartialspends`
is set. (#14582)
Wallet
------
The wallet will now avoid partial spends (APS) by default, if this does not result
in a difference in fees compared to the non-APS variant. The allowed fee threshold
can be adjusted using the new `-maxapsfee` configuration option. (#14582)

9
src/wallet/wallet.cpp
View file

@ -3890,16 +3890,17 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
}
if (gArgs.IsArgSet("-maxapsfee")) {
const std::string max_aps_fee{gArgs.GetArg("-maxapsfee", "")};
CAmount n = 0;
if (gArgs.GetArg("-maxapsfee", "") == "-1") {
if (max_aps_fee == "-1") {
n = -1;
} else if (!ParseMoney(gArgs.GetArg("-maxapsfee", ""), n)) {
error = AmountErrMsg("maxapsfee", gArgs.GetArg("-maxapsfee", ""));
} else if (!ParseMoney(max_aps_fee, n)) {
error = AmountErrMsg("maxapsfee", max_aps_fee);
return nullptr;
}
if (n > HIGH_APS_FEE) {
warnings.push_back(AmountHighWarn("-maxapsfee") + Untranslated(" ") +
_("This is the maximum transaction fee you pay to prioritize partial spend avoidance over regular coin selection."));
_("This is the maximum transaction fee you pay (in addition to the normal fee) to prioritize partial spend avoidance over regular coin selection."));
}
walletInstance->m_max_aps_fee = n;
}

53
test/functional/wallet_groups.py
View file

@ -15,8 +15,14 @@ from test_framework.util import (
class WalletGroupTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 4
self.extra_args = [[], [], ['-avoidpartialspends'], ["-maxapsfee=0.0001"]]
self.num_nodes = 5
self.extra_args = [
[],
[],
["-avoidpartialspends"],
["-maxapsfee=0.00002719"],
["-maxapsfee=0.00002720"],
]
self.rpc_timeout = 480
def skip_test_if_missing_module(self):
@ -50,8 +56,8 @@ class WalletGroupTest(BitcoinTestFramework):
# one output should be 0.2, the other should be ~0.3
v = [vout["value"] for vout in tx1["vout"]]
v.sort()
assert_approx(v[0], 0.2)
assert_approx(v[1], 0.3, 0.0001)
assert_approx(v[0], vexp=0.2, vspan=0.0001)
assert_approx(v[1], vexp=0.3, vspan=0.0001)
txid2 = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 0.2)
tx2 = self.nodes[2].getrawtransaction(txid2, True)
@ -61,8 +67,8 @@ class WalletGroupTest(BitcoinTestFramework):
# one output should be 0.2, the other should be ~1.3
v = [vout["value"] for vout in tx2["vout"]]
v.sort()
assert_approx(v[0], 0.2)
assert_approx(v[1], 1.3, 0.0001)
assert_approx(v[0], vexp=0.2, vspan=0.0001)
assert_approx(v[1], vexp=1.3, vspan=0.0001)
# Test 'avoid partial if warranted, even if disabled'
self.sync_all()
@ -75,8 +81,8 @@ class WalletGroupTest(BitcoinTestFramework):
# - C0 1.0 - E1 0.5
# - C1 0.5 - F ~1.3
# - D ~0.3
assert_approx(self.nodes[1].getbalance(), 4.3, 0.0001)
assert_approx(self.nodes[2].getbalance(), 4.3, 0.0001)
assert_approx(self.nodes[1].getbalance(), vexp=4.3, vspan=0.0001)
assert_approx(self.nodes[2].getbalance(), vexp=4.3, vspan=0.0001)
# Sending 1.4 btc should pick one 1.0 + one more. For node #1,
# this could be (A / B0 / C0) + (B1 / C1 / D). We ensure that it is
# B0 + B1 or C0 + C1, because this avoids partial spends while not being
@ -90,8 +96,8 @@ class WalletGroupTest(BitcoinTestFramework):
# ~0.1 and 1.4 and should come from the same destination
values = [vout["value"] for vout in tx3["vout"]]
values.sort()
assert_approx(values[0], 0.1, 0.0001)
assert_approx(values[1], 1.4)
assert_approx(values[0], vexp=0.1, vspan=0.0001)
assert_approx(values[1], vexp=1.4, vspan=0.0001)
input_txids = [vin["txid"] for vin in tx3["vin"]]
input_addrs = [self.nodes[1].gettransaction(txid)['details'][0]['address'] for txid in input_txids]
@ -104,13 +110,38 @@ class WalletGroupTest(BitcoinTestFramework):
self.nodes[0].sendtoaddress(addr_aps, 1.0)
self.nodes[0].generate(1)
self.sync_all()
txid4 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
with self.nodes[3].assert_debug_log(['Fee non-grouped = 2820, grouped = 4160, using grouped']):
txid4 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
tx4 = self.nodes[3].getrawtransaction(txid4, True)
# tx4 should have 2 inputs and 2 outputs although one output would
# have been enough and the transaction caused higher fees
assert_equal(2, len(tx4["vin"]))
assert_equal(2, len(tx4["vout"]))
addr_aps2 = self.nodes[3].getnewaddress()
[self.nodes[0].sendtoaddress(addr_aps2, 1.0) for _ in range(5)]
self.nodes[0].generate(1)
self.sync_all()
with self.nodes[3].assert_debug_log(['Fee non-grouped = 5520, grouped = 8240, using non-grouped']):
txid5 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 2.95)
tx5 = self.nodes[3].getrawtransaction(txid5, True)
# tx5 should have 3 inputs (1.0, 1.0, 1.0) and 2 outputs
assert_equal(3, len(tx5["vin"]))
assert_equal(2, len(tx5["vout"]))
# Test wallet option maxapsfee with node 4, which sets maxapsfee
# 1 sat higher, crossing the threshold from non-grouped to grouped.
addr_aps3 = self.nodes[4].getnewaddress()
[self.nodes[0].sendtoaddress(addr_aps3, 1.0) for _ in range(5)]
self.nodes[0].generate(1)
self.sync_all()
with self.nodes[4].assert_debug_log(['Fee non-grouped = 5520, grouped = 8240, using grouped']):
txid6 = self.nodes[4].sendtoaddress(self.nodes[0].getnewaddress(), 2.95)
tx6 = self.nodes[4].getrawtransaction(txid6, True)
# tx6 should have 5 inputs and 2 outputs
assert_equal(5, len(tx6["vin"]))
assert_equal(2, len(tx6["vout"]))
# Empty out node2's wallet
self.nodes[2].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=self.nodes[2].getbalance(), subtractfeefromamount=True)
self.sync_all()