sweep: account for aux extra budget when filtering inputs#10897
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an accounting issue in the BudgetAggregator that caused certain inputs to be permanently filtered out of sweep attempts. By failing to account for the extra budget provided by an auxiliary sweeper, the system would incorrectly deem small asset-based inputs as insufficient to cover fees, especially after failed attempts ratcheted up the required starting fee rate. This change ensures the filter correctly considers the total available budget, maintaining consistency with how input sets are formed. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the budget aggregator in sweep/aggregator.go to account for an extra budget contributed by an auxiliary sweeper when filtering inputs. Feedback suggests passing the underlying input.Input interface (pi.Input) instead of the wrapper pi (*SweeperInput) to ExtraBudgetForInputs to avoid type assertion failures in external implementations like Taproot Assets.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
🔴 PR Severity: CRITICAL
🔴 Critical (1 file)
AnalysisThis PR modifies The change is small (1 file, 30 lines), so no severity bump was applied. To override, add a |
6a3e5eb to
b77026e
Compare
The BudgetAggregator filters out inputs whose budget cannot cover the min relay fee or their requested starting fee rate. For inputs that carry a resolution blob (custom channel outputs), the aux sweeper contributes a sizable extra budget to any input set they join, but the filter only considered the input's own budget, which for asset outputs is tiny (their value is carried off-chain). The filter is mostly harmless with default parameters, but the starting fee rate of an input is ratcheted whenever a sweep attempt fails, including failures that have nothing to do with fees: e.g. when a concurrent sweep transaction spends the wallet UTXO that was backing this input's set (the sweeper currently doesn't lease selected wallet UTXOs, so concurrent input sets can pick the same one). One such collision is enough to push the required starting fee above a small asset input's own budget, after which the input is filtered out of every future input set and the sweep is silently stranded forever. Account for the aux extra budget in the filter, mirroring how the budget input set itself accounts for it when deciding whether wallet inputs are needed. Inputs without a resolution blob (the only kind that exists without an aux sweeper) are unaffected.
b77026e to
75433bb
Compare
(TLDR, fixes an accounting error that can lead to sweep failures after aux channel force closes. Related to lightninglabs/taproot-assets#1888 and lightninglabs/taproot-assets#1976).
The BudgetAggregator filters out inputs whose budget cannot cover the min relay fee or their requested starting fee rate. For inputs that carry a resolution blob (custom channel outputs), the aux sweeper contributes a sizable extra budget to any input set they join, but the filter only considered the input's own budget, which for asset outputs is tiny.
The filter is mostly harmless with default parameters, but the starting fee rate of an input is ratcheted whenever a sweep attempt fails, including failures that have nothing to do with fees, e.g. when a concurrent sweep transaction spends the wallet UTXO that was backing this input's set (#10816 contains a separate, independent fix for the "always ratchet on every error" issue, which I need to tease out from the flawed leasing approach introduced there). One such collision is enough to push the required starting fee above a small asset input's own budget, after which the input is filtered out of every future input set and the sweep is silently stranded forever.
This change accounts for the aux extra budget in the filter, mirroring how the budget input set itself accounts for it when deciding whether wallet inputs are needed. Inputs without a resolution blob (the only kind that exists without an aux sweeper) are unaffected.