fuzz: chanmon_consistency payment tracking and assertions #4321
Conversation
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
Replace the separate `p_id: u8` (for payment hash generation) and `p_idx: u64` (for payment IDs) with a single `p_ctr: u64` counter. The two counters served different purposes but can be unified since `create_inbound_payment_for_hash` doesn't perform duplicate detection with the parameters we use. This simplifies the code by removing redundant state. Co-Authored-By: Claude Opus 4.5 <[email protected]> Co-Authored-By: Matt Corallo <[email protected]>
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Change `get_payment_secret_hash` to return `(PaymentSecret, PaymentHash)` directly instead of `Option<...>`. The function calls `create_inbound_payment_for_hash` with `min_value_msat=None` and `min_final_cltv_expiry=None`, which cannot fail. Remove the retry loop and use `.expect()` since the call will always succeed with these parameters. Co-Authored-By: Claude Opus 4.5 <[email protected]> Co-Authored-By: Matt Corallo <[email protected]>
Rename `middle_chan_id` to `middle_scid` and `dest_chan_id` to `dest_scid` in `send_hop_noret` and `send_hop_payment`. These parameters are short channel IDs (SCIDs), not channel IDs, so the rename improves clarity. Co-Authored-By: Claude Opus 4.5 <[email protected]> Co-Authored-By: Matt Corallo <[email protected]>
joostjager
force-pushed
the
fuzz-improvements
branch
2 times, most recently
from
f4df930 to
486b7cd
Compare
joostjager
changed the title
Add transaction tracking to TestBroadcaster and verify no unexpected broadcasts occur during normal fuzzing operation: - Store all broadcast transactions in TestBroadcaster - Clear the broadcast set after initial channel opens - Assert in test_return! that no transactions were broadcast Co-Authored-By: Claude Opus 4.5 <[email protected]>
joostjager
changed the title
Replace the standalone `send_noret` and `send_hop_noret` functions with closures defined inside the main loop. This allows them to capture the `nodes` array and use simple node indices (0, 1, 2) instead of passing `&nodes[x]` references at each call site. The `send_payment` and `send_hop_payment` functions are modified to take pre-computed `PaymentSecret`, `PaymentHash`, and `PaymentId` parameters, with the closures handling the credential generation. This centralizes where `PaymentId` is constructed, which will make it easier to add payment tracking in a future change. Co-Authored-By: Claude Opus 4.5 <[email protected]> Co-Authored-By: Matt Corallo <[email protected]>
Track payment lifecycle by maintaining pending_payments and resolved_payments arrays per node: - When sending a payment, add its PaymentId to pending_payments - On PaymentSent/PaymentFailed events, move the PaymentId from pending to resolved (or assert it was already resolved for duplicate events) This tracking enables verifying payment state consistency after node restarts. Co-Authored-By: Claude Opus 4.5 <[email protected]> Co-Authored-By: Matt Corallo <[email protected]>
joostjager
force-pushed
the
fuzz-improvements
branch
from
5c9a6e1 to
a25cd85
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4321 +/- ##
=======================================
Coverage 86.61% 86.61%
=======================================
Files 158 158
Lines 102730 102730
Branches 102730 102730
=======================================
Hits 88984 88984
+ Misses 11328 11327 -1
- Partials 2418 2419 +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:
|
Contributor
Author
|
@TheBlueMatt not sure if you want to review your own dissected code |
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.
This refactors the chanmon_consistency fuzzer to simplify payment handling and add verification. The payment counters are consolidated, helper functions are converted to closures using node indices, and transaction broadcast tracking is added with assertions to catch unexpected broadcasts. Finally, payment lifecycle tracking is introduced to enable verifying payment state consistency.
Picked from https://github.com/TheBlueMatt/rust-lightning/tree/2025-04-fuzz-pending-payments