Commit Graph

4432 Commits

Author SHA1 Message Date
Olaoluwa Osuntokun
ec8e3b626d
htlcswitch: update unit tests to account for recent API changes 2018-04-06 14:52:00 -07:00
Olaoluwa Osuntokun
0a47b2c4ad
htlcswitch: remove linkControl in favor of a mutex guarding all channel indexes
In this commit, we simplify the switch's code a bit. Rather than having
a set of channels we use to mutate or query for the set of current
links, we'll instead now just use a mutex to guard a set of link
indexes. This serves to simplify the ode, and also make it such that we
don't need to block forwarding in order to add/remove a link.
2018-04-06 14:52:00 -07:00
Olaoluwa Osuntokun
7037d55f65
htlcswitch: perform fee related checks at forwarding time
In this commit, we fix a very old, lingering bug within the link. When
accepting an HTLC we are meant to validate the fee against the
constraints of the *outgoing* link. This is due to the fact that we're
offering a payment transit service on our outgoing link. Before this
commit, we would use the policies of the *incoming* link. This would at
times lead to odd routing errors as we would go to route, get an error
update and then route again, repeating the process.

With this commit, we'll properly use the incoming link for timelock
related constraints, and the outgoing link for fee related constraints.
We do this by introducing a new HtlcSatisfiesPolicy method in the link.
This method should return a non-nil error if the link can carry the HTLC
as it satisfies its current forwarding policy. We'll use this method now
at *forwarding* time to ensure that we only forward to links that
actually accept the policy. This fixes a number of bugs that existed
before that could result in a link accepting an HTLC that actually
violated its policy. In the case that the policy is violated for *all*
links, we take care to return the error returned by the *target* link so
the caller can update their sending accordingly.

In this commit, we also remove the prior linkControl channel in the
channelLink. Instead, of sending a message to update the internal link
policy, we'll use a mutex in place. This simplifies the code, and also
adds some necessary refactoring in anticipation of the next follow up
commit.
2018-04-06 14:52:00 -07:00
Olaoluwa Osuntokun
9d4cea93f0
discovery: fix deadlock in processChanPolicyUpdate
In this commit, we fix an existing deadlock in the
processChanPolicyUpdate method. Before this commit, within
processChanPolicyUpdate, we would directly call updateChannel *within*
the ForEachChannel closure. This would at times result in a deadlock, as
updateChannel will itself attempt to create a write transaction in order
to persist the newly updated channel.

We fix this deadlock by simply performing another loop once we know the
set of channels that we wish to update. This second loop will actually
update the channels on disk.
2018-04-06 14:51:57 -07:00
Olaoluwa Osuntokun
80d57f3ddf
build: udpate to version of btcwallet w/ re-org fix 2018-04-06 14:40:22 -07:00
Olaoluwa Osuntokun
3575aef7e2
Merge pull request #1040 from Roasbeef/write-buf-peer
peer: re-use a static writeBuf within writeMessage optimize memory usage
2018-04-06 14:31:28 -07:00
Olaoluwa Osuntokun
7cbe78eeee
peer: re-use a static writeBuf within writeMessage optimize memory usage
In this commit, we might a very small change to the way writing messages
works in the peer, which should have large implications w.r.t reducing
memory usage amongst chatty nodes.

When profiling the heap on one of my nodes earlier, I noticed this
fragment:
```
Showing top 20 nodes out of 68
      flat  flat%   sum%        cum   cum%
         0     0%     0%    75.53MB 54.61%  main.(*peer).writeHandler
   75.53MB 54.61% 54.61%    75.53MB 54.61%  main.(*peer).writeMessage
```

Which points to an inefficiency with the way we handle allocations when
writing new messages, drilling down further we see:
```
(pprof) list writeMessage
Total: 138.31MB
ROUTINE ======================== main.(*peer).writeMessage in /root/go/src/github.com/lightningnetwork/lnd/peer.go
   75.53MB    75.53MB (flat, cum) 54.61% of Total
         .          .   1104:   p.logWireMessage(msg, false)
         .          .   1105:
         .          .   1106:   // As the Lightning wire protocol is fully message oriented, we only
         .          .   1107:   // allows one wire message per outer encapsulated crypto message. So
         .          .   1108:   // we'll create a temporary buffer to write the message directly to.
   75.53MB    75.53MB   1109:   var msgPayload [lnwire.MaxMessagePayload]byte
         .          .   1110:   b := bytes.NewBuffer(msgPayload[0:0:len(msgPayload)])
         .          .   1111:
         .          .   1112:   // With the temp buffer created and sliced properly (length zero, full
         .          .   1113:   // capacity), we'll now encode the message directly into this buffer.
         .          .   1114:   n, err := lnwire.WriteMessage(b, msg, 0)
(pprof) list writeHandler
Total: 138.31MB
ROUTINE ======================== main.(*peer).writeHandler in /root/go/src/github.com/lightningnetwork/lnd/peer.go
         0    75.53MB (flat, cum) 54.61% of Total
         .          .   1148:
         .          .   1149:                   // Write out the message to the socket, closing the
         .          .   1150:                   // 'sentChan' if it's non-nil, The 'sentChan' allows
         .          .   1151:                   // callers to optionally synchronize sends with the
         .          .   1152:                   // writeHandler.
         .    75.53MB   1153:                   err := p.writeMessage(outMsg.msg)
         .          .   1154:                   if outMsg.errChan != nil {
         .          .   1155:                           outMsg.errChan <- err
         .          .   1156:                   }
         .          .   1157:
         .          .   1158:                   if err != nil {
```

