Skip to content

Closing fee_range uses invalid max value #1071

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

Closed
t-bast opened this issue Sep 8, 2021 · 4 comments · Fixed by #1072
Closed

Closing fee_range uses invalid max value #1071

t-bast opened this issue Sep 8, 2021 · 4 comments · Fixed by #1072

Comments

@t-bast
Copy link

t-bast commented Sep 8, 2021

Hey,

I've been testing the quick close mechanism with the latest LDK, and there's one (small) incompatibility between eclair and rust-lightning. It's exactly similar to ElementsProject/lightning#4599 (comment). When rust-lightning doesn't care about the upper bound it sets max_fee_satoshis = 0xffffffffffffffff.

This is rejected by eclair, because we're strict on what values a satoshi field can hold. 0xffffffffffffffff is bigger than 21 million BTC so we consider it an invalid value. We could relax this but I'd like to avoid it if possible. That happens when rust-lightning is simply accepting a value inside eclair's proposed fee range, so I don't think it's necessary to set this to 0xffffffffffffffff, you could just return the same max_fee_satoshis you received.

WDYT?

@TheBlueMatt
Copy link
Collaborator

Sure, happy to change it to whatever, though ideally the spec would say that we have to keep it within a range if you're gonna check for it - much more sustainable than opening issues on all the implementations :)

@t-bast
Copy link
Author

t-bast commented Sep 8, 2021

ideally the spec would say that we have to keep it within a range

I agree, there's no obvious place in the spec right now where we could require that "all bitcoin amounts sent in LN messages must be <= 21 million btc", but I'll add this before the next spec meeting.

@rustyrussell
Copy link

Set it to the channel size?

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Sep 9, 2021 via email

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Sep 9, 2021
When communicating the maximum fee we're willing to accept on a
cooperative closing transaction to our peer, we currently tell them
we'll accept `u64::max_value()` if they're the ones who have to pay
it. Spec-wise this is fine - they aren't allowed to try to claim
our balance, and we don't care how much of their own funds they
want to spend on transaction fees.

However, the Eclair folks prefer to check all values on the wire
do not exceed 21 million BTC, which seems like generally good
practice to avoid overflows and such issues. Thus, our close
messages are rejected by Eclair.

Here we simply relax our stated maximum to be the real value - our
counterparty's current balance in satoshis.

Fixes lightningdevkit#1071
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Sep 10, 2021
When communicating the maximum fee we're willing to accept on a
cooperative closing transaction to our peer, we currently tell them
we'll accept `u64::max_value()` if they're the ones who have to pay
it. Spec-wise this is fine - they aren't allowed to try to claim
our balance, and we don't care how much of their own funds they
want to spend on transaction fees.

However, the Eclair folks prefer to check all values on the wire
do not exceed 21 million BTC, which seems like generally good
practice to avoid overflows and such issues. Thus, our close
messages are rejected by Eclair.

Here we simply relax our stated maximum to be the real value - our
counterparty's current balance in satoshis.

Fixes lightningdevkit#1071
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 a pull request may close this issue.

3 participants