diff options
| -rw-r--r-- | doc/release-notes-14582.md | 14 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 9 | ||||
| -rwxr-xr-x | test/functional/wallet_groups.py | 53 |
3 files changed, 61 insertions, 15 deletions
diff --git a/doc/release-notes-14582.md b/doc/release-notes-14582.md new file mode 100644 index 0000000000..28b0abecd7 --- /dev/null +++ b/doc/release-notes-14582.md @@ -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) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ac03b3ca9c..c132a4be42 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -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; } diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 9b6230f674..e5c4f12f20 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -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() |