Ah hah! We create a _new_ buffer each time we want to write a message
out. This is unnecessary and _very_ wasteful (as seen by the profile).
The fix is simple: re-use a buffer unique to each peer when writing out
messages. Since we know what the max message size is, we just allocate
one of these 65KB buffers for each peer, and keep it around until the
peer is removed.
2018-04-06 12:55:17 -07:00
Olaoluwa Osuntokun
e30881a14c
Merge pull request #1033 from stevenroose/close-offline
rpcserver: Clarify failure closing offline channel
2018-04-06 12:20:30 -07:00
Steven Roose
445924b7a9 rpcserver: Clarify failure closing offline channel 2018-04-06 10:42:14 +02:00
Johan T. Halseth
ed2cfd74c1
fundingmanager test: test that MinHtlc is preserved during flow 2018-04-06 10:02:19 +02:00
Johan T. Halseth
3d55767838
lnwallet/reservation: remove RegisterMinHTLC
We remove this method, as our minHtlc value is set using the
CommitConstraints method.
2018-04-06 10:02:19 +02:00
Johan T. Halseth
ca0b4cb8c5
fundingmanager: preserve remote MinHtlc during funding flow
This commit fixes a bug within the funding manager, where we would use
the wrong min_htlc_value parameter. Instead of attributing the custom
passed value for MinHtlc to the remote's constraints, we would add it to
our own constraints.
2018-04-06 10:02:19 +02:00
Johan T. Halseth
cbfba79f46
fundingmanager test: add test for custom channel parameters
This commit adds TestFundingManagerCustomChannelParameters, which checks
that custom channel parameters specified at channel creation is
preserved and recorded correctly on both sides of the channel.
2018-04-06 10:02:19 +02:00
Johan T. Halseth
7a4817b066
fundingmanager: preserve custom remoteCsvDelay
This commit fixes a bug that would cause the local and remote commitment
to be incompatible when using custom remote CSV delay when opening a
channel. This would happen because we wouldn't store the CSV value
before we received the FundingAccept message, and here we would use the
default value.

This commit fixes this by making the csv value part of the
reservationWithCtx struct, such that it can be recorded for use when the
FundingAccept msg comes back.
2018-04-06 10:02:19 +02: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
b7875fce4c
Merge pull request #969 from halseth/chainntnfs-chain-spends
chainntnfs: optionally only notify spends on block inclusion
2018-04-05 21:40:38 -07:00
Wilmer Paulino
19a2bd9c7d
chainntnfs: send incremental update notifications for tx confirmations
In this commit, we introduce the ability for the different ChainNotifier
implements to send incremental updates to the subscribers of transaction
confirmations. These incremental updates represent how many
confirmations are left for the transaction to be confirmed. They are
sent to the subscriber at every new height of the chain.
2018-04-06 00:30:25 -04:00
Wilmer Paulino
09f8a72897
chainntnfs: modify watched transactions by height to use a set
In this commit, we avoid storing extra copies of a transaction when
multiple clients register to be notified for the same transaction. We do
this by using a set, which only stores unique elements.
2018-04-06 00:30:23 -04:00
Wilmer Paulino
486694a84e
chainntnfs: add Updates channel field to ConfirmationEvent
In this commit, we add a new Updates channel to our ConfirmationEvent
struct. This channel will be used to deliver updates to a subscriber of
a confirmation notification. Updates will be delivered at every
incremental height of the chain with the number of confirmations
remaining for the transaction to be considered confirmed by the
subscriber.
2018-04-06 00:30:19 -04:00
Olaoluwa Osuntokun
706cc02639
Merge pull request #958 from vapopov/pending_balance
Include a pending_balance in channelbalance response
2018-04-05 21:16:22 -07:00
Adam Soltys
078b4ebec6 lnd: use single asterisk for gitignore wildcards
This stops ripgrep from complaining about

  invalid use of **; must be one path component

See https://github.com/BurntSushi/ripgrep/issues/373
2018-04-05 20:57:10 -07:00
Olaoluwa Osuntokun
e0adb0a4dd
Merge pull request #1007 from wpaulino/htlc-switch-stats
htlcswitch: fix periodic calculation of satoshis sent/received
2018-04-05 20:44:27 -07:00
Everton Melo
fb070bf2a3 LICENSE: update year to 2018 2018-04-05 19:59:48 -07:00
Olaoluwa Osuntokun
28737474ce
Merge pull request #929 from guggero/aezeed-test
aezeed: add tests and README
2018-04-05 19:52:00 -07:00
Antonin Hildebrand
132c67d414 docker: replace bash shells with last command
It is better to replace bash shell with potentially long-running
last script command. This way the running command will receive all
potential unix process signals directly.

