Fix chanmon_consistency fuzz test for splice RBF#4536
Draft
jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
Draft
Fix chanmon_consistency fuzz test for splice RBF#4536jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
chanmon_consistency fuzz test for splice RBF#4536jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
Conversation
…ency The process_events! macro only handled DiscardFunding events with FundingInfo::Contribution, but splice RBF replacements can produce DiscardFunding with FundingInfo::Tx when the original splice transaction is discarded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The SplicePending event handler was immediately confirming splice transactions, which caused force-closes when RBF splice replacements were also confirmed for the same channel. Since both transactions spend the same funding UTXO, only one can exist on a real chain. Model this properly by adding a mempool-like pending pool to ChainState. Splice transactions are added to the pending pool instead of being confirmed immediately. Conflicting RBF candidates coexist in the pool until chain-sync time, when one is selected deterministically (by txid sort order) and the rest are rejected as double-spends. If a conflicting transaction was already confirmed, new candidates are dropped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The HandleError assertion only accepted "timeout awaiting response" warnings. However, the fuzzer can deliver messages in unusual orders that trigger legitimate splice and RBF validation failures (e.g., attempting RBF after splice_locked, splicing before quiescence, or splicing a channel that is not live). These produce DisconnectPeerWithWarning with different messages that the assertion needs to accept. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When process_msg_events processes a single message for node B (via OneMessage or OnePendingMessage mode), remaining events are passed to push_excess_b_events for routing. Timer ticks can generate BroadcastChannelUpdate events from marking channels disabled on disconnect, which may be left over after the single processed message. Since it is a broadcast not directed at a specific peer, it can be safely skipped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
👋 Hi! I see this is a draft PR. |
Contributor
Author
|
This should fix the test for the following inputs: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4536 +/- ##
=======================================
Coverage 87.12% 87.12%
=======================================
Files 163 163
Lines 108740 108765 +25
Branches 108740 108765 +25
=======================================
+ Hits 94735 94765 +30
+ Misses 11520 11514 -6
- Partials 2485 2486 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
chanmon_consistencyfuzz target panicked on various inputs involvingsplice RBF because it immediately confirmed splice transactions in the
SplicePendinghandler. When RBF replacements were also confirmed for thesame channel, both transactions ended up in the chain despite spending the
same funding UTXO -- an impossible scenario that caused force-closes.
ChainState. Splice transactions are held in the pool until chain-synctime, when one is selected deterministically (by txid sort order) among
conflicting candidates. Double-spends with already-confirmed transactions
are rejected.
DiscardFundingwithFundingInfo::Txvariant, which is producedwhen a splice transaction is discarded during RBF replacement.
HandleErrorassertion,which can occur when the fuzzer delivers messages in unusual orders.
BroadcastChannelUpdateinpush_excess_b_events, which can beleft over when
process_msg_eventsprocesses a limited number of messages.See #4504