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.
In this commit, we extend the CloseChannelSummary by also storing: the
current unrevoked revocation for the remote party, the next pending
unused revocation, and also the local channel config. We move to store
these as the provide an extra level of defense against bugs as we'll
always store information required to derive keys for any current and
prior states.
In this commit, we fix an existing flake within the set of revocation
integration tests. Right after Bob's restart, we attempt to force close
the channel. However, it may be the case that the chain arbitrator
hasn't yet been created. As a result, the request to force close the
channel will fail. We easily fix this by wrapping the force close
attempt in a WaitPredicate.
This commit is similar to a recent commit which attempts to account for
internal block races by mining a second block if the initial assertion
for HTLC state fails. This can happen again if by the time that the
sweeping transaction is broadcast, it doesn't make it into the next
block mine.
In this commit, we modify the
testMultHopRemoteForceCloseOnChainHtlcTimeout test slightly to attempt
to account for a block race between the arrival of a message betwen the
contract resolver and the utxo nursery. If this message arrives "late"
(relative to the speed with which we mine blocks in test), then it'll be
detected as such by the utxo nursery. However, since we attempt to mine
a precise number of blocks, if this happens, then we'll never actually
mine that extra block to trigger a broadcast of the sweep transaction.
In this commit, we fix a race in the set of TestChannelLinkTrimCircuits*
tests. Before this commit, we would trim the circuits in the htlcManager
goroutine. However, this was problematic as the scheduling order of
goroutines isn't predictable. Instead, we'll now trim the circuits in
the Start method.
Additionally, we fix a series of off-by-2 bugs in the tests themselves.
In this commit, we fix a bug that could at times cause a deadlock when a
peer is attempting to disconnect. The issue was that when a peer goes to
disconnect, it needs to stop any active msgStream instances. The Stop()
method of the msgStream would block until an atomic variable was set to
indicate that the stream had fully exited. However, in the case that we
disconnected lower in the msgConsumer loop, we would never set the
streamShutdown variable, meaning that msgStream.Stop() would never
unblock.
The fix for this is simple: set the streamShutdown variable within the
quit case of the second select statement in the msgConsumer goroutine.
This commit inserts an initial set of HodlFlags into
their correct places within the switch. In lieu of the
existing HtlcHodl mode, it is been replaced with a
configurable HodlMask, which is a bitvector representing
the desired breakpoints. This will allow for fine grained
testing of the switch's internals, since we can create
arbitrary delays inside a otherwise asynchronous system.
This commit make the server populate the ChainArbitrator's
ContractBreach method, by a method that will reliably handoff the breach
event ot the breachArbiter. The server will now forward the breach event
to the breachArbiter, and only let the closure return a non-nil error
when the breachArbiter ACKs this event.
This commit adds the new function closure option ContractBreach to the
ChainArbitrator config, a closure that is again used by the ChainWatcher
to reliably handoff a breach event to the breachArbiter.
This commit changes how the ChainWatcher notifies the breachArbiter
about a channel breach. Instead of assuming the breachArbiter is among
the clients subscibing to channel events, it will call a new method
contractBreach(), and assume the breachArbiter has reliably gotten the
breach info when this method returns with a non-nil error.
Since the breachArbiter was the only sybsystem having a sync chain
subsciption, we also remove the (now) unused syncDispatch option.
This commit changes how the breachArbiter gets notified about channel
breaches. Previously it would need to SubscribeToChannelEvents to get
get notified if any breach happened, now we send all seen breaches on a
new channel ContractBreaches.
By having the breachArbiter subscribe to channel events, we risked
events getting lost when we were either starting up or shutting down,
since events could happen before we had been able to subscribe, or right
after we had cancelled our subscription.
Now it is the server's responsibility to reliably forward events from
the ChainArbitrator to the breachArbiter, and forward the ACK the
breachArbiter responds with. This makes sure that the messages aren't
lost in the event of starting up or shutting down, since the connection
between the subsystems now are static.
A result of this change is that the internals of the breachArbiter can
be simplified significantly, as we will get all channel breaches
forwarded on one channel. This lets us get rid of the observer
goroutines, and we spin up goroutines handling the channel breaches only
when they happen.
This move the log message "channel marked pending-closed" to the point
where the channel actually has been marked pending closed, instead of
before the database transaction has been done.
This commit removes the for loop in the closeObserver, as it wasn't
serving any purpose. After receiving a spend notification we would
return, breaking out of the loop. When getting a quit signal we would
also return, making the loop only do one iteration in any case.
