Commit Graph

542 Commits

Author SHA1 Message Date
Conner Fromknecht
2f8021d8db
routing/result_interpretation: fix off-by-one for incoming failure
Previously we would not mark a success for the first hop if the fail
source index was 2. We also add a test to assert this behavior.
2019-10-30 21:18:52 -07:00
Joost Jager
a8f077a841
Merge pull request #3558 from joostjager/non-strict-pathfinding
routing: non-strict path finding
2019-10-30 08:42:28 +01:00
Lars Lehtonen
6254c53a4a
routing: fix use of T.Fatal() in test goroutine 2019-10-29 13:21:53 -07:00
Joost Jager
729c3a6bd6
routing: use unified policy for build route 2019-10-29 19:17:58 +01:00
Joost Jager
a347237e7a
routing: use unified policy for path finding
In this commit we change path finding to no longer consider all channels
between a pair of nodes individually. We assume that nodes forward
non-strict and when we attempt a connection between two nodes, we don't
want to try multiple channels because their policies may not be identical.
Having distinct policies for channel to the same peer is against the
recommendation in the spec, but it happens in the wild. Especially since
we recently changed the default cltv delta value.

What this commit introduces is a unified policy. This can be looked upon
as the greatest common denominator of all policies and should maximize
the probability of getting the payment forwarded.
2019-10-25 11:22:28 +02:00
Joost Jager
6b391d04d0
routing/test: mark function as test helper 2019-10-25 11:22:26 +02:00
Juan Pablo Civile
818c302d46 routing: use nodeWithDist instead of vertex to avoid map access
The same nodeWithDist was fetched from the map for every channel it has.
This struct is not mutated so it can be fetched before and reused.
2019-10-24 21:38:07 -03:00
Juan Pablo Civile
df70095ad0 routing: optimize path finding structures
distance map now holds the edge the current path is coming from,
removing the need for next map.

Both distance map and distanceHeap now hold pointers instead of the full
struct to reduce allocations and copies.

Both these changes reduced path finding time by ~5% and memory usage by
~2mb.
2019-10-24 21:38:07 -03:00
Juan Pablo Civile
fc36df0e60 routing: avoid unneeded map access
`processEdge` basically had 4 expensive operations: 3 map accesses and
updating the heap. This removes one of those for a small performance
gain.
2019-10-24 21:31:30 -03:00
Juan Pablo Civile
3e60a23632 routing: pre-allocate the distance map to an estimated node count
Pre-sizing these structures avoids a lot of map resizing, which causes
copies and rehashing of entries. We mostly know that the map won't
exceed that size, and it doesn't affect memory usage in any significant
way.
2019-10-24 21:31:30 -03:00
Juan Pablo Civile
2141713936 routing: avoid walking all nodes for path finding if we don't need to
Calling `ForEachNode` hits the DB, and allocates and parses every node
in the graph. Walking the channels also loads nodes from the DB, so this
meant that each node was read/parsed/allocated several times per run.

This reduces runtime by ~10ms and memory usage by ~4mb.
2019-10-24 21:31:30 -03:00
Juan Pablo Civile
5389161162 routing: make log in findPath hot path use logClosure
It generates heap allocations for it's params even if it won't end up
using them.

Reduces memory usage by 2mb
2019-10-24 21:31:30 -03:00
Joost Jager
1fac41deed
routing+routerrpc: improve prob. estimation for untried connections
This commit changes mission control to partially base the estimated
probability for untried connections on historical results obtained in
previous payment attempts. This incentivizes routing nodes to keep all
of their channels in good shape.
2019-10-22 15:52:38 +02:00
Joost Jager
fab13900e2
routing+routerrpc: remove probability estimates from mc state snapshot
Probability estimates are amount dependent. Previously we assumed an
amount, but that starts to make less sense when we make probability more
dependent on amounts in the future.
2019-10-22 15:52:36 +02:00
Joost Jager
559d3c0910
routing: also fail pairs for node-level failures
This commit modifies the interpretation of node-level failures.
Previously only the failing node was marked. With this commit, also the
incoming and outgoing connections involved in the route are marked as
failed.

