To simplify the message signing API even further, we refactor the
lnwallet.MessageSigner interface to use a key locator instead of the
public key to identify which key should be signed with.
This commit adds a fetchMinMempoolFee function to the BitcoindEstimator
that fetches the current min mempool fee from the bitcoind backend. The
commit then also updates the BitcoindEstimator to use a minFeeManager
for it's minFeeManager member and uses the fetchMinMempoolFee function
to initialise this.
The validateFeeRate function uses the availableBalance function to get
the current spendable balance of a channel, adds the old fee and then
ensures that the new fee is not larger than the amount we have available
to spend. This commit also removes the local reserve check in the
validateFeeRate function since the balance returned from
availableBalance already takes the local reserve into acccount.
In this commit we ensure that the max fee calculated in the MaxFeeRate
function takes the local reserve amount into account along with any
pending HTLCs. This is done by calling the avaialbeBalance function.
The FundingPsbtFinalize step is a safety measure that assures the final
signed funding transaction has the same TXID as was registered during
the funding flow and was used for the commitment transactions.
This step is cumbersome to use if the whole funding process is completed
external to lnd. We allow the finalize step to be skipped for such
cases. The API user/script will need to make sure things are verified
(and possibly cleaned up) properly.
During the final part of the channel funding negotiation we only need to
assemble the full funding TX with the witness if we are going to publish
the transaction ourselves. If the final funding TX is published
externally we don't need this information. This will make it possible to
skip the verify process for fully externally funded PSBT channels.
It over-estimates the local or remote commitment's dust sum by
counting all updates in both updateLogs that are dust using the
trimmed-to-dust mechanism if applicable. The over-estimation is done
because ensuring an accurate counting is a trade-off between code
simplicity and accuracy.
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 is necessary and is implied by BOLT#02. Both ChannelReserve
parameters should be above both DustLimit parameters. Otherwise,
it is possible for one side to have nothing at stake.
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.
Adds a new Brontide struct method tryLinkShutdown that attempts to
fetch the target link and calls ShutdownIfChannelClean on it. This
allows the coop close process to guarantee atomicity of the underlying
channel state. Also removes the UnregisterChannel method from the
chancloser's config as the link is shut down before the chancloser
is created.
Adds a method to the LightningChannel struct called IsChannelClean
that returns a boolean telling the caller whether the channel state
is clean or not. Clean in this case means there are no lingering
updates to be signed for, no HTLC's active on either sides commitment
transaction, and no pending commitments on either side. This can be
used for dynamic commitments or during a strict cooperative close
process that ensures atomicity of the channel.
In this commit, we start to optimistically use the new private key cache
that was added to btcwallet. As is, btcwallet will cache the decrypted
account keys for each scope in memory. However, the existing methods
to derive a child key from those account keys requires a write database
transaction, and will re-derive the private key using BIP-32 each time.
The newly added `DeriveFromKeyPathCache` lets us skip all this and
directly use a cache assuming the account info is already cached. The
new logic will try to use this method, but if it fails fall back to the
existing `DeriveFromKeyPath` method. All calls after this will use this
new cached key.
Fixes https://github.com/lightningnetwork/lnd/issues/5125.
Basic benchmark before the btcwallet change and after:
```
benchmark old ns/op new ns/op delta
BenchmarkDerivePrivKey-8 22840583 125 -100.00%
benchmark old allocs new allocs delta
BenchmarkDerivePrivKey-8 89 2 -97.75%
benchmark old bytes new bytes delta
BenchmarkDerivePrivKey-8 10225 24 -99.77%
```
Depends on btcsuite/btcwallet#757.
Pulls in the updated version of btcwallet and walletdb that have the DB
interface enhanced by their own View() and Update() methods with the
reset callback/closure supported out of the box. That way the global
package-level View() and Update() functions now become pure redirects.
Improve 'ErrReservedValueInvalidated' error string to explain that the
error is triggered by a transaction that would deplete funds reserved for
potential future anchor channel closings (via CPFP)
Add hint that further details can be found in the debug log
Update strings in 'lntest/itest/log_error_whitelist.txt' correspondingly
We only want to register the bbolt DB backend ("bdb") when we're not
compiling for a JS/WASM build targets. That's why we want to have that
import in only one file that we can add a build tag to. We remove it in
two other places since only one import is enough anyway.
In this commit, in order to allow the test added in the prior commit to
pass, we'll increment the mockHTLCAmt value by 1 to ensure we never
attempt to add a zero value HTLC.
Fixes#5468
As is, if the remote party proposes a min HTLC of 0 `mSat` to us, then
we won't ever be able to _send outgoing_ in the channel as the
`MayAddOutgoingHtlc` will attempt to add a zero-value HTLC, which isn't
allowed within the protocol.
The default channels created actually already use a min HTLC value of
zero within the tests, so this test case fails as is.
This commit adds a new struct AnchorResolutions which wraps the anchor
resolutions for local/remote/pending remote commitment transactions. It
is then returned from NewAnchorResolutions. Thus the caller knows how to
retrieve a certain anchor resolution.
In this commit, we add a check inside EstimateFeePerKW for bitcoind so
that when the conf target exceeds the maxBlockTarget, we will use
maxBlockTarget instead.
This commit aims to address a flaw in our anchor reserve enforcement
logic in which an inbound "legacy" channel (i.e. a channel with a
commitment type that precedes anchors) would be rejected by the
recipient if they have at least one opened channel using the anchors
commitment type and do not have enough on-chain funds to meet the
anchors reserve.
Otherwise, we would get non-standard txn's and fail to broadcast
them when cooperatively closing a channel. This wouldn't affect
funds security as no HTLCs would be active to steal. This is just
a safety measure as we should only generate standard txn's.
Fixes#5287.
The PSBT spec is a bit vague when it comes to the WitnessUtxo field of
an input as it's not strictly required for witness inputs. Therefore
Electrum for example does not include the field.
We need to make the SegWit input check a bit more elaborate by looking
at the output script of the UTXO and also the redeem script in case it's
a nested SegWit spend.
Since private channels (most likely) won't be used for routing other
than as last hop, they bear a smaller risk that we must quickly force
close them in order to resolve a HTLC up/downstream.
In the case where it actually has to be force to resolve a payment (if
it is the first/last hop on a routed payment), we can assume that the
router will have UTXOs available from the reserved value from the
incoming public channel.
We cap the maximum value we'll reserve for anchor channel fee bumping at
10 times the per-channel amount such that nodes with a high number of
channels don't have to keep around a very large amount for the unlikely
scanario that they all close at the same time.
This commit ensures that for the neutrino implementation of
lnwallet.BlockChainIO, the neutrino GetBlock method is called directly
(since it already uses the blockcache). It also ensures that the block
cache mutex for the given hash is locked before the call to GetBlock.
The previous behavior would allow updates to be overwritten in some
scenarios. Upon restart, this would lead to a missing settle/fail in
the update logs.
Add a dust-limit to `CoinSelect` and let the fee estimation consider if a change output is needed or not, assuring that if the change is below the dust-limit it will go towards the fee instead.
This commit adds a RevocationKeyLocator field to the OpenChannel
struct so that the SCB derivation doesn't have to brute-force the
sha chain root key and match the public key. ECDH derivation is now
used to derive the key instead of regular private key derivation a
la DerivePrivKey. The legacy can still be used to recover old
channels.
Messages:
- UpdateFulfillHTLC
- UpdateFee
- UpdateFailMalformedHTLC
- UpdateFailHTLC
- UpdateAddHTLC
- Shutdown
- RevokeAndAck
- ReplyShortChanIDsEnd
- ReplyChannelRange
- QueryShortChanIDs
- QueryChannelRange
- NodeAnnouncement
- Init
- GossipTimestampRange
- FundingSigned
- FundingLocked
- FundingCreated
- CommitSig
- ClosingSigned
- ChannelUpdate
- ChannelReestablish
- ChannelAnnouncement
- AnnounceSignatures
lnwire: update quickcheck tests, use constant for Error
multi: update unit tests to pass deep equal assertions with messages
In this commit, we update a series of unit tests in the code base to now
pass due to the new wire message encode/decode logic. In many instances,
we'll now manually set the extra bytes to an empty byte slice to avoid
comparisons that fail due to one message having an empty byte slice and
the other having a nil pointer.
This commit caps the update fee the initiator will send when the anchors
channel type is used. We do not limit anything on the receiver side.
10 sat/vbyte is the current default max fee rate we use. This should be
enough to ensure propagation before anchoring down the commitment
transaction.
* mod: bump btcwallet version to accept db timeout
* btcwallet: add DBTimeOut in config
* kvdb: add database timeout option for bbolt
This commit adds a DBTimeout option in bbolt config. The relevant
functions walletdb.Open/Create are updated to use this config. In
addition, the bolt compacter also applies the new timeout option.
* channeldb: add DBTimeout in db options
This commit adds the DBTimeout option for channeldb. A new unit
test file is created to test the default options. In addition,
the params used in kvdb.Create inside channeldb_test is updated
with a DefaultDBTimeout value.
* contractcourt+routing: use DBTimeout in kvdb
This commit touches multiple test files in contractcourt and routing.
The call of function kvdb.Create and kvdb.Open are now updated with
the new param DBTimeout, using the default value kvdb.DefaultDBTimeout.
* lncfg: add DBTimeout option in db config
The DBTimeout option is added to db config. A new unit test is
added to check the default DB config is created as expected.
* migration: add DBTimeout param in kvdb.Create/kvdb.Open
* keychain: update tests to use DBTimeout param
* htlcswitch+chainreg: add DBTimeout option
* macaroons: support DBTimeout config in creation
This commit adds the DBTimeout during the creation of macaroons.db.
The usage of kvdb.Create and kvdb.Open in its tests are updated with
a timeout value using kvdb.DefaultDBTimeout.
* walletunlocker: add dbTimeout option in UnlockerService
This commit adds a new param, dbTimeout, during the creation of
UnlockerService. This param is then passed to wallet.NewLoader
inside various service calls, specifying a timeout value to be
used when opening the bbolt. In addition, the macaroonService
is also called with this dbTimeout param.
* watchtower/wtdb: add dbTimeout param during creation
This commit adds the dbTimeout param for the creation of both
watchtower.db and wtclient.db.
* multi: add db timeout param for walletdb.Create
This commit adds the db timeout param for the function call
walletdb.Create. It touches only the test files found in chainntnfs,
lnwallet, and routing.
* lnd: pass DBTimeout config to relevant services
This commit enables lnd to pass the DBTimeout config to the following
services/config/functions,
- chainControlConfig
- walletunlocker
- wallet.NewLoader
- macaroons
- watchtower
In addition, the usage of wallet.Create is updated too.
* sample-config: add dbtimeout option
outdated local state
This commit fixes a bug that would cause us to not sweep our local
output in case we force closed, then lost state or attempted recovery.
The reason being that we would use or local commit height when deriving
our scripts, which would be incorrect. Instead we use the extracted
state number to derive the correct scripts, allowing us to sweep the
output.
Allthough being an unlikely scenario, we would leave money on chain in
this case without any warning (since we would just end up with an empty
delay script) and forget about the spend.
Similar to the previous commit, we fix a bug resulting in the wrong
commit weight being calculated when an HTLC just above the remote's
duslimit was added from the remote. This was a result of using the
successFee instead of the timeoutFee when checking whether it was dust,
making us consider it dust when it shouldn't have been.
In this commit we fix a bug resulting in the wrong commit weight being
calculated when an HTLC just below the remote's duslimit was added. This
was a result of using the timeoutFee instead of the successFee when
checking whether it was dust, making us consider it non-dust when it
should have been.
Without this change the high-fee logic is never tested as it is instead caught by the dust-output logic. This change uses a higher fee rate to ensure an output value above the dust limit, while still spending 20% on fees.
To allow nodes more control over the amount of time that their funds
will be locked up, we add a MaxLocalCSVDelay option which sets the
maximum csv delay we will accept for all channels. We default to the
existing constant of 10000, and set a sane minimum on this value so that
clients cannot set unreasonably low maximum csv delays which will result
in their node rejecting all channels.
A while back, changes were made to the wallet such that it waits for the
backend to be synced before beginning to store the latest 10,000 blocks
of the chain. This inherently broke sync progress implementations based
on the best_header_timestamp result from the GetInfo RPC for
neutrino-based nodes as the wallet is no longer tracking all blocks in
the chain. To work around this, we now make sure to return the backend's
best header timestamp instead of the wallet's, allowing said sync
progress implementations to work again.
For unconfirmed commit tx anchors, supply the sweeper with cpfp info and
a confirmation target fee estimate.
The sweeper will try to pay for the parent commit tx as long as the
current fee estimate exceeds the pre-signed commit tx fee rate.
- let users specify their MAXIMUM WUMBO with new config option which sets the maximum channel size lnd will accept
- current implementation is a simple check by the fundingManager rather than anything to do with the ChannelAcceptor
- Add test cases which verify that maximum channel limit is respected for wumbo/non-wumbo channels
- use --maxchansize 0 value to distinguish set/unset config. If user sets max value to 0 it will not do anything as 0 is currently used to indicate to the funding manager that the limit should not be enforced. This seems justifiable since --maxchansize=0 doesn't seem to make sense at first glance.
- add integration test case to ensure that config parsing and valiation is proper. I simplified the funding managers check electing to rely on config.go to correctly parse and set up either i) non wumbo default limit of 0.16 BTC OR ii) wumbo default soft limit of 10 BTC
Addresses: https://github.com/lightningnetwork/lnd/issues/4557
To be spec compliant, we require the initiator to not pay the anchor
values into fees on coop close. We extract the balance calculation into
commitment.go, and add back the value of the anchors to the initiator's
balance.
Externally funded channels are expected by the user and explicitly
registered through the use of a funding shim and should therefore not
count towards the max pending channel count which is primarily there to
mitigate DoS attacks.
Previously whether or not to add test htlcs was implictly controlled by
a nil value of the HtlcDescs test parameter. With the conversion to
json, that nil value got lost.
The reason that the test still passed is because with the fee rate of
the no-htlc test case, the htlcs were trimmed. Also because in the test
json, balances are specified after applying htlcs, the test didn't fail
with a mismatching balance.
In this commit, we make a new wrapper method around the internal
`WalletController` method to ensure it holds the coin select mutex while
the balance is being computed.
This commit includes a regression test that checks that we remember
to restore updates that we sent to the peer but they haven't sent
us a signature for yet.
This fixes a long-standing force close bug. When we receive a
revocation, store the updates that the remote should sign next under
a new database key. Previously, these were not persisted which would
lead to force closure.
Follow up labelling of external transactions with labels for the
transaction types we create within lnd. Since these labels will live
a life of string matching, a version number and rigid format is added
so that string matching is less painful. We start out with channel ID,
where available, and a transaction "type". External labels, added in a
previous PR, are not updated to this new versioned label because they
are not lnd-initiated transactions. Label matching can check this case,
then check for a version number.
Previously, we could sign a pending commitment for the remote party,
disconnect, and not restore these signed remote updates as having been
removed at the pending commitment height. This commit fixes that to
look up whether the update under the unsigned acked updates key is
present on the pending commitment or not and appropriately set
the remove commit heights.
This commit moves the deletion of all updates under the unsigned
acked updates key from AppendRemoteCommitChain to
AdvanceCommitChainTail. This is done because if we went down after
signing for these updates but before receiving a revocation, we would
incorrectly reject their commitment signature:
Alice Bob
-----add----->
-----sig----->
<----rev------
<----sig------
-----rev----->
<----fail-----
<----sig------
-----rev----->
-----sig----->
*reconnect*
<----rev------
<----add------
x----sig------
It is also important to note that filtering is required when we
receive a revocation to ensure that we aren't erroneously deleting
remote updates. Take the following state transitions:
Alice Bob
-----add----->
-----sig----->
<----rev------
<----sig------
-----rev----->
-----add----->
-----sig----->
<----fail-----
<----sig------
-----rev-----> (alice stores updates here)
<----rev------
In the above case, if Alice deleted all updates rather than filtering
when receiving the final revocation from Bob, then Alice would have
to force close the channel due to missing updates. Since Alice hasn't
signed for any of the unsigned acked updates, she should not filter any
of them out.
The `restoreStateLogs` function now properly restores the
`addCommitHeightLocal` field of a settle or fail's parent add.
Previously, any updates' parent in unsignedAckedUpdates would have
the field set to the default value of 0. This would cause a force
closure when receiving a commitment due to our belt-and-suspenders
checks for update logs during commitment validation.
The bug in question occurs because the `addCommitHeightLocal` field
is only populated for a restored add if the add is on the local
commitment. `TestChannelRestoreCommitHeight` is expanded in
`lnwallet/channel_test.go` to demonstrate restoration now works.
The faulty state transition:
```
<----fail----
<----sig-----
-----rev----> (add no longer on Alice's commitment)
*Alice restores* (addCommitHeightLocal of failed htlc is 0)
```
NOTE: Alice dies after sending a revocation but before signing a
commitment. This is possible because there is a select block in the link
that can potentially exit after sending over the revocation but before
signing the next commitment state for the counterparty.
The first channels of a batch shouldn't publish the batch TX
to avoid problems if some of the funding flows can't be completed.
Only the last channel of a batch should publish. We set the channel flag
accordingly depending on the flag in the assembler.
This is useful when we wish to have a channel frozen for a specific
amount of blocks after its confirmation. This could also be done with an
absolute thaw height, but it does not suit cases where a strict block
delta needs to be enforced, as it's not possible to know for certain
when a channel will be included in the chain. To work around this, we
add a relative interpretation of the field, where if its value is below
500,000, then it's interpreted as a relative height. This approach
allows us to prevent further database modifications to account for a
relative thaw height.
This commit changes the verification of our code against the spec test
vectors to use a more black box approach. It exercises the channel state
machine via its external interface as much as possible, making this test
more robust. A consequence of this is that the test now runs from the
'root' data from which the test vectors are also derived, meaning that
more code is covered too.
Running from the root data is also a preparation for _producing_ test
vectors for the new anchor commitment format. This will be a matter of
changing the channel type and recording the produced commitment and htlc
txes.
Previously the success transaction was skipped during verification. With
this commit, the proper preimage insertion is carried out, allowing the
success tx to be checked too.
Introduces a new chancloser package which exposes a ChanCloser
struct that handles the cooperative channel closure negotiation
and is meant to replace chancloser.go in the lnd package. Updates
all references to chancloser.go to instead use chancloser package.
The current implementation of LeaseOutput already checked whether the
output had already been leased by the persisted implementation, but not
the in-memory one used by lnd internally. Without this check, we could
potentially end up with a double spend error if lnd acquired the UTXO
internally before the LeaseOutput call.
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.
Add start and end height parameters to the rpc and cli GetTransactions
endpoints. Default to returning all transactions from genesis to tip,
including unconfirmed transactions to maintain backwards compatibility.
This enforces the _actualized_ fee rate of the commitment transaction,
rather than the fee floor used for estimation. The new value of 250
sat/kw corresponds to 1 sat/byte, rather than 253 which is only rounded
up during estimation to account for the fact that BOLT 3 rounds down to
the nearest satoshi and that the vbyte fee estimation is lossy.
Previously we would incorrectly fail to sign the next commitment even
though the fee was technically high enough. Restarting with this commit
should solve the issue as long as the channel hasn't already gone to
chain.
This commit introduces the Signature interface which will be used by our
witness construction methods instead of passing in raw byte slices. This
will be used later to inject various kinds of mock signatures, e.g.
73-byte signatures for simulating worst-case witness weight.
This commit adds an additional santity check that rejects zero-value
HTLCs, preventing them from being added to the channel state even if the
channel config's minhtlc value is zero.
This commit adds a test to exercise that HTLC signatures are sent in the
correct order, i.e. they match the sorting of the HTLC outputs on the
commitment after applying BOLT 3's BIP69+CLTV sort.
This commit fixes#4118 by properly sorting the HTLC signatures sent
over the wire to match the BOLT3 BIP69+CLTV sorting of the commitment
outputs.
To do so, we expose the slice of cltv deltas for HTLCs on the unsigned
commitment after applying the commitment sorting. This will be used to
locate the proper output index, as the CLTV serves as a tie breaker
between HTLCs that otherwise have the same payment hash and amount.
Note that #3412 fixed the issue partially by ensuring the commitment was
constructed properly (and the second-level prev outpoint's txid was
correct), but failed to address that the HTLC signatures were still sent
out in the incorrect order. With this, we pass the test case introduce
in the next commit.
We currently write each HTLCs OutputIndex to disk, but we don't use it
when restoring. The restoration is modified to use these directly, since
we will have lost access to the sorting of CLTVs after the initial
signing process.
In case the funding manager detects that a funding flow is requested
to be executed with the help of a PsbtIntent, the normal channel
negotiation with the remote peer is interrupted, as soon as the
accept_channel message was received. With the remote peer's funding
multisig key and our local key, we can derive the funding output
script and its address. This is enough to start the PSBT funding
and signing process which the user will do externally to the daemon.
We add a new funding assembler and intent type that handle channel
funding through the use of a PSBT. The PsbtIntent is in itself a
simple state machine that can be stepped through the process of
assembling the required information for the funding output, verifying
a user supplied PSBT for correctness, accepting a fully signed PSBT
and then assembling the funding wire message.
