diff options
author | fanquake <fanquake@gmail.com> | 2023-02-20 17:11:53 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-02-20 17:20:37 +0000 |
commit | 94070029fb6b783833973f9fe08a3a871994492f (patch) | |
tree | d5932dc2e537c37e6c48811a8c7caa49bc4f6673 /src | |
parent | 0f670e0eae43de2766866563fc54f188d18d004c (diff) | |
parent | 14b4921a91920df25b19ff420bfe2bff8c56f71e (diff) | |
download | bitcoin-94070029fb6b783833973f9fe08a3a871994492f.tar.xz |
Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX with avoidpartialspends
14b4921a91920df25b19ff420bfe2bff8c56f71e wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/27051
When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Otherwise, the first change address remains reserved, will not be used in the second attempt, and then will never be used by the wallet, leaving gaps in the BIP44 chain.
If the user had `avoidpartialspends` set to true, there is no second version of the created transaction and the change addresses are not affected.
I believe this behavior was introduced in https://github.com/bitcoin/bitcoin/pull/14582
ACKs for top commit:
achow101:
ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e
Tree-SHA512: a3d56f251ff4b333fc11325f30d05513e34ab0a2eb703fadd0ad98d167ae074493df1a24068298336c6ed2da6b31aa2befa490bc790bbc260ed357c8f2397659
Diffstat (limited to 'src')
-rw-r--r-- | src/wallet/spend.cpp | 7 |
1 files changed, 7 insertions, 0 deletions
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 0ef56ab8ed..1a79b59d12 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1090,6 +1090,13 @@ util::Result<CreatedTransactionResult> CreateTransaction( TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; + + // Re-use the change destination from the first creation attempt to avoid skipping BIP44 indexes + const int ungrouped_change_pos = txr_ungrouped.change_pos; + if (ungrouped_change_pos != -1) { + ExtractDestination(txr_ungrouped.tx->vout[ungrouped_change_pos].scriptPubKey, tmp_cc.destChange); + } + auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; |