Skip to content
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.

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

sweep: group pinnable and non-pinnable inputs separately #9427

Open
morehouse opened this issue Jan 17, 2025 · 3 comments
Open

sweep: group pinnable and non-pinnable inputs separately #9427

morehouse opened this issue Jan 17, 2025 · 3 comments
Assignees
Labels
enhancement Improvements to existing features / behaviour security General label for issues/PRs related to the security of the software utxo sweeping
Milestone

Comments

@morehouse
Copy link
Collaborator

The sweeper currently has no concept of pinnable and unpinnable inputs, so HTLC-Timeout (pinnable) and HTLC-Success (not pinnable) inputs can be aggregated into the same transaction if they have the same deadline. As a result, the HTLC-Success inputs effectively become pinnable too.

Another example of this is expired anchors (pinnable) being aggregated with HTLC-Success (not pinnable), though this could also be solved by removing expired anchor sweeping.

I think @yyforyongyu's proposed "sweeper groups" would be all we need to properly implement this.

@morehouse morehouse added enhancement Improvements to existing features / behaviour security General label for issues/PRs related to the security of the software utxo sweeping labels Jan 17, 2025
@yyforyongyu yyforyongyu added this to the v0.19.0 milestone Jan 18, 2025
@yyforyongyu yyforyongyu self-assigned this Jan 18, 2025
@TheBlueMatt
Copy link

TheBlueMatt commented Jan 21, 2025

We did this in LDK in the latest release (I assume that's where @morehouse saw this). One subtle issue, though, is an HTLC can become pinnable once it (gets close to) expiring as suddenly both parties can claim it. Thus it's useful to track the pinnable state rather than making it fixed.

@morehouse
Copy link
Collaborator Author

@TheBlueMatt Will transactions actually relay with an nLockTime in the future? If not, then this isn't a problem until the HTLC actually expires, in which case we're partially screwed already. Of course, splitting expired inputs out at that point helps control the damage.

@TheBlueMatt
Copy link

They will not, no, indeed, for non-revoked transactions its really only an issue after expiry. If you do this for expired counterparty broadcasts, though, it needs consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features / behaviour security General label for issues/PRs related to the security of the software utxo sweeping
Projects
None yet
Development

No branches or pull requests

3 participants