Skip to content

Add deserialize+load steps to chanmon_fail_consistency (fixes #327) #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

TheBlueMatt
Copy link
Collaborator

@TheBlueMatt TheBlueMatt commented Jul 23, 2019

This expands the chanmon_fail_consistency test quite a bit by serializing/loading ChannelManagers/monitors, including potentially stale-but-UpdateTemporaryFailed monitors!

This simplifies chanmon_fail_consistency greatly and makes it a
tiny bit more realistic. It also sets us up for more
disconnect-relatex events without the complexity blowing up.
@TheBlueMatt TheBlueMatt force-pushed the 2019-07-more-chanmon-fuzz branch 3 times, most recently from 87e3856 to 27ac85b Compare July 24, 2019 02:27
@TheBlueMatt TheBlueMatt force-pushed the 2019-07-more-chanmon-fuzz branch from 27ac85b to 20efabd Compare July 24, 2019 19:33
Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, working on data_loss_protect test, I realized at which point reload from serialization can be buggy, so good to test it more!

@@ -363,13 +364,18 @@ pub fn do_test(data: &[u8]) {

macro_rules! process_msg_events {
($node: expr, $corrupt_forward: expr) => { {
for event in nodes[$node].get_and_clear_pending_msg_events() {
let events = if $node == 1 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so in fact we test only test A <--> B and B <--> C but never A <--> B, doing so would let us hit new cases ? Like maybe cyclic payment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, seems like a reasonable next step to try to fuzz, but not 100% sure it'll add too much.

@TheBlueMatt TheBlueMatt merged commit 3f557cb into lightningdevkit:master Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants