8000 wallet: skip APS when no partial spend exists · bitcoin/bitcoin@d6edd15 · GitHub
[go: up one dir, main page]
Skip to content

Commit d6edd15

Browse files
81442253098144225309
committed
wallet: skip APS when no partial spend exists
APS runs a second coin selection to fully spend UTXOs sharing a scriptPubKey. Currently runs unconditionally, even when the first selection has no partial spend and APS cannot help. Detect partial spends by comparing selected vs available UTXO counts per scriptPubKey inside CreateTransactionInternal, reusing available_coins. Add has_partial_spend to CreatedTransactionResult. Skip APS if false. Update interface_usdt_coinselection test to expect APS to be skipped when no partial spend exists. Add new test that creates a partial spend scenario so the full APS tracepoint flow is verified. Fixes #25150
1 parent 5f66fca commit d6edd15

File tree

3 files changed

+62
-15
lines changed

3 files changed

+62
-15
lines changed

‎src/wallet/spend.cpp‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,28 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
12111211
result.GetWaste(),
12121212
result.GetSelectedValue());
12131213

1214+
// Check for partial spend: spending some but not all UTXOs from any scriptPubKey
1215+
bool has_partial_spend{false};
1216+
if (coin_control.m_allow_other_inputs) {
1217+
std::map<CScript, std::pair<size_t, size_t>> spk_counts; // {available, selected}
1218+
for (const auto& coin : available_coins.All()) {
1219+
spk_counts[coin.txout.scriptPubKey].first++;
1220+
}
1221+
// Also count preset inputs as available (they're wallet UTXOs too)
1222+
for (const auto& coin : preset_inputs.coins) {
1223+
spk_counts[coin->txout.scriptPubKey].first++;
1224+
}
1225+
for (const auto& coin : result.GetInputSet()) {
1226+
spk_counts[coin->txout.scriptPubKey].second++;
1227+
}
1228+
for (const auto& [spk, counts] : spk_counts) {
1229+
if (counts.second > 0 && counts.second < counts.first) {
1230+
has_partial_spend = true;
1231+
break;
1232+
}
1233+
}
1234+
}
1235+
12141236
// vouts to the payees
12151237
txNew.vout.reserve(vecSend.size() + 1); // + 1 because of possible later insert
12161238
for (const auto& recipient : vecSend)
@@ -1404,7 +1426,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
14041426
feeCalc.est.fail.start, feeCalc.est.fail.end,
14051427
(feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0,
14061428
feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool);
1407-
return CreatedTransactionResult(tx, current_fee, change_pos, feeCalc);
1429+
return CreatedTransactionResult(tx, current_fee, change_pos, feeCalc, has_partial_spend);
14081430
}
14091431

14101432
util::Result<CreatedTransactionResult> CreateTransaction(
@@ -1432,8 +1454,11 @@ util::Result<CreatedTransactionResult> CreateTransaction(
14321454
res && res->change_pos.has_value() ? int32_t(*res->change_pos) : -1);
14331455
if (!res) return res;
14341456
const auto& txr_ungrouped = *res;
1435-
// try with avoidpartialspends unless it's enabled already
1457+
// try with avoidpartialspends unless it's enabled already, or if there's no partial spend to avoid
14361458
if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
1459+
if (!txr_ungrouped.has_partial_spend) {
1460+
return res;
1461+
}
14371462
TRACEPOINT(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str());
14381463
CCoinControl tmp_cc = coin_control;
14391464
tmp_cc.m_avoid_partial_spends = true;

‎src/wallet/spend.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,11 @@ struct CreatedTransactionResult
208208
CAmount fee;
209209
FeeCalculation fee_calc;
210210
std::optional<unsigned int> change_pos;
211+
/** Whether the selection spends only some UTXOs from any scriptPubKey (relevant for APS) */
212+
bool has_partial_spend{false};
211213

212-
CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, std::optional<unsigned int> _change_pos, const FeeCalculation& _fee_calc)
213-
: tx(_tx), fee(_fee), fee_calc(_fee_calc), change_pos(_change_pos) {}
214+
CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, std::optional<unsigned int> _change_pos, const FeeCalculation& _fee_calc, bool _has_partial_spend = false)
215+
: tx(_tx), fee(_fee), fee_calc(_fee_calc), change_pos(_change_pos), has_partial_spend(_has_partial_spend) {}
214216
};
215217

