Skip to content

More aggressive confirmation target when low on utxos#2141

Merged
t-bast merged 2 commits intomasterfrom
bump-fee-check-utxo-count
Jan 24, 2022
Merged

More aggressive confirmation target when low on utxos#2141
t-bast merged 2 commits intomasterfrom
bump-fee-check-utxo-count

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Jan 21, 2022

Now that #2113 is out of the way, we can start tweaking and making small improvements to the fee-bumping mechanism.

An optimistic schedule for confirmation during force-close scenarios could in theory help malicious peers starve us of our utxos, making it hard to fee-bump HTLC transactions in high-fee environments.

If we detect that our safe utxos count is low, we try to get force-close transactions confirmed as quickly as possible, which will yield a new utxo that can be used to fund other transactions.

This PR is a deterministic alternative to #2137

An optimistic schedule for confirmation during force-close scenarios
could in theory help malicious peers starve us of our utxos, making it
hard to fee-bump HTLC transactions in high-fee environments.

If we detect that our safe utxos count is low, we try to get force-close
transactions confirmed as quickly as possible, which will yield a new utxo
that can be used to fund other transactions.
@t-bast t-bast requested a review from pm47 January 21, 2022 11:01
@codecov-commenter
Copy link

Codecov Report

Merging #2141 (1352c21) into master (2f07b3e) will decrease coverage by 0.04%.
The diff coverage is 87.80%.

@@            Coverage Diff             @@
##           master    #2141      +/-   ##
==========================================
- Coverage   87.47%   87.43%   -0.05%     
==========================================
  Files         170      170              
  Lines       13200    13216      +16     
  Branches      546      579      +33     
==========================================
+ Hits        11547    11555       +8     
- Misses       1653     1661       +8     
Impacted Files Coverage Δ
.../fr/acinq/eclair/blockchain/fee/FeeEstimator.scala 100.00% <ø> (ø)
...clair/channel/publish/ReplaceableTxPublisher.scala 74.49% <87.17%> (-0.88%) ⬇️
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala 92.34% <100.00%> (+0.03%) ⬆️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala 95.93% <0.00%> (-1.63%) ⬇️
...main/scala/fr/acinq/eclair/router/Validation.scala 90.76% <0.00%> (-1.54%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 87.97% <0.00%> (+0.07%) ⬆️
...main/scala/fr/acinq/eclair/io/PeerConnection.scala 85.82% <0.00%> (+0.38%) ⬆️
...ir/blockchain/bitcoind/rpc/BitcoinCoreClient.scala 96.15% <0.00%> (+1.92%) ⬆️

pm47
pm47 previously approved these changes Jan 24, 2022
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 easier to understand and more foolproof than #2137, and also more optimal in a normal low fees scenario.

feeEstimator.getFeeratePerKw(blockTarget)
} else {
// We don't have many safe utxos so we want the transaction to confirm quickly.
feeEstimator.getFeeratePerKw(1)
Copy link
Member

Choose a reason for hiding this comment

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

I know it makes sense to target one block here, but I'm afraid we overpay. How about two blocks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 901b565

@t-bast t-bast merged commit 2d64187 into master Jan 24, 2022
@t-bast t-bast deleted the bump-fee-check-utxo-count branch January 24, 2022 10:27
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.

3 participants