This commit is a preparation for the implementation of remote spend
detection. Remote spends may happen before we broadcast our own sweep
tx. This calls for accurate height hints.
This commit introduces a common interface for sweep
inputs. It eliminates the type checking from UtxoSweeper.
Also the formerly present Amount() getter is removed. It was redundant
because this value is present in SignDesc().Output.Value as well.
Due to a recent change within the codebase to return estimated fee rates
in sat/kw, this commit ensures that we use this fee rate properly by
calculing a transaction's fees using its weight. This includes all of
the different transactions that are created within lnd (funding, sweeps,
etc.). On-chain transactions still rely on a sat/vbyte fee rate since it's
required by btcwallet.
In this commit, we update the convertToSecondLevelRevoke function to
also upgrade the second level pkScript as well. This is required as
we'll need to use this for a confirmation notification once we sweep the
output in the case that the remote party goes to the second layer.
This commit handles a racy condition within the breacharbiter's justice
tx procedure. For backends that have no mempool we would check if an
HTLC output was spent and then try broadcasting the justice tx, but this
would fail since we wouldn't detect the spend before it was in a block.
The result was that we would continuously attempt to broadcast the
transaction, effectively ending up in an endless (until the second-level
tx actually comfirmed) loop.
Instead we now register for spend notifications in case broadcasting the
transaction fails, and then wait for any of the notifications to be
sent before trying again.
This is a necessary step to be able to make lnd work well only with
confimed transactions, and was a better solution than introducing
timeouts within the broadcast loop (which complicates integration
tests).
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 commit fixes a broadcast loop within the breach arbiter, that would
occur when HTLC outputs had been taken to the second level. The breach
arbiter would register for a spend event, but would immediately go on to
create and publish the justice, without waiting for the response to be
received on the spend channel. This lead to a race, where the outpoint
could actually already have been spent, but the notification would
arrive after the breach arbiter checked the channel, and publishing the
justice TX would fail because it was a double spend. This would create a
"broadcast loop", as seen in the logs from the integration test
revoked_uncooperative_close_retribution_remote_hodl.
This is fixed by reusing an existing spend event for the outputs,
meaning we will actually receive on the first channel we initiated,
making the broadcast loop more likely to terminate.
After a shutdown has been initiated, both registrations
for spend ntfns and publishing txns can fail. The current
behavior in the face of such failures is to continue trying,
which is fine if we are online. However, this causes an
infinite loop during shutdown, and lnd cannot exit since
the routine is tracked by the brar's waitgroup.
A simple fix is to select on the brar's quit channel after
detecting a failure from either, allowing the breach arbiter
to break out of this death cycle.
In this commit, we address an un accounted for case during the breach
remedy process. If the remote node actually went directly to the second
layer during a channel breach attempt, then we wouldn’t properly be
able to sweep with out justice transaction, as some HTLC inputs may
actually be spent at that point.
In order to address this case, we’ll now catch the transaction
rejection, then check to see which input was spent, promote that to a
second level spend, and repeat as necessary. At the end of this loop,
any inputs which have been spent to the second level will have had the
prevouts and witnesses updated.
In order to perform this transition, we now also store the second level
witness script in the database. This allow us to modify the sign desc
with the proper input value, as well as witness script.
Before this commit, we had the weight estimates flipped. When sweeping
w/o any delay, we’re spending a regular P2WKH output. When we’re
sweeping their CSV delayed output, we’ll using the entire to-local
script, so we need to properly account for that.
In this commit, we modify the breach arbiter to no longer require
holding a channel object directly in order to receive new notifications
about possible breaches. Instead, we’ll contact the chain arbiter to
request a new channel event subscription.
As a result of the new architecture, we no longer need to receive a
handoff once the new channel comes online, as the chainWatcher will
always be active and watching the channel until it’s been closed.
In this commit, we remove all the code win the BreachArbiter that was
dedicated to sweeping output on the remote party’s commitment
transaction, and also responding to unilateral channel closes. We no
longer need to do this, as this is now the duty of the
ChannelArbitrator.
With the new negotiation policy, we instead just need to ensure that
our fee inches closer to the other party’s with each iteration, and
that it’s within the proper bounds.
In this commit we add a chainhash field to the retributionInfo struct
as within the database, channels are now further namespaced by their
chain hash, and all ChannelCloseSummary structs now also carry the
chain hash of their respective chain.
This commit refactors the breach arbiter such that it ignores
commitment values below the remote party's dust limit when trying
to sweep funds after a channel breach. The wallet is now
permitted to pass nil sign descriptors for commitment outputs,
which are then ignored by the arbiter.
All non-dust outputs are accumulated into a single slice of
outputs inside the breach arbiter to simplify the internals.
Doing so permitted a work flow that reduces the total number
of allocations made while processing breaches.
The SpendableOutputs is also expanded to include public methods
to access the witness type and sign descriptors, which was useful
in refactoring the craft justice transaction logic.
This commit introduces a RetributionStore interface, which
establishes the methods used to access persisted information
regarding breached channels. A RetributionStore is used to
persist retributionInfo regarding all channels for which
the wallet has signaled a breach.
The current design could be improved by moving certain
functionality, e.g. closing channels and htlc links, such
that they are handled by upstream by their respective
subsystems. This was investigated, but deemed preferable to
postpone to a later update to prevent the current
implementation from sprawling amongst too many packages.
The test suite creates a mockRetributionStore and ensures that
it exhibits the same behavior as the retribution store backed
by a channeldb.DB.
