Skip to content

Check Bolt 2 spec - Closing Negotiation: closing_signed #250

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

Conversation

neonknight64
Copy link

Added more appropriate actions for the errors in the closing_signed function of channel. Out of bound fee updates have disconnect peer actions as specified by the bolt 2 specs. Error messages where sent when out of order instructions were received, I wasn't sure about this as the bolt 2 specs didn't specify what actions should be taken in these cases.

@TheBlueMatt
Copy link
Collaborator

I'd like to move to just using ChannelError instead of filling out actions everywhere since ChannelError is much simpler and lets ChannelManager make more decisions about when to disconnect/just send an error. That said, I think closing_signed negotiation is somewhat borked right now - I went to go write tests and make sure we're compliant but ended up deciding the spec is unimplementable and confused - the whole "drop your own output" thing is kinda nonsense given that the message is sending the total fee and you cannot exceed the base fee on the last commitment transaction (which is also undefined since there are two commitment transactions and the spec doesn't stop update_fee updates after shutdown). Given its likely gonna get rewritten for 1.1 I kinda just decided to leave it as-is for now and make sure 1.1 is more clear and then just move on requiring 1.1.

@neonknight64
Copy link
Author

Thank you for the comments TheBlueMatt. Based on your comments I reverted the changes. Here is a checklist of all the Bolt 2 specs I checked for the Closing Negotiation - closing_signed functionality.
All of them seem to be implemented according to the spec except the two not failing the connection.

Checking Bolt 2 spec for Closing Negotiation - closing_signed
[Y] Message structure correct - ClosingSigned
[Y] Check shutdown is complete before fee negotiation
[Y] Check channel is empty of HTLCs before fee negotiation
[Y] Funding node should send closing_signed message after receiving shutdown and no HTLCs remain
[Y] Sending node must set fee less than or equal to the base fee of the final commitment transaction
[Y] Sending node should set initial fee according to its estimate of cost of inclusion in a block
[Y] Sending node must set signature to the signature of the close transaction
[Y] Receiving node must fail connection when signature of close transactions invalid
[Y] Receiving node should broadcast final closing transaction when current and previous fee are the same
[-] Receiving node must fail connection when fee is greater than the base fee of the final commitment transaction
Comment: Implemented check with error but not failing connection
[-] Receiving node should fail connection when fee not between previous sent and received fee, except when reconnected
Comment: Implemented check with error but not failing connection
[Y] Receiving node should send closing_signed with same fee when agreement is reached
[Y] Receiving node can propose new fee between previous received and sent fee

@neonknight64 neonknight64 deleted the 204-check-bolt2-closing-negotiation branch November 14, 2018 12:14
@TheBlueMatt TheBlueMatt mentioned this pull request Nov 15, 2018
26 tasks
@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Nov 21, 2018 via email

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