A concrete example which motivated this change:
Exec of btcd is needed for graceful shutdown of btcd during
`docker-compose down`. Docker Compose properly sends this signal to our
start-btcd.sh bash shell but it is not further signalled to the running
btcd process. Docker Compose then kills whole container forcefully after
some timeout.

An alternative solution would be to trap SIGTERM in our bash script and
forward it to running btcd. Which would be IMO ugly and error prone.
2018-04-05 18:17:46 -07:00
Wilmer Paulino
c8e0eeaa83 gitignore: exclude local lnd+lncli binaries and log outputs 2018-04-05 18:17:17 -07: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
f07b1cc267
Merge pull request #1026 from Roasbeef/link-sig-errors
htlcswitch+lnwallet: ensure the Error is sent to remote peer before d/c, add detailed err for htlc sig rejection
2018-04-05 16:28:25 -07:00
Olaoluwa Osuntokun
7da8cb2910
lnwallet: add new TestNewBreachRetributionSkipsDustHtlcs test 2018-04-05 16:20:22 -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
406fdbbf64
brontide: increase timeout to 5s 2018-04-05 15:54:47 -07:00
Johan T. Halseth
d6b9907cf5
Merge pull request #1014 from bretton/master
Correcting install docs to add path for upgrade instructions section
2018-04-05 10:15:34 +02: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
e91dff44e6
test: extend testRevokedCloseRetributionRemoteHodl to have HTLCs in both directions
In this commit, we extend the testRevokedCloseRetributionRemoteHodl so
that the final broadcast revoked transaction has incoming *and* outgoing
HTLC's. As is, this test fails as there's a lingering bug in the way we
generate htlc resolutions. A follow up commit will remedy this issue.
2018-04-04 18:41:33 -07:00
Olaoluwa Osuntokun
ca9174e166
peer: extend SendMessage to allow callers to block until msg is sent 2018-04-04 17:43:57 -07:00
Olaoluwa Osuntokun
b3bc374ba1
htlcswitch: send a direct Error if we get a known channel error on validate commit 2018-04-04 17:41:47 -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
0dbd325fd0
htlcswitch: synchronously send the error to the peer on commitment verify fail
In this commit, we fix a slight bug in lnd. Before this commit, we would
send the error to the remote peer, but in an async manner. As a result,
it was possible for the connections to be closed _before_ the error
actually reached the remote party. The fix is simple: wait for the error
to be returned when sending the message. This ensures that the error
reaches the remote party before we kill the connection.
2018-04-04 17:38:35 -07:00
Olaoluwa Osuntokun
f53a99e18e
htlcswitch: modify the SendMessage method on the Peer interface to optionally block
In this commit, add a new argument to the SendMessage method to allow
callers to request that the method block until the message has been sent
on the socket to the remote peer.
2018-04-04 17:36:44 -07:00
Oliver Gugger
16ad6aed09 aezeed: add README with the text of the PR 2018-04-04 15:39:47 +02:00
Oliver Gugger
0c7451c97c aezeed: add test vectors and test birthday calculation 2018-04-04 15:38:37 +02:00
Johan T. Halseth
ed6682ea4a
multi test: make mock adhere to api change 2018-04-03 22:04:02 +02:00
Johan T. Halseth
6b6a616b1e
chainntfnfs test: add testSpendNotificationMempoolSpends for btcd backend
The test is only run for the btcd backend for now, as notifying on
mempool spends doesn't work for neutrino and bitcoind.
2018-04-03 11:24:08 +02:00
Johan T. Halseth
a36683e5e0
chainntfnfs test: check spend notification only on confirmation
This commit changes the chainntnfs tests to adhere to the new
RegisterSpendNtfn signature. It also makes sure that for the test
testSpendNotification, we are only getting notified when a spend is
mined, as previously btcd would notify on mempool inclusion, while
neutrino and bitcoind would notify only on confirmation, and the test
wouldn't catch this.
2018-04-03 11:24:07 +02:00
Johan T. Halseth
8b02064c7b
multi: provide mempool=true to RegisterSpendNtfn 2018-04-03 11:24:07 +02:00
Johan T. Halseth
9e7e023194
chainntnfs/bitcoind: add bool to RegisterSpendNtfn 2018-04-03 11:24:07 +02:00
Johan T. Halseth
5283b6d210
chainntnfs/neutrino: add bool to RegisterSpendNtfn 2018-04-03 11:24:07 +02:00
Johan T. Halseth
cc17bc1636
chainntnfs/btcd: add logic for mempool argument to RegisterSpendNtfn
This commit adds a boolean to RegisterSpendNtfn, giving the caller the
option to only register for notifications on confirmed spends. This is
implemented for the btcd backend using logic similar to what is in used
for Neutrino, paving the way for later unifying them.
2018-04-03 11:24:07 +02:00
Johan T. Halseth
fa29adf9a3
chainntnfs interface: add mempool argument to RegisterSpendNtfn 2018-04-03 11:24:07 +02:00