diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index f9d846823..5aec3c2e4 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -909,10 +909,6 @@ func TestEdgeInfoUpdates(t *testing.T) { } // TestEdgePolicyCRUD tests basic CRUD operations for edge policies. -// -// NOTE: this currently demonstrates a bug in the SQL backend where -// Channel Flags and Message Flags are not properly stored. This will be fixed -// in an upcoming commit. func TestEdgePolicyCRUD(t *testing.T) { t.Parallel() ctx := context.Background() @@ -925,7 +921,7 @@ func TestEdgePolicyCRUD(t *testing.T) { // Create an edge. Don't add it to the DB yet. edgeInfo, edge1, edge2 := createChannelEdge(node1, node2) - updateAndAssertPolicies := func(expErr bool) { + updateAndAssertPolicies := func() { // Make copies of the policies before calling UpdateEdgePolicy // to avoid any data race's that can occur during async calls // that UpdateEdgePolicy may trigger. @@ -945,17 +941,6 @@ func TestEdgePolicyCRUD(t *testing.T) { policy1 *models.ChannelEdgePolicy, policy2 *models.ChannelEdgePolicy) error { - if expErr { - require.Error( - t, compareEdgePolicies(edge1, policy1), - ) - require.Error( - t, compareEdgePolicies(edge2, policy2), - ) - - return nil - } - require.NoError(t, compareEdgePolicies(edge1, policy1)) require.NoError(t, compareEdgePolicies(edge2, policy2)) @@ -971,12 +956,12 @@ func TestEdgePolicyCRUD(t *testing.T) { // Now add the edge. require.NoError(t, graph.AddChannelEdge(ctx, edgeInfo)) - updateAndAssertPolicies(false) + updateAndAssertPolicies() // Update one of the edges to have no extra opaque data. edge1.ExtraOpaqueData = nil - updateAndAssertPolicies(false) + updateAndAssertPolicies() // Update one of the edges to have ChannelFlags include a bit unknown // to us. @@ -986,12 +971,7 @@ func TestEdgePolicyCRUD(t *testing.T) { // us. edge2.MessageFlags |= 1 << 4 - // NOTE: If the backend is SQL, then we expect an error here as - // there is currently a bug in the SQL backend where - // ChannelFlags and MessageFlags are not properly stored. This will - // be fixed in an upcoming commit. - _, isSQLImp := graph.V1Store.(*SQLStore) - updateAndAssertPolicies(isSQLImp) + updateAndAssertPolicies() } func assertNodeInCache(t *testing.T, g *ChannelGraph, n *models.LightningNode, diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index 5ff8b7a1d..8f3a48b85 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -1271,7 +1271,7 @@ func (s *SQLStore) ForEachChannelCacheable(cb func(*models.CachedEdgeInfo, var pol1, pol2 *models.CachedEdgePolicy if dbPol1 != nil { policy1, err := buildChanPolicy( - *dbPol1, edge.ChannelID, nil, node2, true, + *dbPol1, edge.ChannelID, nil, node2, ) if err != nil { return err @@ -1281,7 +1281,7 @@ func (s *SQLStore) ForEachChannelCacheable(cb func(*models.CachedEdgeInfo, } if dbPol2 != nil { policy2, err := buildChanPolicy( - *dbPol2, edge.ChannelID, nil, node1, false, + *dbPol2, edge.ChannelID, nil, node1, ) if err != nil { return err @@ -2900,7 +2900,7 @@ func forEachNodeDirectedChannel(ctx context.Context, db SQLQueries, var p1, p2 *models.CachedEdgePolicy if dbPol1 != nil { policy1, err := buildChanPolicy( - *dbPol1, edge.ChannelID, nil, node2, true, + *dbPol1, edge.ChannelID, nil, node2, ) if err != nil { return err @@ -2910,7 +2910,7 @@ func forEachNodeDirectedChannel(ctx context.Context, db SQLQueries, } if dbPol2 != nil { policy2, err := buildChanPolicy( - *dbPol2, edge.ChannelID, nil, node1, false, + *dbPol2, edge.ChannelID, nil, node1, ) if err != nil { return err @@ -3138,6 +3138,8 @@ func updateChanEdgePolicy(ctx context.Context, tx SQLQueries, Valid: edge.MessageFlags.HasMaxHtlc(), Int64: int64(edge.MaxHTLC), }, + MessageFlags: sqldb.SQLInt16(edge.MessageFlags), + ChannelFlags: sqldb.SQLInt16(edge.ChannelFlags), InboundBaseFeeMsat: inboundBase, InboundFeeRateMilliMsat: inboundRate, Signature: edge.SigBytes, @@ -4135,7 +4137,7 @@ func getAndBuildChanPolicies(ctx context.Context, db SQLQueries, var pol1, pol2 *models.ChannelEdgePolicy if dbPol1 != nil { pol1, err = buildChanPolicy( - *dbPol1, channelID, dbPol1Extras, node2, true, + *dbPol1, channelID, dbPol1Extras, node2, ) if err != nil { return nil, nil, err @@ -4143,7 +4145,7 @@ func getAndBuildChanPolicies(ctx context.Context, db SQLQueries, } if dbPol2 != nil { pol2, err = buildChanPolicy( - *dbPol2, channelID, dbPol2Extras, node1, false, + *dbPol2, channelID, dbPol2Extras, node1, ) if err != nil { return nil, nil, err @@ -4156,8 +4158,8 @@ func getAndBuildChanPolicies(ctx context.Context, db SQLQueries, // buildChanPolicy builds a models.ChannelEdgePolicy instance from the // provided sqlc.ChannelPolicy and other required information. func buildChanPolicy(dbPolicy sqlc.ChannelPolicy, channelID uint64, - extras map[uint64][]byte, toNode route.Vertex, - isNode1 bool) (*models.ChannelEdgePolicy, error) { + extras map[uint64][]byte, + toNode route.Vertex) (*models.ChannelEdgePolicy, error) { recs, err := lnwire.CustomRecords(extras).Serialize() if err != nil { @@ -4165,19 +4167,6 @@ func buildChanPolicy(dbPolicy sqlc.ChannelPolicy, channelID uint64, "fields: %w", err) } - var msgFlags lnwire.ChanUpdateMsgFlags - if dbPolicy.MaxHtlcMsat.Valid { - msgFlags |= lnwire.ChanUpdateRequiredMaxHtlc - } - - var chanFlags lnwire.ChanUpdateChanFlags - if !isNode1 { - chanFlags |= lnwire.ChanUpdateDirection - } - if dbPolicy.Disabled.Bool { - chanFlags |= lnwire.ChanUpdateDisabled - } - var inboundFee fn.Option[lnwire.Fee] if dbPolicy.InboundFeeRateMilliMsat.Valid || dbPolicy.InboundBaseFeeMsat.Valid { @@ -4194,8 +4183,12 @@ func buildChanPolicy(dbPolicy sqlc.ChannelPolicy, channelID uint64, LastUpdate: time.Unix( dbPolicy.LastUpdate.Int64, 0, ), - MessageFlags: msgFlags, - ChannelFlags: chanFlags, + MessageFlags: sqldb.ExtractSqlInt16[lnwire.ChanUpdateMsgFlags]( + dbPolicy.MessageFlags, + ), + ChannelFlags: sqldb.ExtractSqlInt16[lnwire.ChanUpdateChanFlags]( + dbPolicy.ChannelFlags, + ), TimeLockDelta: uint16(dbPolicy.Timelock), MinHTLC: lnwire.MilliSatoshi( dbPolicy.MinHtlcMsat, @@ -4259,6 +4252,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy1InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy1InboundFeeRateMilliMsat, Disabled: r.Policy1Disabled, + MessageFlags: r.Policy1MessageFlags, + ChannelFlags: r.Policy1ChannelFlags, Signature: r.Policy1Signature, } } @@ -4277,6 +4272,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy2InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy2InboundFeeRateMilliMsat, Disabled: r.Policy2Disabled, + MessageFlags: r.Policy2MessageFlags, + ChannelFlags: r.Policy2ChannelFlags, Signature: r.Policy2Signature, } } @@ -4299,6 +4296,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy1InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy1InboundFeeRateMilliMsat, Disabled: r.Policy1Disabled, + MessageFlags: r.Policy1MessageFlags, + ChannelFlags: r.Policy1ChannelFlags, Signature: r.Policy1Signature, } } @@ -4317,6 +4316,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy2InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy2InboundFeeRateMilliMsat, Disabled: r.Policy2Disabled, + MessageFlags: r.Policy2MessageFlags, + ChannelFlags: r.Policy2ChannelFlags, Signature: r.Policy2Signature, } } @@ -4339,6 +4340,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy1InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy1InboundFeeRateMilliMsat, Disabled: r.Policy1Disabled, + MessageFlags: r.Policy1MessageFlags, + ChannelFlags: r.Policy1ChannelFlags, Signature: r.Policy1Signature, } } @@ -4357,6 +4360,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy2InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy2InboundFeeRateMilliMsat, Disabled: r.Policy2Disabled, + MessageFlags: r.Policy2MessageFlags, + ChannelFlags: r.Policy2ChannelFlags, Signature: r.Policy2Signature, } } @@ -4379,6 +4384,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy1InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy1InboundFeeRateMilliMsat, Disabled: r.Policy1Disabled, + MessageFlags: r.Policy1MessageFlags, + ChannelFlags: r.Policy1ChannelFlags, Signature: r.Policy1Signature, } } @@ -4397,6 +4404,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy2InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy2InboundFeeRateMilliMsat, Disabled: r.Policy2Disabled, + MessageFlags: r.Policy2MessageFlags, + ChannelFlags: r.Policy2ChannelFlags, Signature: r.Policy2Signature, } } @@ -4419,6 +4428,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy1InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy1InboundFeeRateMilliMsat, Disabled: r.Policy1Disabled, + MessageFlags: r.Policy1MessageFlags, + ChannelFlags: r.Policy1ChannelFlags, Signature: r.Policy1Signature, } } @@ -4437,6 +4448,8 @@ func extractChannelPolicies(row any) (*sqlc.ChannelPolicy, *sqlc.ChannelPolicy, InboundBaseFeeMsat: r.Policy2InboundBaseFeeMsat, InboundFeeRateMilliMsat: r.Policy2InboundFeeRateMilliMsat, Disabled: r.Policy2Disabled, + MessageFlags: r.Policy2MessageFlags, + ChannelFlags: r.Policy2ChannelFlags, Signature: r.Policy2Signature, } } diff --git a/sqldb/sqlutils.go b/sqldb/sqlutils.go index bafba8416..8745fe2f7 100644 --- a/sqldb/sqlutils.go +++ b/sqldb/sqlutils.go @@ -68,3 +68,10 @@ func SQLTime(t time.Time) sql.NullTime { Valid: true, } } + +// ExtractSqlInt16 turns a NullInt16 into a numerical type. This can be useful +// when reading directly from the database, as this function handles extracting +// the inner value from the "option"-like struct. +func ExtractSqlInt16[T constraints.Integer](num sql.NullInt16) T { + return T(num.Int16) +}