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

option_closing_rejected: turn-based fee_range coop close (feature 60/61) #1016

Closed
wants to merge 1 commit into from

Conversation

Crypt-iQ
Copy link
Contributor

Adds a feature bit. It's optional, but does simplify some of the spec logic and I think it's nice to have. The naming can change. It works with simple-taproot-channels to allow sending musig2 nonces in a turn-based way.

Fixes #1013

@Roasbeef
Copy link
Collaborator

Commentary from the latest spec meeting:

  • This splits off a bit into paths of adding more information to the warning message (error code, context information, etc), and also a possible greater re-org for co-op close in general.
  • Instead of the new message, we can add more info to warnings, then also add a new bit to help ppl recognize/enforce that
    • In addition to an error code, we might also want to add: suggested value, the field itself, maybe the first 16 bytes of message (or hash or something like that).
  • Also a chance to make the "negotiation" part of co-op closing more explicit

@t-bast
Copy link
Collaborator

t-bast commented Aug 29, 2022

Concept ACK. I would add a feerate_range tlv in the closing_rejected message, it provides more information to the initiator even though it may be obsolete by the time the initiator acts on it (and of course, this will also later contain a tlv with musig2 nonces).

I think it would be confusing to use a warning or an error here. This is an explicit step of the protocol that will require protocol-related handling (e.g. extract musig2 nonces / analyze responder's proposed feerates), so it should use a message tailored for that protocol. It's easy to understand/debug a state machine by only looking at what messages are exchanged, and using a warning for this would force introspection of the message (adds one layer). For example we could have very confusing message flows such as:

  • A sends closing_signed to B
  • B sends warnings to A for a completely unrelated issues
  • B send closing_signed to accept A's closing_signed

If we had a shortage of available message types I would understand the need to create that additional layer of indirection, but since we don't have such a shortage adding such indirection is creating unnecessary complexity?

@Crypt-iQ
Copy link
Contributor Author

Crypt-iQ commented Aug 29, 2022

@t-bast

if there is no overlap between that and its own fee_range:
- SHOULD send a warning

Should the above be changed so that if you are sending closing_rejected, you don't send a warning?

@t-bast
Copy link
Collaborator

t-bast commented Aug 29, 2022

Should the above be changed so that if you are sending closing_rejected, you don't send a warning?

Yes, I believe we should replace the use of warning with this new closing_rejected message now that we have it (it lets us provide more structured data to the sender).

Of course, if others disagree with my previous comment and think we should extend warning/error instead of introducing a new message, this point would be moot.


1. type: 40 (`closing_rejected`)
2. data:
* [`channel_id`:`channel_id`]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment from spec meeting: should this also include a fee range in this message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we were going with the whole warnings approach for telling what parameters you didn't like about any message?

@rustyrussell rustyrussell changed the title option_closing_rejected: turn-based fee_range coop close option_closing_rejected: turn-based fee_range coop close (feature 60/61) Dec 5, 2022
@t-bast
Copy link
Collaborator

t-bast commented Sep 18, 2024

Superseded by #1096

@t-bast t-bast closed this Sep 18, 2024
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.

bolt2: fee_range clarification
3 participants