Commit Graph

790 Commits

Author SHA1 Message Date
Johan T. Halseth
78a4a15bb4
lnwallet/channel: check validity of received commitPoint
This commit adds a check for the LocalUnrevokedCommitPoint sent to us by
the remote during channel reestablishment, ensuring it is the same point
as they have previously sent us.
2018-07-31 15:16:23 +02:00
Johan T. Halseth
a2f2d28d0b
lnwallet/channel: enumerate error cases from remote chain desync
This commit enumerates the various error cases we can encounter when we
compare our remote commit chain to the view the remote communicates to us
via msg.NextLocalCommitHeight.

We now compare this height to our remote tail and tip height, returning
relevant error in case of a unrecoverable desync, and re-send a
commitment signature (including log updates) in case we owe one.
2018-07-31 15:16:23 +02:00
Johan T. Halseth
f1757d6da4
lnwallet/channel: enumerate error cases from local chain desync
This commit enumerates the various error cases we can encounter when we
compare our local commit chain to the view the remote communicates to us
via msg.RemoteCommitTailHeight.

We now compare this height to our local tail height (note that there's
never a local "tip" at this point), returning relevant error in case of
a unrecoverable desync, and re-send a revocation in case we owe one.
2018-07-31 15:16:23 +02:00
Johan T. Halseth
7fb3be84df
lnwallet/channel test: rename ErrCommitSyncDataLoss->ErrCommitSyncLocalDataLoss 2018-07-31 15:16:23 +02:00
Johan T. Halseth
48f1458ae5
lnwallet/channel: define channel sync errors
This commit defines a few new errors that we can potentially encounter
during channel reestablishment:
* ErrInvalidLocalUnrevokedCommitPoint
* ErrCommitSyncLocalDataLoss
* ErrCommitSyncRemoteDataLoss

in addition to the already defined errors
* ErrInvalidLastCommitSecret
* ErrCannotSyncCommitChains
2018-07-31 15:16:23 +02:00
Johan T. Halseth
3825ca71dd
lnwallet/channel: reduce scope of commitSecretCorrect 2018-07-31 15:16:22 +02:00
Johan T. Halseth
eed052eba5
lnwallet/channel: extract local balance from spend instead of stored commit 2018-07-31 08:27:03 +02:00
Johan T. Halseth
d9e9b6197c
lnwallet/channel test: take commitPoint in NewUnilateralCloseSummary 2018-07-31 08:27:03 +02:00
Johan T. Halseth
06ceba429f
lnwallet/channel: make NewUnilateralCloseSummary take commitPoint 2018-07-31 08:27:03 +02:00
Conner Fromknecht
ee2f2573c1
lnwallet/size: correct commit to-local and 2nd stage script sizes
In this commit, we correct our size estimates for to-local scripts,
which are used on the commitment transaction and the htlc
success/timeout transactions. There have been observed cases of
transactions getting stuck because our estimates were too low, and cause
the transactions to not be relayed.

Our previous estimate for the commitment to-local script was derived
from an older version of the script. Though the estimate is greater than
the actual size, this has been updated with the current estimate of 79
bytes.

This estimates makes the assumption that CSV delays will be at most
4 bytes when serialized. Since this value is expressed in relative block
heights, this should be more than sufficient for our needs, even though
the maximum possible size for the little-endian int64 is 9 bytes (plus
an OP_DATA).

The other correction is to use the ToLocalScriptSize as our estimate for
htlc timeout/success scripts, as they are the same script. Previously,
our estimate was derived from the proper script, though we were 6 bytes
shy of the new to-local estimate, since we counted the csv_delay as 1
byte, and missed some other OP_DATAs.

All derived estimates have been updating depending on the new and
improved ToLocalScriptSize estimate, and fix some estimates that did not
include the witness length in the estimate.

Finally, we correct some weight miscalculations in:
 - AcceptedHtlcTimeoutWitnessSize: missing data push lengths
 - OfferedHtlcSuccessWitnessSize: extra 73 byte sig, missing data push lengths
 - OfferedHtlcPenaltyWitnessSize: missing 33 byte pubkey
2018-07-24 22:53:13 -07:00
Olaoluwa Osuntokun
9ad9af4f7a
build: update glide files to point to btcsuite/* not roasbeef/* 2018-07-13 17:15:12 -07:00
Olaoluwa Osuntokun
6f60f139f4 multi: switch over import paths from roasbeef/* to btcsuite/* 2018-07-13 17:05:39 -07:00
Conner Fromknecht
2843df88c0
lnwallet/script_utils_test: use public commit script helpers 2018-07-03 17:07:48 -07:00
Conner Fromknecht
a66c488546
lnwallet/wallet: use public witness script hash helper 2018-07-03 17:07:48 -07:00
Conner Fromknecht
bb808a1937
lnwallet/channel: use public variant of script helpers 2018-07-03 17:07:48 -07:00
Conner Fromknecht
b507ea0ca3
lnwallet/script_utils: expose commit script helpers
Makes the helper methods for constructing witness script
hash and to-local outputs. This will allow watchtowers to
import and reuse this logic when sweeping outputs.
2018-07-03 17:07:48 -07:00
Oliver Gugger
24a085bf7d btcwallet: extend config to allow passing of an unlocked wallet 2018-06-12 19:27:56 -07:00
Oliver Gugger
4cefb6b8dc btcwallet: use already unlocked wallet if available 2018-06-12 19:27:56 -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
Wilmer Paulino
1a47d182d3
lnd+lnwallet: extract default wallet password vars into lnwallet 2018-05-31 17:23:58 -07: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
d4700640e0
lnwallet/channel test: add TestChannelRestoreCommitHeight
This commit adds a test which will restore a channel from an OpenChannel
struct at various stages of the state transation cycle, ensuring the
HTLC local and remote add heights are restored properly.
2018-05-29 08:35:01 +02: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
c285c32678
lnwallet/channel_test: test locked in settle/fail 2018-05-28 16:13:07 -07:00
Conner Fromknecht
e693f89cb0
lnwallet/channel: locked in settle/fail 2018-05-28 13:17:28 -07:00
Olaoluwa Osuntokun
376d332dbd
lnwallet: add new tests to ensure we reject duplicate settles and fails 2018-05-25 20:26:02 -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
c7afb867bc
lnwallet: add additional assertions to TestChanReserve, fix bug before final case
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.
2018-05-21 16:51:39 -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
fad7821e45
lnwallet/channel test: add TestChannelRestoreUpdateLogsFailedHTLC
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.
2018-05-16 21:02:14 +02:00
Johan T. Halseth
d69c358dc6
lnwallet/channel_test: add TestChannelRestoreUpdateLogs
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.
2018-05-16 21:01:32 +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
Wilmer Paulino
3a982063a0
fundingmanager+lnwallet: ensure proposed channel reserve is above dust limit 2018-05-14 14:20:20 -04:00
Wilmer Paulino
3c33a8cb67
lnwallet: enforce backend node's min relay fee floor
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.
2018-05-14 14:20:18 -04: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