Commit Graph

333 Commits

Author SHA1 Message Date
Conner Fromknecht
bb808a1937
lnwallet/channel: use public variant of script helpers 2018-07-03 17:07:48 -07:00
Olaoluwa Osuntokun
8885c3de8d
server: defer cancelling the outbound connection until the tie-breaker 2018-06-10 23:02:22 -07:00
maurycy
3be08e69cf multi: 64bit aligment of atomic vars on arm/x86-32 2018-06-04 20:02:34 -07:00
Johan T. Halseth
07629e5da1
lnwallet/channel: use FullySynced to check chanSync special case
We check if the channel is FullySynced instead of comparing the local
and remote commit chain heights, as the heights might not be in sync.
Instead we call FullySynced which recently was modified to use compare
the message indexes instead, which is _should_ really be in sync between
the chains.

The test TestChanSyncOweRevocationAndCommitForceTransition is altered to
ensure the two chains at different heights before the test is started, to
trigger the case that would previously fail to resend the commitment
signature.
2018-06-04 15:09:51 +02:00
Olaoluwa Osuntokun
31aa8553e7
Merge pull request #1294 from halseth/add-commit-height
lnwallet/channel: set add heights based on the locked in commits
2018-05-29 15:46:39 -07:00
Johan T. Halseth
cb52151123
lnwallet/channel: set add heights based on the locked in commits
This commit fixes a bug which would cause the add heights of the HTLCs
in the update log to be set wrongly. At times, an add height could be
incorrecly set, leading to the HTLCs not being accounted for correctly
during evaluating the HTLC views. This was caused by the assumption that
if the HTLC was not on the pending remote commit, then it was locked in
on both the local and the remote commit, which is not always true.

Instead of making this assumption, we instead now inspect the three
commits: the local, remote and pending remote; and set the add heights
accordingly. This should ensure that HTLCs are subtracted from the
balances only when they are first added.
2018-05-29 08:35:01 +02:00
Conner Fromknecht
e693f89cb0
lnwallet/channel: locked in settle/fail 2018-05-28 13:17:28 -07:00
Olaoluwa Osuntokun
04f576ee6e
lnwallet: reject duplicate fails for the same HTLC 2018-05-25 20:26:01 -07:00
Olaoluwa Osuntokun
28f37def7c
lnwallet: reject duplicate settles for the same HTLC 2018-05-25 20:26:01 -07:00
Olaoluwa Osuntokun
b6d046ad84
lnwallet: mark HTLCs modified for updates restored in restoreStageLogs 2018-05-25 20:26:01 -07:00
Olaoluwa Osuntokun
1f1f22cfe8
lnwallet: clear modified HTLCs within updateLog.removeHtlcs 2018-05-25 20:26:00 -07:00
Olaoluwa Osuntokun
56ebd6de8d
lnwallet: add new modified HTLC index to the updateLog
In this commit, we add a new index to the HTLC log. This new index is
meant to ensure that we don't attempt to modify and HTLC twice. An HTLC
modification is either a fail or a settle. This is the first in a series
of commits to fix an existing bug in the state machine that can cause a
panic if a remote node attempts to settle an HTLC twice.
2018-05-25 20:25:55 -07:00
Johan T. Halseth
9ddd484524
lnwallet/channel: export method MarkCommitmentBroadcasted 2018-05-22 12:03:35 +02:00
Johan T. Halseth
7cd783d7bb
lnwallet/channel: remove unused method DeleteState 2018-05-22 11:51:30 +02:00
Olaoluwa Osuntokun
95293f5102
lnwallet: add assertion in createCommitmentTx to detect overdraw attempts
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.
2018-05-21 16:52:57 -07:00
Olaoluwa Osuntokun
0570e80fd2
lnwallet: check commitment sanity within createCommitmentTx
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.
2018-05-21 16:47:08 -07:00
Johan T. Halseth
9c7163187b
lnwallet/channel: don't restore htlcs uneccessary
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.
2018-05-16 21:02:17 +02:00
Johan T. Halseth
c1e96e01e6
lnwallet/channel: remove restoreHtlc from pendingRemoteCommit
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.
2018-05-16 21:02:17 +02:00
Johan T. Halseth
902e1e77cb
lnwallet/channel: init localUpdateLog with index from remoteCommit,
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.
2018-05-16 20:53:33 +02:00
Conner Fromknecht
f963f91a3c
multi: use mutex-guarded ShortChanID() on OpenChannel 2018-05-09 16:06:49 -07:00
Johan T. Halseth
71228a6b06
lnwallet/channel: don't use commit height to determine FullySynced
This commit removes a faulty check we did to determine if the channel
commitments were fully synced. We assumed that if out local commitment
chain had a height higher than the remote, then we would have state
updates present in our chain but not in theirs, and owed a commitment.
However, there were cases where this wasn't true, and we would send a
new commitment even though we had no new updates to sign. This is a
protocol violation.

