This commit implements some minor coding style, commenting and naming
clean up after the recent major discovery service was merged into the
codebase.
Highlights of the naming changes:
* fundingManager.SendToDiscovery -> SendAnnouncement
* discovery.Discovery -> discovery.AuthenticatedGossiper
The rest of the changes consist primary of grammar fixes and proper
column wrapping.
Added the signer which will be needed in the funding manager to sign
the lnwaire announcement message before sending them to discovery
package. Also in the future the message signer will be used to sign
the users data.
This commit fixes a slight oversight in the current state machine which
assumes that both commitment chains are always at the same height. In a
future where we move back to allowing nodes to pipeline commitment
updates, this will not always be the case.
This commit fixes a lingering TODO within the wallet portion of the
codebase by properly adhering to the set dust limits when closing a
channel. With this new commit if a party’s current settled balance is
below their current dust-limit, then it will be omitted from the
commitment transaction.
The prior test that asserted negative outputs are rejected has been
removed as they’ll now be avoided by ensuring we omit dust outputs from
the commitment transaction.
This commit does some minor shuffling around and also adds some
additional comments to the restoreStateLogs method within the channel
state machine. After the latest merge in this area, the code has
diverged slightly from what’s considered typical within the rest of the
codebase.
It is possible that that there are multiple HTLCs with different values,
but the same public key script. As such, a check against the value should
be performed when looking for HTLC outputs in a commitment transaction.
Create a new helper method called genHtlcScript which will
generate the public key scripts for a supplied HTLC. This functionality
from addHTLC is removed, and addHTLC will instead call this new
method.
In restoreStateLogs we will regenerate the public key scripts for the
HTLCs with genHtlcScript and restore the proper values.
When an HTLC is either cancelled or settled we must properly set the
pkScript for the HTLC on the remote commitment, such that we can
generate a valid ChannelDelta.
Description of bug:
When calling ReceiveNewCommitment() we will progress through methods
fetchCommitmentView and addHTLC which will add HTLC outputs to the
commitment transaction in the local commitment chain and save the
pkScript to the relevant PaymentDescriptor which resides in the
corresponding updateLog. Finally the local commitment will be added
to the local commitment chain.
When the same user next calls SignNextCommitment we will again
progress through fetchCommitmentView and addHTLC. In addHTLC we will
now overwrite the pkScripts in the PaymentDescriptors with the
pkScript from the context of the remote commitment. When we later
call RevokeCurrentCommitment and proceed into toChannelDelta, we
will not be able to find the correct pkScript in the PaymentDescriptor
to match it against the outputs in the commitment transaction.
This will lead to the nested function locateOutputIndex returning
incorrect values.
Fixing the bug:
We introduce three new fields in PaymentDescriptor:
* ourPkScript
* theirPkScript
* theirPrevPkScript
ourPkScript will include the pkScript for the HTLC from the context
of the local commitment.
theirPkScript will take the value of the latest pkScript for the HTLC
from the context of the remote commitment.
theirPrevPkScript will take the second-latest pkScript for the HTLC
from the context of the remote commitment. This is the value we use
in toChannelDelta when we save a revoked commitment from our peer.
The appropriate value of these fields are set in the addHTLC method.
Additionally we pass a boolean value to toChannelDelta so we know
whether we are operating on a local or remote commitment and grab
the correct pkScript in locateUpdateIndex.
This commit removes all instances of the fastsha256 library and
replaces it with the sha256 library in the standard library. This
change should see a number of performance improvements as the standard
library has highly optimized assembly instructions with use vectorized
instructions as the platform supports.
If the value of the to-local output is below the dust limit, the
ForceCloseSummary should not include a sign descriptor for this output.
We also find the proper to-self output by looking for the expected public
key script and not assume that no HTLC outputs exist.
Currently non-HTLC outputs will be accepted in the commitment
transaction as long as it is non-zero. We change this by not allowing
outputs with a value lower than the dust limit. The value of such
an output will go towards transaction fees.
This commit fixes a class of bug that currently exists within the
cooperative closure methods for the channel state machine. As an
example, due to the current hard coded fees, if one of the outputs
generated within the generated closure transaction has a negative
output, then the initiating node would gladly forward this to the
remote node. The remote node would then reject the closure as the
transaction is invalid. However, the act of completing the closure
would cause the remote node’s state machine to shift into a “closed”
state. As a result, any further closure attempts by the first node
(force or regular) would go unnoticed by the remote node.
We fix this issue by ensuring the transaction is “sane” before
initiating of completing a cooperative channel closure.
At test case has been added exercising the particular erroneous case
reported by “moli” on IRC.
This commit avoids a class of bug wherein the state of the channel
would be marked as closing enough though an error occurred somewhere in
the function. The bug was due to the fact that the channel `status` was
shifted before any actual logic within the function(s) were executed.
We fix this bug by _only_ shifting the channel status once the function
has completed without any error.
This commit adds a new method to the channel’s state machine:
NextRevocationKey. This method is being added in preparation for the
upcoming change to switch to the commitment transaction format outlined
in the spec. When this comes to pass, the ExtendRevocationWindow method
will be removed, as it will no longer be needed.
The NextRevocationKey method will be needed as to conform to the spec,
we’ll need to send the next revocation key within the `fundingLocked`
message.
This commit adds the FundingManagerPersistence test to ensure that the
funding process completes as expected when nodes shutdown after the the
funding transaction has been broadcast. Note that the final parts of
several wallet tests have been removed, as functionality has been moved
to the Funding Manager and should now be tested there.
Once a channel funding process has advanced to the point of broadcasting
the funding transaction, the state of the channel should be persisted
so that the nodes can disconnect or go down without having to wait for the
funding transaction to be confirmed on the blockchain.
Previously, the finalization of the funding process was handled by a
combination of the funding manager, the peer and the wallet, but if
the remote peer is no longer online or no longer connected, this flow
will no longer work. This commit moves all funding steps following
the transaction broadcast into the funding manager, which is available
as long as the daemon is running.
When a pending channel is persisted and then reloaded upon system startup
it's necessary to also persist the number of confirmations that will be required
before the pending channel can be opened.
In order to facilitate persistence during the funding process, added
the isPending flag to channels so that when the daemon restarts, we can
properly re-initialize the chain notifier and update the state of
channels that were going through the funding process.
In this commit the initial implementation of revocation hash
generation 'elkrem' was replaced with 'shachain' Rusty Russel
implementation which currently enshrined in the spec. This alghoritm has
the same asymptotic characteristics but has more complex scheme
to determine wish hash we can drop and what needs to be stored
in order to be able to achive full compression.
Introduce TimelockShift which is used to make sure the commitment
transaction is spendable by setting the locktime with it so that
it is larger than 500,000,000, thus interpreting it as Unix epoch
timestamp and not a block height. It is also smaller than the current
timestamp which has bit (1 << 30) set, so there is no risk of having
the commitment transaction be rejected. This way we can safely use
the lower 24 bits of the locktime field for part of the obscured
commitment transaction number.
Add tests to assert maximum state can be used. Also test
that more than one input in the commitment transaction
will fail and that having state number larger than
maxStateHint will fail.
Fix SetStateNumHint and GetStateNumHint to properly
set and get the stateNumHints using the lower 24 bits
of the locktime of the commitment transaction as the
lower 24 bits of the obfuscated state number and the
lower 24 bits of the sequence field as the higher 24
bits.
This commit fixes a bug in the wallet’s internal reservation manager
that prevented it from cleaning up the resources used by a reservation
after it was finished running through the workflow.
We fix this issue by ensuring the reservations context is deleted from
the funding limbo.
It is the callers responsibility to properly .Cancel() a reservation in
the case of an error during the funding workflow.
This commit adds an additional case of the closeObserver that will
properly handle the case of a channel being closed by a de-sync’d
commitment transaction from the PoV of the local node. In the case of a
minor 1-state divergence, the commitment transaction broadcast by the
remote node will be 1 state ahead of the commitment transaction we have
locally. This should be seen as a regular unilateral close as they
remote peer didn’t violate the channel contract in any way.
We address this case by changing the `==` to a `>=`.
This commit updates the set of channel state machine tests to properly
compile and execute with the latest set of changes to the state
machine.
Most of the changes within this commit are just renaming and field
changes as a result of the new wire messages.
The more substantial change is due to the change in semantics of the
new state machine w.r.t what has and has not been ACK’d when a new
state transition is implemented. In the case of a concurrent update
(both sides add to the log before a state transition), both sides need
to trigger a state update in order to ensure their updates have been
included.
This commit updates the internal channel state machine to the one as
described within the spec and currently implemented within the rest of
the other Lightning implementations.
At a high level the following modifications have been made:
* When signing we no loner include the index of the remote party’s
log
that our signature covers. Instead we include ALL of our current
updates, but only the updates of the remote party that we’ve
ACK’d.
* A pending change is considered ACK’d once a revocation message
has been received, locking in the changes in the remote party’s
commitment transaction.
* When sending a new commitment, we remember the index of our
log at that point so we can mark that portion of the log as ACK’d
once we receive a revocation message from the remote party.
* When receiving a new commitment signature, we include ALL of
the remote party’s changes that we’ve received but only our set
of changes that’ve been ACK’d by the remote party.
* Implicitly a revocation message now also implicitly serves to ACK
all the changes that were included in the CommitSig message
received before it.
The resulting change is a rather minor diff. However, with this state
machine it’s important to note that the order to sig/revoke messages
has been swapped. A proper exchange now looks like the following:
* Alice -> Add, Add, Add
* Alice -> Sig
* Revoke <- Bob
* Sig <- Bob
* Alice -> Revoke
One other thing that’s worth noting is that with this state machine,
since what’s included in an update is implicit, both side may need to
at times send a new commitment update in the case of a concurrent state
transition initiated by both sides.
Finally, all counters/indexes have been made 64-bit integers in order
to properly match the spec.
This commit adds a new struct to the channel state machine: updateLog.
updateLog encapsulates the update log linked list itself, a series of
new counters we’ll need in order to switch to the spec’s state machine
and also the index into the log itself. This new struct serves to
simplify much of the logic surrounding the update log and also
elminates a bit of code duplication within the current state machine.
This commit only adds the new struct. The rest of the state machine
will be updated in a later commit to use the new log and its new
counters.
This commit fixes a bug in the LightingChannel commitment state machine
which could occasionally result in the total number of satoshis sent or
received being counted twice if a redundant state transition were
initiated.
To fix this bug, we now only increment the number of satoshi
sent/recv’d iff it’s the first time the HTLC has been processed.
