Commit Graph

10913 Commits

Author SHA1 Message Date
Steven Roose
09719626e5 htlcswitch: Fix typo in logging
Because the .String() method of an amount already includes a unit string
("BTC" usually), it was printing "0.00000001 BTC satoshi".
2017-07-31 21:45:15 -07:00
Conner Fromknecht
9f85eadde1 chainntnfs/btcd+neutrino: sync epoch cancel 2017-07-31 21:44:23 -07:00
Conner Fromknecht
a9b1af4c73 chainntnfs/btcd+neurtino: unify + sync ntfn cancels 2017-07-31 21:44:23 -07:00
Conner Fromknecht
14a06526b8 routing/notifs: order invariant testing of ntfn delivery (#238)
Modifies the test cases in `TestEdgeUpdateNotification` and
`TestNodeUpdateNotification` to check for the possibility of notifications
being delivered out of order.  This addresses some sporadic failures that
were observed when running the test suite. 

I looked through some of the open issues but didn't see any addressing this
issue in particular, but if someone could point me to any relevant issues
that would be much appreciated!

Issue
-----
Currently the test suite validates notifications received in the order they
are submitted. The check fails because the verification of each
notification is statically linked to the order in which they are delivered,
seen
[here](1be4d67ce4/routing/notifications_test.go (L403))
and
[here](1be4d67ce4/routing/notifications_test.go (L499))
in `routing/notifications_test.go`.  The notifications are typically
delivered in this order, but causes the test to fail otherwise.

Proposed Changes
-------------------
Construct an index that maps a public key to its corresponding edges and/or
nodes.  When a notification is received, use its identifying public key and
the index to look up the edge/node to use for validation. Entries are
removed from the index after they are verified to ensure that the same
entry is validated twice. The logic to dynamically handle the verification
of incoming notifications rests can be found here
[here](https://github.com/cfromknecht/lnd/blob/order-invariant-ntfns/routing/notifications_test.go#L420)
and
[here](https://github.com/cfromknecht/lnd/blob/order-invariant-ntfns/routing/notifications_test.go#L539).

Encountered Errors
--------------------
 * `TestEdgeUpdateNotification`: notifications_test.go:379: min HTLC of
   edge doesn't match: expected 16.7401473 BTC, got 19.4852751 BTC
 * `TestNodeUpdateNotification`: notifications_test.go:485: node identity
   keys don't match: expected
   027b139b2153ac5f3c83c2022e58b3219297d0fb3170739ee6391cddf2e06fe3e7, got
   03921deafb61ee13d18e9d96c3ecd9e572e59c8dbd0bb922b5b6ac609d10fe4ee4


Recreating Failing Behavior
---------------------------
The failures can be somewhat difficult to recreate, I was able to reproduce
them by running the unit tests repeatedly until they showed up.  I used the
following commands to bring them out of hiding:

```
./gotest.sh -i
go test -test.v ./routing && while [ $? -eq 0 ]; do go test -test.v ./routing; done
```

I was unable to recreate these errors, or any others in this package, after
making the proposed changes and leaving the script running continuously for
~30 minutes. Previously, I could consistently generate an error after ~20
seconds had elapsed on the latest commit in master at the time of writing:
78f6caf5d2e570fea0e5c05cc440cb7395a99c1d. Moar stability ftw!
2017-07-31 21:38:03 -07:00
Olaoluwa Osuntokun
0377a4f99d
peer: ensure the chanMsgStream for a channel exists on peer d/c 2017-07-31 21:31:24 -07:00
Olaoluwa Osuntokun
c47408119e
peer: abandon the prior activeChanStreams scheme in favor of chanMsgStream
This commit fixes a bug that existed in the prior scheme we used to
synchronize between the funding manager and the peer’s readHandler.
Previously, it was possible for messages to be re-ordered before the
reached the target ChannelLink. This would result in commitment
failures as the state machine assumes a strict in-order message
delivery. This would be manifested due to the goroutine that was
launched in the case of a pending channel funding.

The new approach using the chanMsgStream is much simpler, and easier to
read. It should also be a bit snappier, as we’ll no longer at times
create a goroutine for each message.
2017-07-31 21:25:54 -07:00
Olaoluwa Osuntokun
5e4b368348
peer: introduce chanMsgStream to provide a concurrent safe, in-order stream of msgs 2017-07-31 21:20:42 -07:00
Olaoluwa Osuntokun
5425eff09c
multi: ensure all lnwallet.LightningChannel's are stopped
This commit ensures that we always clean up the resources that are
created when a new instance of a lnwallet.LightningChannel is
instantiated. The is necessary due to the sigPool that’s now present as
an internal goroutine.
2017-07-31 21:07:02 -07:00
Olaoluwa Osuntokun
f963859524
funding+peer: rename processFundingResponse to processFundingAccept 2017-07-31 21:04:58 -07:00
Olaoluwa Osuntokun
812ebe6fe6
lnwallet: type htlcWeight and commitWeight as int64's 2017-07-31 21:02:30 -07:00
Olaoluwa Osuntokun
5240953de0
lnwallet: temporarily ensure TestStateUpdatePersistence doesn't make dust
Note that this commit is temporary, and should be reverted once #231 is
merged. The reason we need to do this for now, is that we don’t
properly track the exact state of the remote party’s commitment. In
this test case, the resulting HTLC’s added are dust to one party, but
non-dust to another. So upon restart, the states (balance wise) has
diverged.
2017-07-31 20:53:08 -07:00
Olaoluwa Osuntokun
8c6a83a67d
lnwallet: account for second-level HTLC fee in TestChannelBalanceDustLimit 2017-07-31 20:50:54 -07:00
Olaoluwa Osuntokun
4aacf2b51d
lnwallet: ensure TestHTLCDustLimit also asserts for bob's commit chain 2017-07-31 20:50:12 -07:00
Olaoluwa Osuntokun
350e6d5c9f
lnwallet: extend TestDustHTLCFees to ensure dust HTLC's go to miners 2017-07-31 20:49:13 -07:00
Olaoluwa Osuntokun
00ca97c789
lnwallet: in TestForceClose properly assign Bob's balance 2017-07-31 20:44:22 -07:00
Olaoluwa Osuntokun
1b61341410
lnwallet: when creating channels in tests, properly account for commitment fee 2017-07-31 20:43:38 -07:00
Olaoluwa Osuntokun
db793991ac
lnwallet: don't add trimmed HTLC value to initiator commitment fees
This commit fixes a lingering bug in the way the internal channel state
machine handled fee calculation. Previously, we would count the dust
HTLC’s that were trimmed towards the fee that the initiator paid. This
is invalid as otherwise, the initiator would always benefit from dust
HTLC’s. Instead, we now simply “donate” the dust HTLC’s to the miner in
the commitment transaction. This change puts us in compliance with
BOLT-0003.
2017-07-31 20:41:23 -07:00
Olaoluwa Osuntokun
5ece7fec2d
lnwallet: add a String() method to updateType 2017-07-31 20:37:51 -07:00
Olaoluwa Osuntokun
628c5cd63a
features: add a new feature bit for recent changes 2017-07-30 20:27:56 -07:00
Olaoluwa Osuntokun
0f2fcf68b2
test: update testChannelFundingPersistence to use --defaultchanconfs 2017-07-30 20:26:38 -07:00
Olaoluwa Osuntokun
fa3fd9a19a
test: update integration tests framework API usage to recent change
OpenPendingChannel and OpenChannel no longer take a number of
confirmations as a parameter.
2017-07-30 20:24:54 -07:00
Olaoluwa Osuntokun
d4a5eaa6ad
lnwallet: modify CommitSpendNoDelay to directly craft witness
This commit modifies the CommitSpendNoDelay script witness generation
function. We must modify this function as all non-delayed outputs now
also require a key derivation. The current default
signer.ComputeInputScript implementation is unable to directly look up
the public key required as it attempt to target the pub key using the
pkScript.
2017-07-30 20:23:31 -07:00
Olaoluwa Osuntokun
e840f43db4
build: update glide to track latest lightning-onion commit 2017-07-30 20:22:21 -07:00
Olaoluwa Osuntokun
828c650bbc
config: add new command line param --defaultchanconfs
This commit adds a new command line option that allows clients to
specify a default value to use when responding to a new channel funding
request. In a future change, a pure mapping will be added, with the
command line option having higher precedence.
2017-07-30 20:21:51 -07:00
Olaoluwa Osuntokun
747e0f57d1
multi: fix linter errors 2017-07-30 18:22:45 -07:00
Olaoluwa Osuntokun
563fac84cc
breacharbiter: sweep commitment output in case of unilateral close
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.
2017-07-30 17:52:32 -07:00
Olaoluwa Osuntokun
8eadd09403
htlcswitch: add additional logging statement when forwarding HTLC's 2017-07-30 17:52:25 -07:00
Olaoluwa Osuntokun
07b0604458
lnwallet: add TweakPubKeyWithTweak helper function
This commit adds a new helper function which is identical to
TweakPubkey, but lets the caller specify their own hash tweak.
2017-07-30 17:52:22 -07:00
Olaoluwa Osuntokun
899fa1ea3e
lnwallet: properly populate the signDesc within UnilateralCloseSummary
This commit modifies the closeObserver code to populate the signDesc in
the case we have a non-trimmed balance. Additionally, we now also add a
*wire.OutPoint field to the struct in order to allow receivers of the
message to construct a witness that can spend the newly created output
to their wallet.
2017-07-30 17:52:18 -07:00
Olaoluwa Osuntokun
8a23de5303
lnwallet: consult current state instead of revocation log during unilateral closure 2017-07-30 17:52:14 -07:00
Olaoluwa Osuntokun
db10772bd0
lnd: add initial skeleton functions for RequiredRemoteDelay+ NumRequiredConfs 2017-07-30 17:52:11 -07:00
Olaoluwa Osuntokun
a73589f3e9
lnd: properly initialize LightningWallet with new config 2017-07-30 17:52:07 -07:00
Olaoluwa Osuntokun
522712b9a6
breacharbiter: update wallet/signer API usage to due recent changes 2017-07-30 17:52:04 -07:00
Olaoluwa Osuntokun
458138f6fa
utxonursery: update serialization, API usage for recent SignDescriptor changes 2017-07-30 17:52:00 -07:00
Olaoluwa Osuntokun
07212588fc
funding: update funding workflow to adhere to BOLT-0002
This commit updates the main single-funder funding workflow within the
fundingManager (initiated via the rpcserver or by a message from a
connected peer) to fully adhere to the funding protocol outlined in
BOLT-0002.

The major changes are as follows:
  * All messages modified to use the new funding messages in BOLT-0002.
  * The initiator of a funding workflow no longer decides how many
confirmations must elapse before the channel can be considered open.
  * Rather than each side specifying their desired CSV delay, both
sides now specify the CSV delay for the _other_ party.
2017-07-30 17:51:57 -07:00
Olaoluwa Osuntokun
216cc3f919
test: update open channel methods in the testing framework 2017-07-30 17:51:54 -07:00
Olaoluwa Osuntokun
5ed9375e94
rpcserver: update RPC function impls to account for new API changes 2017-07-30 17:51:51 -07:00
Olaoluwa Osuntokun
0dfe73386b
Cmd/lncli: remove num_confs as a param to the openchannel command 2017-07-30 17:51:48 -07:00
Olaoluwa Osuntokun
43b40c2ba2
lnrpc: remove num_confs as a parameter within OpenChannelRequest
This commit removes the num_confs parameter within the
OpenChannelRequest struct as it’s no longer applicable with the new
funding workflow. In the new funding workflow, it’s the responder that
decides how many confirmations are required.
2017-07-30 17:51:44 -07:00
Olaoluwa Osuntokun
34bb0ec36f
server: within peerTerminationWatcher exit early if daemon is exiting 2017-07-30 17:51:41 -07:00
Olaoluwa Osuntokun
cd7b3290a8
peer: modify channel closing negotiation to create new delivery scripts
This commit modifies the channel close negotiation workflow to instead
take not of the fat that with the new funding workflow, the delivery
scripts are no longer pre-committed to at the start of the funding
workflow. Instead, both sides present their delivery addresses at the
start of the shutdown process, then use those to create the final
cooperative closure transaction.

To accommodate for this new change, we now have an intermediate staging
area where we store the delivery scripts for both sides.
2017-07-30 17:51:37 -07:00
Olaoluwa Osuntokun
01fe9adff0
peer: update logWireMessage to account for new lnwire messages 2017-07-30 17:51:34 -07:00
Olaoluwa Osuntokun
976dcb7b97
htlcswitch: update TestChannelLinkSingleHopMessageOrdering to ignore rev window 2017-07-30 17:51:31 -07:00
Olaoluwa Osuntokun
1d771eeb1b
htlcswitch: ensure payments in TestChannelLinkBidirectionalOneHopPayments aren't dust
This commit modifies the TestChannelLinkBidirectionalOneHopPayments
test to ensure that each payment sent is safely above the dust
threshold. Note that the dust threshold itself is now higher due to the
existence of the HTLC covenant transactions which the HTLC values
themselves must cover.

This change ensure that this test operates under “normal” operation
conditions in order to catch any bugs introduced during a major change.
2017-07-30 17:51:27 -07:00
Olaoluwa Osuntokun
f8e536dff5
htlcswitch: update messageToString helper test func to new msgs 2017-07-30 17:51:23 -07:00
Olaoluwa Osuntokun
402112e6ee
htlcswitch: update tests utilities to be aware of new commitment design 2017-07-30 17:51:20 -07:00
Olaoluwa Osuntokun
2d1a598b66
htlcswitch: modify channelLink to use new create+verify commitment API 2017-07-30 17:51:16 -07:00
Olaoluwa Osuntokun
f7c4237686
htlcswitch: remove initial revocation window extension in channelLink
We can safely remove the initial revocation window extension as this
has gone away with the new state machine. We instead now just fill the
window once the channel has been opened, and then maintain a fixed
window size of 2 from there on.
2017-07-30 17:51:13 -07:00
Olaoluwa Osuntokun
6ad9d218b0
lnwallet: update all channel unittests due to new commitment design 2017-07-30 17:51:09 -07:00
Olaoluwa Osuntokun
8121bc77ce
lnwallet: add delivery scripts as a param to CreateCloseProposal+ CompleteCooperativeClose
This commit modifies the methods that transition the state of the
channel into an active closing state. With the new commitment design,
the delivery scripts are no longer pre-committed to the initial funding
messages. Instead, the scripts are sent at the instant that either side
decides to shutdown within the Shutdown message.
2017-07-30 17:51:05 -07:00