Now we don't longer check the heights to determine if we are fully
synced. A consequence of this is that we also need to check if we have
any pending fee updates that are nopt yet signed, as those are
considered non-empty updates.
2018-05-09 08:47:21 +02:00
Johan T. Halseth
9324bf7b1d
lnwallet/channel: return error in case of htlc index mismatch
This commit make us return an error in case a restored HTLC from a
pending remote commit has an index that is different from our local
update log index. It is appended with the assumption that these indexes
are the same, and if they are not we cannot really continue.
2018-05-08 13:41:52 +02:00
Johan T. Halseth
7e1f2a7dc3
lnwallet/channel: remove unused arguments 2018-05-08 13:41:52 +02:00
Johan T. Halseth
aa1c2cdf81
lnwallet/channel: add more info in case of crash
This commit adds a call to panic in case the HTLC we are looking for is
not found in the update log. It _should_ always be there, but we have
seen crashes resulting from it not being found. Since it will crash with
a nil pointer dereference below, we instead call panic giving us a bit
more information to work with.
2018-05-08 13:41:52 +02:00
Johan T. Halseth
1d676b77e1
lnwallet/channel: log pending remote commit during restore 2018-05-08 13:41:51 +02:00
Johan T. Halseth
1d172428c4
lnwallet/channel: return missing errors
This commit corrects a few cases where we would encounter errors, but
return nil by mistake.
2018-05-08 13:41:51 +02:00
Olaoluwa Osuntokun
8b06817482
lnwallet: modify NewUnilateralCloseSummary to be aware of pending remote commits
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.
2018-05-03 21:29:28 -07:00
Conner Fromknecht
cae1d468e7
lnwallet/channel: expose NextLocalHtlcIndex 2018-05-02 01:12:17 -07:00
Olaoluwa Osuntokun
c7c25445eb
lnwallet: add new test to ensure channel is able to update fees in both directions
Closes #1145.
2018-05-01 17:35:42 -07:00
Vegard Engen
d7a254328e Take into account that the new fee will not come in addition to old fee when updating commit fee 2018-04-29 22:42:20 +00:00
Johan T. Halseth
b7bb53a8b8
channeldb/channel: add property ChanStatus in place of IsBorked to OpenChannel
This commit changes the bool `IsBorked` in OpenChannel to a `ChanStatus`
struct, of type ChannelStatus. This is used to indicated that a channel
that is technically still open, is either borked, or has had a
commitment broadcasted, but is not confirmed on-chain yet.

The ChannelStatus type has the value 1 for the status Borked, meaning it
is backwards compatible with the old database format.
2018-04-25 09:37:23 +02:00
Johan T. Halseth
3d0cb16677
lnwallet/channel: clearify that the spendDetails is spending funding outpoint 2018-04-25 09:37:22 +02:00
Dimitris Tsapakidis
4009f7f874 multi: fix typos in comments 2018-04-17 19:03:27 -07:00
practicalswift
663c396235 multi: fix a-vs-an typos 2018-04-17 19:02:04 -07:00
Johan T. Halseth
8e77e1e6eb
lnwallet/channel: add NewLocalForceCloseSummary
This commit renames ForceCloseSummary to LocalForceCloseSummary, and
adds a new method NewLocalForceCloseSummary that can be used to derive a
LocalForceCloseSummary if our commitment transaction gets confirmed
in-chain. It is meant to accompany the NewUnilateralCloseSummary method,
which is used for the same purpose in the event of a remote commitment
being seen in-chain.
2018-04-06 09:46:30 +02:00
Olaoluwa Osuntokun
065b39bc0b
Merge pull request #1025 from Roasbeef/incoming-htlc-breach-retribution
lnwallet+test: properly generate the sender HTLC script in a contract breach scenario
2018-04-05 17:58:31 -07:00
Olaoluwa Osuntokun
f79af461d3
lnwallet: in NewBreachRetribution create the htlcRetributions slice to capacity, not length
In this commit, we fix an existing bug in the NewBreachRetribution
method. Rather than creating the slice to the proper length, we instead
now create it to the proper _capacity_. As we'll now properly filter out
any dust HTLCs, before this commit, even if no HTLCs were added, then
the slice would still have a full length, meaning callers could actually
interact with _blank_ HtlcRetribution structs.

