sweep: return all inputs in PendingSweeps#9772
sweep: return all inputs in PendingSweeps#9772yyforyongyu merged 5 commits intolightningnetwork:masterfrom
PendingSweeps#9772Conversation
Previously we'd skip returning immature inputs to stay backwards compatible. This has the downside as the node operator cannot take actions on this input, like some customized batching strategy via `bumpfee` RPC. After this change, it means when calling `bumpfee` RPC, it will now update the params for the immature input instead of returning an error saying `output does not belong to wallet`. We also add a `MaturityHeight` field so the user knows when this input will be swept.
4c2ca3f to
c30de5c
Compare
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ziggie1984
left a comment
There was a problem hiding this comment.
Looks good, had some nits and a question.
c30de5c to
df8cd90
Compare
Since we now return all sweeps, we need to update the call to include immature sweeps.
|
Just a drive-by question: Do we know how many inputs a sweep has at any given time? Exposing the number of target inputs (e.g. the inputs we want to sweep) and the number of sponsoring inputs (e.g. the inputs purely added to pay for fees) would be quite useful to have in the TAP integration tests. So if they're easy to add, perhaps we could do that in this PR as well. |
Yes we do know that internally in the sweeper, before we aggregate them and potentially add more wallet inputs.
I think we need this too, to make fee bumping a bit easier. This is the 7th item in #8680, which can be a bit involved as we need to change the sweeper to remember all the input sets (atm it's created over-the-fly), then maybe a new RPC to return that info. But yeah def will find time to add it. |
df8cd90 to
b610678
Compare
Previously when calling
PendingSweeps, if the outputs being swept have a locktime in the future, they will be filtered out. This is now changed such that all outputs registered in the sweeper will be returned in the RPC response regardless of their locktime, which enables users to plan ahead about upcoming sweeps and implement customized aggregation logic.A new field,
MaturityHeight,has been added toPendingSweepto show the absolute locktime value.Thanks @Liongrass for the feedback!