bolts
bolts copied to clipboard
lightning
clarification: when does max_accepted_htlcs apply?
This came up in https://github.com/lightningnetwork/lnd/pull/3910. If we have two parties Alice and Bob who exchange the following asynchronous state updates:
Alice Bob
1. ------add----->
2. <--settle/fail--- (covers a locked-in add)
3. <-----sig------
4. ------sig-----> (covers the add in step 1, Bob's commitment has max htlcs now)
5. <-----rev------
6. ------rev----->
7. ------add----->
where the add Alice sends in step 1 gives Bob the max htlcs once locked-in, then is the add that Alice sends in step 7 allowed? My rationale for allowing it is that the settle/fail in step 2 will make room for the add in step 7. Alice's next signature for Bob that covers the add in step 7 must also cover the settle/fail and thus make room on Bob's commitment transaction (in the end Bob will have max htlcs).
There are two places this is mentioned in the spec:
if result would be offering more than the remote's max_accepted_htlcs HTLCs, in the remote commitment transaction:
MUST NOT add an HTLC.
and
if a sending node adds more than receiver max_accepted_htlcs HTLCs to its local commitment transaction [...]:
SHOULD fail the channel.
Since both passages are concerned with the effect on the commitment transaction, which is the hard constraint here, I think allowing the add in step 7 is appropriate and therefore the correct behavior. If an add is sent in step 8, however, that should be rejected as this would overflow Bob's commitment. Thoughts?
fwiw, seems you addressed it in LND by running validateCommitmentSanity earlier https://github.com/lightningnetwork/lnd/pull/3910
fwiw, seems you addressed it in LND by running validateCommitmentSanity earlier lightningnetwork/lnd#3910
Yup, just need to see that everyone agrees
Ping @t-bast , I think this could be a topic of discussion at a spec meeting. Of course if this is the obvious behavior, then there's nothing to talk about. But I think we should all agree on exactly when it applies.
Ping @t-bast , I think this could be a topic of discussion at a spec meeting.
Agreed, we've all spent some time on this recently, so it would be nice to summarize that. I won't be able to attend tonight's meeting though, but if you can attend it feel free to raise the topic, otherwise I'll add this to the next meeting agenda.
@t-bast Forgot to attend the meeting, could be added to the next meeting agenda
where the add Alice sends in step 1 gives Bob the max htlcs once locked-in, then is the add that Alice sends in step 7 allowed?
That's my understanding too that Alice's RAA in step 6 irremediably drops settle/fail HTLC from her commitment transaction and as such Bob is free to forget about it. At least this is what RL is doing : https://github.com/rust-bitcoin/rust-lightning/blob/84967faf52b992206176e58020dfed80b1093c35/lightning/src/ln/channel.rs#L2562.
Since both passages are concerned with the effect on the commitment transaction, which is the hard constraint here, I think allowing the add in step 7 is appropriate and therefore the correct behavior. If an add is sent in step 8, however, that should be rejected as this would overflow Bob's commitment. Thoughts?
+1 to clarify the spec on this point. AFAICT, the two excerpts you're mentioning aren't saying when a counterparty is supposed to drop a settled/failed HTLC otherwise a HTLC sender could prune out the number of HTLCs announced to remote as soon as update_fail/commitment_signed are received but without waiting to send a RAA ?
That's my understanding too that Alice's RAA in step 6 irremediably drops settle/fail HTLC from her commitment transaction and as such Bob is free to forget about it. At least this is what RL is doing : https://github.com/rust-bitcoin/rust-lightning/blob/84967faf52b992206176e58020dfed80b1093c35/lightning/src/ln/channel.rs#L2562.
Just to make sure we're on the same page -- Bob must validate that Alice's next sig "covers" this removed HTLC otherwise invalid commitment sig occurs.
+1 to clarify the spec on this point. AFAICT, the two excerpts you're mentioning aren't saying when a counterparty is supposed to drop a settled/failed HTLC otherwise a HTLC sender could prune out the number of HTLCs announced to remote as soon as update_fail/commitment_signed are received but without waiting to send a RAA ?
Yeah so I guess the spec doesn't specifically mention that. In lnd the following is not allowed (but is technically valid):
6. ---add---> (max_accepted_htlcs on commitment + 1 in-memory)
7. ---rev--->
8. ---sig---> (must have max_accepted_htlcs since rev effectively dropped an htlc)
We do some pre-validation when receiving an htlc to make sure it doesn't bypass max_accepted_htlcs, but we may need to rework this because atm we assume that a RAA won't occur after an add but before a sig in this specific case of pre-commitment validating max_accepted_htlcs.
Yes, this is my understanding too. If you ever end up constructing a commitment tx which has more than max_accepted_htlcs, your peer has violated the constraint.
Thus, you could choose a more conservative approach when sending, if you wanted, but not when receiving.
Side note: option_simplified_commitment fixes this, of course :)
Yeah so I guess the spec doesn't specifically mention that. In lnd the following is not allowed (but is technically valid):
6. ---add---> (max_accepted_htlcs on commitment + 1 in-memory) 7. ---rev---> 8. ---sig---> (must have max_accepted_htlcs since rev effectively dropped an htlc)
I don't think step 6 is valid? You haven't acked the removal yet, so you'd be over. You're allows in the spec to validate either as-you-go or defer to when you receive commitment_signed, but it has to be valid both ways!
I don't think step 6 is valid?
At least we will reject it, so I'm gonna call it invalid :)
Yeah so I guess the spec doesn't specifically mention that. In lnd the following is not allowed (but is technically valid):
6. ---add---> (max_accepted_htlcs on commitment + 1 in-memory) 7. ---rev---> 8. ---sig---> (must have max_accepted_htlcs since rev effectively dropped an htlc)I don't think step 6 is valid? You haven't acked the removal yet, so you'd be over. You're allows in the spec to validate either as-you-go or defer to when you receive commitment_signed, but it has to be valid both ways!
Ah I see, then step 6 would be invalid and lnd has the correct behavior for this case
Yeah so I guess the spec doesn't specifically mention that. In lnd the following is not allowed (but is technically valid):
6. ---add---> (max_accepted_htlcs on commitment + 1 in-memory) 7. ---rev---> 8. ---sig---> (must have max_accepted_htlcs since rev effectively dropped an htlc)I don't think step 6 is valid? You haven't acked the removal yet, so you'd be over. You're allows in the spec to validate either as-you-go or defer to when you receive commitment_signed, but it has to be valid both ways!
Is the validate as-you-go point explicit or implicit in the spec right now? If it's implicit, might be good to have it explicit?
Actually, I was wrong: we have that language for update_fee (...but MAY delay this check until the update_fee is committed.) but the other checks are immediate:
### Adding an HTLC: `update_add_htlc`
...
A receiving node:
...
- if a sending node adds more than receiver `max_accepted_htlcs` HTLCs to
its local commitment transaction, OR adds more than receiver `max_htlc_value_in_flight_msat` worth of offered HTLCs to its local commitment transaction:
- SHOULD fail the channel.
Which AFAICT is where everyone checks anyway.