Skip to content
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

channeld: send warning, not error if peer has old commitment number. #5152

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

rustyrussell
Copy link
Contributor

This is the minimal change to meet the desired outcome of lightning/bolts#934
which wants to give obsolete-db nodes a chance to fix things up, before we
close the channel.

We need to dance around a bit here, since we will close the channel if
we receive an ERROR, so we suppress that.

@rustyrussell rustyrussell requested a review from cdecker as a code owner April 1, 2022 02:05
@t-bast
Copy link

t-bast commented Apr 1, 2022

This title of the PR is misleading, shouldn't it be exactly the opposite (ie send warning, not error)?

@rustyrussell rustyrussell changed the title channeld: send error, not warning, if peer has old commitment number. channeld: send warning, not error if peer has old commitment number. Apr 2, 2022
# l1 should NOT drop to chain, since it didn't receive an error.
time.sleep(5)
assert bitcoind.rpc.getrawmempool(False) == []

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
l1.daemon.wait_for_logs(["channeld WARNING: bad reestablish revocation_number: 0 vs 3"])

@rustyrussell rustyrussell added this to the v0.12 milestone Apr 12, 2022
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 3532d3d

@cdecker cdecker force-pushed the test-reestablish-fail branch from 3532d3d to 9841485 Compare July 19, 2022 10:17
@cdecker
Copy link
Member

cdecker commented Jul 19, 2022

Rebased on top of master to get CI to rerun and ready the PR for merge.

@rustyrussell rustyrussell modified the milestones: v0.12, v22.10 Jul 26, 2022
@cdecker cdecker force-pushed the test-reestablish-fail branch from 9841485 to 0697009 Compare September 19, 2022 11:49
This is the minimal change to meet the desired outcome of lightning/bolts#934
which wants to give obsolete-db nodes a chance to fix things up, before we
close the channel.

We need to dance around a bit here, since we *will* close the channel if
we receive an ERROR, so we suppress that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker cdecker force-pushed the test-reestablish-fail branch from 0697009 to a551f70 Compare September 21, 2022 10:15
@cdecker cdecker merged commit b698a5a into ElementsProject:master Sep 26, 2022
@cdecker cdecker deleted the test-reestablish-fail branch September 26, 2022 09:36
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.

5 participants