Commit Graph

4614 Commits

Author SHA1 Message Date
Johan T. Halseth
c5169a79f5
contractcourt/channel_arbitrator: add missing return on resolver exit
This commit adds a missing return to the resolveContract method, that
will ensure the goroutine exits if the ChannelArbitrator shuts down.
This fixes a potential deadlock during the integration tests.

We also promote some of the logs to Debug from Trace.
2018-04-13 11:33:09 +02:00
Johan T. Halseth
25d56eda6f
lntest: timeout if process not exited in 1 minute 2018-04-13 11:33:08 +02:00
Olaoluwa Osuntokun
86ad6d318e
Merge pull request #889 from wpaulino/chainnotifier-updates
chainntnfs: add incremental update notifications within ChainNotifier
2018-04-12 19:21:26 -07:00
Olaoluwa Osuntokun
aaa8fa33b1
contractcourt: when creating resolveContract closure don't bind to loop variable
In this commit, we fix a long standing bug where at times a co-op
channel closure wouldn't be properly marked as fully closed in the
database. The culprit was a re-occurring code flaw we've seen many times
in the codebase: a closure variable that closes over a loop iterator
variable. Before this instance, I assumed that this could only pop up
when goroutines bind to the loop iterator within a  closure. However,
this instance is the exact same issue, but within a regular closure that
has _delayed_ execution. As the closure doesn't execute until long after
the loop has finished executing, it may still be holding onto the _last_
item the loop iterator variable was assigned to.

The fix for this issue is very simple: re-assign the channel point
before creating the closure. Without this fix, we would go to call
db.MarkChanFullyClosed on a channel that may not have yet actually be in
the pending close state, causing all executions to fail.

Fixes #1054.
Fixes #1056.
Fixes #1075.
2018-04-12 18:54:33 -07:00
Olaoluwa Osuntokun
f052f18312
test: extend closeChannelAndAssert to also check that channel is no longer pending close
In this commit, we extend the closeChannelAndAssert testing utility
function to ensure that the channel is no longer marked as "pending
close" in the database. With this change, we hop to catch a recently
reported issue wherein users report that a co-op close channel has been
fully confirmed, yet it still pops up in the `pendingchannels` command.
2018-04-12 18:54:32 -07:00
Francisco Calderon
f61a71b6fc docs/macaroons: add reference to invoice.macaroon 2018-04-12 17:25:52 -07:00
Andrew Fuller
26bf3fcb1c README: update Slack invite link 2018-04-12 17:22:52 -07:00
Olaoluwa Osuntokun
7a13378671
channeldb+contractcourt: add additional logging around co-op channel closes 2018-04-12 17:13:37 -07:00
Olaoluwa Osuntokun
f667e3b29f
Merge pull request #1079 from cfromknecht/chain-watcher-unlock
contractcourt/chain_watcher: release mutex on return
2018-04-11 16:16:24 -07:00
Conner Fromknecht
218293db4a
contractcourt/chain_watcher: release mutex on return 2018-04-11 14:50:05 -07:00
Olaoluwa Osuntokun
bc029b9cd4
contractcourt: properly log error if unable to update chan close state 2018-04-10 16:55:32 -07:00
mecampbellsoup
921f505b12 [.gitignore] Ignore *.iml files from IntelliJ 2018-04-10 09:27:44 -04:00
mecampbellsoup
bd398540c1 Add version field to GetInfo RPC endpoint
This is inspired by the Bitcoin RPC `getnetworkinfo` endpoint which also
includes version information:
https://bitcoin.org/en/developer-reference#getnetworkinfo
2018-04-10 09:27:44 -04:00
Olaoluwa Osuntokun
12cb35a6c9
Merge pull request #1067 from cfromknecht/fmgr-scope-fix
fundingmanager: fixes range-goroutine scoping bug
2018-04-09 20:49:50 -07:00
Jim Posen
4b2cb68fe6 discovery: Fix formatting issue in log line. 2018-04-09 20:49:23 -07:00
Olaoluwa Osuntokun
32ad632aa1
Merge pull request #956 from halseth/channel-newlocalforceclosesummary
[refactor] lnwallet/channel: add NewLocalForceCloseSummary
2018-04-09 20:47:53 -07:00
Olaoluwa Osuntokun
9d29c4f43d
server: only swap out the port for inbound connections
Note that the check is actually reversed to the quirk atm in the server
logic, where inbound and outbound are reversed.

