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

Set anchor output feerates when force-closing #1702

Merged
merged 5 commits into from
Feb 24, 2021

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Feb 22, 2021

When using anchor outputs, the commitment feerate is kept low (<10 sat/byte).
When we need to force-close a channel, we must ensure the commit tx and htlc txs confirm before a given deadline, so we need to increase their feerates.

This is currently done only once, at broadcast time. We use CPFP for the commit tx and RBF for the htlc txs.
If publishing fails because we don't have enough utxos available, it will be retried after the next block is found.

The RBF/CPFP logic requires asynchronous calls to bitcoind, which is why we couldn't just do it in the Channel directly without making the channel state machine more complex (and it's already complex enough).

This PR is a good starting point, but it's still not recommended to activate anchor outputs on your node. More re-architecting works needs to happen to make the CPFP/RBF story more seamless; in the longer term the channel should not care at all about how txs get confirmed. But it would be nice to get this PR included in the next release, as it still hits a first important milestone.

Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

This is a first pass.

Note to myself: need to thoroughly check publishCommitWithAnchor and publishHtlcTx.

@pm47

This comment has been minimized.

@t-bast
Copy link
Member Author

t-bast commented Feb 24, 2021

Noting this here because the file hasn't been modified as part as this PR:

Good catch, this is a bit counter-intuitive, I remember we already had a discussion around it when it was first introduced.
I clarified this in d1542c5, I think I will refactor this afterwards to move this outside of this function (or rename this function to make it more clear).

Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

I have just spent two hours on publishCommitWithAnchor 😓

pm47
pm47 previously approved these changes Feb 24, 2021
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

🎉

When using anchor outputs, the commitment feerate is kept low (<10 sat/byte).
When we need to force-close a channel, we must ensure the commit tx and htlc
txs confirm before a given deadline, so we need to increase their feerates.

This is currently done only once, at broadcast time.
We use CPFP for the commit tx and RBF for the htlc txs.
If publishing fails because we don't have enough utxos available, it will
be retried after the next block is confirmed.

Note that it's still not recommended to activate anchor outputs.
More work needs to be done on this fee bumping logic and utxos management.
It's a bit counter-intuitive that we deduce anchor amounts in this function,
because it's not technically a fee.
@t-bast t-bast merged commit 5d662fc into master Feb 24, 2021
@t-bast t-bast deleted the anchor-output-closing-fees branch February 24, 2021 18:28
tompro pushed a commit to tompro/eclair that referenced this pull request Mar 7, 2021
When using anchor outputs, the commitment feerate is kept low (<10 sat/byte).
When we need to force-close a channel, we must ensure the commit tx and htlc
txs confirm before a given deadline, so we need to increase their feerates.

This is currently done only once, at broadcast time.
We use CPFP for the commit tx and RBF for the htlc txs.
If publishing fails because we don't have enough utxos available, it will
be retried after the next block is confirmed.

Note that it's still not recommended to activate anchor outputs.
More work needs to be done on this fee bumping logic and utxos management.
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.

2 participants