diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4323,18 +4323,20 @@ } if (gArgs.IsArgSet("-maxapsfee")) { + const std::string max_aps_fee{gArgs.GetArg("-maxapsfee", "")}; Amount n = Amount::zero(); - if (gArgs.GetArg("-maxapsfee", "") == "-1") { + if (max_aps_fee == "-1") { n = -1 * SATOSHI; - } 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 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -15,9 +15,13 @@ class WalletGroupTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 4 + self.num_nodes = 5 self.extra_args = [ - [], [], ['-avoidpartialspends'], ["-maxapsfee=100"]] + [], + [], + ['-avoidpartialspends'], + ["-maxapsfee=2.93"], + ["-maxapsfee=2.94"]] self.rpc_timeout = 120 self.supports_cli = False @@ -52,8 +56,8 @@ assert_equal(2, len(tx1["vout"])) # one output should be 0.2, the other should be ~0.3 v = sorted([vout["value"] for vout in tx1["vout"]]) - assert_approx(v[0], 200000) - assert_approx(v[1], 300000, 100) + assert_approx(v[0], vexp=200_000, vspan=100) + assert_approx(v[1], vexp=300_000, vspan=100) txid2 = self.nodes[2].sendtoaddress( self.nodes[0].getnewaddress(), 200000) @@ -63,8 +67,8 @@ assert_equal(2, len(tx2["vout"])) # one output should be 0.2, the other should be ~1.3 v = sorted([vout["value"] for vout in tx2["vout"]]) - assert_approx(v[0], 200000) - assert_approx(v[1], 1300000, 100) + assert_approx(v[0], vexp=200_000, vspan=100) + assert_approx(v[1], vexp=1_300_000, vspan=100) # Test 'avoid partial if warranted, even if disabled' self.sync_all() @@ -77,8 +81,8 @@ # - C0 1.0 - E1 0.5 # - C1 0.5 - F ~1.3 # - D ~0.3 - assert_approx(self.nodes[1].getbalance(), 4300000, 100) - assert_approx(self.nodes[2].getbalance(), 4300000, 100) + assert_approx(self.nodes[1].getbalance(), vexp=4_300_000, vspan=100) + assert_approx(self.nodes[2].getbalance(), vexp=4_300_000, vspan=100) # 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 @@ -92,8 +96,8 @@ # the accumulated value should be 1.5, so the outputs should be # ~0.1 and 1.4 and should come from the same destination values = sorted([vout["value"] for vout in tx3["vout"]]) - assert_approx(values[0], 100000, 100) - assert_approx(values[1], 1400000) + assert_approx(values[0], vexp=100_000, vspan=100) + assert_approx(values[1], vexp=1_400_000, vspan=100) input_txids = [vin["txid"] for vin in tx3["vin"]] input_addrs = [self.nodes[1].gettransaction( @@ -107,14 +111,44 @@ self.nodes[0].sendtoaddress(addr_aps, 1000000) self.nodes[0].generate(1) self.sync_all() - txid4 = self.nodes[3].sendtoaddress( - self.nodes[0].getnewaddress(), 100000) + with self.nodes[3].assert_debug_log([ + 'Fee non-grouped = 225, grouped = 372, using grouped']): + txid4 = self.nodes[3].sendtoaddress( + self.nodes[0].getnewaddress(), 100_000) 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_000_000) for _ in range(5)] + self.nodes[0].generate(1) + self.sync_all() + with self.nodes[3].assert_debug_log([ + 'Fee non-grouped = 519, grouped = 813, using non-grouped']): + txid5 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), + 2_950_000) + 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_000_000) for _ in range(5)] + self.nodes[0].generate(1) + self.sync_all() + with self.nodes[4].assert_debug_log([ + 'Fee non-grouped = 519, grouped = 813, using grouped']): + txid6 = self.nodes[4].sendtoaddress(self.nodes[0].getnewaddress(), + 2_950_000) + 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) @@ -125,7 +159,7 @@ # scriptPubKey for _ in range(5): raw_tx = self.nodes[0].createrawtransaction( - [{"txid": "0" * 64, "vout": 0}], [{addr2[0]: 50000}]) + [{"txid": "0" * 64, "vout": 0}], [{addr2[0]: 50_000}]) tx = FromHex(CTransaction(), raw_tx) tx.vin = [] tx.vout = [tx.vout[0]] * 2000