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!
Within the network, it's important that when an HTLC forwarding failure
occurs, the recipient is notified in a timely manner in order to ensure
that errors are graceful and not unknown. For that reason with
accordance to BOLT №4 onion failure obfuscation have been added.
The btclog package has been changed to defining its own logging
interface (rather than seelog's) and provides a default implementation
for callers to use.
There are two primary advantages to the new logger implementation.
First, all log messages are created before the call returns. Compared
to seelog, this prevents data races when mutable variables are logged.
Second, the new logger does not implement any kind of artifical rate
limiting (what seelog refers to as "adaptive logging"). Log messages
are outputted as soon as possible and the application will appear to
perform much better when watching standard output.
Because log rotation is not a feature of the btclog logging
implementation, it is handled by the main package by importing a file
rotation package that provides an io.Reader interface for creating
output to a rotating file output. The rotator has been configured
with the same defaults that btcd previously used in the seelog config
(10MB file limits with maximum of 3 rolls) but now compresses newly
created roll files. Due to the high compressibility of log text, the
compressed files typically reduce to around 15-30% of the original
10MB file.
This commit fixes a send on closed channel panic by adding additional
synchronization when cancelling the notifications for a particular
topology client. We now ensure that all goroutines belonging to a
particular topology client exit fully before we close the notification
channel in order to avoid a panic.
This commit adds a new method to the routing.Route struct:
ToHopPayloads. This function will converts a complete route into the
series of per-hop payloads that is to be encoded within each HTLC using
an opaque Sphinx packet.
We can now use this function when creating the sphinx packet to
properly encoded the hop payload for each hop in the route.
This commit inches towards fully validation+adherance of the per-hop
payloads within an HTLC’s route by properly calculating the outgoing
time lock value for each hop according to the current draft
specification.
This commit fixes a possible race condition wherein a call to
FilterBlock after a call to UpdateFilter would result in the call to
FilterBlock not yet using the updated filter. We fix this by ensuring
the internal chain filter is updated by the time the call to
FilterBlock returns.
This commit optimizes the neutrino implementation of FilterBlock method
of the ChainView interface. The old implementation would _always_ fetch
the entire block and manually scan through it. Instead, we can just
fetch the filter, and then if the items match, fetch the block itself.
This will save bandwidth during a lnd node’s pruning of the channel
graph after a period of dormancy.
This commit adds an initial rough implementation father ChainNotifier
interface for neutrino, our new light client implementation. This
implementation largely borrows from the existing BtcdNotifier
implementation. As a result, a follow up commit will perform two
refactoring in order to further consolidate code.
This commit adds a new implementation of the FilteredChainView
interface. This implementation speaks purely to the p2p network and is
backed by a new experimental light client implementation.
This commit replaces the hard-coded 5000 satoshi fees with calls to the
FeeEstimator interface. This should provide a way to cleanly plug in
additional fee calculation algorithms in the future. This change
affected quite a few tests. When possible, the tests were changed to
assert amounts sent rather than balances so that fees wouldn't need to
be taken into account. There were several tests for which this wasn't
possible, so calls to the static fee calculator were made.
This commit fixes a panic due to a send on a closed channel that could
possibly occur depending on the order of channel closes when a client
goes to cancel a topology notification client.
Previously we closed the ntfnChan first, this would possible result in
a panic as the goroutine may have succeeded on a send at the same time
the channel was closed. Instead, we now close the `exit` channel first
which is meant to be a signal to the goroutine that the client has been
canceled.
This commit modifies the processing in the routing package eo new
announcements. Previously, if we cgot a cnew channel announcement but
didn’t yet know of the verses that the chanell connected, the
cnnounacment would be accepted. This behavior was eronoues as if the
channel were to be queried for, the DB query would fail as we would be
unable to retrieve the two nodes involved int he channel.
To avoid such an error case, we will now _reject_ any channel
announcements in which we don’t yet have a valid node announcement for
the connected nodes. This case has been inserted into the handling of
channel announcement, a new test has been added, and finally older
tests have also been updated to ensure that nodes are added to the
database _before_ the edge is.
This commit modifies the routing package to no longer use the
ChainNotifier for pruning the channel graph. Instead, we now use the
FilteredChainView interface to more (from the ChannelRouter’s PoV)
efficiently maintain the channel graph.
Rather than scanning the _entire_ block manually, we now rely on the
FilteredChainView to provide us with FilteredBlocks which include
_only_ the relevant transactions that we care about.
This commit adds a new set of behavioral interface level tests to the
chain view package. This set of tests can now be used in order to check
proper conformity to this “specification” for all future
implementations of the chain view package.
This commit adds the first concrete implementation of the
chainview.FilteredChainView interface. The implementation of this
interface, BtcdFilteredChainView is backed by a web sockets connection
to an active btcd instance.
This commit creates a new package as sub-package within the routing
package: chainview. This package is centered around a single interface
definition: the FilteredChainView. This interface is to be used to
allow the routing package to watch a _subset_ of the UTXO set for any
modifications. In the case of LN, the subset of the UTXO set that we
care about is the set of currently opened channels.
In a future commit the routing package will be modified to remove the
current full block scanning with processing of FilteredBlock
notification, and proper updates to the filter as observed by the
FilteredChainView.
This commit fixes a pretty nasty unnoticed bug within the main
k-shortest paths algorithm loop. After a new candidate path is found,
the rootPath (the path up to the pivot node) and the spurPath (the
_new_ path after the pivot node) are to be combined into a new candiate
shortest path. The prior logic simply appended the spurPath onto the
end of the rootPath to create a slice. However, if the case that the
currnet rootPath is really a sub-path in a larger slice, then this will
mutate the underlying slice.
This bug would manifest when doing path finding and cause an infinite
loop as the slice kept growing with new spurPaths, causing the loop to
never terminate. We remedy this bug by properly create a new backing
slice, and adding the elements to them rather than incorrectly mutating
an underlying slice.
This commit fixes a bug within the k-shortest paths routine which could
result in a daemon panic when traversing a graph with particular
characteristics. Before referencing the path to create a sub-slice, we
we’re properly asserting that the length of the path was at least as
long as the current rootPath in question. We fix this by simply
ensuring the length of the slice is adequate before proceeding with the
operation.
This commit implements some minor coding style, commenting and naming
clean up after the recent major discovery service was merged into the
codebase.
Highlights of the naming changes:
* fundingManager.SendToDiscovery -> SendAnnouncement
* discovery.Discovery -> discovery.AuthenticatedGossiper
The rest of the changes consist primary of grammar fixes and proper
column wrapping.
Originally we adding the edge without proof in order to able to use it
for payments path constrcution. This method will allow us to populate
the announcement proof after the exchange of the half proofs and
constrcutrion of full proof finished.
Change the name of fields of messages which are belong to the discovery
subsystem in a such way so they were the same with the names that are
defined in the specification.
Add usage of the 'discovery' package in the lnd, now discovery service
will be handle all lnwire announcement messages and send them to the
remote party.
In this commit the routing package was divided on two separete one,
this was done because 'routing' package start take too much responsibily
on themself, so with following commit:
Routing pacakge:
Enitites:
* channeldb.ChannelEdge
* channeldb.ChannelPolicy
* channeldb.NodeLightning
Responsibilities:
* send topology notification
* find payment paths
* send payment
* apply topology changes to the graph
* prune graph
* validate that funding point exist and corresponds to given one
* to be the source of topology data
Discovery package:
Entities:
* lnwire.AnnounceSignature
* lnwire.ChannelAnnouncement
* lnwire.NodeAnnouncement
* lnwire.ChannelUpdateAnnouncement
Responsibilities:
* validate announcement signatures
* sync topology with newly connected peers
* handle the premature annoucement
* redirect topology changes to the router susbsystem
* broadcast announcement to the rest of the network
* exchange channel announcement proofs
Before that moment all that was in the 'routing' which is quite big for
one subsystem.
split
This commit modifies address handling in the NodeAnnouncement struct,
switching from net.TCPAddr to []net.Addr. This enables more flexible
address handling with multiple types and multiple addresses for each
node. This commit addresses the first part of issue #131 .
