In this commit, we fix an existing source of a panic, that could at
times lead to a deadlock. If the circuit returned from closeCircuit
didn't have an outgoing key (as it was an incomplete forward), then we
would attempt to de-ref a nil pointer. This would trigger a panic, and
the runtime would start to unwind the stack, and execute each defer in
line. A deadlock can arise here, as in the defer at the root goroutine,
we need to grab the fwdingEventMtx. However, we already have it at the
panic site.
We fix this issue by ensuring we only attempt to add the event if it's a
_settle_ and also actually has an outgoing circuit (which it should
already, just a defensive check).
In this commit, we modify the existing updateChanBucket function to no
longer auto-create buckets if they don't exist. We do this in order to
fix a class of bug that could arise wherein after a channel has actually
be closed (and the parent buckets removed) a method that mutates the
channel state is called, which then re-creates the relevant set of
buckets. As a result, subsequent calls to any RPCs which need to read
all the channels will fail as most of the fields won't actually be
populated.
After this commit, the fullSync method is the only one that's able to
create the full bucket hierarchy.
We no longer have to mark the channel as fully closed in the database,
as it is done directly in the chainWatcher. Instead, we stop the watcher
and delete it from the set of active watchers.
This commit makes the dispatchCooperativeClose method mark the channel
fully closed directly, without registering for confirmation
notifications first. We can do this as recent changes to the
contractcourt changed the definition of a closed channel in the database
to have had its closing tx confirmed, and we only dispatch the
cooperative close once the transaction has 1 confirmation.
We also rename the markChanClosed method to notifyChanClosed, to more
clearly indicate that the ChainArbitrator no longer has to mark the
channel fully closed in the database.
This commit stops the chan closer from sending the potential coop close
transactions to the chainwatcher, as this is no longer needed. The
chainwatcher recently was modified to watch for any potential close, and
will because of this handle the close regardless of which one appears in
chain.
When the chancloser broadcast the final close transaction, we mark it as
CommitmentBroadcasted in the database.
In this commit, we add a precautionary assertion at the end of
createCommitmentTx. This assertion is meant to ensure that we don't
accept or propose a commitment transaction that attempts to send out
more than it was funded with.
In this commit, we add a series of additional balance assertions to
ensure that the balance of the two channels at each stage match up with
our expectations. Additionally, we also fix a bug at the end of the test
which would result in Alice accidentally overdrawing her balance in the
channel. The issue was that the test attempted to settle HTLCs that
weren't yet fully locked in. We fix this by adding an additional state
transition before settling the final set of HTLCs.
In this commit, we move the check to CheckTransactionSanity into
createCommitmentTx. We do this as within wallet.go (during the funding
process) we actually end up calling this helper function twice, and also
moving it up until right when we create the fully commitment transaction
ensures we making our assertion against the final version.
In this commit, we attempt to fix a bug that's possible within the
Start() method of the ChainArbiter. We pass the channel pointer directly
into the newActiveChannelArbitrator function causing it to close over
the loop variable. We later use the channel point directly to send
messages to other sub-systems. It's possible that we actually have the
shadowed loop variable and will send an incorrect message. Defensively,
we now re-bind the loop variable in order to ensure we point to the
proper channel.
This commit satisfies the auto-build functionality desired in #722 without changing anything in the docker folder. This allows development and testing to continue to build and use images locally, but external users get a very lean Docker image they can use. The size difference is appreciable.
REPOSITORY TAG IMAGE ID CREATED SIZE
lnd latest 3634dcf5df68 17 seconds ago 1.19GB
lnd-single latest b295cc248447 30 minutes ago 56MB
Right before merging this PR, an Automated Build repository should be configured on the appropriate account (lightningnetwork?) on Docker Hub with the name of lnd in order to match documentation.
I see there are already some repositories created, however I think best practice for Docker is to have a single repository and different tags for different architectures. The included image is based off of Alpine, but in the future we could extend into multiple Dockerfiles (example: ARM support, or being Ubuntu based).
So in this proposed scenario, lightningnetwork/lnd:latest would be alpine, but lightningnetwork/lnd:ubuntu would be an ubuntu-based image, for example.
(With thanks to @alingenhag for https://github.com/f-u-z-z-l-e/docker-lnd/ from which I borrowed heavily)
This commit removes redundant HTLC restoring. We don't have to restore
outgoing HTLCs from the local commitment, as we _know_ they will always
be added to the remote commitment first. Also, when receiving
Settles/Fails, they will be removed from the local commitment first.
This way we can be sure that outgoing HTLCs found on the local
commitment always will be found on the remote commitment
Similarly we don't have to restore incoming HTLCs from the remote
commitment, as they will be added to the local commitment first.
This commit removes the stage during updateLog restoration where we
would attempt to restore incoming HLTCs from the pendingRemoteCommit, in
addition to update our log and htlc counter to reflect this state. The
reason we can safely remove this is to observe that a pending remote
commit is always created from a commitDiff which only contains updates
made by _us_, and thus only taken from the localUpdateLog. The same can
be said for the counters, when creating a commitDiff we'll always use
the remoteACKedIndex as the index into the remoteUpdateLog, meaning that
all potential updates will already be included in the remote commit that
has been ACKed.
This commit adds a test that runs through a scenario where an HTLC is
added then failed, making sure the update logs are properly restored at
any point during the process.
This commit adds a test ensuring that the fix applied in the previous
commit works as expected. The test exercises the scenario where the
HTLCs on the local, remote and pending remote commitment differ, and we
attempt to restore the update logs. We now check that in this case the
logs before and after restart are equivalent.
remoteUpdateLog from localCommit
This commit fixes a bug within channel.go that would lead to the
content of the update logs and their indexes getting out of sync during
restores.
The scenario that could occur was that the localUpdateLog was initiated
with a log index taken from the localCommitment. Updates we send (which
are added to the localUpdateLog) will be added to the remote commitment
first. The problem happened when an update was sent and added to the
remote commitment, but not ACKed. Since it was not ACKed, we would not
add it to our local commitment. During a restart/restore we would init
the localUpdateLog with a height too low, such that when going through
the outgoing HTLCs on the remote commitment, we would restore an HTLC at
an index higher than our local log HTLC counter.
The symmetric change is done to the remoteUpdateLog.
In this commit, we introduce a new method to the channel router's config
struct: QueryBandwidth. This method allows the channel router to query
for the up-to-date available bandwidth of a particular link. In the case
that this link emanates from/to us, then we can query the switch to see
if the link is active (if not bandwidth is zero), and return the current
best estimate for the available bandwidth of the link. If the link,
isn't one of ours, then we can thread through the total maximal
capacity of the link.
In order to implement this, the missionControl struct will now query the
switch upon creation to obtain a fresh bandwidth snapshot. We take care
to do this in a distinct db transaction in order to now introduced a
circular waiting condition between the mutexes in bolt, and the channel
state machine.
The aim of this change is to reduce the number of unnecessary failures
during HTLC payment routing as we'll now skip any links that are
inactive, or just don't have enough bandwidth for the payment. Nodes
that have several hundred channels (all of which in various states of
activity and available bandwidth) should see a nice gain from this w.r.t
payment latency.
In this commit, we fix an issue where sometimes transactions wouldn't
provide enough of a fee to be relayed by the backend node. This would
especially cause issues when sweeping outputs after a contract breach,
etc.
Now, we'll fetch the minimum relay fee from the backend node and ensure
it is set as a lower bound if the estimated fee ends up dipping below
it.
This commit adds a test for the case where the ChannelArbitrator fails
to broadcast its commitment during a force close because of
ErrDoubleSpend. We test that in this case it will still wait for a
commitment getting confirmed in-chain, then resolve.
