In this commit, we add a new option for the existing confirmation
notification system that optionally allows the caller to specify that a
block should be included as well.
The only quirk w/ the implementation here is the neutrino backend:
usually we get filtered blocks, we so need to first fetch the block
again so we can deliver the full block to the notifier. On the notifier
end, it'll only be checking for the transactions we care about, to
sending a full block doesn't affect the correctness.
We also extend the `testBatchConfirmationNotification` test to assert
that a block is only included if the caller specifies it.
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.
With go 1.17 a change to the build flags was implemented:
https://go.googlesource.com/proposal/+/master/design/draft-gobuild.md
The formatter now automatically adds the forward-compatible build tag
format and the linter checks for them, so we need to include them in our
code.
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.
This to more easily track mismatches if constants and get more accurate
fee estimates for the two channel types.
The non-anchor weight estimates will now be smaller, this is okay since
these constants are only being used for fee estimation (and will now be
more accurate).
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.
This add a test for inputs that gets re-ordered because the inputs with
required TxOuts must be added first.
We add a new step to the test that checks that all inputs were signed at
the correct tx input index.
This test would fail without the previous commit.
This commit fixes an issue that would arise if inputs without required
TxOuts would be swept together with inputs with required TxOuts. In this
case we would add the required ones first to the transaction, but did
not change the order we signed the inputs, resulting in signing the
wrong input index.
Now that inputs might have accompanied outputs to be added to the sweep
tx, we add them to the sweep transaction first, and account for it when
calculating the change amount.
If inputs require outputs to be added at the same time, this will
change the weight and amount calculations, so we must account for that.
We wait to get the weight estimator for the sweep tx until needed,
such that we can easily choose whether to include a change output or not
in the estimate. This is needed for the case where the second level
transactions can pay for their own fee, so no change output is needed.
Similarly as with kvdb.View this commits adds a reset closure to the
kvdb.Update call in order to be able to reset external state if the
underlying db backend needs to retry the transaction.
This commit adds a reset() closure to the kvdb.View function which will
be called before each retry (including the first) of the view
transaction. The reset() closure can be used to reset external state
(eg slices or maps) where the view closure puts intermediate results.
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.
Preparation for a cpfp-aware weight estimator. For cpfp, a regular
weight estimator isn't sufficient, because it needs to take into account
the weight of the parent transaction(s) as well.
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.
The add function tries to add an input to the current set. It therefore
calculates what the new set would look like before actually adding. This
commit isolates the state of the tentative set so that there is less
opportunity for bugs to creep in.
This commit moves all localized instances of mock implementations of
the Signer interface to the lntest/mock package. This allows us to
remove a lot of code and have it housed under a single interface in
many cases.
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.
This commit introduces a new test case that asserts all of the witness
size constants currently in the codebase. We also reintroduce the
AcceptedHtlcSuccessWitnessSize and OfferedHtlcTimeoutWitnessSize
constants that were recently removed for the sake of completeness.
In asserting the witnes sizes, there were three uncovered discrepancies:
* OfferedHtlcSuccessWitnessSize overestimated by about 30% because it
included an extra signature in the calculation.
* ToLocalPenaltyWitnessSize was underestimated by one byte, because it
was missing the length byte for the OP_TRUE. This has implications
the watchtower protocol since the client and server are assumed to
share the same weight estimates used for signing. This commit keeps
the current behavior, with the intention of rolling out negotiation
for which weight estimate to use for a given session.
* AcceptedHtlcScriptSize was underestimated by one byte because it was
missing a length byte for the value 32 pushed on the stack when
asserting the preimage's length. This affects all AcceptedHtlc*
witness sizes.
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.
We also increase the witness size for these types to account for the 3
extra bytes. The size won't be correct in all cases, but it is just an
upper bound in any case.
Allows certain sweep inputs to be kept in separate transactions at all
times. This is a preparation for anchor outputs. Before the commitment
tx confirms, there are three potential anchors that can be cpfp'ed. We
want to cpfp them all, but if done in the same transaction, the
transaction would guaranteed to be invalid. Exponential backoff would
eventually get the txes published, but having exclusive groups makes the
process faster.
Previously only the fee rate used for the last sweep (the sweep bucket
average) was reported. This commit adds the request fee preference to
the report, which is used to select a bucket and the sweep tx fee rate.
This commit allows sweeper to sweep inputs that on its own are not able
to form a sweep transaction that meets the dust limit.
This functionality is useful for sweeping small outputs. In the future,
this will be particularly important to sweep anchors. Anchors will
typically be spent with a relatively large fee to pay for the parent tx.
It will then be necessary to attach an additional wallet utxo.
A refactoring that introduces no functional changes. This prepares for
the addition of wallet utxos to push the sweep tx above the dust limit.
It also enabled access to input-specific sweep parameters during tx
generation. This will be used in later commits to control the sweep
process.
In this commit, we create a new chainfee package, that houses all fee
related functionality used within the codebase. The creation of this new
package furthers our long-term goal of extracting functionality from the
bloated `lnwallet` package into new distinct packages. Additionally,
this new packages resolves a class of import cycle that could arise if a
new package that was imported by something in `lnwallet` wanted to use
the existing fee related functions in the prior `lnwallet` package.
Because the BestBlock method of ChainIO is not exposed through any
RPC we want to get rid of it so we can use the sweeper outside of
lnd too. Since the chain notifier now also delivers the current best
block we don't need the BestBlock method any more.
In this commit, we update the `CommitSpendNoDelay` method to be aware of
the alternate spending mechanism for commitments that don't have a tweak
for the remote party's non-delay output. We also add a new witness type
so callers can convey their expected signing path.
In this commit, we introduce the ability to bump the fee of an input
within the UtxoSweeper. Once its fee rate is bumped, a replacement
transaction (RBF) will be broadcast with the newer fee rate (assuming
the newer fee rate is high enough to be valid), replacing any
conflicting lower fee rate transactions.
Note that this currently doesn't validate the fee preference of the
bump. This responsibility is delegated to the caller, so care must be
taken to ensure the new fee preference is sufficient.
In this commit, we address another issue that arose with the
introduction of the fee rate buckets. We'll use an example to explain
the problem space:
Let's say we have inputs A, B, and C within the same fee rate bucket. If
A's fee rate is bumped to a higher bucket, then it's currently possible
for the lower fee rate bucket to be swept first, which would produce an
invalid RBF transaction since we're removing an input from the original
without providing a higher fee. By the time we get to the higher fee
rate bucket, we broadcast a valid RBF transaction _only_ sweeping input
A, which would evict the transaction sweeping inputs B and C from the
mempool.
To prevent this eviction, we can simply broadcast the higher fee rate
sweep transactions first, to ensure we have valid RBF transactions.
In this commit, we introduce support for arbitrary client fee
preferences when accepting input sweep requests. This is possible with
the addition of fee rate buckets. Fee rate buckets are buckets that
contain inputs with similar fee rates within a specific range, e.g.,
1-10 sat/vbyte, 11-20 sat/vbyte, etc. Having these buckets allows us to
batch and sweep inputs from different clients with similar fee rates
within a single transaction, allowing us to save on chain fees.
With this addition, we can now get rid of the UtxoSweeper's default fee
preference. As of this commit, any clients using the it to sweep inputs
specify the same fee preference to not change their behavior. Each of
these can be fine-tuned later on given their use cases.
This commit is a step to split the lnwallet package. It puts the Input
interface and implementations in a separate package along with all their
dependencies from lnwallet.
In this commit, we add a new function, CraftSweepAllTx. This function
allows callers to craft a transaction which sweeps ALL outputs from the
wallet to a single target address. It can either be used for UTXO
consolidation (at the cost of privacy by co-mingling inputs), or simply
to sweep all funds out of a wallet for various reasons.
In an attempt to ensure this method is loosely coupled and testable, for
all behavior structs, we create brand new interface to accept. This
ensures that we only rely on the minimal number of methods needed to
perform our duty.
In this commit, we update the `getInputWitnessSizeUpperBound` and all
its callers to be aware of nested p2sh witness inputs. We do so by
adding another bool which is true if the output is a nested p2sh output.
If so, then in order to properly estimate the total weight, the caller
needs to factor in the non-witness data of the additional sigScript data
push.
In this commit, we extend the WitnessGenerator type to now return an
InputScript. This allows it to be more encompassing, as now callers can
expect a sigScript to be populated if the input being swept requires a
sigScript field.
Along the way, we've also renamed input.BuildWitness to
input.CraftInputScript. We also take a step towards allowing the
sweeper to sweep transactions for n2pwkh outputs. We do so by modifying
the BuiltWitness method to instead return an InputScript. Additionally,
when populating inputs if a sigScript is present, it will now be
populated.
In this commit, we extract the existing determineFeePerKw method on the
RPC server into a new file in the sweep package. Along the way, we
consolidate code by introducing a new FeePreference struct, which allows
the caller to express their fee preference either in blocks to
confirmation, or a direct fee rate. This move takes a small step to
father decoupling calls in the main RPC server.
This commit fixes a test flake caused by a race condition. Using the
real sweeper in the nursery test created too complex concurrent
behaviour to reliably assert on and made the tests difficult to
comprehend.
In this commit, the sweep package is extended from just tx generation to
an active sweeper that collects sweep inputs and autonomously proceeds
to publish the sweep tx after the batch window time interval has passed
without new inputs being added.
This commit is a preparation for the implementation of remote spend
detection. Remote spends may happen before we broadcast our own sweep
tx. This calls for accurate height hints.
In this commit, we modify the newly introduced UtxoSweeper.CreateSweepTx
to accept the confirmation target as a param of the method rather than a
struct level variable. We do this as this allows each caller to decide
at sweep time, what the fee rate should be, rather than using a global
value that is meant to work in all scenarios. For example, anytime
we're sweeping an output with a CLTV lock that's has a dependant
transaction we need to sweep/cancel, we may require a higher fee rate
than a regular force close with a CSV output.
This commit restores the sweep behavior of
filtering inputs with unknown witness types
from the final sweep transaction. Currently,
such inputs will be included without adding
anything to the weight estimate, possibly
creating a transaction with insufficient fees.
This commit introduces a common interface for sweep
inputs. It eliminates the type checking from UtxoSweeper.
Also the formerly present Amount() getter is removed. It was redundant
because this value is present in SignDesc().Output.Value as well.