Fixes #1063.
2018-04-09 19:55:01 -07:00
Conner Fromknecht
5c43e0ad02
fundingmanager: fixes range-goroutine scoping bug
This commit fixes an issue in funding manager startup,
where a goroutine reads from a range value. The method in
question could cause a channel to be announced at the
wrong time.

This may have been a cause for certain channels having
phantom HTLCs before they had even received the funding
locked message from the remote peer.

This is fixed simply by using the locally scoped
variable passed in as an argument to the goroutine.
2018-04-09 16:55:41 -07:00
Karlson Lee
16c304a4c1 docs/grpc/python remove witness_only 2018-04-09 13:01:39 -07:00
Olaoluwa Osuntokun
13945de806
Merge pull request #1041 from aakselrod/bitcoind-cn-ooo-fix
chainntnfs/bitcoindnotify: rescan blocks manually instead of rewinding
2018-04-06 20:32:24 -07:00
Alex
89e2ba41c9 chainntnfs/bitcoindnotify: rescan blocks manually instead of rewinding 2018-04-06 20:35:27 -06:00
Olaoluwa Osuntokun
5b5491338b
Merge pull request #1019 from Roasbeef/proper-link-constraints
htlcswitch: properly apply the fee constraints of the *outgoing* link when accepting HTLC's
2018-04-06 18:31:13 -07:00
Olaoluwa Osuntokun
31d2f4595f
Merge pull request #1035 from Roasbeef/bip-compliant-neutrino
build: update to point to BIP158+BIP157 compliant btcd+btcutil+neutrino
2018-04-06 18:11:15 -07:00
Olaoluwa Osuntokun
3b47a24a7e
build: update to point to BIP158+BIP157 compliant btcd+btcutil+neutrino 2018-04-06 16:29:30 -07:00
Olaoluwa Osuntokun
fb3c488d3c
build: update to version of btcd w/ mempool spend ntfns 2018-04-06 16:21:09 -07:00
Olaoluwa Osuntokun
1034bdf9e0
Merge pull request #1034 from halseth/funding-custom-csv-delay-bugfix
[bugfix] Custom remote_csv_delay and min_htlc_msat
2018-04-06 15:41:04 -07:00
vegardengen
5eed171187 config: make log rotation configurable 2018-04-06 15:11:42 -07:00
Olaoluwa Osuntokun
a6ffe999c6
routing: prune vertex, not ege after repeated FeeInsufficientErrors
In this commit, we modify the way we handle FeeInsufficientErrors to
more aggressively route around nodes that repeatedly return the same
error to us. This will ensure we skip older nodes on the network which
are running a buggier older version of lnd. Eventually most nodes will
upgrade to this new version, making this change less needed.

We also update the existing test to properly use a multi-hop route to
ensure that we route around the offending node.
2018-04-06 14:52:02 -07:00
Olaoluwa Osuntokun
aa7f83d72e
lnwire: remove pointer receiver from ToUint64 for ShortChannelID 2018-04-06 14:52:02 -07:00
Olaoluwa Osuntokun
3fa2e08665
test: update testUpdateChannelPolicy to ensure Bob's link uses the proper policies
In this commit, we update the testUpdateChannelPolicy to exercise the
recent set of changes within the switch. If one applies this test to a
fresh branch (without those new changes) it should fail. This is due to
the fact that before, Bob would attempt to apply the constraints of the
incoming link (which we updated) instead of the outgoing link. With the
recent set of changes, the test now properly passes.
2018-04-06 14:52:01 -07:00
Olaoluwa Osuntokun
ffabb17ce6
peer: use new fetchLastChanUpdate method to populate the ChannelLinkConfig 2018-04-06 14:52:01 -07:00
Olaoluwa Osuntokun
8b520377bb
htlcswitch: fix TestUpdateForwardingPolicy
In this commit, we fix the TestUpdateForwardingPolicy test case after
the recent modification in the way we handling validating constraints
within the link. After the recent set of changes, Bob will properly use
his outgoing link to validate the set of fee related constraints rather
than the incoming link. As a result, we need to modify the second
channel link, not the first for the test to still be applicable.
2018-04-06 14:52:01 -07:00
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