Skip to content

sweep: roll back wallet inputs on all AddWalletInputs error paths #10723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Bahtya wants to merge 4 commits into
base: master
Choose a base branch
from
+14 −1
Open

sweep: roll back wallet inputs on all AddWalletInputs error paths #10723

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
50a74b7
sweep: roll back wallet inputs on all AddWalletInputs error paths
Bahtya Apr 9, 2026
01d69f9
sweep: refine comment and remove redundant rollback in AddWalletInputs
Bahtya Apr 9, 2026
976196f
chore: add release note for AddWalletInputs rollback fix
Bahtya Apr 27, 2026
1c35f4d
merge: resolve release notes conflict with upstream master
Bahtya Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sweep/tx_input_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,14 @@ func (b *BudgetInputSet) AddWalletInputs(wallet Wallet) error {
return utxos[i].Value < utxos[j].Value
})

// Save the current number of inputs so we can roll back on error.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment partially restates the code (1:1 explanation), which is discouraged by the repository's style guide. It should instead focus on the intention or the "why" behind the logic, providing more context for the rollback mechanism.

Suggested change
// Save the current number of inputs so we can roll back on error.
// We'll track the initial number of inputs to ensure we can restore the
// set to its original state if we fail to add the required wallet
// inputs.
References
  1. Comments must not explain the code 1:1 but instead explain the why behind a certain block of code. In-body comments should explain the intention of the code. (link)

inputsBefore := len(b.inputs)

// Add wallet inputs to the set until the specified budget is covered.
for _, utxo := range utxos {
err := b.addWalletInput(utxo)
if err != nil {
b.inputs = b.inputs[:inputsBefore]
return err
}

Expand All @@ -382,6 +386,7 @@ func (b *BudgetInputSet) AddWalletInputs(wallet Wallet) error {

// Exit if there are no inputs can contribute to the fees.
if !b.hasNormalInput() {
b.inputs = b.inputs[:inputsBefore]
return ErrNotEnoughInputs
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This rollback operation is redundant in this specific error path. If the loop at line 374 successfully added any wallet inputs, b.hasNormalInput() would return true (since wallet inputs are "normal" inputs with no required outputs), and this block would not be entered. If the loop added no inputs (e.g., because utxos was empty), then len(b.inputs) is already equal to inputsBefore, making the truncation a no-op. Removing it simplifies the logic without affecting correctness.

Suggested change
b.inputs = b.inputs[:inputsBefore]
return ErrNotEnoughInputs
return ErrNotEnoughInputs

}

Expand Down
Loading