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.
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 fixes the TestMaxAcceptedHTLCs, TestMaxPendingAmount,
TestMinHTLC, & TestChanReserve unit tests to pass with the new
ReceiveHTLC logic. Instead of asserting specific failures upon
receiving a new commitment signature, the various assertions were
moved to assert on the error returned from ReceiveHTLC.
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.
This test asserts that remote updates that are locked-in on the local
commitment, but haven't been signed for on the remote commitment, are
properly restored after a restart.
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.
