Fix "Bad commit_sig signature" error. #3215
Merged
Conversation
This file contains 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
rustyrussell
force-pushed
the
guilt/stress-feechange
branch
from
de51fd9
to
8c87c1a
Compare
A 'Bad commit_sig signature' was reported by @javier on Telegram and @darthcoin. This was between two c-lightning peers, so definitely our fault. Analysis of this message revealed the signature was using the wrong feerate. I finally managed to make a test case which triggered this. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Feerate changes are asymmetric, as they can only be sent by the funder. For FUNDER, the remote feerate is set when upon send of commitment_signed, and the local feerate is set on receipt of revoke_and_ack. For non-funder, the local feerate is set on receipt of commitment_signed, and the remote feerate set on send of revoke_and_ack. In our code, these two happen together. channeld gets this right, but lightningd ignored the funder/fundee distinction, and as a result, receipt of a commitment_signed by the funder altered fees in the database. If there was a reconnection event or restart, then these (incorrect) values would be used, causing us to complain about a 'Bad commit_sig signature' and close the channel. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/stress-feechange
branch
from
8c87c1a
to
907364b
Compare
|
I think this fixes #2661 |
darosior
approved these changes
Oct 28, 2019
|
I am one of the affected, if you require more information that I can provide, just ask. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Thanks to DarthCoin and Javier for reporting this; simple to diagnose once I reproduced.
@cdecker had reports of similar issues with LND under stress, which we assumed was their end. Oops, it was us!