contractcourt: fix HTLC success resolver phantom-input bug (#10840)#10869
contractcourt: fix HTLC success resolver phantom-input bug (#10840)#10869yyforyongyu wants to merge 13 commits into
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 a bug in the HTLC success resolver where foreign spends of commitment HTLC outputs were incorrectly treated as valid success transactions. By implementing stricter validation of the spending transaction's output and ensuring that expired HTLCs do not trigger the success resolution path, the changes improve the robustness of the contract resolution process and prevent unnecessary sweeper activity. 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
|
🔴 PR Severity: CRITICAL
🔴 Critical (2 files)
🟢 Low (1 file)
AnalysisThis PR modifies two files in the Severity bump check: 3 non-test/non-generated files (threshold: >20), 139 lines changed (threshold: >500), single distinct critical package — no bump triggered. Three test/mock files ( To override, add a |
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where an incoming HTLC resolver could incorrectly treat a foreign spend of a commitment HTLC output as its own success transaction. It introduces validation of the spending transaction's expected output before sweeping, and checkpoints the resolver as failed (timeout) if a foreign spend is detected. Additionally, the resolver now avoids launching the success path for incoming HTLCs that are already expired by checking the best block height at launch. Comprehensive unit tests have been added to verify these behaviors. No review comments were provided, so there is no additional feedback.
f82a83c to
0a2f9d3
Compare
0a2f9d3 to
61fb9b4
Compare
61fb9b4 to
70db302
Compare
|
LGTM 🎯 ( |
70db302 to
2c8fa1c
Compare
|
LGTM 🚀 ( |
2c8fa1c to
f3cad96
Compare
f3cad96 to
7b2bf4b
Compare
7b2bf4b to
eee8ed9
Compare
eee8ed9 to
b6dc107
Compare
a647572 to
318c461
Compare
318c461 to
1b8995c
Compare
1b8995c to
615c42f
Compare
|
LGTM 🦊 |
10d2e07 to
079f197
Compare
Clear launched or resolved flags when sweep offers or terminal checkpoints fail, so resolvers can retry instead of remaining stuck in an in-memory terminal state that was not persisted.
Use invoice lookups during Launch to restore preimages only for the exact settled circuit, persist learned preimages before creating success resolvers, and continue launched success resolvers after expiry.
Cover restart cases where incoming HTLC contest resolvers learn preimages from the durable preimage store or settled invoice state after expiry, while ignoring open invoices and other circuits.
Merge taproot close-summary fields into incoming HTLC resolvers without replacing resolver-owned state such as preimages learned after the close summary was written.
Treat resolvers that reach a terminal checkpoint during Launch as resolved before starting their Resolve goroutine, and avoid starting resolution after shutdown interrupts concurrent launch.
Restore persisted preimages before offering success resolver sweeps and clear launched state when Launch fails, so later blockbeats can retry without a restart.
Validate direct incoming HTLC success spends by the revealed preimage and checkpoint mismatched spends as failed HTLC outcomes instead of treating them as successful claims.
Only derive second-level success outputs from spends that create the expected output, and persist the confirmed success txid so resolver reports stay correct after sweeper re-signing or aggregation.
Cover direct incoming HTLC success classification by witnessed preimage and assert foreign direct spends checkpoint a failed HTLC outcome instead of a settled claim.
Cover taproot augmentation preserving learned preimages, Launch restoring preimages from the preimage DB, and failed terminal launch checkpoints returning the resolver to retryable state.
Cover second-level success output validation, including re-signed success transactions and foreign spends observed before and after restart.
Cover resolvers that reach terminal state during initial or nested Launch, and assert shutdown before observing Launch results leaves the unresolved contract intact for retry.
079f197 to
4540e8a
Compare
Fixes #10840.
This fixes the HTLC success resolver path that could treat any spend of the original HTLC output as our own second-level HTLC success transaction. If a remote timeout spend is observed instead, the resolver now validates the spender output against the stored sweep descriptor and fails the final HTLC outcome instead of handing a phantom output to the sweeper.
The incoming contest resolver also avoids launching the success resolver after CLTV expiry and passes the current height into the immediate invoice-registry lookup.
Companion sweeper blast-radius fix: #10842.
Testing: