Skip to content

Commit 5f961e4

Browse files
guggeroguggero
authored
Merge pull request #9972 from ellemouton/graphSQL17-complete-unit-tests
[17] multi: run all graph unit tests against SQL backends & run itest suite against SQL graph backend
2 parents 3493b08 + cc0be0b commit 5f961e4

19 files changed

+210
-163
lines changed

‎.github/workflows/main.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,10 +368,14 @@ jobs:
368368
args: backend=bitcoind dbbackend=sqlite
369369
- name: bitcoind-sqlite-nativesql
370370
args: backend=bitcoind dbbackend=sqlite nativesql=true
371+
- name: bitcoind-sqlite=nativesql-experiment
372+
args: backend=bitcoind dbbackend=sqlite nativesql=true tags=test_native_sql
371373
- name: bitcoind-postgres
372374
args: backend=bitcoind dbbackend=postgres
373375
- name: bitcoind-postgres-nativesql
374376
args: backend=bitcoind dbbackend=postgres nativesql=true
377+
- name: bitcoind-postgres-nativesql-experiment
378+
args: backend=bitcoind dbbackend=postgres nativesql=true tags=test_native_sql
375379
steps:
376380
- name: git checkout
377381
uses: actions/checkout@v4

‎config_builder.go

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,23 +1046,6 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
10461046
)
10471047
}
10481048

1049-
graphStore, err := graphdb.NewKVStore(
1050-
databaseBackends.GraphDB, graphDBOptions...,
1051-
)
1052-
if err != nil {
1053-
return nil, nil, err
1054-
}
1055-
1056-
dbs.GraphDB, err = graphdb.NewChannelGraph(graphStore, chanGraphOpts...)
1057-
if err != nil {
1058-
cleanUp()
1059-
1060-
err = fmt.Errorf("unable to open graph DB: %w", err)
1061-
d.logger.Error(err)
1062-
1063-
return nil, nil, err
1064-
}
1065-
10661049
dbOptions := []channeldb.OptionModifier{
10671050
channeldb.OptionDryRunMigration(cfg.DryRunMigration),
10681051
channeldb.OptionKeepFailedPaymentAttempts(
@@ -1098,6 +1081,10 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
10981081
return nil, nil, err
10991082
}
11001083

1084+
// The graph store implementation we will use depends on whether
1085+
// native SQL is enabled or not.
1086+
var graphStore graphdb.V1Store
1087+
11011088
// Instantiate a native SQL store if the flag is set.
11021089
if d.cfg.DB.UseNativeSQL {
11031090
migrations := sqldb.GetMigrations()
@@ -1156,17 +1143,27 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
11561143
// the base DB and transaction executor for the native SQL
11571144
// invoice store.
11581145
baseDB := dbs.NativeSQLStore.GetBaseDB()
1159-
executor := sqldb.NewTransactionExecutor(
1146+
invoiceExecutor := sqldb.NewTransactionExecutor(
11601147
baseDB, func(tx *sql.Tx) invoices.SQLInvoiceQueries {
11611148
return baseDB.WithTx(tx)
11621149
},
11631150
)
11641151

11651152
sqlInvoiceDB := invoices.NewSQLStore(
1166-
executor, clock.NewDefaultClock(),
1153+
invoiceExecutor, clock.NewDefaultClock(),
11671154
)
11681155

11691156
dbs.InvoiceDB = sqlInvoiceDB
1157+
1158+
graphStore, err = d.getGraphStore(
1159+
baseDB, databaseBackends.GraphDB, graphDBOptions...,
1160+
)
1161+
if err != nil {
1162+
err = fmt.Errorf("unable to get graph store: %w", err)
1163+
d.logger.Error(err)
1164+
1165+
return nil, nil, err
1166+
}
11701167
} else {
11711168
// Check if the invoice bucket tombstone is set. If it is, we
11721169
// need to return and ask the user switch back to using the
@@ -1188,6 +1185,23 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
11881185
}
11891186

11901187
dbs.InvoiceDB = dbs.ChanStateDB
1188+
1189+
graphStore, err = graphdb.NewKVStore(
1190+
databaseBackends.GraphDB, graphDBOptions...,
1191+
)
1192+
if err != nil {
1193+
return nil, nil, err
1194+
}
1195+
}
1196+
1197+
dbs.GraphDB, err = graphdb.NewChannelGraph(graphStore, chanGraphOpts...)
1198+
if err != nil {
1199+
cleanUp()
1200+
1201+
err = fmt.Errorf("unable to open channel graph DB: %w", err)
1202+
d.logger.Error(err)
1203+
1204+
return nil, nil, err
11911205
}
11921206

11931207
// Wrap the watchtower client DB and make sure we clean up.

‎config_prod.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//go:build !test_native_sql
2+
3+
package lnd
4+
5+
import (
6+
graphdb "github.com/lightningnetwork/lnd/graph/db"
7+
"github.com/lightningnetwork/lnd/kvdb"
8+
"github.com/lightningnetwork/lnd/sqldb"
9+
)
10+
11+
// getGraphStore returns a graphdb.V1Store backed by a graphdb.KVStore
12+
// implementation.
13+
func (d *DefaultDatabaseBuilder) getGraphStore(_ *sqldb.BaseDB,
14+
kvBackend kvdb.Backend,
15+
opts ...graphdb.StoreOptionModifier) (graphdb.V1Store, error) {
16+
17+
return graphdb.NewKVStore(kvBackend, opts...)
18+
}

‎config_test_native_sql.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//go:build test_native_sql
2+
3+
package lnd
4+
5+
import (
6+
"database/sql"
7+
8+
graphdb "github.com/lightningnetwork/lnd/graph/db"
9+
"github.com/lightningnetwork/lnd/kvdb"
10+
"github.com/lightningnetwork/lnd/sqldb"
11+
)
12+
13+
// getGraphStore returns a graphdb.V1Store backed by a graphdb.SQLStore
14+
// implementation.
15+
func (d *DefaultDatabaseBuilder) getGraphStore(baseDB *sqldb.BaseDB,
16+
_ kvdb.Backend, opts ...graphdb.StoreOptionModifier) (graphdb.V1Store,
17+
error) {
18+
19+
graphExecutor := sqldb.NewTransactionExecutor(
20+
baseDB, func(tx *sql.Tx) graphdb.SQLQueries {
21+
return baseDB.WithTx(tx)
22+
},
23+
)
24+
25+
return graphdb.NewSQLStore(
26+
&graphdb.SQLStoreConfig{
27+
ChainHash: *d.cfg.ActiveNetParams.GenesisHash,
28+
},
29+
graphExecutor, opts...,
30+
)
31+
}

‎discovery/bootstrapper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func (c *ChannelGraphBootstrapper) SampleNodeAddrs(_ context.Context,
236236

237237
return errFound
238238
})
239-
if err != nil && err != errFound {
239+
if err != nil && !errors.Is(err, errFound) {
240240
return nil, err
241241
}
242242

‎docs/release-notes/release-notes-0.20.0.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ circuit. The indices are only available for forwarding events saved after v0.20.
9191
* [8](https://github.com/lightningnetwork/lnd/pull/9938)
9292
* [9](https://github.com/lightningnetwork/lnd/pull/9939)
9393
* [10](https://github.com/lightningnetwork/lnd/pull/9971)
94+
* [11](https://github.com/lightningnetwork/lnd/pull/9972)
9495

9596
## RPC Updates
9697

‎graph/builder_test.go

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -892,15 +892,27 @@ func TestPruneChannelGraphDoubleDisabled(t *testing.T) {
892892
}
893893

894894
func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
895+
timestamp := time.Now()
896+
897+
// nextTimeStamp is a helper closure that will return a new
898+
// timestamp each time it's called, this helps us create channel updates
899+
// with new timestamps so that we don't run into our SQL DB constraint
900+
// which only allows an update to a channel edge if the last update
901+
// timestamp is greater than the previous one.
902+
nextTimeStamp := func() time.Time {
903+
timestamp = timestamp.Add(time.Second)
904+
905+
return timestamp
906+
}
907+
895908
// We'll create the following test graph so that only the last channel
896909
// is pruned. We'll use a fresh timestamp to ensure they're not pruned
897910
// according to that heuristic.
898-
timestamp := time.Now()
899911
testChannels := []*testChannel{
900912
// Channel from self shouldn't be pruned.
901913
symmetricTestChannel(
902914
"self", "a", 100000, &testChannelPolicy{
903-
LastUpdate: timestamp,
915+
LastUpdate: nextTimeStamp(),
904916
Disabled: true,
905917
}, 99,
906918
),
@@ -918,7 +930,7 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
918930
Node1: &testChannelEnd{
919931
Alias: "a",
920932
testChannelPolicy: &testChannelPolicy{
921-
LastUpdate: timestamp,
933+
LastUpdate: nextTimeStamp(),
922934
Disabled: true,
923935
},
924936
},
@@ -932,7 +944,7 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
932944
Node1: &testChannelEnd{
933945
Alias: "a",
934946
testChannelPolicy: &testChannelPolicy{
935-
LastUpdate: timestamp,
947+
LastUpdate: nextTimeStamp(),
936948
Disabled: false,
937949
},
938950
},
@@ -946,14 +958,14 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
946958
Node1: &testChannelEnd{
947959
Alias: "a",
948960
testChannelPolicy: &testChannelPolicy{
949-
LastUpdate: timestamp,
961+
LastUpdate: nextTimeStamp(),
950962
Disabled: true,
951963
},
952964
},
953965
Node2: &testChannelEnd{
954966
Alias: "b",
955967
testChannelPolicy: &testChannelPolicy{
956-
LastUpdate: timestamp,
968+
LastUpdate: nextTimeStamp(),
957969
Disabled: false,
958970
},
959971
},
@@ -963,13 +975,13 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
963975

964976
// Both edges enabled.
965977
symmetricTestChannel("c", "d", 100000, &testChannelPolicy{
966-
LastUpdate: timestamp,
978+
LastUpdate: nextTimeStamp(),
967979
Disabled: false,
968980
}, 2),
969981

970982
// Both edges disabled, only one pruned.
971983
symmetricTestChannel("e", "f", 100000, &testChannelPolicy{
972-
LastUpdate: timestamp,
984+
LastUpdate: nextTimeStamp(),
973985
Disabled: true,
974986
}, 3),
975987
}
@@ -1363,7 +1375,9 @@ func parseTestGraph(t *testing.T, useCache bool, path string) (
13631375
testAddrs = append(testAddrs, testAddr)
13641376

13651377
// Next, create a temporary graph database for usage within the test.
1366-
graph := graphdb.MakeTestGraph(t, graphdb.WithUseGraphCache(useCache))
1378+
graph := graphdb.MakeTestGraph(
1379+
t, graphdb.WithUseGraphCache(useCache),
1380+
)
13671381

13681382
aliasMap := make(map[string]route.Vertex)
13691383
privKeyMap := make(map[string]*btcec.PrivateKey)
@@ -1441,6 +1455,13 @@ func parseTestGraph(t *testing.T, useCache bool, path string) (
14411455
}
14421456

14431457
source = dbNode
1458+
1459+
// Set the selected source node.
1460+
if err := graph.SetSourceNode(ctx, source); err != nil {
1461+
return nil, err
1462+
}
1463+
1464+
continue
14441465
}
14451466

14461467
// With the node fully parsed, add it as a vertex within the
@@ -1450,13 +1471,6 @@ func parseTestGraph(t *testing.T, useCache bool, path string) (
14501471
}
14511472
}
14521473

1453-
if source != nil {
1454-
// Set the selected source node
1455-
if err := graph.SetSourceNode(ctx, source); err != nil {
1456-
return nil, err
1457-
}
1458-
}
1459-
14601474
// With all the vertexes inserted, we can now insert the edges into the
14611475
// test graph.
14621476
for _, edge := range g.Edges {
@@ -1739,14 +1753,16 @@ func createTestGraphFromChannels(t *testing.T, useCache bool,
17391753
testAddrs = append(testAddrs, testAddr)
17401754

17411755
// Next, create a temporary graph database for usage within the test.
1742-
graph := graphdb.MakeTestGraph(t, graphdb.WithUseGraphCache(useCache))
1756+
graph := graphdb.MakeTestGraph(
1757+
t, graphdb.WithUseGraphCache(useCache),
1758+
)
17431759

17441760
aliasMap := make(map[string]route.Vertex)
17451761
privKeyMap := make(map[string]*btcec.PrivateKey)
17461762

17471763
nodeIndex := byte(0)
1748-
addNodeWithAlias := func(alias string, features *lnwire.FeatureVector) (
1749-
*models.LightningNode, error) {
1764+
addNodeWithAlias := func(alias string,
1765+
features *lnwire.FeatureVector) error {
17501766

17511767
keyBytes := []byte{
17521768
0, 0, 0, 0, 0, 0, 0, 0,
@@ -1776,26 +1792,26 @@ func createTestGraphFromChannels(t *testing.T, useCache bool,
17761792

17771793
// With the node fully parsed, add it as a vertex within the
17781794
// graph.
1779-
if err := graph.AddLightningNode(ctx, dbNode); err != nil {
1780-
return nil, err
1795+
if alias == source {
1796+
err = graph.SetSourceNode(ctx, dbNode)
1797+
require.NoError(t, err)
1798+
} else {
1799+
err := graph.AddLightningNode(ctx, dbNode)
1800+
require.NoError(t, err)
17811801
}
17821802

17831803
aliasMap[alias] = dbNode.PubKeyBytes
17841804
nodeIndex++
17851805

1786-
return dbNode, nil
1806+
return nil
17871807
}
17881808

17891809
// Add the source node.
1790-
dbNode, err := addNodeWithAlias(source, lnwire.EmptyFeatureVector())
1810+
err = addNodeWithAlias(source, lnwire.EmptyFeatureVector())
17911811
if err != nil {
17921812
return nil, err
17931813
}
17941814

1795-
if err = graph.SetSourceNode(ctx, dbNode); err != nil {
1796-
return nil, err
1797-
}
1798-
17991815
// Initialize variable that keeps track of the next channel id to assign
18001816
// if none is specified.
18011817
nextUnassignedChannelID := uint64(100000)
@@ -1813,7 +1829,7 @@ func createTestGraphFromChannels(t *testing.T, useCache bool,
18131829
features =
18141830
node.testChannelPolicy.Features
18151831
}
1816-
_, err := addNodeWithAlias(
1832+
err := addNodeWithAlias(
18171833
node.Alias, features,
18181834
)
18191835
if err != nil {

‎graph/db/errors.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ var (
6666
// ErrUnknownAddressType is returned when a node's addressType is not
6767
// an expected value.
6868
ErrUnknownAddressType = fmt.Errorf("address type cannot be resolved")
69+
70+
// ErrCantCheckIfZombieEdgeStr is an error returned when we
71+
// attempt to check if an edge is a zombie but encounter an error.
72+
ErrCantCheckIfZombieEdgeStr = fmt.Errorf("unable to check if edge " +
73+
"is a zombie")
6974
)
7075

7176
// ErrTooManyExtraOpaqueBytes creates an error which should be returned if the

‎graph/db/graph.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ func (c *ChannelGraph) UpdateEdgePolicy(ctx context.Context,
594594
return nil
595595
}
596596

597-
// MakeTestGraphNew creates a new instance of the ChannelGraph for testing
597+
// MakeTestGraph creates a new instance of the ChannelGraph for testing
598598
// purposes. The backing V1Store implementation depends on the version of
599599
// NewTestDB included in the current build.
600600
//
@@ -603,7 +603,7 @@ func (c *ChannelGraph) UpdateEdgePolicy(ctx context.Context,
603603
// implemented, unit tests will be switched to use this function instead of
604604
// the existing MakeTestGraph helper. Once only this function is used, the
605605
// existing MakeTestGraph function will be removed and this one will be renamed.
606-
func MakeTestGraphNew(t testing.TB,
606+
func MakeTestGraph(t testing.TB,
607607
opts ...ChanGraphOption) *ChannelGraph {
608608

609609
t.Helper()

0 commit comments

Comments
 (0)