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.
This commit isolates preimages of forwarded htlcs from invoice
preimages. The reason to do this is to prevent the incoming contest
resolver from settling exit hop htlcs for which the invoice isn't marked
as settled.
One of the first things the incoming contest resolver does is checking
if the preimage is available and if it is, convert itself into a success
resolver.
This behaviour makes it unnecessary to already determine earlier in the
process whether an incoming contest or a success resolver is needed.
By having all incoming htlcs go through the incoming contest resolver,
the number of execution paths is reduced and it becomes easier to
ascertain that the implemented logic is correct.
The only functional change in this commit is that a forwarded htlc for
which is the preimage is known, is no longer settled when the htlc is
already expired. Previously a success resolver would be instantiated
directly, skipping the expiry height check.
This created a risk that the success resolver would never finish,
because an expired htlc could already have been swept by the remote
party and there is no detection of this remote spend in the success
resolver currently.
With the new change, the general direction that an expired htlc
shouldn't be settled and instead given up on is implemented more
consistently.
This commit prepares for fixing edges cases related to hodl
invoice on-chain resolution.
This commit adds logging of the reason to go to chain for a channel.
This can help users to find out the reason why a channels forced closed.
To get all go to chain reasons, an optimization to break early is
removed. This optimization was not significant, because the normal flow
already examined all htlcs. In the exceptional case where we need to go
to chain, it does not weigh up against logging all go to chain reasons.
This commits exposes the various parameters around going to chain and
accepting htlcs in a clear way.
In addition to this, it reverts those parameters to what they were
before the merge of commit d1076271456bdab1625ea6b52b93ca3e1bd9aed9.
The multiplier doesn't make sense because funds may be equally at risk
by failing to broadcast to chain regardless of whether the HTLC is a
redeem or a timeout.
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.
Previously the arbitrator wasn't advanced to the final stage after
the last contract resolved.
Also channel arbitrator now does not ignore a log error anymore
unresolved contracts cannot be retrieved.
Previously, contract resolvers that needed to publish a second level tx,
did not have access to the original htlc amount.
This commit reconstructs this amount from data that is already persisted
in arbitrator log.
Co-authored-by: Joost Jager <joost.jager@gmail.com>
In this commit, we prevent the ChainArbitrator from sending a force
close request for a channel if it has previously already sent one. We do
this to prevent blocking the caller of ForceCloseContract.
At ChannelArbitrator startup we now check the database close status of
the channel. If we detect that the channel is closed, but our state
machine hasn't advanced to reflect that (possibly because of a shutdown
before the state transition was finished), we manually trigger the state
transition to recover.
This commit moves the responsibility for closing local and remote force
closes in the database from the chain watcher to the channel arbitrator.
We do this because we previously would close the channel in the
database, before sending the event to the channel arbitrator. This could
lead to a situation where the channel was marked closed, but the channel
arbitrator didn't receive the event before shutdown. As we don't listen
for chain events for channels that are closed, those channels would be
stuck in the pending close state forever, as the channel arbitrator
state machine wouldn't progress.
We fix this by letting the ChannelArbitrator close the channel in the
database. After the contract resolutions are logged (in the state
callback before transitioning to StateContractClosed) we mark the
channel closed in the database. This way we make sure that it is marked
closed only if the resolutions have been successfully persisted.
This commit removes the state callback, and instead logs the contract
resolutions directly after receiving the unilateral close event. The
resolutions won't change so there's not really necessary to wait to log
them, and this greatly simplifies the code.
In this commit, we alter cooperative channel closures to also use
MarkChannelResolved in order to unify the logic for the different types
of channel closures.
This commit changes the channel arbitrator state machine to only care
about commitment transactions that are being confirmed on-chain
according to the chain_watcher. This is meant to handles the cases where
we would broadcast our commitment, expecting it to get confirmed, but
instead a competing transaction was confirmed.
This commit readies the ChannelArbitrator state machine for the change
that will make the ChainWatcher only notify on confirmed commitments.
The state machine has gotten a new state, StateCommitmentBroadcasted,
which we'll transition to after we have broadcasted our own commitment.
From this state we'll go to the StateContractClosed state regardless of
which commitment the ChainWatcher notifies about, unifying the contract
resolution betweee the local and remote force close.
This commit removes a short circuit checking if the contract resolver
after a unilateral close is empty. After removing this, the state
machine will advance the state from StateDefault->ContractClosed, in
which the stateCallback will be called, logging the state needed to
advance. Since this logged state is empty, the state machine will go
directly to StateFullyResolved, which will trigger the
MarkChannelResolved call. This means the behaviour is kept.
This commit adds a missing return to the resolveContract method, that
will ensure the goroutine exits if the ChannelArbitrator shuts down.
This fixes a potential deadlock during the integration tests.
We also promote some of the logs to Debug from Trace.
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.
This commit mitigates a problem within the ChannelArbitrator, where
after a restart we would start up in the state StateBroadcastCommit but
fail to broadcast out commitment because a conflicting transaction (most
likely our own commitment) was already broadcast. A more complete fix
for this case will be added later, but this commit let the
ChannelArbitrator continue, trying to close out the channel.
In this commit, we modify the way that notifications are dispatched
within the chainWatcher. Before we would *always* wait for an ack back
before we started to clean up he database state. This would at times
lead to deadlocks. To remedy this, we now allow callers to decide if
they want notifications to be sync or not. The only current caller that
requires this is the breach arbiter.
In this commit, we modify the construction of the channel arbitrator to
accept a pointer to an event stream from the chain watcher that’s been
assigned to that channel. As a result, we no longer need a fresh
unilateral close signal, as the one we get from the chain watcher will
*always* be up to date.
For each active channel, we’ll now create a chainWatcher instance that
will be around until the channel is fully closed on chain.
In this commit, we add the primary struct of the package with a full
implementation. The duty of the ChannelArbitrator is to watch the set
of active contracts on a comment transaction and act accordingly if any
of their redemption criteria have been met. Potential criteria include:
an HTLC about to time out, and HTLC about to time out that we know the
preiamge to, or the remote party going to chain (forcing us to resolve
all pending contracts on chain).
The primary goroutine of this struct implements a persistent state
machine in order to ensure that mid contract resolution, we’re able to
properly survive restarts without losing our place, or forgetting about
a pending contract.
A ChannelArbitrator will stay alive until all contracts have been fully
resolved. This means that outside sub-systems no longer need to worry
about remembering to mark a channel as fully resolved, as it’s the job
of the ChannelArbitrator to do this task.
