Refactor FeeEstimator to introduce local target variants
#352
Conversation
tnull
force-pushed
the
2024-08-fee-estimator-refactor
branch
from
1930aad
to
a007882
Compare
tnull
force-pushed
the
2024-08-fee-estimator-refactor
branch
2 times, most recently
from
ecfa9f6
to
cebf11b
Compare
| ConfirmationTarget::OnchainPayment => 6, | ||
| ConfirmationTarget::ChannelFunding => 12, |
There was a problem hiding this comment.
Seems like in practice the values are the same as before. Is this change to allow us to vary them in the future?
There was a problem hiding this comment.
Yes, I intentionally left the behavior unchanged for now. This is really only an overdue refactor and also prepares for the LDK upgrade where we'll get more ConfirmationTargets and in particular will split OnChainSweep.
At some point in the future (probably when tackling #176), we'll revisit if the chosen defaults could be improved in any way, or if they are fine as is.
| pub struct Wallet<D, B: Deref, E: Deref, L: Deref> | ||
| pub(crate) struct Wallet<D, B: Deref, E: Deref, L: Deref> |
There was a problem hiding this comment.
Visibility control for Wallet and WalletKeysManager (below) seem unrelated to to the commits purpose.
There was a problem hiding this comment.
I thought so, too, but rustc 1.63 seemed to think otherwise (as the new FeeEstimator is pub(crate), it complained that I couldn't leak a pub(crate) trait in a pub struct, nevermind that in fact the module itself is pub(crate)). I now split the visibility changes out to their own commit though.
tnull
force-pushed
the
2024-08-fee-estimator-refactor
branch
2 times, most recently
from
138bf61
to
9b1ea5a
Compare
.. previously we used LDK's `FeeEstimator` and `ConfirmationTarget` and ~misused some of the latter's variants for our non-Lightning operations. Here, we introduce our own `FeeEstimator` and `ConfirmationTarget` allowing to add specific variants for `ChannelFunding` and `OnchainPayment`s, for example.
tnull
force-pushed
the
2024-08-fee-estimator-refactor
branch
from
9b1ea5a
to
42a695e
Compare
Squashed without further changes. |
.. previously we used LDK's
FeeEstimatorandConfirmationTargetand ~misused some of the latter's variants for our non-Lightning operations.Here, we introduce our own
FeeEstimatorandConfirmationTargetallowing to add specific variants forChannelFundingandOnchainPayments, for example.(This is can also be viewed as a prefactor to #176, which we'll do as part of / after the upcoming rust-bitcoin/BDK upgrade)