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.
This commit adds a breached contract retribution storage layer using
boltdb to the breach arbiter. The breach arbiter now stores retribution
state on disk between detecting a contract breach, broadcasting a
justice transaction that sweeps the channel, and finally witnessing the
justice transaction confirm on the blockchain. It is critical that such
state is persisted on disk, so that if our node restarts at any point
during the retribution procedure, we can recover and continue from the
persisted state.
This commit adds a new responsibility to the breach arbiter: the
service is now responsible for sweeping the commitment outputs to-self,
in the case of a unilateral commitment broadcast by the remote party.
In this new commitment design, this output won’t be immediately
recognized by the wallet due to using a tweaked public key. As a
result, we need to sweep this output into the wallet manually.
A prior commit modified the walletbalance RPC to return satoshi instead
of BTC. As a result, we need to update the SetUp method in the
networkHarness to ensure we expect the proper value when asserting
wallet balances.
In current commit big shift have been made in direction of unit testable
payments scenarios. Previosly two additional structures have been added
which had been spreaded in the lnd package before, and now we apply
them in the lnd itself:
1. ChannelLink - is an interface which represents the subsystem for
managing the incoming htlc requests, applying the changes to the
channel, and also propagating/forwarding it to htlc switch.
2. Switch - is a central messaging bus for all incoming/outgoing htlc's.
The goal of the switch is forward the incoming/outgoing htlc messages
from one channel to another, and also propagate the settle/fail htlc
messages back to original requester.
With this abtractions the folowing schema becomes nearly complete:
abstraction
^
|
| - - - - - - - - - - - - Lightning - - - - - - - - - - - - -
|
| (Switch) (Switch) (Switch)
| Alice <-- channel link --> Bob <-- channel link --> Carol
|
| - - - - - - - - - - - - - TCP - - - - - - - - - - - - - - -
|
| (Peer) (Peer) (Peer)
| Alice <----- tcp conn --> Bob <---- tcp conn -----> Carol
This commit adds the FeeEstimator interface, which can be used for
future fee calculation implementations. Currently, there is only the
StaticFeeEstimator implementation, which returns the same fee rate for
any transaction.
This commit fixes a slight logic error in the breachArbiter. Previously
we wouldn’t watch a pending channel for closure if the regular
(settled) balance was non-zero. However, this was incorrect, as it’s
possible for us to be on the receiving side of a channel force closure.
This error would leave certain channels as “pending close zombies”
forever until a user manually deleted the entry (or promoted it to be
fully closed).
To fix this, we now utilize the new `TimeLockedBalance` field to make a
better judgment as to if the utxoNursery is watching over a channel or
not.
This commit adds a start up check to the breachArbiter: it will now
watch all channels which are in the “pending closed” state, to ensure
that state of the database is up to date at all times. Once any of the
closing transactions for these channels have been confirmed, then they
will properly be marked as such within the database.
This commit modifies the breachArbiter to properly mark a channel as
_fully_ closed once the transaction which force closed the channel has
been confirmed within the chain.
This commit modifies the logic of the contract hand-off within the
breachArbiter to ensure that only a single internal closeObserver
goroutine is ever active for a given contract.
This modification cleans up the logs in the case of multiple
reconnections and may possible eliminate some non-deterministic bugs
related to channel force closures.
This commit moves the fetching of active channels from the
contractObserver goroutine up to the Start() method on the
breachArbiter. By doing this, we ensure that the user receives an error
(under the current set up) if the btcd node that lnd is connected to
doesn’t have their txindex enabled.
This commit makes a large number of minor changes concerning API usage
within the deamon to match the latest version on the upstream btcsuite
libraries.
The major changes are the switch from wire.ShaHash to chainhash.Hash,
and that wire.NewMsgTx() now takes a paramter indicating the version of
the transaction to be created.
This commit fixes s bug in the breachArbiter which was introduced
during the change that added the live channel hand off from an active
peer. Rather than closing the settle channel of the _older_
breachObserver, the settle channel of the newer beachObserver was being
closed. This would result in a panic once the channel itself was
actually settled as the channel would be closed again.
To fix this bug, we now properly close the channel of the old settle
signal instead of the new one.
This commit modifies the interaction between the breachArbiter and the
peer struct such that the breachArbiter _always_ has the latest version
of a contract.
Prior to this commit once we connected out to a peer which we had an
active contract with and the breachArbiter was watching, we’d have two
copies of the channel in memory, instead of just a single one. This was
wasteful and caused some duplicated log messages due to two instances
of the channel being active.
This commit introduces a new sub-system into the daemon whose job it is
to vigilantly watch for any potential channel breaches throughout the
up-time of the daemon. The logic which was moved from the utxoNursery
in a prior commit now resides within the breachArbiter.
Upon start-up the breachArbiter will query the database for all active
channels, launching a goroutine for each channel in order to be able to
take action if a channel breach is detected. The breachArbiter is also
responsible for notifying the htlcSwitch about channel breaches in
order to black-list the breached linked during any multi-hop forwarding
decisions.
