Drive splices to completion in chanmon_consistency#4411
Drive splices to completion in chanmon_consistency#4411wpaulino wants to merge 6 commits intolightningdevkit:mainfrom
Conversation
This commit adds new opcodes to enable/disable signer operations one by one. Note that this only covers signer operations post-funding.
There were two bugs preventing splices from negotiating up to the `tx_signatures` exchange: 1. A `serial_id` collision because its generation used the first 4 bytes of `get_secure_random_bytes`. 2. Opcodes 0xa2 and 0xa3 used the wrong vout for the input to splice in. This commit fixes both, while also adding support for locking splices. This required confirming transactions, which this target previously didn't consider.
The `short_channel_id` is no longer guaranteed to be stable with splicing now that the fuzzer can actually lock splices.
Even though we don't explicitly send probes, because probes are detected based on hashing the payment hash+preimage, it's rather trivial for the fuzzer to build payments that accidentally end up looking like probes.
This requires calling `timer_tick_occurred`. As a result, disabled/enabled updates may be sent based on the connection status when `timer_tick_occurred` is called.
This regressed at some point, making the logs harder to parse on a failed test run.
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
|
||
| // Since this fuzzer is only concerned with live-channel operations, we don't need to | ||
| // worry about any signer operations that come after a force close. | ||
| 0xc0 => keys_manager_a.disable_op_for_all_signers(SignerOp::SignCounterpartyCommitment), |
There was a problem hiding this comment.
We're actually starting to run a bit tight on one-byte commands. I'm not sure we care that much about supporting partially-async signers all that much anyway - should we just move the disable operations to only three bytes that disable all operations on the given nodes, then leave the enables split (because operations may complete out of order or at any time)?
| make_channel!(nodes[1], nodes[2], monitor_b, monitor_c, keys_manager_c, 5), | ||
| make_channel!(nodes[1], nodes[2], monitor_b, monitor_c, keys_manager_c, 6), | ||
| ]; | ||
| let _ = make_channel!(nodes[0], nodes[1], monitor_a, monitor_b, keys_manager_b, 1); |
There was a problem hiding this comment.
I mean presumably we should stop returning anything from the macro lol.
|
👋 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4411 +/- ##
==========================================
- Coverage 86.05% 86.05% -0.01%
==========================================
Files 156 156
Lines 103384 103384
Branches 103384 103384
==========================================
- Hits 88971 88966 -5
- Misses 11900 11908 +8
+ Partials 2513 2510 -3
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:
|
3 participants
This PR also includes support for fuzzing async signing (though not during initial channel open) and a few miscellaneous fixes as a result of #4367.
After running the fuzzer for a bit with these changes, a few bugs were discovered that will be addressed separately.