The change prepares for the removal of node-level failures in mission
control probability estimation.
2019-10-22 15:52:34 +02:00
Joost Jager
843c87a203
routing: create mc getWeight method 2019-10-22 15:52:32 +02:00
Joost Jager
a3f7dbc633
routing: change representation of pair results in mc
This commit changes the in-memory structure of the mission control
state. It prepares for calculation of a node probability. For this we
need to be able to efficiently look up the last results for all channels
of a node.
2019-10-22 15:52:30 +02:00
Joost Jager
aefbee78d6
routing: use pairResult constructors
To make it explicit whether a failure or a success result is
instantiated.
2019-10-22 15:52:28 +02:00
Joost Jager
37ef46bc48
routing/test: extract mission control test parameters 2019-10-22 15:52:26 +02:00
Johan T. Halseth
610a93e2ff
routing/chainview: check GetCFilter error
Since the filter will never be nil in non-error cases now, remove the
nil-check.
2019-10-15 10:31:25 +02:00
Wilmer Paulino
0fc401de19
routing+routerrpc: take max cltv limit into account within path finding
With the introduction of the max CLTV limit parameter, nodes are able to
reject HTLCs that exceed it. This should also be applied to path
finding, otherwise HTLCs crafted by the same node that exceed it never
left the switch. This wasn't a big deal since the previous max CLTV
limit was ~5000 blocks. Once it was lowered to 1008, the issue became
more apparent. Therefore, all of our path finding attempts now have a
restriction of said limit in in order to properly carry out HTLCs to the
network.
2019-10-11 18:04:49 -04:00
Olaoluwa Osuntokun
5ef1f85998
Merge pull request #3578 from wpaulino/no-freelist-sync-option
config: add sync freelist cli flag
2019-10-04 15:16:30 -07:00
Olaoluwa Osuntokun
c03fb8aca0
Merge pull request #3561 from Roasbeef/chan-validation-cleanup
chanvalidate: create new channel validation package
2019-10-04 12:32:07 -07:00
Wilmer Paulino
194a9dea81
multi: support sync freelist option within btcwallet 2019-10-04 12:19:40 -04:00
Olaoluwa Osuntokun
6ebada112f
routing: update 3d party channel verification to use new chanvalidate package
In the process of moving to use the new package, we no longer need to
fetch the outpoint directly, and instead only need to pass the funding
transaction into the new verification logic.
2019-10-03 16:23:17 -07:00
Wilmer Paulino
5ed1084130
Merge pull request #3572 from joostjager/canceled-spelling
multi: fix canceled spelling
2019-10-03 16:50:06 -04:00
Joost Jager
b58dbb2d70
multi: fix canceled spelling 2019-10-03 17:27:36 +02:00
Joost Jager
9f3656efeb
routing: log performance metrics 2019-10-03 15:07:11 +02:00
Olaoluwa Osuntokun
18f88cbd8d
Merge pull request #3440 from joostjager/buildroute
routing: add build route functionality
2019-09-24 20:24:24 -07:00
Joost Jager
299821152a
routing+routerrpc: add BuildRoute function 2019-09-24 10:00:44 +02:00
Valentine Wallace
9a52cb6dab
multi: update internals to support updating max htlc
In this commit, we update the router and link to support users
updating the max HTLC policy for their channels. By updating these internal
systems before updating the RPC server and lncli, we protect users from
being shown an option that doesn't actually work.
2019-09-23 13:07:10 +02:00
Joost Jager
c80feeb4b3
routing+discovery: extract local channel manager
The policy update logic that resided part in the gossiper and
part in the rpc server is extracted into its own object.

This prepares for additional validation logic to be added for policy
updates that would otherwise make the gossiper heavier.

It is also a small first step towards separation of our own channel data
from the rest of the graph.
2019-09-23 13:07:08 +02:00
Joost Jager
fd63ed13ff
routing/route: add vertex constructor for hex string 2019-09-17 11:11:00 +02:00
Joost Jager
8c44cf4a22
channeldb: add ComputeFee function 2019-09-17 11:10:58 +02:00
Joost Jager
f60e4b1e14
lnwire+htlcswitch: report height for invalid payment details failure
Extends the invalid payment details failure with the new accept height
field. This allows sender to distinguish between a genuine invalid
details situation and a delay caused by intermediate nodes.
2019-09-16 10:10:20 +02:00
Lars Lehtonen
3b29ecb921 routing: Fix dropped errors in tests.
lnd: Fix dropped errors in tests.

contractcourt: Fix dropped errors in tests.

htlcswitch: Fix dropped errors in tests.

invoices: Fix dropped error in tests.

lnwallet: Fix dropped errors in tests.

