-
Notifications
You must be signed in to change notification settings - Fork 492
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
Update closing_signed fee requirement #847
Conversation
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.
I agree on the advantage of this change but I believe we should relax receiver-side first otherwise, you will have unilateral channel closure for nothing between upgraded-more-tolerant peers and non-upgrades ones ?
cc |
bb10122
to
b050092
Compare
I added a I chose flat fees instead of feerates to be consistent with the existing fields. |
OK, perhaps we should split this further, to make it explicit that there's no back-and-forth if both sides support this?
That will be a nice simplification in future... |
3566bcf
to
9a0b1f6
Compare
Updated in 9a0b1f6 |
As described in lightning/bolts#847 We also refactor the negotiating state, add many tests and fix #1742.
Ok, so reading the spec as is this intended exchange isn't obvious. If I understand correctly, the 3 message exchange would be as follows:
If I'm reading @rustyrussell's comment correctly, A should at this point sign + broadcast AS WELL AS replying with
The spec, as written, only details two of these three requirements
I'd propose updating this section to:
Or something along these lines. I think the language Rusty proposed above is a clearer way to specify this fwiw. |
Implementation of lightning/bolts#847 In particular see comment: OK, perhaps we should split this further, to make it explicit that there's no back-and-forth if both sides support this? If you both include the TLV, there's no back-and-forth, it's either two messages or three. If nodes receive the tlv, it picks from that range for its reply: if it doesn't care, uses the same rate. If reply has different rate (and it's acceptable), reply with using that rate. Otherwise it's complete. That will be a nice simplification in future... Needs tests, and interop check. Waiting for finalized wording before importing more spec notes.
You're right, it was too implicit, I added one step in 1684acc, let me know if that's better. The main difficulty in making this clear comes from the fact that we keep specifying the legacy negotiation and the requirements must reflect that. The flow you describe is what I expect implementations to do, so hopefully we won't have surprises when doing interop testing! |
@niftynei did you have time to finalize this on the c-lightning side? |
Not yet :(
…On Tue, Jun 8, 2021 at 08:03 Bastien Teinturier ***@***.***> wrote:
@niftynei <https://github.com/niftynei> did you have time to finalize
this on the c-lightning side?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#847 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIMAKNX3XL2QNXXPOUXQ6TTRYII7ANCNFSM4XVAZ7IQ>
.
|
Implementation of lightning/bolts#847 In particular see comment: OK, perhaps we should split this further, to make it explicit that there's no back-and-forth if both sides support this? If you both include the TLV, there's no back-and-forth, it's either two messages or three. If nodes receive the tlv, it picks from that range for its reply: if it doesn't care, uses the same rate. If reply has different rate (and it's acceptable), reply with using that rate. Otherwise it's complete. That will be a nice simplification in future... Needs tests, and interop check. Waiting for finalized wording before importing more spec notes.
OK, I'm working on implementing this now, at @niftynei request. There are two things here:
Now, LND is doing option_zero_htlc_tx_fee instead, so not sure how that fits (we need more internal work before we can support this). I don't know if they allow 2? So I am going with:
Normally we don't do 2, since we don't reduce fees for option_anchor_outputs commitments (yet) anyway, Hopefully that will allow reasonable transition on any existing (experimental!) deployments. |
Yes, at first I opened the PR only for the second point (which is a spec defect with anchor outputs), then we decided it was a good opportunity to improve closing negotiation, hence the first point. But they are really two separate things.
I think this PR is fully orthogonal to what fees are applied to htlc txs, only the commit tx fee is important and it's not different between
Why? I think this is a change that can benefit all channels right now, I don't see a compelling reason to only apply it to anchor output channels? And if you do, does that mean you would simply ignore the TLVs if you receive them for a standard channel?
In that case you don't need to change anything when you're funder, but you still need to change something when you're fundee, because your counterparty may reduce fees for |
Agreed, logic applies to either.
Our RPC interface to "close" allows you to specify a "fee_negotiation_step". I'd really like to know, up-front, whether we should allow them to specify that (or, perhaps in future a new "feerange" argument). But thinking harder, I think we can say "always send if anchor-outputs applies (and don't let them specify fee_negotiation_step), otherwise, send unless they specified fee_negotiation_step, implying they wanted old-style negotiation)?
Yes. We will always allow lower, but not actually send lower. Testing now... Thanks! |
That sounds perfectly reasonable. That would work for me, and would allow me to easily test interop with eclair. |
OK, I think the language needs some tweaking, in particular, there should be either 2 or 3 packets sent if both are sending the feerange tlvs?
As written, it seems like there's still a negotiation possible, since opener could reply with YA different fee? Which we would like to get away from! Actually, this is tricky. The non-opener doesn't have to send its range, but if it doesn't, then the opener doesn't know if it should negotiate or not (might be old client), so I guess it should send it anyway, even though the information is useless to the opener? |
Here's a diff, which I think makes it clearer. Even more clear might be a complete separate section, but maybe with this explanation it's OK? diff --git a/02-peer-protocol.md b/02-peer-protocol.md
index 58c0579..78de62e 100644
--- a/02-peer-protocol.md
+++ b/02-peer-protocol.md
@@ -542,6 +542,11 @@ the other node then replies similarly, using a fee it thinks is fair. This
exchange continues until both agree on the same fee or when one side fails
the channel.
+In the modern method, the funder sends its permissable fee range, and
+the non-funder has to pick a fee in this range. If it chooses the
+same value, negotiation is complete after two messages, otherwise the
+funder will reply with the same value (completing after three messages).
+
1. type: 39 (`closing_signed`)
2. data:
* [`channel_id`:`channel_id`]
@@ -589,10 +594,20 @@ The receiving node:
and the channel does not use `option_anchor_outputs`:
- MUST fail the connection.
- if the message contains a `fee_range`:
- - if it disagrees with that `fee_range`:
+ - if there is no overlap between that and its own `fee_range`:
- SHOULD fail the connection
- otherwise:
- - MUST propose a `fee_satoshis` in that range
+ - if it is the funder:
+ - if `fee_satoshis` is not in the overlap between the sent and received `fee_range`:
+ - SHOULD fail the connection
+ - otherwise:
+ - MUST reply with the same `fee_satoshis`.
+ - otherwise (it is not the funder):
+ - if it has already sent a `closing_signed`:
+ - if `fee_satoshis` is not the same as the value it sent:
+ - SHOULD fail the connection.
+ - otherwise:
+ - MUST propose a `fee_satoshis` in the overlap between received and (about-to-be) sent `fee_range`.
- otherwise, if `fee_satoshis` is not strictly between its last-sent `fee_satoshis`
and its previously-received `fee_satoshis`, UNLESS it has since reconnected:
- SHOULD fail the connection. |
Thanks, I've applied these suggestions in 9ee62c4 I tried to be clever and not explicitly expand the funder vs fundee case when receiving the |
This follows lightning/bolts#847. For anchor_outputs, we pass down a max_feerate to closingd, and set the fee ceiling to MAX. It uses that to estimate the desired closing fee. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-EXPERIMENTAL: Anchor output mutual close allow a fee higher than the final commitment transaction (as per lightning-rfc ElementsProject#847)
This is now allowed for anchors (as per lightning/bolts#847). We need to play with feerates, since we don't put a discount on anchor commitments yet. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
Updates the closing fee negotiation to comply with most recent spec changes, see lightning/bolts#847 The closing negotiation is backwards compatible with the old negotiation.
- if `fee_satoshis` is not in the overlap between the sent and received `fee_range`: | ||
- MUST fail the channel | ||
- otherwise: | ||
- MUST reply with the same `fee_satoshis`. |
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.
Sorry to gravedig, but what is meant by overlap here? Is it the union of the two ranges or the intersection? If it's the intersection of the two ranges, then the statement a bit above if fee_satoshis matches its previously sent fee_range:
could make this "overlap" statement redundant by changing the SHOULD to a MUST?
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.
Yes, overlap means intersection. We can make the SHOULD
a MUST
, that would make sense.
- if the message contains a `fee_range`: | ||
- if there is no overlap between that and its own `fee_range`: | ||
- SHOULD fail the connection | ||
- MUST fail the channel if it doesn't receive a satisfying `fee_range` after a reasonable amount of time |
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.
Does this imply that you are allowed to send multiple closing_signed
messages without "waiting your turn" if you send bad fee ranges? If so, this interacts with musig2+taproot channels
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.
Yes, this is not a turn-based protocol, the initiator may send another closing_signed
with a different fee range.
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.
Just to clarify, legacy coop close is turn-based, but fee_range isn't?
What is the rationale to make this not turn-based? Shouldn't the recipient of a fee_range ensure that the sent fee_range has overlap, or else don't send one?
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.
Just to clarify, legacy coop close is turn-based, but fee_range isn't?
Yes, fee_range
allows the funder to update its fee_range
if the fundee doesn't respond or sent a warning.
What is the rationale to make this not turn-based?
If you make it strictly turn-based you end up with exactly the previous closing protocol, which fee_range
tries to simplify.
Shouldn't the recipient of a fee_range ensure that the sent fee_range has overlap, or else don't send one?
But that's exactly the "don't send one" part that creates the issue and requires the protocol to allow the sender to send a new one out of turn. The only other option is to force-close, which is undesirable.
I don't see why this is an issue for Taproot. If a sender sends a second closing_signed
, it just throws away the previous state and the receiver does the same. There is an issue if the receiver was concurrently sending its closing_signed
, but in practice 1) it's very unlikely 2) it will result in a force-close which is okay-ish.
A better longer term plan is to migrate closing to use the interactive-tx
protocol.
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.
reading the spec the non-turn-based aspect wasn't immediately clear to me.
specifically for taproot there is no issue, but for the proposal here #995 it is incompatible with fee_range unless some amount of nonces are shared up front or a deterministic nonce scheme is used (I think determinism is undesirable since if you mess up the scheme and re-use a nonce, you've leaked your private key).
the issue boils down to:
- the sender cannot send a second
closing_signed
until the receiver has sent theirclosing_signed
. This is because the sender has effectively used up one of the receiver's nonces and does not know which nonce to use next. This isn't a problem if the flow is turn-based as the receiver will send over the nonces that the sender can use next. This issue doesn't exist during regular channel operation since a sender cannot send twocommit_sig
in a row without arevoke_and_ack
in between.
that said, my question is answered so i'll have to think a bit more on what to do re #995
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.
I see now that you've answered my question in that A (funder) is the one that sends multiple closing_signed if B sends a warning
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.
Yes, sorry for the late reply. There are two possible flows for fee_range
closing. The first is the happy flow:
- A sends
closing_signed
- B responds with
closing_signed
inside thefee_range
proposed by A (B is not allowed to answer with a fee outside of A'sfee_range
) - A responds with
closing_signed
with the same fee as the previous message
The second possible flow is the unhappy flow where B isn't satisfied with A's fee_range
:
- A sends
closing_signed
- B doesn't like the
fee_range
, sends awarning
- A sends another
closing_signed
- B still doesn't like it, sends another
warning
- ...
- A sends another
closing_signed
- B likes that
fee_range
, responds withclosing_signed
- A responds with
closing_signed
with the same fee as the previous message
We can make that strictly turn-based by having Bob send a new message (e.g. reject_closing_signed
) to express his disagreement with the fee_range
(instead of a warning
), in which case we could have nonces in both messages.
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.
I think there is also the 2-message case where:
- A sends
fee_range
[100, 200],fee_satoshis
=150 - B replies with
fee_range
[120, 180],fee_satoshis
=150
The previous idea I was working with was to get rid of signatures in some cases, but it turns out to be complicated given the different message flows. I think a reject message is actually better, but A must only be able to send closing_signed
after receipt, so it is essentially coop close v3.
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.
I don't mind doing a coop close v3 for taproot, with a more strictly specified protocol if it's easier than tacking it on to the existing one!
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.
Right I dont mind either, I can draft some coop changes to add onto @Roasbeef taproot proposal
In this commit, we stop clamping the ideal fee rate to the commitment fee of the channel. This catches us up to this PR of the spec: lightning/bolts#847. We also do away with the old 3x ideal fee "max fee", and replace that with an explicit max fee. This new max fee will either be the default multiplier of the ideal fee, or a new user specified max fee value.
In this commit, we stop clamping the ideal fee rate to the commitment fee of the channel. This catches us up to this PR of the spec: lightning/bolts#847. We also do away with the old 3x ideal fee "max fee", and replace that with an explicit max fee. This new max fee will either be the default multiplier of the ideal fee, or a new user specified max fee value.
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
This commit removes the check that compares mutual close fee with commit tx fee, this check is outdated and has been changed by this spec PR [1]. To prevent unreasonably high fee, this commit introduces MutualCloseMaxFeeMultiplier setting. [1] lightning/bolts#847
With anchor outputs, we can keep the commit tx feerate lower than the real on-chain feerate.
That means that when closing the channel, the closing fee will not necessarily be lower than the current commit tx fee, the existing requirement doesn't make sense to be that strict (at least for anchor outputs).
Imagine for example that the commitment feerate is kept at 10 sat/byte (what lnd currently does when using anchor outputs).
Imagine you want to do a mutual close on that channel and target a confirmation in N blocks, which results in a feerate higher than 10 sat/byte.
That's perfectly reasonable and should be allowed, otherwise you have no way of negotiating a feerate higher than 10 sat/byte (which may confirm in weeks if the mempool is quite full, like it is nowadays).