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 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.
This commit adds two new channel statuses which indicate the party that
initatited closing the channel. These statuses are set in conjunction
with the existing commit broadcast status so that we do not need to
migrate existing logic to handle multiple types of closes. This status
is set for locally initiated force closes in this commit because they
follow a similar pattern to cooparative closes, marking the commitment
broadcast then proceeding with tx broadcast. Remote force closes are
added in the following commit, as they are handled differently.
Since our HTLC must also be added to the remote commitment, we do the
balance caluclation also from the remote chain perspective and report
our minimum balance from the two commit views as our available balance.
When we send non-dust HTLCs as the non-initiator, the remote party will
have to pay the extra commitment fee. To account for this we figure out
if they can afford paying this fee, if not we report that we only have
balance available for dust HTLCs, since these HTLCs won't increase the
commitment fee.
Since we want to handle the edge case where paying the HTLC fee would
take the initiator below the reserve, we move the subtraction of the
reserve into availableBalance where this calculation will be performed.
This commit adds an extra validation step when adding HTLCs. Previously
we would only validate the remote commitment resulting from adding an
HTLC, which in most cases is enough. However, there are situations where
the dustlimits are different, which could lead to the resulting remote
commitment from adding the HTLC being valid, but not the local
commitment.
Now we also validate the local commitment. A test to trigger the case is
added.
add
To ba able to validate the commitment sanity both for remote and local
commitments, and at the same time predict both our and their add, we let
validateCommitmentSanity take an extra payment descriptor to make this
possible.
This commit checks the commitment sanity when receiving an HTLC so
that if a commitment transaction will overflow from an ADD, it is
caught earlier rather than in ReceiveNewCommitment.
The unit test TestNewBreachRetributionSkipsDustHtlcs triggered a state
transition from Bob, even though it was Alice that had added the HTLCs.
This is wrong since it will lead to Bob still owing Alice a commitment,
which is not accounted for in the unit tests.
We add a sanity check that the add heights has been set for all entries
found in the logs, and return an error otherwise. This won't happen
during normal operation, but it does reveal the mistake in the unit
test, which is fixed by making Alice trigger the transition.
In addition we resolve a long standing TODO by removing a (purposeful)
panic in the channel state machine. Old version of lnd had a bug that
could lead to the parent entries being lost during channel restore. A
panic was added to get to the bottom of if.
This is now fixed, so new nodes shouldn't encounter it. However, to be
on the safe side, instead of panicking we return an error back to
gracefully exit the channel state machine.
Updates were always restored with the same log index. This could cause a
crash when the logs were compacted and possibly other problems
elsewhere.
Extended unit test to cover the crash scenario.
This commit updates the channel state machine to
persistently store remote updates that we have received a
signature for, but that we haven't yet included in a commit
signature of our own.
Previously those updates were only stored in memory and
dropped across restarts. This lead to the production of
an invalid signature and channel force closure. The remote
party expects us to include those updates.
When creating the keyring, the tweak is already calculated in the remote
commitment case. We add the calculation also for our own commitment, so
we can use it in all cases without deriving the tweak.
Based on the current channel type, we derive the script used for the
to_remote output. Currently only the unencumbered p2wkh type is used,
but that will change with upcoming channel types.
To make the channel state machine less concerned about the type of
commitment, we nil the local tweak when creating the keyring, depending
on the commitment type.
We abstract away how keys are generated for the different channel types
types (currently tweak(less)).
Intention is that more of the logic that is unique for each commitment
type lives in commitment.go, making the channel state machine oblivious
to the keys and outputs being created on the commitment tx for a given
channel state.
createCommitmentTx would earlier mutate the passed commitment struct
after evaluating the htlc view and calculating the final balances, which
was confusing since the balances are supposed to only be *after*
subtracting fees.
Instead we take the needed parameters as arguments, and return the final
balances, tx and fee to populate the commitment struct in a proper way.
PURE CODE MOVE:
Moving createCommitmentTx, CreateCommitTx, createStateHintObfuscator,
CommitmentKeyRing, DeriveCommitmentKeys, addHTLC, genHtlcScripts
We move the methods and structs to a new file commitment.go in
preparation for defining all the logic that is dependent on the channel
type in this new file.
Instead of passing delays and dustlimits separately, we pass the correct
channel config to CreateCommitTx from the POV of the local party that
owns the commit tx.
To make it more clear which commitment we are actually creating, we
rename variables to denote local and remote, to prepare for the case
when both outputs might be delayed.
