In this commit, we fix an existing bug that would cause us to be unable
to derive the very first key in a key family if the wallet hadn't
already derived it in the past. This can happen if a user keeps their
same `channel.db`, but restores their wallet resulting in fresh
`wallet.db` state.
This is an existing issue due to the fact that we don't properly
distinguish between an empty key locator, and the very first key in a
`KeyFamily`: `(0, 0)`. Atm, `KeyLoactor{0, 0}.IsEmpty() == True`,
causing us to be unable to retrieve this key in certain cases since we
fall through and attempt address based derivation.
In order to remedy this, we add a new special case (until we upgrade
`KeyLoactor` formats, but needed for legacy reasons) to _try_ a regular
`KeyLoactor` based derivation if we fail to derive via address, and this
is an "empty" key loc. This has been tested in the field and shown to
work, with the one downside that in this "hot swap restoration" case,
we'll hit the database twice to derive the key.
In this commit, we fix a logic flaw in the testCreateSimpleTx test case
which emerged once we the bug fix for dust outputs landed. Before this
commit, we would erroneously fail during valid test execution.
In this commit we fix a hidden bug in the transaction creating logic
that was only manifested recently due to higher fees on Bitcoin's
mainnet. Before this commit, we would use the target fee rate to
determine if an output was dust or not. However, this is incorrect, as
instead the relay fee should be used as this matches the policy checks
widely deployed in Bitcoin full node today.
To fix this issue we now properly use the relay fee when computing dust.
This fixes the issue for the `EstimateFee` call, but the `SendOutputs`
call also has a similar issue. However, this must be fixed within
`btcwallet` itself, so it has been left out of this commit
Fixes#3217.
In this commit, we patch a small bug in the newly added raw tx hex field
for ListTransactions. We now ensure that we also set the raw tx hex
field for unconfirmed transactions.
In this commit, we fix a lingering TOOD statement in the channel arb.
Before this commitment, we would simply wipe our our local HTLC set of
the HTLC set that was on the remote commitment transaction on force
close. This was incorrect as if our commitment transaction had an HTLC
that the remote commitment didn't, then we would fail to cancel that
back, and cause both channels to time out on chain.
In order to remedy this, we introduce a new `HtlcSetKey` struct to track
all 3 possible in-flight set of HTLCs: ours, theirs, and their pending.
We also we start to tack on additional data to all the unilateral close
messages we send to subscribers. This new data is the CommitSet, or the
set of valid commitments at channel closure time. This new information
will be used by the channel arb in an upcoming commit to ensure it will
cancel back HTLCs in the case of split commitment state.
Finally, we start to thread through an optional *CommitSet to the
advanceState method. This additional information will give the channel
arb addition information it needs to ensure it properly cancels back
HTLCs that are about to time out or may time out depending on which
commitment is played.
Within the htlcswitch pakage, we modify the `SignNextCommitment` method
to return the new set of pending HTLCs for the remote party's commitment
transaction and `ReceiveRevocation` to return the latest set of
commitment transactions on the remote party's commitment as well. This
is a preparatory change which is part of a larger change to address a
lingering TODO in the cnct.
Additionally, rather than just send of the set of HTLCs after the we
revoke, we'll also send of the set of HTLCs after the remote party
revokes, and we create a pending commitment state for it.
Now that the success resolver preimage field is always populated by the
incoming contest resolver, preimage lookups earlier in the
process (channel and channel arbitrator) can mostly be removed.
This enables users to specify an external API for fee estimation.
The API is expected to return fees in the JSON format:
`{
fee_by_block_target: {
a: x,
b: y,
...
c: z
}
}`
where a, b, c are block targets and x, y, z are fees in sat/kb.
Note that a, b, c need not be contiguous.
In this commit, we add a new interface which will allow callers to drop
in an arbitrary Web API for fee estimation with an arbitrary
request/response schema.
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
In this commit, we modify the main `closeObserver` dispatch loop to only
look for the local force close if we didn't recover the channel. We do
this, as for a recovered channel, it isn't possible for us to force
close from a recovered channel.
In this commit, we modify the `ChanSyncMsg` to send an invalid
commitment secret in `ChanSyncMsg`. We do this in order to force the
remote party to force close off-chain, if we're restoring a channel from
scratch and we never had any state updates within the channel. We need
to do this, as otherwise the remote party will think we can resume as
they're able to verify their own commit secret for state zero.
The checks to determine whether the transaction broadcast failed due to
it already existing in the mempool/chain are no longer needed since the
underlying btcwallet PublishTransaction call will not return an error
when running into these cases.
In this commit, we update the `TestChanSyncFailure` method to pass given
the new behavior around updating borked channel states. In order to do
this, we add a new method to allow the test to clear an existing channel
state. This method may be of independent use in other areas in the
codebase in the future as well.
In this commit, we add a new test: `TestForceCloseBorkedState`. This
ensures that it isn't possible to update the channel state once a
channel has been marked as borked. This assumes that all calls to
`ForceClose` will also mark the channel as borked. This isn't the case
yet, so this test fails as is.
In this commit, we add a new `LastUnusedAddress` method to the
`WalletController` interface. Callers can use this new method to graph
the last unused address, which can be useful for UIs that want to
refresh the address, but not cause nearly unbounded address generation.
The implementation for `btcwallet` uses the existing `CurrentAddress`
method. We've also added a new integration tests to exercise the new
functionality.
In this commit, we set a default max HTLC in the forwarding
policies of newly open channels.
The ForwardingPolicy's MaxHTLC field (added in this commit)
will later be used to decide whether an HTLC satisfies our policy before
forwarding it.
To ensure the ForwardingPolicy's MaxHTLC default matches the max HTLC
advertised in the ChannelUpdate sent out for this channel, we also add
a MaxPendingAmount() function to the lnwallet.Channel.
In this commit, we modify the WitnessCache's
AddPreimage method to accept a variadic number
of preimages. This enables callers to batch
preimage writes in performance critical areas
of the codebase, e.g. the htlcswitch.
Additionally, we lift the computation of the
witnesses' keys outside of the db transaction.
This saves us from having to do hashing inside
and blocking other callers, and limits extraneous
blocking at the call site.
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.
To avoid more bugs slipping through where the logIndex is not set, we
panic to catch this. This was earlier done for Adds and the htlcCounter,
which did lead us to find the resulting retoration bug.
Earlier versions did not write the log index to disk for fee updates, so
they will be unset. To account for this we set them to to current update
log index.
This reverts commit 4aa52d267f000f84caf912c62fc14a5b8e7cacb5.
It turns out that the other implementations set values for this field
which aren't based on the actual capacity of the channel. As a result,
we'll no reject most of their channel offerings, since they may offer a
value of a max `uint64` or something else hard coded that's above the
size of the channel. As a result, we're reverting this check for now to
maintain proper compatibility.
In this commit, we ensure that if a channel is detected to have local
data loss, then we don't allow a force close attempt, as this may not be
possible, or cause us to play an invalid state.
This commit removes the breach transaction from the
arguments passed to NewBreachRetribution. We already
keep all prior remote commitments on disk in the
commitment log, and load that transaction from disk
inside the method. In practice, the one loaded from
disk will be the same one that is passed in by the
caller, so there should be no change in behavior
as we've already derived the appropriate state number.
This changes makes integration with the watchtower
client simpler, since we no longer need to acquire
the breaching commitment transaction to be able to
construct the BreachRetribution. This simplifies
not only the logic surrounding transient backsups,
but also on startup (and later, retroactively
backing up historic updates).
This tests make sure we don't reset our expected fee upate after signing
our next commitment. This test would fail without the previous set of
commits.
Instead of special casing the UpdateFee messages, we instead add them to
the update logs like any other HTLC update message. This lets us avoid
having to keep an extra set of variables to keep track of the fee
updates, and instead reuse the commit/ack logic used for other updates.
This fixes a bug where we would reset the pendingFeeUpdate variable
after signing our next commitment, which would make us calculate the new
fee incorrectly if the remote sent a commitment concurrently.
When restoring state logs, we also make sure to re-add any fee updates.
When compacting the update logs we remove any fee updates when they
remove height is passed. We do this since we'll assume fee updates are
added and removed at the same commit height, as they will apply for all
commitments following the fee update.
This commit adds conversion between the lnwire.UpdateFee message and the
new FeeUpdate PaymentDescriptor. We re-purpose the existing Amount field
in the PaymentDescriptor stuct to hold the feerate.
This commit adds a new updateType that can be used for
PaymentDescriptors: FeeUpdate. We repurpose the fields of the existing
PaymentDescriptor struct such that we can easily re-use the commit/ack
logic used for other update types also for fee updates.
In this commit, we add a new method WithCoinSelectLock. This method will
allow us to fix bugs in the project atm that can arise if a channel
funding is attempted (either manually or by autopilot) while a users is
attempting to send an on-chain transaction. If this happens
concurrently, then both contexts will grab the set of UTXOs and attempt
to lock them one by one. However, since they didn't obtain an exclusive
snapshot of the UTXO set of the wallet, they may both attempt to lock
the same input.
We also ensure that calls to SendMany cannot run into this issue by
using the WithCoinSelectLock synchronization when attempting to instruct
the internal wallet to send payments.
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 remove the per channel `sigPool` within the
`lnwallet.LightningChannel` struct. With this change, we ensure that as
the number of channels grows, the number of gouroutines idling in the
sigPool stays constant. It's the case that currently on the daemon, most
channels are likely inactive, with only a hand full actually
consistently carrying out channel updates. As a result, this change
should reduce the amount of idle CPU usage, as we have less active
goroutines in select loops.
In order to make this change, the `SigPool` itself has been publicly
exported such that outside callers can make a `SigPool` and pass it into
newly created channels. Since the sig pool now lives outside the
channel, we were also able to do away with the Stop() method on the
channel all together.
Finally, the server is the sub-system that is currently responsible for
managing the `SigPool` within lnd.
Returns a brief json summary of each utxo found by calling
ListUnspentWitness in the wallet. The two arguments are the
minimum and maximum number of conrfirmations (0=include
unconfirmed)
One way applications built on top of lnd can estimate sync percentage is
through comparing the current time to the best known timestamp of the
lnd wallet's sync state. Therefore, we should always return this
information even if the the wallet is not synced.
In this commit, we add an additional check to btcwallet's IsSynced
method to ensure that it is not currently undergoing a rescan. We do
this to block upon starting the server and all other dependent
subsystems until the rescan is complete.
In this commit, we add the lightning address scope before the wallet
starts to prevent a race condition between the wallet syncing and adding
the scope itself. This became more apparent with the recent btcwallet
fixes, as several database transactions now occur between the wallet
being started and it syncing.
In this commit, we add a new test to the existing set of wallet tests to
ensure we can properly detect the confirmation of transactions that
spend our change outputs. We do this as a measure to prevent future
regressions from happening where the wallet doesn't request its backend
to be notified of when an on-chain transaction pays to a change address,
like with the recently discovered SendOutputs bug.
As is, this test will not pass until we update the btcwallet dependency
in the next commit.
In this commit, we add an additional check to btcwallet's FetchInputInfo
method to ensure the output is actually under control of the wallet.
Previously, the wallet would assume the output was under its control if
the txid of the output was found within the wallet. This is not a safe
assumption to make however, because if we happened to be the sender of
this transaction, it would be found within the wallet but it's not
actually under our control. To fix this, we explicitly check that there
exists an address in our wallet for this output.