216218
/**

‎test/functional/interface_usdt_coinselection.py‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,14 @@ def skip_test_if_missing_module(self):
119119
self.skip_if_no_wallet()
120120
self.skip_if_running_under_valgrind()
121121

122-
def get_tracepoints(self, expected_types):
122+
def get_tracepoints(self, expected_types, wallet_name=None):
123+
if wallet_name is None:
124+
wallet_name = self.default_wallet_name
123125
events = []
124126
try:
125127
for i in range(0, len(expected_types) + 1):
126128
event = self.bpf["coin_selection_events"].pop()
127-
assert_equal(event.wallet_name.decode(), self.default_wallet_name)
129+
assert_equal(event.wallet_name.decode(), wallet_name)
128130
assert_equal(event.type, expected_types[i])
129131
events.append(event)
130132
else:
@@ -180,15 +182,35 @@ def run_test(self):
180182
self.generate(self.nodes[0], 101)
181183
wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
182184

183-
self.log.info("Sending a transaction should result in all tracepoints")
185+
self.log.info("Sending a transaction skips APS when no partial spend exists")
186+
# Default wallet has UTXOs at different addresses (from mining), so no
187+
# partial spend exists. APS is skipped. We should have 2 tracepoints:
188+
# 1. selected_coins (type 1)
189+
# 2. normal_create_tx_internal (type 2)
190+
wallet.sendtoaddress(wallet.getnewaddress(), 10)
191+
events = self.get_tracepoints([1, 2])
192+
success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
193+
assert_equal(success, True)
194+
assert_greater_than(change_pos, -1)
195+
196+
self.log.info("APS is attempted when a partial spend exists")
197+
# Create a wallet with multiple UTXOs at the same address to trigger APS.
198+
self.nodes[0].createwallet("aps_test")
199+
aps_wallet = self.nodes[0].get_wallet_rpc("aps_test")
200+
addr = aps_wallet.getnewaddress()
201+
wallet.sendtoaddress(addr, 5)
202+
wallet.sendtoaddress(addr, 5)
203+
self.generate(self.nodes[0], 1)
204+
# Spending less than the total creates a partial spend (using one of two
205+
# UTXOs at the same address). APS should be attempted.
184206
# We should have 5 tracepoints in the order:
185207
# 1. selected_coins (type 1)
186208
# 2. normal_create_tx_internal (type 2)
187209
# 3. attempting_aps_create_tx (type 3)
188210
# 4. selected_coins (type 1)
189211
# 5. aps_create_tx_internal (type 4)
190-
wallet.sendtoaddress(wallet.getnewaddress(), 10)
191-
events = self.get_tracepoints([1, 2, 3, 1, 4])
212+
aps_wallet.sendtoaddress(wallet.getnewaddress(), 2)
213+
events = self.get_tracepoints([1, 2, 3, 1, 4], "aps_test")
192214
success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
193215
assert_equal(success, True)
194216
assert_greater_than(change_pos, -1)
@@ -212,15 +234,13 @@ def run_test(self):
212234
assert_equal(success, True)
213235
assert_equal(use_aps, None)
214236

215-
self.log.info("Change position is -1 if no change is created with APS when APS was initially not used")
216-
# We should have 2 tracepoints in the order:
237+
self.log.info("Sending entire balance skips APS (no partial spend)")
238+
# Sending entire balance spends all UTXOs, so no partial spend exists.
239+
# APS is skipped. We should have 2 tracepoints:
217240
# 1. selected_coins (type 1)
218241
# 2. normal_create_tx_internal (type 2)
219-
# 3. attempting_aps_create_tx (type 3)
220-
# 4. selected_coins (type 1)
221-
# 5. aps_create_tx_internal (type 4)
222242
wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True, avoid_reuse=False)
223-
events = self.get_tracepoints([1, 2, 3, 1, 4])
243+
events = self.get_tracepoints([1, 2])
224244
success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
225245
assert_equal(success, True)
226246
assert_equal(change_pos, -1)

0 commit comments

Comments
 (0)
0