macaroons: Fix dropped error in tests.
2019-09-13 06:28:05 +00:00
Joost Jager
3d7de2ad39
multi: remove dead code 2019-09-10 17:21:59 +02:00
Conner Fromknecht
947cc50c13
routing/route/route: prevent modification of hop.TLVRecords
Currently the underlying array backing the hop's TLVRecords is modified
when combining custom records with the primitive forwarding info. This
commit uses a fresh slice to prevent modifications from mutating the
hop itself.
2019-09-04 14:14:34 -07:00
Conner Fromknecht
278e10a2fd
routing/route/route: omit next_hop_id for final hop
BOLT04 says to omit this field for final hops, but must be present on
all other hops.
2019-09-04 12:05:10 -07:00
Conner Fromknecht
dc0f429271
htlcswitch+routing: use TLV hop constructors 2019-09-04 12:05:09 -07:00
Joost Jager
ff0c5a0d5e
routing: process successes in mission control
This commit modifies paymentLifecycle so that it not only feeds
failures into mission control, but successes as well.
This allows for more accurate probability estimates. Previously,
the success probability for a successful pair and a pair with
no history was equal. There was no force that pushed towards
previously successful routes.
2019-08-23 09:15:41 +02:00
Joost Jager
2594abfba1
routing/test: remove unused parameter in mission control test 2019-08-23 09:15:39 +02:00
Olaoluwa Osuntokun
4697cfde30
routing: extend path finding to be TLV-EOB aware, allow dest TLV records
In this commit, we extend the path finding to be able to recognize when
a node needs the new TLV format, or the legacy format based on the
feature bits they expose. We also extend the `LightningPayment` struct
to allow the caller to specify an arbitrary set of TLV records which can
be used for a number of use-cases including various variants of
spontaneous payments.
2019-08-22 18:53:01 -07:00
Olaoluwa Osuntokun
5b4c8ac232
routing/route+tlv: add new TLV-EOB awareness to Hop+Route
In this commit, we extend the Hop struct to carry an arbitrary set of
TLV values, and add a new field that allows us to distinguish between
the modern and legacy TLV payload.

We add a new `PackPayload` method that will be used to encode the
combined required routing TLV fields along any set of TLV fields that
were specified as part of path finding.

Finally, the `ToSphinxPath` has been extended to be able to recognize if
a hop needs the modern, or legacy payload.
2019-08-22 18:53:00 -07:00
Joost Jager
d9ec158412
routing: stricter payment result interpretation
This commit overhauls the interpretation of failed payments. It changes
the interpretation rules so that we always apply the strongest possible
set of penalties, without making assumptions that would hurt good nodes.

Main changes are:

- Apply different rule sets for intermediate and final nodes. Both types
of nodes have different sets of failures that we expect. Penalize nodes
that send unexpected failure messages.

- Distinguish between direct payments and multi-hop payments. For direct
payments, we can infer more about the performance of our peer because we
trust ourselves.

- In many cases it is impossible for the sender to determine which of
the two nodes in a pair is responsible for the failure. In this
situation, we now penalize bidirectionally. This does not hurt the good
node of the pair, because only its connection to a bad node is
penalized.

- Previously we always penalized the outgoing connection of the
reporting node. This is incorrect for policy related failures. For
policy related failures, it could also be that the reporting node
received a wrongly crafted htlc from its predecessor. By penalizing the
incoming channel, we surely hit the responsible node.

- FailExpiryTooSoon is a failure that could have been caused by any node
up to the reporting node by delaying forwarding of the htlc. We don't
know which node is responsible, therefore we now penalize all node pairs
in the route.
2019-08-17 10:24:09 +02:00
Joost Jager
e7a457f1ce
routing: query bandwidth hints before each payment attempt
Previously the bandwidth hints were only queried once per payment. This
did not allow for concurrent payments changing channel balances.
2019-08-17 10:24:07 +02:00
Joost Jager
e135cf7326
routing: penalize all node pairs for unknown outcomes
When an undecryptable failure comes back for a payment attempt, we
previously only penalized our own outgoing connection. However,
any node could have caused this failure. It is therefore better to
penalize all node connections along the route. Then at least we know for
sure that we will hit the responsible node.
2019-08-17 10:24:03 +02:00
Joost Jager
c39d7a29cd
routing/test: do not test local channel mission control
This commit updates existing tests to not rely on mission control for
pruning of local channels. Information about local channels should
already be up to date before path finding starts. If not, the problem
should be fixed where bandwidth hints are set up.
2019-08-17 10:24:01 +02:00
Joost Jager
45dacd0df1
routing: isolate failure interpretation from mission control
This commit moves the payment outcome interpretation logic into a
separate file. Also, mission control isn't updated directly anymore, but
results are stored in an interpretedResult struct. This allows the
mission control state to be locked for a minimum amount of time and
makes it easier to unit test the result interpretation.
2019-08-17 10:23:59 +02:00
Joost Jager
e7af6a077a
routing: convert to nillable failure reason
This commit converts several functions from returning a bool and a
failure reason to a nillable failure reason as return parameter. This
will take away confusion about the interpretation of the two separate
values.
2019-08-17 10:23:57 +02:00