The fix is simple: create the slice with the proper capacity, and append
to the end of it.
2018-04-05 16:19:49 -07:00
Olaoluwa Osuntokun
c393475d39
lnwallet: ensure that we skip dust HTLC's in NewBreachRetribution 2018-04-04 18:41:38 -07:00
Olaoluwa Osuntokun
4e44528bff
lnwallet: fix ordering of keys when we generate the sender script during a breach
In this commit, we fix an existing within lnd. Before this commit,
within NewBreachRetribution the order of the keys when generating the
sender HTLC script was incorrect. As in this case, the remote party is
the sender, their key should be first. However, the order was swapped,
meaning that at breach time, our transaction would be rejected as it had
the incorrect witness script.

The fix is simple: swap the ordering of the keys. After this commit, the
test extension added in the prior commit now passes.
2018-04-04 18:41:37 -07:00
Olaoluwa Osuntokun
1c5d62a804
lnwallet: add new concrete error InvalidHtlcSigError for failed htlc sig validation
In this commit we add a new error: InvalidHtlcSigError. This error will
be returned when we're unable to validate an HTLC signature sent by the
remote party. This will allow other nodes to more easily debug _why_ the
signature was rejected.
2018-04-04 17:41:10 -07:00
Olaoluwa Osuntokun
6103ccb081
lnwallet: ensure we only accept/produce sane commitments
In this commit, we add an additional check within CreateCommitTx to
ensure that we will never create or accept a commitment transaction that
wasn't valid by consensus. To enforce this check, we use the
blockchain.CheckTransactionSanity method.
2018-03-27 15:37:15 -07:00
Johan T. Halseth
a6e7dce7b7
lnwallet/channel: reject received commitment with too many htlc sigs
This commit adds a check that will make LightningChannel reject a
received commitment if it is accompanied with too many HTLC signatures.
This enforces the requirement in BOLT-2, saying:

if num_htlcs is not equal to the number of HTLC outputs in the local commitment transaction:
    * MUST fail the channel.

A test exercising the behaviour is added.
2018-03-22 14:58:22 +01:00
Johan T. Halseth
70b86e596e
lnwallet/channel: use remote dustlimit when generating HTLC sigs
This commit fixes an issue which would arise in some cases when the
local and remote dust limits would differ, resulting in lnd not
producing the expected number of HTLC signatures. This was a result of
checking dust against the local instead of the remote dust limit.

A test exercising the scenario is added.
2018-03-22 14:58:22 +01:00
Johan T. Halseth
263d6b9c1f
lnwallet/channel: don't accept immediately on empty htlc sigs
This commit fixes an issue where we would blindly accept a commitment
which came without any accompanying HTLC signatures. A test exercising
the scenario is added.
2018-03-22 14:58:22 +01:00
Johan T. Halseth
241c79397f
lnwallet/channel: fix crash on receiving too few HTLC sigs
This commit fixes an out of bounds error that would occur in the case
where we received a new commitment where the accompanying HTLC sigs were
too few. Now we'll just reject such an commitment.

A test exercising the behavior is also added.
2018-03-22 14:58:22 +01:00
Conner Fromknecht
d420266911
lnwallet/channel: expose API changes to channel state transitions 2018-03-09 17:35:53 -08:00
Conner Fromknecht
53e4422a2e
lnwallet/channel: integrate fwdpkgs w/ in-mem buffering 2018-03-09 14:45:30 -08:00
Olaoluwa Osuntokun
b8d0df998a
lnwallet: when validating fee updates, ensure newFee < balance 2018-03-08 12:50:49 -05:00
Olaoluwa Osuntokun
ac90a8288e
lnwallet: precalculate fees in mSAT to avoid multiple conversions 2018-03-08 12:50:49 -05:00
Olaoluwa Osuntokun
217166fb10
lnwallet: within validateCommitmentSanity check for balance underflow
In this commit, we add an additional check within
validateCommitmentSanity due to the recent change to unsigned integers
for peer balances in the channel state machine. If after evaluation
(just applying HTLC updates), the balances are negative, then we’ll
return ErrBelowChanReserve.
2018-03-08 12:50:48 -05:00