-
Notifications
You must be signed in to change notification settings - Fork 894
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
Bugfix: Fix 'extreme cases' logging of many commit timer failures #5881
Bugfix: Fix 'extreme cases' logging of many commit timer failures #5881
Conversation
What is this about? |
I'm fixing dead code. This log appears to be a sanity check which will show up whenever a commitment timer is set before it has received the corresponding revoke and ack for the previous commitment, which isn't allowed by the protocol. Currently it's impossible to actually trigger because of the bug as described. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack 7cfbaf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 7cfbaf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to initialize commit_timer_attempts at start time:
Valgrind error file: valgrind-errors.74244
==74244== Conditional jump or move depends on uninitialised value(s)
==74244== at 0x110AF6: send_commit (channeld.c:1239)
==74244== by 0x13A03F: timer_expired (timeout.c:62)
==74244== by 0x117ED3: main (channeld.c:4042)
==74244==
{
<insert_a_suppression_name_here>
Memcheck:Cond
fun:send_commit
fun:timer_expired
fun:main
}
7cfbaf6
to
a224587
Compare
good catch valgrind; fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK a224587
Currently it is impossible for the log line to be hit since the value is set to 1, then reset immediately after to 0. Makes it hard to see if this is happening in a degenerate way.
Changelog-None