-
Notifications
You must be signed in to change notification settings - Fork 265
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
Conversation
e61504c
to
fa345d3
Compare
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.
This is a first pass.
Note to myself: need to thoroughly check publishCommitWithAnchor
and publishHtlcTx
.
eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Good catch, this is a bit counter-intuitive, I remember we already had a discussion around it when it was first introduced. |
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 have just spent two hours on publishCommitWithAnchor
😓
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Show resolved
Hide resolved
090f7cb
to
a0d48a8
Compare
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala
Outdated
Show resolved
Hide resolved
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.
🎉
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.
378108a
to
5b072b1
Compare
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.
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 theChannel
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.