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

channeld: defer first update_fee until we have an HTLC to send. #3634

Merged

Conversation

rustyrussell
Copy link
Contributor

Sending update_fee immediately after channel establishment seems to
upset LND, so work around it by deferring it. The reason we increase
the fee after establishment is because now we might need to close the
channel in a hurry due to htlcs, but until there are htlcs that's
unnecessary.

Workaround for #3596
Signed-off-by: Rusty Russell [email protected]

@rustyrussell rustyrussell added this to the 0.8.2 milestone Apr 8, 2020
@darosior
Copy link
Collaborator

darosior commented Apr 8, 2020

I think this fixes #3341 too.

@cdecker
Copy link
Member

cdecker commented Apr 8, 2020

Seems some more tests require porting to the new behavior:

  • test_update_fee_reconnect: this one seems to actually crash c-lightning

@darosior
Copy link
Collaborator

darosior commented Apr 8, 2020

test_update_fee_reconnect: this one seems to actually crash c-lightning

iirc in this one we make bitcoind fail to answer fee est, so I guess because we wait for the UPDATE_FEE log bcli will timeout on failed attempts (and throw a lovely backtrace) before TIMEOUT.

@rustyrussell
Copy link
Contributor Author

Hmm, somehow I missed that failure. Easy to fix, just added a HTLC to make update_fee work.

@rustyrussell rustyrussell force-pushed the guilt/defer-fee-update branch 2 times, most recently from f575115 to 6c4d5c9 Compare April 11, 2020 02:49
Sending update_fee immediately after channel establishment seems to
upset LND, so work around it by deferring it.  The reason we increase
the fee after establishment is because now we might need to close the
channel in a hurry due to htlcs, but until there are htlcs that's
unnecessary.

Fixes: ElementsProject#3596
Changelog-Changed: Added workaround for lnd rejecting our commitment_signed when we send an update_fee after channel confirmed.
Signed-off-by: Rusty Russell <[email protected]>
@niftynei
Copy link
Collaborator

ACK 462e62e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants