In this commit, the `AddChannelEdge` method of the SQLStore is
implemented. Like the KVStore implementation, it makes use of the
available channel `batch.Scheduler` and also updates the reject and
channel caches.
This then lets us convert the following 2 unit tests to run against the
SQL backends:
- TestPartialNode
- TestAddChannelEdgeShellNodes
Expand the existing TestAddChannelEdgeShellNodes test so that we have
coverage for error we expect when AddChannelEdge is called a second time
if we already know of a channel.
In preparation for having consistency with the structs created by the
SQLStore and the KVStore (so that they have the same behaviour when
tested by the unit tests), here we make sure not to init the
ExtraOpaqueData field of the LightningNode struct unless there are
actualy bytes to set.
In this commit, we add the `source_nodes` table. It points to entries in
the `nodes` table. This table will store one entry per protocol version
that we are announcing a node_announcement on.
With this commit, we can run the TestSourceNode unit test against our
SQL backends.
In this commit we add the necessary SQL queries and then implement the
SQLStore's NodeUpdatesInHorizon method. This lets us run the
TestNodeUpdatesInHorizon unit tests against SQL backends.
In this commit, we add the various sqlc queries that we need in order
to implement the following V1Store methods:
- AddLightningNode
- FetchLightningNode
- HasLightningNode
- AddrsForNode
- DeleteLightningNode
- FetchNodeFeatures
These are implemented by SQLStore which then lets us use the SQLStore
backend for the following unit tests:
- TestNodeInsertionAndDeletion
- TestLightningNodePersistence
Let all the NewTestDB functions return the V1Store interface type
instead of pointers. Then add a manual skip in the TestGraphLoading test
for any non-bbolt backend. We can remove this once all the methods used
by the test have been implemented by the SQLStore. We only need the
manual skip for this one test since it is the only one that doesnt use
MakeGraphTest to init the graph db.
In this commit we add more test coverage for the persistence of the
addresses of a LightningNode. This is so that we have unit test coverage
that ensures that all the various address types can be persisted and
that the order of the addresses (within a type) are preserved.
Here we expand TestEdgeInsertionDeletion to assert that the expected
error is returned when AddChannelEdge is called for a channel that has
already been persisted. We also take the opportunity to convert some of
the error checks in the test to use strict error type matching.
Finally, we test that the expected error is returned if
DeleteLightningNode is called for a node that is no longer in the DB.
Expand an existing test for ForEachNodeDirectedChannel so that it also
tests the DB method and not just the ChannelGraph method which will use
the in-memory graph cache for the query.
In this commit, we add a `test_kvdb.go` file with a single definition of
the `NewTestDB` function. A new version of `MakeTestGraph` (called
`MakeTestGraphNew` is added which makes use of this `NewTestDB` function
to create the backing `V1Store` passed to the `ChannelGraph` for tests.
Later on, we will add new implementations of this method backed by
sqlite and postgres. When those are added, then build flags will
control which version of `NewTestDB` is called.
With this change, the only test call-site of `NewKVStore` is the new
`test_kvdb.go` file.
Instead of returning an error and needing to call `require.NoError` for
each call to `MakeTestGraph`, rather just used the available testing
variable to require no error within the function itself.
In preparation for our SQL Graph store which wont explicitly store the
chain hash but will instead obtain it from the runtime config, we
replace the test chainhash value with that of the mainnet genesis hash.
In preparation for a different store impl which may wrap errors, we
use `require.ErrorIs` for error assertions rather than direct "="
comparisons in tests.
Expand this existing test so that it also tests that a node's addresses
and feature are fetched correctly after insertion. With this, we ensure
that the `FetchNodeFeatures` and `AddrsForNodes` methods of the
`V1Store` interface are properly covered by unit tests.
Later when we introduce our SQL version of the graph store, we will
normalise the persistence of the ExtraOpaqueData using the fact that it
is always made up of TLV entries. So we update our tests here to ensure
that they use valid TLV streams as examples.
Use the new `V1Store` interface as a field in the `ChannelGraph` rather than
a pointer to the `KVStore` implementation in preparation for allowing a
SQL implementation of `V1Store` to be used by the `ChannelGraph`.
Note that two tests are adjusted in this commit to be skipped if the
V1Store is not the `KVStore` implementation since the tests are very
bbolt specific.
- Let it do a proper comparison of the full structs passed in.
- Pass in a testing parameter so we can remove the returned error.
- Make sure the callers are passing in the expected and result
parameters in the correct order.
- Fix a bug: the compareNodes was not comparing the Features field of
the LightningNode structs. Now that it does, one test needed to be
updated to properly set the expected Features fields.
Later on we will store the Alias as a Text field in our sql impls of the
graph db. For Postgres, this field then MUST be a valid UTF-8 string.
This is also the case in general for the alias according to [bolt
7](e1fa25cf00/07-routing-gossip.md (L313))
Since we have not removed all call-sites that make use of this
parameter, we can remove it. This helps hide DB-specific details from
the interface we will introduce for the graph store.
In preparation for creating a clean interface for the graph store, we
want to hide anything that is DB specific from the exposed methods on
the interface. Currently the `ForEachNodeChannel` and the
`FetchOtherNode` methods of the `KVStore` expose a `kvdb.RTx` parameter
which is bbolt specific. There is only one call-site of
`ForEachNodeChannel` actually makes use of the passed `kvdb.RTx`
parameter, and that is in the `establishPersistentConnections` method of
the `server` which then passes the tx parameter to `FetchOtherNode`.
So to clean-up the interface such that the `kvdb.RTx` is no longer
exposed: we instead create one new method called
`ForEachSourceNodeChannel` which can be used to replace the above
mentioned call-site. So as of this commit, all the remaining call-site
of `ForEachNodeChannel` pass in a nil param for `kvdb.RTx` - meaning we
can remove the parameter in a future commit.
Replace all tests calls to the private `forEachNode` method on the
`KVStore` with the exported ForEachNode method. This is in preparation
for having the tests run against an abstract DB backend.
Remove the kvdb.Backend parameter from the `createChannelEdge` helper.
This is all in preparation for having the unit tests run against any DB
backend.
This commit cleans up the graph test code by removing unused kvdb type
parameters from the `createTextVertex` and `createLightningNode` helper
methods. We also pass in the testing parameter now so that we dont need
to check the error each time we call `createTestVertex`.
We plan to later on add an option for a remote graph source which will
be managed from the ChannelGraph. In such a set-up, a node would rely on
the remote graph source for graph updates instead of from gossip sync.
In this scenario, however, our topology subscription logic should still
notify clients of all updates and so it makes more sense to have the
logic as part of the ChannelGraph so that we can send updates we receive
from the remote graph.
The test as it stands today does not make sense as it adds a
Partial/Shell node to the graph via AddLightningNode which will never
happen since this is only ever triggered by the gossiper which only
calls the method with a full node announcement. Shell/Partial nodes are
only ever added via AddChannelEdge which will insert a partial node if
we are adding a channel edge which has node pub keys that we dont have a
node entry for. So we adjust the test to use this more accurate flow.
We do this in preparation for moving channel cache population logic out
of the constructor and into the Start method. We also will later on
(when topology subscription is moved to the ChannelGraph), have a
goroutine that will need to be kicked off and stopped.
This commit moves the graph cache checks for FetchNodeFeatures,
ForEachNodeDirectedChannel, GraphSession and ForEachNodeCached from the
KVStore to the ChannelGraph. Since the ChannelGraph is currently just a
pass-through for any of the KVStore methods, all that needs to be done
for calls to go via the ChannelGraph instead directly to the KVStore is
for the ChannelGraph to go and implement those methods.
In this commit, we let the ChannelGraph be responsible for populating
the graphCache and then passing it to the KVStore. This is a first step
in moving the graphCache completely out of the KVStore layer.
And use this struct to pass NewChannelGraph anything it needs to be able
to init the KVStore that it houses. This will allow us to add
ChannelGraph specific options.
Add the `Tx` suffix to both ForEachNodeDirectedChannelTx and
FetchNodeFeatures temporarily so that we free up the original names for
other use. The renamed methods will be removed or unexported in an
upcoming commit. The aim is to have no exported methods on the
ChannelGraph that accept a kvdb.RTx as a parameter.
With the previous commit, the AddNode method was removed and since that
was the only method making use of the ForEachChannel on the
GraphCacheNode interface, we can remove that method. Since the only two
methods left just expose the node's pub key and features, it really is
not required anymore and so the entire thing can be removed along with
the implementation of it.
Find and replace all nolint instances refering to the `lll` linter and
replace with `ll` which is the name of our custom version of the `lll`
linter which can be used to ignore log lines during linting.
The next commit will do the configuration of the custom linter and
disable the default one.
