For anchor channels and neutrino backends we need to make sure
that sweeps of the same exclusive group are removed when one of
them is confirmed. Otherwise for neutrino backends those sweep
transaction are always rebroadcasted and are blocking funds in
the worst case scenario.
This commit changes how we create the input sets which are used to
construct the sweeping transactions. Assume the sweeper has two inputs,
one is new and one is retried, we'd end up having two transactions,
- tx1: which spends both the new and old inputs.
- tx2: which spends the new inputs only.
When publishing these txes, depending on which one gets into the mempool
first, the other one will be viewed as an RBF for the first one since
they both spending the same input(the new input).
This is now fixed by only attempt to publish the second tx when there
isn't a first tx - when there is a tx1, it means the new inputs are
already used in this tx along with retried inputs, hence there's no need
to publish tx2 which spends the new inputs only.
This commit attempts to make the polling logic in sweeper more linear.
Previously, the sweep's timer is reset/restarted in multiple places,
such as when a new input comes in, or a new block comes in, or a
previous input being spent, making it difficult to follow. We now remove
the old timer and replaces it with a simple polling logic - we will
schedule sweeps every 5s(default), and if there's no input to be swept,
we'd skip, just like the previous `scheduleSweep` does.
It's also worthy noting that, although `scheduleSweep` triggers the
timer to tick, by the time we do the actual sweep in `sweepCluster`,
conditions may have changed. This is now also fixed because we only have
one place to create the clusters and sweeps.
This commit makes sure an input is only added to the cluster when it has
successfully estimated its fee rate. Previously, when an error is
returned from `feeRateForPreference`, we'd still add this input to the
cluster, resulting a **lower** fee rates being used because when
averaging the fee rates, we'd think this input has zero fee rate
specified.
An unit test is patched to make the method `clusterByLockTime` more
robust.
* sweep: use longer variable name for clarity in `addToState`
* sweeper: add more docs and debug logs
* sweep: prioritize smaller inputs when adding wallet UTXOs
This commit sorts wallet UTXOs by their values when using them for
sweeping inputs. This way we'd avoid locking large UTXOs when sweeping
inputs and also provide an opportunity to aggregate wallet UTXOs.
* contractcourt+itest: relax anchor sweeping for CPFP purpose
This commit changes from always sweeping anchor for a local force close
to only do so when there is an actual time pressure. After this change,
a forced anchor sweeping will only be attempted when the deadline is
less than 144 blocks.
* docs: update release notes
* itest: update test `testMultiHopHtlcLocalChainClaim` to skip CPFP
Since we now only perform CPFP when both the fee rate is higher and the
deadline is less than 144, we need to update the test to reflect that
Bob will not CPFP the force close tx for the channle Alice->Bob.
* itest: fix `testMultiHopRemoteForceCloseOnChainHtlcTimeout`
* itest: update related tests to reflect anchor sweeping
This commit updates all related tests to reflect the latest anchor
sweeping behavior. Previously, anchor sweeping is always attempted as
CPFP when a force close is broadcast, while now it only happens when the
deadline is less than 144. For non-CPFP purpose sweeping, it will happen
after one block is mined after the force close transaction is confirmed
as the anchor will be resent to the sweeper with a floor fee rate, hence
making it economical to sweep.
This commit updates the `fee()` method in `weightEstimator` to make sure
when doing CPFP we are not exceeding the max allowed fee rate. In order
to use the max fee rate, we need to modify several methods to pass the
configured value to the estimator.
We remove the publishing of the last published sweep tx during the
startup of the sweeper. This republishing can lead to situations
where funds of the default wallet might be locked for neutrino
backend clients.
Moreover all related tests are removed as well.
In this commit, we an existing gap in our rebroadcast handling logic. As
is, if we're trying to sweep a transaction and a conflicting transaction
is mined (timeout lands on chain, anchor swept), then we'll continue to
try to rebroadcast the tx in the background.
To resolve this, we give the sweeper a new closure function that it can
use to mark conflicted transactions as no longer requiring rebroadcast.
Before this commit, we we were trying to sweep an anchor output, and
that output was spent by someone else (not the sweeper), then we would
report this back to the original resolver (allowing it to be cleaned
up), and also remove the set of inputs spent by that transaction from
the set we need to sweep.
However, it's possible that if a user is spending unconfirmed outputs,
then the wallet is holding onto an invalid transaction, as the outputs
that were used as inputs have been double spent elsewhere.
In this commit, we fix this issue by recursively removing all descendant
transactions of our past sweeps that have an intersecting input set as
the spending transaction. In cases where a user spent an unconfirmed
output to funding a channel, and that output was a descendant of the now
swept anchor output, the funds will now properly be marked as available.
Fixes#6241
This commit was previously split into the following parts to ease
review:
- 2d746f68: replace imports
- 4008f0fd: use ecdsa.Signature
- 849e33d1: remove btcec.S256()
- b8f6ebbd: use v2 library correctly
- fa80bca9: bump go modules
This aims to cover an edge case and also serves as an optimization of
what happens when an input that was offered to the Sweeper with an
exclusive group is re-offered without one. This happens every time we
attempt to sweep the different possible anchors of a channel at the time
of broadcast, as we don't know which commitment transaction will end up
confirming in the chain. Once the commitment transaction confirms
however, we know which anchor output to act upon and re-offer it to the
Sweeper without an exclusive group. At this point, the Sweeper will
continue to attempt sweeping the other anchor output versions even know
we know they are not valid.
This commit updates call-sites to use the proper dust limits for
various script types. This also updates the default dust limit used
in the funding flow to be 354 satoshis instead of 573 satoshis.
We risked deadlocking on shutdown if a client (in our case a contract
resolver) attempted to schedule a sweep of an input after the
ChainNotifier had been shut down. This would cause the `collector`
goroutine to exit, and not handle incoming requests, causing a deadlock
(since the ChainArbitrator is being stopped before the Sweeper in the
server).
To fix this we could change the order these subsystems are stopped, but
this doesn't ensure there aren't other clients that could end up in the
same deadlock scenario. So instead we keep handling the incoming
requests even after the collector has exited (immediatly returning an
error), until the sweeper is signalled to shutdown.
Extend the fee estimator to take into account parent transactions with
their weights and fees.
Do not try to cpfp parent transactions that have a higher fee rate than
the sweep tx fee rate.
The sweeper call UpdateParams does not update the exclusive group
property of a pending sweep. This led to anchor outputs being swept
after confirmation with an exclusive group restriction, which is not
necessary.
This commit changes the anchor resolver to not use UpdateParams anymore,
but instead always re-offer the anchor input to the sweeper. The sweeper
is modified so that a re-offering also updates the sweep parameters.
Add label parameter to PublishTransaction in WalletController
interface. A labels package is added to store generic labels that are
used for the different types of transactions that are published by lnd.
To keep commit size down, the two endpoints that require a label
parameter be passed down have a todo added, which will be removed in
subsequent commits.
Exclusive group is a static property that doesn't need to be updated.
Requiring the exclusive group to be passed into UpdateParams creates a
burden for the caller to make sure they supply the existing group.
This change will be beneficial for users that bump anchor sweeps that
have exclusive groups set.
