Repository navigation
staticaddr/loopin: sweep the htlc tx that spent the deposits - #1269
claravanstaden wants to merge 3 commits into
Conversation
|
Open question before this is ready: do swaps that are already stuck in the timeout sweep need recovery at all? A swap only gets there if a backup fee variant of the HTLC tx confirmed and the swap invoice was never paid. We're checking whether that has actually happened. If it hasn't, I'll keep this PR to the sweep fix plus a dust check on the sweep output. I'd drop the recovery path for already stuck swaps, since review found it adds more state machine complexity than it's worth: an unpersisted sweep hash, a retry path without backoff, and no coverage for neutrino backends. |
fb585e6 to
aa09dd4
Compare
The client co-signs a standard, a high fee and an extremely high fee variant of the static address loop-in htlc tx, and the server may publish any of them. The timeout sweep always rebuilt the standard fee variant and spent its htlc outpoint. When a backup variant confirmed, the sweep spent an output that does not exist, and the hourly retry and recovery rebuilt the same invalid transaction. Rebuilding the published variant locally is not enough, because only the standard fee rate is persisted. Take the htlc tx from the chain instead: wait for the spend of the first deposit, accept the spending tx only if it spends exactly the swap's deposits and pays the htlc script once, and sweep that htlc outpoint with its actual value. The inputs are matched as a set because recovery can load the deposits in a different order than they were signed in. This replaces the rebuild's index-0 layout check, and createHtlcSweepTx no longer takes the htlc fee limit, which only guarded the rebuild. The server's htlc fees are still checked when signing. The lookup can take a while, so it runs before the fee rate and block height are fetched, it is logged at info and bounded by the new HtlcTxLookupTimeout config so that a deposit that is never spent fails the attempt like any other error. Shutdown while waiting keeps the state without logging a failed attempt.
The timeout sweep now spends whichever htlc tx variant confirmed. The backup variants leave a smaller htlc output, so a high sweep fee rate can leave a dust or negative sweep output. Such a sweep was signed and only rejected by the backend with an unclear error on every retry. Check the sweep output against the dust limit before signing and return an error that names the htlc output, its value and the fee.
The timeout sweep of a static address loop-in could not sweep a confirmed backup fee variant of the htlc tx. Note the fix and the new dust check so that users with affected swaps know to upgrade.
aa09dd4 to
4341f68
Compare
Good question. This hasn't happen yet since it requires manual trigger from the server. We hence can remove the recovery path. |
The client co-signs three variants of the static address loop-in HTLC tx (standard, high and extremely high fee). The timeout sweep always rebuilt the standard fee variant, so if a backup variant confirmed, the sweep couldn't spend it. Only the standard fee rate is persisted, so rebuilding the confirmed variant after a restart isn't possible.
This PR takes the HTLC tx from the chain instead. It waits for the spend of the first deposit, checks that the spending tx spends exactly the swap's deposits and has one HTLC output, and sweeps that outpoint with its actual value. The lookup runs before the fee rate is estimated and is bounded by a new
HtlcTxLookupTimeoutconfig (default 30 min). A second commit rejects sweeps whose output would be dust, since the backup variants leave a smaller HTLC output.Recovering swaps that are already stuck from the old behavior isn't included. See the comment below; it can follow separately if needed.
An alternative to #1217 that doesn't depend on the multi-address stack. Thanks @hieblmi for the original approach.
Testing
TestHtlcTimeoutSweepSpendsPublishedVariant: all three variants after a simulated restartTestHtlcTimeoutSweepFeeRateAfterLookup: fee rate estimated after the lookupTestFetchHtlcTxFailures: notifier error, spend without tx, lookup timeout, shutdownTestHtlcOutput: input and output validationTestCreateHtlcSweepTxRejectsDust: dust boundarygo test -race ./staticaddr/...Pull Request Checklist
docs/release-notes/release-notes-next.md, or apply theno-changeloglabel (required by CI)