Fuzz multi-part payments #4367
Fuzz multi-part payments #4367
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
joostjager
force-pushed
the
fuzz-multi-channels
branch
from
e287a1f to
3038033
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4367 +/- ##
==========================================
- Coverage 86.09% 86.00% -0.10%
==========================================
Files 156 156
Lines 102804 102781 -23
Branches 102804 102781 -23
==========================================
- Hits 88508 88394 -114
- Misses 11788 11878 +90
- Partials 2508 2509 +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:
|
|
@TheBlueMatt we keep expanding the fuzz space. If we want to stick to byte commands to not make it unnecessarily difficult for the fuzzer, it seems we have to accept less variation. In particular when there are multiple channels, fuzzing operations on each of them individually is quickly using up all byte values. For the same reason, I selected only four variations of mpp. Do you think the trade-off is still right, or should we consider something like 1 byte command + 1 byte parameter? |
joostjager
force-pushed
the
fuzz-multi-channels
branch
from
3038033 to
81c11a0
Compare
This expands the channel monitor consistency fuzz test from 2 channels to 6 channels (3 between A-B and 3 between B-C), enabling future MPP payment testing. Changes: - Extract `connect_peers!` macro from `make_channel!` to avoid duplicate peer connections - Create channel arrays `chan_ab_ids[3]` and `chan_bc_ids[3]` - Store SCIDs in `chan_ab_scids[3]` and `chan_bc_scids[3]` - Use funding transaction versions 1-6 to avoid txid collisions under fuzz hashing (which XORs all bytes to a single byte, causing versions 0-5 to collide between A-B and B-C channel pairs) - Update `test_return!` assertions to expect 3/6/3 channels Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add multi-path payment (MPP) fuzzing commands that split payments across multiple channels: - send_mpp_payment: direct MPP from source to dest using multiple channels - send_mpp_hop_payment: MPP via intermediate node with multiple channels on either or both hops New fuzz commands: - 0x70: direct MPP 0->1 (uses all 3 A-B channels) - 0x71: MPP 0->1->2, multi channels on first hop (A-B) - 0x72: MPP 0->1->2, multi channels on both hops (A-B and B-C) - 0x73: MPP 0->1->2, multi channels on second hop (B-C) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
joostjager
force-pushed
the
fuzz-multi-channels
branch
from
81c11a0 to
e0ba40e
Compare
joostjager
requested review from
TheBlueMatt and
valentinewallace
and removed request for
valentinewallace
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
IMO one of the biggest coverage gaps that we can fix here is what happens to payments when a channel was closed. Its obviously not trivial (we have to handle clearing HTLCs via the monitor path) but I'm not actually sure the changes here would have found the bulk of the issues we've run into on 0.1, whereas closing some of the channels would.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Yes I can definitely see that. My idea was to first add multiple channels and mpp in this PR, and then continue with force closing in the next. Do you think doing it the other way around is better? |
|
Oh, no, this is definitely fine, i just wanted to check if that was the next step/intended as a separate PR or if it wasn't planned. Happy to land this as-is first. |
|
Intended next step. I had added it to the PR description to avoid the question :) |
|
Maybe that'll teach me to read (it won't). |
3 participants
This PR is aiming to increase fuzz coverage generally, and for async persistence of channel monitors specifically. Helpful for increasing confidence that #4345 (which makes use of async persistence) is stable to use.
Changes
chanmon_consistencyfuzz test from 2 to 6 channels (3 per peer pair)New fuzz commands
0x70: Direct MPP A→B using all 3 channels0x71-0x73: MPP via intermediate node with multi-channel variants on first hop, both hops, or second hopFollow-up
Force-closing one of the channels