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 update the batch schedular so that it has the ability
to do read-only calls. It will do a best effort attempt at keeping a
transaction in read-only mode and then if any requests get added to a
batch that require a read-write tx, then the entire batch's tx will be
upgraded to use a read-write tx.
In preparation for using the same logic for non-bbolt backends, we adapt
the batch.Schedular to be more generic.
The only user of the scheduler at the moment is the KVStore in the
`graph.db` package. This store instantiates the bbolt implementation of
the scheduler.
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 implement the postgres and sqlite versions of the
NewTestDB function. We add the various build flags so that only one of
the three versions of this function can be active at a time.
We also introduce the SQLStore struct which is the SQL implementation of
the V1Store interface.
NOTE: it currently temporarily embeds the KVStore struct so that we can
implement the V1Store interface incrementally. For any method not
implemented, things will fall back to the KVStore. This is ONLY the
case for the time being while this struct is purely used in unit tests
only. Once all the methods have been implemented, the KVStore field will
be removed from the SQLStore struct.
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.
In this commit, we unify how all unit tests that make use of the graph
create their test ChannelGraph instance. This will make it easier to
ensure that once we plug in different graph DB implementations, that all
unit tests are run against all variants of the graph DB.
With this commit, `NewChannelGraph` is mainly only called via
`MakeTestGraph` for all tests _except_ for `TestGraphLoading` which
needs to be able to reload a ChannelGraph with the same backend. This
will be addressed in a follow-up commit once more helpers are defined.
Note that in all previous packages where we created a test graph using
`kvdb.GetBoltBackend`, we now need to add a `TestMain` function with a
call to `kvdb.RunTest` since the `MakeTestGraph` helper uses
`GetTestBackend` instead of `kvdb.GetBoltBackend` which requires an
embedded postgres instance to be running.
Currently none of the calls to MakeTestGraph make use of the
KVStoreOptionModifier options but later on we will want to make use of
the `WithUseGraphCache` ChannelGraphOption and so we take this
opportunity to switch out the functional parameters that the helper
function takes.
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.
In this commit, we introduce the `V1Store` interface which the existing
`graphdb.KVStore` implements today. The idea is to eventually create a
SQL DB backed implementation of this interface.
Currently, a few of the graph KVStore methods take the
`batch.SchedulerOptions` param. This is only used to set the LazyAdd
option. A SchedulerOption is a functional option that takes a
`batch.Request` which has bolt-specific fields in it. This commit
restructures things a bit such that the `batch.Request` type is no
longer part of the `batch.SchedulerOptions` - this will make it easier
to implement the graph store with a different DB backend.
Remove the kvdb.Backend return type of the `makeTestGraph` helper. This
is in preparation for the helper being used to create a test graph
backed by a DB other than bbolt.
- 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.
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.
Later on we will create an interface for the persisted graph data. We
want this interface to be as small and as neat as possible. In
preparation for this, we remove this unused `Wipe` method.
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`.
In this commit, we ensure that any topology update is forced to go via
the `handleTopologySubscriptions` handler so that client subscriptions
and updates are handled correctly and in the correct order.
This removes a bug that could result from a client missing a
notification about a channel being closed if the client is subscribed
and shortly after, `PruneGraph` is called which would notify all
subscribed clients and possibly do so before the client subscription has
actually been persisted.
We remove the mutex that was previously held between DB calls and calls
that update the graphCache. This is done so that the underlying DB calls
can make use of any batch requests which they currently cannot since the
mutex prevents multiple requests from calling the methods at once.
The reason the cacheMu was originally added here was during a code
refactor that moved the `graphCache` out of the `KVStore` and into the
`ChannelGraph` and the aim was then to have a best effort way of
ensuring that updates to the DB and updates to the graphCache were as
consistent/atomic as possible.
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.
Elle Mouton