In this commit, we fix an issue where sometimes transactions wouldn't
provide enough of a fee to be relayed by the backend node. This would
especially cause issues when sweeping outputs after a contract breach,
etc.
Now, we'll fetch the minimum relay fee from the backend node and ensure
it is set as a lower bound if the estimated fee ends up dipping below
it.
This commit removes a faulty check we did to determine if the channel
commitments were fully synced. We assumed that if out local commitment
chain had a height higher than the remote, then we would have state
updates present in our chain but not in theirs, and owed a commitment.
However, there were cases where this wasn't true, and we would send a
new commitment even though we had no new updates to sign. This is a
protocol violation.
Now we don't longer check the heights to determine if we are fully
synced. A consequence of this is that we also need to check if we have
any pending fee updates that are nopt yet signed, as those are
considered non-empty updates.
This commit make us return an error in case a restored HTLC from a
pending remote commit has an index that is different from our local
update log index. It is appended with the assumption that these indexes
are the same, and if they are not we cannot really continue.
This commit adds a call to panic in case the HTLC we are looking for is
not found in the update log. It _should_ always be there, but we have
seen crashes resulting from it not being found. Since it will crash with
a nil pointer dereference below, we instead call panic giving us a bit
more information to work with.
In this commit, we modify the NewUnilateralCloseSummary to be able to
distinguish between a unilateral closure using the lowest+highest
commitment the remote party possesses. Before this commit, if the remote
party broadcast their highest commitment, when they have a lower
unrevoked commitment, then this function would fail to find the proper
output, leaving funds on the chain.
To fix this, it's now the duty of the caller to pass remotePendingCommit
with the proper value. The caller should use the lowest unrevoked
commitment, and the height hint of the broadcast commitment to discern
if this is a pending commitment or not.
In this commit, we move a set of useful functions for testing channels
into a new file. The old createTestChannels has been improved as it will
now properly set the height hint on the first created commitments, and
also no longer accepts any arguments as the revocation window no longer
exists.
This commit changes the bool `IsBorked` in OpenChannel to a `ChanStatus`
struct, of type ChannelStatus. This is used to indicated that a channel
that is technically still open, is either borked, or has had a
commitment broadcasted, but is not confirmed on-chain yet.
The ChannelStatus type has the value 1 for the status Borked, meaning it
is backwards compatible with the old database format.
This commit renames ForceCloseSummary to LocalForceCloseSummary, and
adds a new method NewLocalForceCloseSummary that can be used to derive a
LocalForceCloseSummary if our commitment transaction gets confirmed
in-chain. It is meant to accompany the NewUnilateralCloseSummary method,
which is used for the same purpose in the event of a remote commitment
being seen in-chain.
In this commit, we fix an existing bug in the NewBreachRetribution
method. Rather than creating the slice to the proper length, we instead
now create it to the proper _capacity_. As we'll now properly filter out
any dust HTLCs, before this commit, even if no HTLCs were added, then
the slice would still have a full length, meaning callers could actually
interact with _blank_ HtlcRetribution structs.
The fix is simple: create the slice with the proper capacity, and append
to the end of it.
In this commit, we fix an existing within lnd. Before this commit,
within NewBreachRetribution the order of the keys when generating the
sender HTLC script was incorrect. As in this case, the remote party is
the sender, their key should be first. However, the order was swapped,
meaning that at breach time, our transaction would be rejected as it had
the incorrect witness script.
The fix is simple: swap the ordering of the keys. After this commit, the
test extension added in the prior commit now passes.
In this commit we add a new error: InvalidHtlcSigError. This error will
be returned when we're unable to validate an HTLC signature sent by the
remote party. This will allow other nodes to more easily debug _why_ the
signature was rejected.
In this commit we add a new command line option (and a sane default) to
allow users to specify the *smallest* inbound channel that they'll
accept. Having a higher-ish limit lets users limit their channels, and
also avoid a series of very low value "spam" channels.
The new option is --minchansize, and expressed in satoshis. If we
receive an inbound channel request for a value smaller than this, then
we'll immediately reject it.
In this commit, we add an additional check within CreateCommitTx to
ensure that we will never create or accept a commitment transaction that
wasn't valid by consensus. To enforce this check, we use the
blockchain.CheckTransactionSanity method.
In this commit, we fix an existing rounding related bug in the codebase.
The RPC interface for btcd and bitcoind return values in BTC rather than
in satoshis. So in several places, we're forced to convert ourselves
manually. The existing logic attempted to do this, but didn't properly
account for rounding. As a result, our values can be off due to not
rounding incorrectly.
The fix for this is easy: simply properly use btcutil.NewAmount
everywhere which does rounding properly.
Fixes#939.
This commit adds a check that will make LightningChannel reject a
received commitment if it is accompanied with too many HTLC signatures.
This enforces the requirement in BOLT-2, saying:
if num_htlcs is not equal to the number of HTLC outputs in the local commitment transaction:
* MUST fail the channel.
A test exercising the behaviour is added.
This commit fixes an issue which would arise in some cases when the
local and remote dust limits would differ, resulting in lnd not
producing the expected number of HTLC signatures. This was a result of
checking dust against the local instead of the remote dust limit.
A test exercising the scenario is added.
This commit fixes an issue where we would blindly accept a commitment
which came without any accompanying HTLC signatures. A test exercising
the scenario is added.
This commit fixes an out of bounds error that would occur in the case
where we received a new commitment where the accompanying HTLC sigs were
too few. Now we'll just reject such an commitment.
A test exercising the behavior is also added.
This commit extends the amount of time we wait
for transaction to enter the mempool from
10 to 30 seconds. The wallet's interface tests
seem to be particularly slow when run with the
race flag, a problem which is only exacerbated
by the slowness of travis.
With 10s and the race flag, I was able to repro
the issues locally fairly consistently.
In this commit, we add an additional check within
validateCommitmentSanity due to the recent change to unsigned integers
for peer balances in the channel state machine. If after evaluation
(just applying HTLC updates), the balances are negative, then we’ll
return ErrBelowChanReserve.
In this commit, we add logic to account for an edge case in the
protocol. If they initiator if unable to pay the fees for a commitment,
then their *entire* output is meant to go to fees. The recent change to
properly interpret balances as unsigned integers (within the protocol)
let to the discovery of this missed edge case.
lnwire.MilliSatoshi is now a signed integer, as a result, we’ll return
a different error if our balances go to negative due to the inability
to pay a the set fee.
In this commit, we fix a bug introduced by the recent change of
lnwire.MilliSatoshi to be an unsigned integer. After this change an
integer underflow was left undetected, as a result we’ll now
momentarily cast to a signed integer in order to ensure that both sides
can pay the proper fee.
In this commit, we modify the way we obtain the current best header
timestamp. In doing this, we fix an intermittent flake that would pop
up at times on the integration tests. This could occur as if the wallet
was lagging behind the chain backend for a re-org, then a hash that the
backend knew of, may not be known by the wallet.
To remedy this, we’ll take advantage of a recent change to btcwallet to
actually include the timestamp in its sync state.
In this commit, we modify the mechanics of the wallet to only allow
derivation of segwit-like addresses. Additionally, the ConfirmedBalance
method on the WalletController now only has a single argument, as it’s
assumed that the wallet is itself only concerned with segwit outputs.
In this commit, we modify the way we generate the secrets for
revocation roots to be fully deterministic. Rather than use a special
key and derive all sub-roots from that (mixing in some “salts”), we’ll
use the proper keychain.KeyFamily instead. This ensures that given a
static description of the channel, we’re able to re-derive our
revocation root properly.
In this commit, we modify the funding flow process to obtain all keys
necessary from the keychain.KeyRing interface. This ensure that all
keys we generate are fully deterministic.
In this commit, we update the SignDescriptor struct to instead use the
keychain.KeyDescriptor struct inplace of a raw public key. We do this
as this allows for a recovery mode, where the caller doesn’t know what
key was used exactly, but remembers the precise KeyDescriptor. A
stateless signer is still able to re-derive the proper key, as it’s
given the full path.
The new version of the internal core of btcwallet now uses KeyScopes
rather than address types to derive particular addresses. As a result,
in this commit, we update our API usage to ensure that proper addresses
are still derived.
In this commit, we remove two methods from the WalletController
interface which were previously used by the funding reservation process
(NewRawKey) and the p2p network (FetchRootKey) in order to derive
various keys for operation. This methods are no longer necessary as the
KeyRing interface implements the functionality in a deterministic
manner.
In this commit, due to the recent changes within lnd itself, it may be
possible that a wallet already exists when the wallet has been signaled
to be created. As a result, *always* open the wallet ourselves, but
allow an existing wallet to already be in place.
Adds an extra case to the select statement to catch
an error produced by btcd. The error is meant to signal
that an output was previously spent, which can appear
under certain race conditions in spending/broadcasting.
This caused our final itest to fail because it would
not try to recraft the justice txn.
This commit adds wallet_best_block_timestamp to the gRPC interface.
This is done in order to allow clients to calculate progress while
lnd syncs to the blockchain. wallet_best_block_timestamp is exposed
via the GetInfo() rpc call. Additionally, IsSynced() returns the
WalletBestBlockTimestamp as the second value in the tuple
that is returned, providing additional detail when querying about the
status of the sync. The BtcWallet interface has also been updated
accordingly.
This commit was created to support the issue to
[Add progress bar for chain sync] (lightninglabs/lightning-app#10) in
lightning-app
This commit introduces changes to the validateCommitmentSanity
function to fully validate all channel constraints.
validateCommitmentSanity now validates that the
MaxPendingAmount, ChanReserve, MinHTLC, & MaxAcceptedHTLCs
limits are all adhered to during the lifetime of a channel.
When applying a set of updates, the channel constraints are
validated from the point-of-view of either the local or the
remote node, to make sure the updates will be accepted.
Co-authored-by: nsa <elzeigel@gmail.com>
