From 239acb406395cb6af9196ad66a3e243396241fbe Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 08:25:58 +0200 Subject: [PATCH 01/11] graph/db: validate edge policy TLV data before persisting In this commit, we start validating the extra opaque data of a channel edge policy before persisting it. We just check that the data is valid TLV. NOTE: we recently [started validating](https://github.com/lightningnetwork/lnd/commit/1410a0949d555e14de11faf81a17fe64917505e3) this at the lnwire level. So really, no new update will reach the DB layer without this already being checked. But we check it again here so that the DB API behaves correctly as its own unit. --- graph/db/errors.go | 4 ++++ graph/db/graph_test.go | 18 ++++++++++++------ graph/db/kv_store.go | 6 ++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/graph/db/errors.go b/graph/db/errors.go index afdbdb457..8201980aa 100644 --- a/graph/db/errors.go +++ b/graph/db/errors.go @@ -12,6 +12,10 @@ var ( ErrEdgePolicyOptionalFieldNotFound = fmt.Errorf("optional field not " + "present") + // ErrParsingExtraTLVBytes is returned when we attempt to parse + // extra opaque bytes as a TLV stream, but the parsing fails. + ErrParsingExtraTLVBytes = fmt.Errorf("error parsing extra TLV bytes") + // ErrGraphNotFound is returned when at least one of the components of // graph doesn't exist. ErrGraphNotFound = fmt.Errorf("graph bucket not initialized") diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index c8a5c54b2..efe1b4642 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -4195,19 +4195,25 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) { directedChan := getSingleChannel() require.NotNil(t, directedChan) - require.Equal(t, directedChan.InboundFee, lnwire.Fee{ + expectedInbound := lnwire.Fee{ BaseFee: 10, FeeRate: 20, - }) + } + require.Equal(t, expectedInbound, directedChan.InboundFee) - // Set an invalid inbound fee and check that the edge is no longer - // returned. + // Set an invalid inbound fee and check that persistence fails. edge1.ExtraOpaqueData = []byte{ 253, 217, 3, 8, 0, } - require.NoError(t, graph.UpdateEdgePolicy(edge1)) + require.ErrorIs( + t, graph.UpdateEdgePolicy(edge1), ErrParsingExtraTLVBytes, + ) - require.Nil(t, getSingleChannel()) + // Since persistence of the last update failed, we should still bet + // the previous result when we query the channel again. + directedChan = getSingleChannel() + require.NotNil(t, directedChan) + require.Equal(t, expectedInbound, directedChan.InboundFee) } // TestGraphLoading asserts that the cache is properly reconstructed after a diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index b08c86022..f68732e81 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -4546,6 +4546,12 @@ func serializeChanEdgePolicy(w io.Writer, edge *models.ChannelEdgePolicy, } } + // Validate that the ExtraOpaqueData is in fact a valid TLV stream. + err = edge.ExtraOpaqueData.ValidateTLV() + if err != nil { + return fmt.Errorf("%w: %w", ErrParsingExtraTLVBytes, err) + } + if len(edge.ExtraOpaqueData) > MaxAllowedExtraOpaqueBytes { return ErrTooManyExtraOpaqueBytes(len(edge.ExtraOpaqueData)) } From 8314bacddab22886cabe851ad24fb0d5f5522eed Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 08:45:25 +0200 Subject: [PATCH 02/11] graph/db: extract InboundFee at deserialisation time Here we add an explicit InboundFee field to the ChannelEdgePolicy struct. Then, in the graph KVStore, at deserialisation time, we extract the InboundFee from the ExtraOpaqueData. Currently we do this at higher levels but we are going to move it to the DB layer so that when we add the SQL implementation of the graph store, we can have explicit columns for inbound fees. We need to account for the fact that we may have invalid TLV already persisted though and we dont want to fail if we deserialise those necessarily. So we return ErrParsingExtraTLVBytes now if we fail to parse the extra bytes as TLV and then we let the callers handle it similarly to how ErrParsingExtraTLVBytes is handled in that we dont necessarily fail if we receive one of these errors. As of this commit, we can now expect the InboundFee field of a ChannelEdgePolicy to be set (if inbound fees are set on the policy) for any update that we read from disk. --- graph/db/kv_store.go | 59 +++++++++++++++++++++----- graph/db/models/channel_edge_policy.go | 10 +++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index f68732e81..290c2f54c 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -22,6 +22,7 @@ import ( "github.com/btcsuite/btcwallet/walletdb" "github.com/lightningnetwork/lnd/aliasmgr" "github.com/lightningnetwork/lnd/batch" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/kvdb" @@ -284,6 +285,13 @@ func (c *KVStore) getChannelMap(edges kvdb.RBucket) ( case errors.Is(err, ErrEdgePolicyOptionalFieldNotFound): return nil + // We don't want a single policy with bad TLV data to stop us + // from loading the rest of the data, so we just skip this + // policy. This is for backwards compatibility since we did not + // use to validate TLV data in the past before persisting it. + case errors.Is(err, ErrParsingExtraTLVBytes): + return nil + case err != nil: return err } @@ -2342,7 +2350,7 @@ func (c *KVStore) FilterChannelRange(startHeight, edge, err := deserializeChanEdgePolicyRaw(r) if err != nil && !errors.Is( err, ErrEdgePolicyOptionalFieldNotFound, - ) { + ) && !errors.Is(err, ErrParsingExtraTLVBytes) { return err } @@ -2357,7 +2365,7 @@ func (c *KVStore) FilterChannelRange(startHeight, edge, err := deserializeChanEdgePolicyRaw(r) if err != nil && !errors.Is( err, ErrEdgePolicyOptionalFieldNotFound, - ) { + ) && !errors.Is(err, ErrParsingExtraTLVBytes) { return err } @@ -4334,15 +4342,20 @@ func putChanEdgePolicy(edges kvdb.RwBucket, edge *models.ChannelEdgePolicy, // need to deserialize the existing policy within the database // (now outdated by the new one), and delete its corresponding // entry within the update index. We'll ignore any - // ErrEdgePolicyOptionalFieldNotFound error, as we only need - // the channel ID and update time to delete the entry. + // ErrEdgePolicyOptionalFieldNotFound or ErrParsingExtraTLVBytes + // errors, as we only need the channel ID and update time to + // delete the entry. + // // TODO(halseth): get rid of these invalid policies in a // migration. + // TODO(elle): complete the above TODO in migration from kvdb + // to SQL. oldEdgePolicy, err := deserializeChanEdgePolicy( bytes.NewReader(edgeBytes), ) if err != nil && - !errors.Is(err, ErrEdgePolicyOptionalFieldNotFound) { + !errors.Is(err, ErrEdgePolicyOptionalFieldNotFound) && + !errors.Is(err, ErrParsingExtraTLVBytes) { return err } @@ -4449,6 +4462,11 @@ func fetchChanEdgePolicy(edges kvdb.RBucket, chanID []byte, case errors.Is(err, ErrEdgePolicyOptionalFieldNotFound): return nil, nil + // If the policy contains invalid TLV bytes, we return nil as if + // the policy was unknown. + case errors.Is(err, ErrParsingExtraTLVBytes): + return nil, nil + case err != nil: return nil, err } @@ -4568,15 +4586,18 @@ func serializeChanEdgePolicy(w io.Writer, edge *models.ChannelEdgePolicy, func deserializeChanEdgePolicy(r io.Reader) (*models.ChannelEdgePolicy, error) { // Deserialize the policy. Note that in case an optional field is not - // found, both an error and a populated policy object are returned. - edge, deserializeErr := deserializeChanEdgePolicyRaw(r) - if deserializeErr != nil && - !errors.Is(deserializeErr, ErrEdgePolicyOptionalFieldNotFound) { + // found or if the edge has invalid TLV data, then both an error and a + // populated policy object are returned so that the caller can decide + // if it still wants to use the edge or not. + edge, err := deserializeChanEdgePolicyRaw(r) + if err != nil && + !errors.Is(err, ErrEdgePolicyOptionalFieldNotFound) && + !errors.Is(err, ErrParsingExtraTLVBytes) { - return nil, deserializeErr + return nil, err } - return edge, deserializeErr + return edge, err } func deserializeChanEdgePolicyRaw(r io.Reader) (*models.ChannelEdgePolicy, @@ -4663,6 +4684,22 @@ func deserializeChanEdgePolicyRaw(r io.Reader) (*models.ChannelEdgePolicy, edge.ExtraOpaqueData = opq[8:] } + // Attempt to extract the inbound fee from the opaque data. If we fail + // to parse the TLV here, we return an error we also return the edge + // so that the caller can still use it. This is for backwards + // compatibility in case we have already persisted some policies that + // have invalid TLV data. + var inboundFee lnwire.Fee + typeMap, err := edge.ExtraOpaqueData.ExtractRecords(&inboundFee) + if err != nil { + return edge, fmt.Errorf("%w: %w", ErrParsingExtraTLVBytes, err) + } + + val, ok := typeMap[lnwire.FeeRecordType] + if ok && val == nil { + edge.InboundFee = fn.Some(inboundFee) + } + return edge, nil } diff --git a/graph/db/models/channel_edge_policy.go b/graph/db/models/channel_edge_policy.go index 365acbfe1..48d748ee0 100644 --- a/graph/db/models/channel_edge_policy.go +++ b/graph/db/models/channel_edge_policy.go @@ -5,6 +5,7 @@ import ( "time" "github.com/btcsuite/btcd/btcec/v2/ecdsa" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/lnwire" ) @@ -65,6 +66,15 @@ type ChannelEdgePolicy struct { // to. Using this pub key, the channel graph can further be traversed. ToNode [33]byte + // InboundFee is the fee that must be paid for incoming HTLCs. + // + // NOTE: for our kvdb implementation of the graph store, inbound fees + // are still only persisted as part of extra opaque data and so this + // field is not explicitly stored but is rather populated from the + // ExtraOpaqueData field on deserialization. For our SQL implementation, + // this field will be explicitly persisted in the database. + InboundFee fn.Option[lnwire.Fee] + // ExtraOpaqueData is the set of data that was appended to this // message, some of which we may not actually know how to iterate or // parse. By holding onto this data, we ensure that we're able to From e170ad44d9495d30eaaf32190b9130053df77ac4 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:33:58 +0200 Subject: [PATCH 03/11] graph/db+rpcserver: remove inbound fee extraction For any call-site where we extract inbound fees from a models.ChannelEdgePolicy object that was deserialised from disk, we can now just use the new InboundFee field on the object since we know that it would have been populated at deserialisation time. Note that for all these call-sites, if a failure previously happened on decoding of the TLV stream, the error would be ignored and the edge would just be skipped. This behaviour is now still the same given how ErrParsingExtraTLVBytes is handled on the DB layer. --- graph/db/kv_store.go | 17 ++++++----------- rpcserver.go | 11 +++-------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index 290c2f54c..51a1d8483 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -482,16 +482,6 @@ func (c *KVStore) forEachNodeDirectedChannel(tx kvdb.RTx, cachedInPolicy.ToNodeFeatures = toNodeFeatures } - var inboundFee lnwire.Fee - if p1 != nil { - // Extract inbound fee. If there is a decoding error, - // skip this edge. - _, err := p1.ExtraOpaqueData.ExtractRecords(&inboundFee) - if err != nil { - return nil - } - } - directedChannel := &DirectedChannel{ ChannelID: e.ChannelID, IsNode1: node == e.NodeKey1Bytes, @@ -499,7 +489,12 @@ func (c *KVStore) forEachNodeDirectedChannel(tx kvdb.RTx, Capacity: e.Capacity, OutPolicySet: p1 != nil, InPolicy: cachedInPolicy, - InboundFee: inboundFee, + } + + if p1 != nil { + p1.InboundFee.WhenSome(func(fee lnwire.Fee) { + directedChannel.InboundFee = fee + }) } if node == e.NodeKey2Bytes { diff --git a/rpcserver.go b/rpcserver.go index 79d91e646..f9f8a5a9c 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -6842,7 +6842,7 @@ func marshalDBRoutingPolicy( disabled := policy.ChannelFlags&lnwire.ChanUpdateDisabled != 0 customRecords := marshalExtraOpaqueData(policy.ExtraOpaqueData) - inboundFee := extractInboundFeeSafe(policy.ExtraOpaqueData) + inboundFee := policy.InboundFee.UnwrapOr(lnwire.Fee{}) return &lnrpc.RoutingPolicy{ TimeLockDelta: uint32(policy.TimeLockDelta), @@ -7698,14 +7698,9 @@ func (r *rpcServer) FeeReport(ctx context.Context, edgePolicy.FeeProportionalMillionths feeRate := float64(feeRateFixedPoint) / feeBase - // Decode inbound fee from extra data. - var inboundFee lnwire.Fee - _, err := edgePolicy.ExtraOpaqueData.ExtractRecords( - &inboundFee, + inboundFee := edgePolicy.InboundFee.UnwrapOr( + lnwire.Fee{}, ) - if err != nil { - return err - } // TODO(roasbeef): also add stats for revenue for each // channel From 1d162216e65873c024b7b2e48417ad9fb4e48106 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:37:09 +0200 Subject: [PATCH 04/11] peer: stop extracting InboundFees Like the previous commit, here we can start directly using the InboundFee on the models.ChannelEdgePolicy object since we know we read it from disk and so the InboundFee field will be populated accordingly. NOTE: unlike the previous commit, behaviour is slightly different here since previously we would error out here if TLV parsing failed whereas now, the DB call will just skip the error and return a nil policy. This should be ok since this is explicitly only dealing with our own updates and so our TLV should always be valid. --- peer/brontide.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/peer/brontide.go b/peer/brontide.go index 26d02d3e7..5d00e14ab 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -1205,27 +1205,17 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( // routing policy into a forwarding policy. var forwardingPolicy *models.ForwardingPolicy if selfPolicy != nil { - var inboundWireFee lnwire.Fee - _, err := selfPolicy.ExtraOpaqueData.ExtractRecords( - &inboundWireFee, - ) - if err != nil { - return nil, err - } - - inboundFee := models.NewInboundFeeFromWire( - inboundWireFee, - ) - forwardingPolicy = &models.ForwardingPolicy{ MinHTLCOut: selfPolicy.MinHTLC, MaxHTLC: selfPolicy.MaxHTLC, BaseFee: selfPolicy.FeeBaseMSat, FeeRate: selfPolicy.FeeProportionalMillionths, TimeLockDelta: uint32(selfPolicy.TimeLockDelta), - - InboundFee: inboundFee, } + selfPolicy.InboundFee.WhenSome(func(fee lnwire.Fee) { + inboundFee := models.NewInboundFeeFromWire(fee) + forwardingPolicy.InboundFee = inboundFee + }) } else { p.log.Warnf("Unable to find our forwarding policy "+ "for channel %v, using default values", From 9890d74622f4b62821c61dfc2aa8da033f1461a5 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:48:59 +0200 Subject: [PATCH 05/11] multi: set the InboundFee on ChannelEdgePolicy In this commit, we make sure to set the new field wherever appropriate. This will be any place where the ChannelEdgePolicy is constructed other than its disk deserialisation. --- discovery/gossiper.go | 20 ++++++++++++++++++++ graph/builder.go | 23 +++++++++++++++++++++-- graph/notifications_test.go | 1 + routing/localchans/manager.go | 10 +++++----- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 0b59c8af9..d79fe7a74 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -3260,6 +3260,26 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context, ExtraOpaqueData: upd.ExtraOpaqueData, } + // Extract the inbound fee from the ExtraOpaqueData, if present. + // + // TODO(elle): this can be removed once we define the optional TLV + // field on the lnwire.ChannelUpdate itself. + var inboundFee lnwire.Fee + typeMap, err := upd.ExtraOpaqueData.ExtractRecords(&inboundFee) + if err != nil { + rErr := fmt.Errorf("%w: %w", graphdb.ErrParsingExtraTLVBytes, + err) + + log.Error(rErr) + nMsg.err <- rErr + return nil, false + } + + val, ok := typeMap[lnwire.FeeRecordType] + if ok && val == nil { + update.InboundFee = fn.Some(inboundFee) + } + if err := d.cfg.Graph.UpdateEdge(update, ops...); err != nil { if graph.IsError( err, graph.ErrOutdated, diff --git a/graph/builder.go b/graph/builder.go index 3350eeb33..32aac9434 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -11,6 +11,7 @@ import ( "github.com/go-errors/errors" "github.com/lightningnetwork/lnd/batch" "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/fn/v2" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnutils" @@ -943,7 +944,7 @@ func (b *Builder) ApplyChannelUpdate(msg *lnwire.ChannelUpdate1) bool { return false } - err = b.UpdateEdge(&models.ChannelEdgePolicy{ + update := &models.ChannelEdgePolicy{ SigBytes: msg.Signature.ToSignatureBytes(), ChannelID: msg.ShortChannelID.ToUint64(), LastUpdate: time.Unix(int64(msg.Timestamp), 0), @@ -955,7 +956,25 @@ func (b *Builder) ApplyChannelUpdate(msg *lnwire.ChannelUpdate1) bool { FeeBaseMSat: lnwire.MilliSatoshi(msg.BaseFee), FeeProportionalMillionths: lnwire.MilliSatoshi(msg.FeeRate), ExtraOpaqueData: msg.ExtraOpaqueData, - }) + } + + // Extract the inbound fee from the ExtraOpaqueData, if present. + // + // TODO(elle): this can be removed once we define the optional TLV + // field on the lnwire.ChannelUpdate itself. + var inboundFee lnwire.Fee + typeMap, err := update.ExtraOpaqueData.ExtractRecords(&inboundFee) + if err != nil { + log.Errorf("%v: %v", graphdb.ErrParsingExtraTLVBytes, err) + return false + } + + val, ok := typeMap[lnwire.FeeRecordType] + if ok && val == nil { + update.InboundFee = fn.Some(inboundFee) + } + + err = b.UpdateEdge(update) if err != nil && !IsError(err, ErrIgnored, ErrOutdated) { log.Errorf("Unable to apply channel update: %v", err) return false diff --git a/graph/notifications_test.go b/graph/notifications_test.go index 71c6f4a34..2f2ec1b4b 100644 --- a/graph/notifications_test.go +++ b/graph/notifications_test.go @@ -121,6 +121,7 @@ func randEdgePolicy(chanID *lnwire.ShortChannelID, FeeBaseMSat: lnwire.MilliSatoshi(prand.Int31()), FeeProportionalMillionths: lnwire.MilliSatoshi(prand.Int31()), ToNode: node.PubKeyBytes, + InboundFee: fn.Some(inboundFee), ExtraOpaqueData: extraOpaqueData, }, nil } diff --git a/routing/localchans/manager.go b/routing/localchans/manager.go index 3beb28525..492f8f18c 100644 --- a/routing/localchans/manager.go +++ b/routing/localchans/manager.go @@ -127,12 +127,10 @@ func (r *Manager) UpdatePolicy(newSchema routing.ChannelPolicy, Edge: edge, }) - // Extract inbound fees from the ExtraOpaqueData. var inboundWireFee lnwire.Fee - _, err = edge.ExtraOpaqueData.ExtractRecords(&inboundWireFee) - if err != nil { - return err - } + edge.InboundFee.WhenSome(func(fee lnwire.Fee) { + inboundWireFee = fee + }) inboundFee := models.NewInboundFeeFromWire(inboundWireFee) // Add updated policy to list of policies to send to switch. @@ -372,6 +370,8 @@ func (r *Manager) updateEdge(chanPoint wire.OutPoint, err = fn.MapOptionZ(newSchema.InboundFee, func(f models.InboundFee) error { inboundWireFee := f.ToWire() + edge.InboundFee = fn.Some(inboundWireFee) + return edge.ExtraOpaqueData.PackRecords( &inboundWireFee, ) From cb16c7177ab2ddbf9d8512bc1ae2a08c54108eab Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:50:31 +0200 Subject: [PATCH 06/11] graph/db: use InboundFee directly from ChannelEdgePolicy Now that we know that the InboundFee on the ChannelEdgePolicy is always set appropriately, we can update the GraphCache UpdatePolicy method to take the InboundFee directly from the ChannelEdgePolicy object. --- graph/db/graph_cache.go | 19 ++++++------------- graph/db/graph_cache_test.go | 7 +++++++ routing/pathfind_test.go | 20 ++++++++++++++++---- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/graph/db/graph_cache.go b/graph/db/graph_cache.go index cefa88a85..141f610b2 100644 --- a/graph/db/graph_cache.go +++ b/graph/db/graph_cache.go @@ -180,17 +180,6 @@ func (c *GraphCache) updateOrAddEdge(node route.Vertex, edge *DirectedChannel) { func (c *GraphCache) UpdatePolicy(policy *models.ChannelEdgePolicy, fromNode, toNode route.Vertex, edge1 bool) { - // Extract inbound fee if possible and available. If there is a decoding - // error, ignore this policy. - var inboundFee lnwire.Fee - _, err := policy.ExtraOpaqueData.ExtractRecords(&inboundFee) - if err != nil { - log.Errorf("Failed to extract records from edge policy %v: %v", - policy.ChannelID, err) - - return - } - c.mtx.Lock() defer c.mtx.Unlock() @@ -216,13 +205,17 @@ func (c *GraphCache) UpdatePolicy(policy *models.ChannelEdgePolicy, fromNode, // policy for node 1. case channel.IsNode1 && edge1: channel.OutPolicySet = true - channel.InboundFee = inboundFee + policy.InboundFee.WhenSome(func(fee lnwire.Fee) { + channel.InboundFee = fee + }) // This is node 2, and it is edge 2, so this is the outgoing // policy for node 2. case !channel.IsNode1 && !edge1: channel.OutPolicySet = true - channel.InboundFee = inboundFee + policy.InboundFee.WhenSome(func(fee lnwire.Fee) { + channel.InboundFee = fee + }) // The other two cases left mean it's the inbound policy for the // node. diff --git a/graph/db/graph_cache_test.go b/graph/db/graph_cache_test.go index 087fb81ac..96318366d 100644 --- a/graph/db/graph_cache_test.go +++ b/graph/db/graph_cache_test.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "testing" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" @@ -37,11 +38,17 @@ func TestGraphCacheAddNode(t *testing.T) { channelFlagA, channelFlagB = 1, 0 } + inboundFee := lnwire.Fee{ + BaseFee: 10, + FeeRate: 20, + } + outPolicy1 := &models.ChannelEdgePolicy{ ChannelID: 1000, ChannelFlags: lnwire.ChanUpdateChanFlags(channelFlagA), ToNode: nodeB, // Define an inbound fee. + InboundFee: fn.Some(inboundFee), ExtraOpaqueData: []byte{ 253, 217, 3, 8, 0, 0, 0, 10, 0, 0, 0, 20, }, diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index bc3a9ef58..361fb83d4 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -666,15 +666,25 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, return nil, err } - getExtraData := func( - end *testChannelEnd) lnwire.ExtraOpaqueData { + getInboundFees := func( + end *testChannelEnd) fn.Option[lnwire.Fee] { - var extraData lnwire.ExtraOpaqueData inboundFee := lnwire.Fee{ BaseFee: int32(end.InboundFeeBaseMsat), FeeRate: int32(end.InboundFeeRate), } - require.NoError(t, extraData.PackRecords(&inboundFee)) + + return fn.Some(inboundFee) + } + getExtraData := func( + end *testChannelEnd) lnwire.ExtraOpaqueData { + + var extraData lnwire.ExtraOpaqueData + + inboundFee := getInboundFees(end) + inboundFee.WhenSome(func(fee lnwire.Fee) { + require.NoError(t, extraData.PackRecords(&fee)) + }) return extraData } @@ -701,6 +711,7 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, FeeBaseMSat: node1.FeeBaseMsat, FeeProportionalMillionths: node1.FeeRate, ToNode: node2Vertex, + InboundFee: getInboundFees(node1), //nolint:ll ExtraOpaqueData: getExtraData(node1), } if err := graph.UpdateEdgePolicy(edgePolicy); err != nil { @@ -731,6 +742,7 @@ func createTestGraphFromChannels(t *testing.T, useCache bool, FeeBaseMSat: node2.FeeBaseMsat, FeeProportionalMillionths: node2.FeeRate, ToNode: node1Vertex, + InboundFee: getInboundFees(node2), //nolint:ll ExtraOpaqueData: getExtraData(node2), } if err := graph.UpdateEdgePolicy(edgePolicy); err != nil { From bf72d9ec191837a06af067d740f362845a1f74e0 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:54:26 +0200 Subject: [PATCH 07/11] graph+rpcserver: add InboundFee to ChannelEdgeUpdate And then use that new field instead of parsing from extra opaque data. --- graph/db/notifications.go | 5 +++++ rpcserver.go | 21 +-------------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/graph/db/notifications.go b/graph/db/notifications.go index d573e0aa5..6c0acba88 100644 --- a/graph/db/notifications.go +++ b/graph/db/notifications.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/wire" "github.com/go-errors/errors" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnutils" "github.com/lightningnetwork/lnd/lnwire" @@ -360,6 +361,9 @@ type ChannelEdgeUpdate struct { // payments. Disabled bool + // InboundFee is the fee that must be paid for incoming HTLCs. + InboundFee fn.Option[lnwire.Fee] + // ExtraOpaqueData is the set of data that was appended to this message // to fill out the full maximum transport message size. These fields can // be used to specify optional data such as custom TLV fields. @@ -442,6 +446,7 @@ func (c *ChannelGraph) addToTopologyChange(update *TopologyChange, AdvertisingNode: aNode, ConnectingNode: cNode, Disabled: m.ChannelFlags&lnwire.ChanUpdateDisabled != 0, + InboundFee: m.InboundFee, ExtraOpaqueData: m.ExtraOpaqueData, } diff --git a/rpcserver.go b/rpcserver.go index f9f8a5a9c..758400b1f 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -6776,23 +6776,6 @@ func marshalExtraOpaqueData(data []byte) map[uint64][]byte { return records } -// extractInboundFeeSafe tries to extract the inbound fee from the given extra -// opaque data tlv block. If parsing fails, a zero inbound fee is returned. This -// function is typically used on unvalidated data coming stored in the database. -// There is not much we can do other than ignoring errors here. -func extractInboundFeeSafe(data lnwire.ExtraOpaqueData) lnwire.Fee { - var inboundFee lnwire.Fee - - _, err := data.ExtractRecords(&inboundFee) - if err != nil { - // Return zero fee. Do not return the inboundFee variable - // because it may be undefined. - return lnwire.Fee{} - } - - return inboundFee -} - func marshalDBEdge(edgeInfo *models.ChannelEdgeInfo, c1, c2 *models.ChannelEdgePolicy) *lnrpc.ChannelEdge { @@ -7349,9 +7332,7 @@ func marshallTopologyChange( customRecords := marshalExtraOpaqueData( channelUpdate.ExtraOpaqueData, ) - inboundFee := extractInboundFeeSafe( - channelUpdate.ExtraOpaqueData, - ) + inboundFee := channelUpdate.InboundFee.UnwrapOr(lnwire.Fee{}) channelUpdates[i] = &lnrpc.ChannelEdgeUpdate{ ChanId: channelUpdate.ChanID, From 7add5bfd4a16c1162609a92bdd9ebd921de01bdc Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 07:55:45 +0200 Subject: [PATCH 08/11] tlv+go.mod: generate inbound fee TLV type Also add a temporariy replace to the tlv package which can be removed as soon as the PR that includes this commit is merged and a new tag for the tlv package has been created. --- go.mod | 3 +++ go.sum | 2 -- tlv/internal/gen/gen_tlv_types.go | 5 +++++ tlv/tlv_types_generated.go | 10 ++++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index d5716a01d..f7f6b08ff 100644 --- a/go.mod +++ b/go.mod @@ -203,6 +203,9 @@ require ( // store have been included in a tagged sqldb version. replace github.com/lightningnetwork/lnd/sqldb => ./sqldb +// TODO(elle): replace once the updated tlv package has been tagged. +replace github.com/lightningnetwork/lnd/tlv => ./tlv + // This replace is for https://github.com/advisories/GHSA-25xm-hr59-7c27 replace github.com/ulikunitz/xz => github.com/ulikunitz/xz v0.5.11 diff --git a/go.sum b/go.sum index 40665b84a..5760d9bd9 100644 --- a/go.sum +++ b/go.sum @@ -377,8 +377,6 @@ github.com/lightningnetwork/lnd/queue v1.1.1 h1:99ovBlpM9B0FRCGYJo6RSFDlt8/vOkQQ github.com/lightningnetwork/lnd/queue v1.1.1/go.mod h1:7A6nC1Qrm32FHuhx/mi1cieAiBZo5O6l8IBIoQxvkz4= github.com/lightningnetwork/lnd/ticker v1.1.1 h1:J/b6N2hibFtC7JLV77ULQp++QLtCwT6ijJlbdiZFbSM= github.com/lightningnetwork/lnd/ticker v1.1.1/go.mod h1:waPTRAAcwtu7Ji3+3k+u/xH5GHovTsCoSVpho0KDvdA= -github.com/lightningnetwork/lnd/tlv v1.3.1 h1:o7CZg06y+rJZfUMAo0WzBLr0pgBWCzrt0f9gpujYUzk= -github.com/lightningnetwork/lnd/tlv v1.3.1/go.mod h1:pJuiBj1ecr1WWLOtcZ+2+hu9Ey25aJWFIsjmAoPPnmc= github.com/lightningnetwork/lnd/tor v1.1.6 h1:WHUumk7WgU6BUFsqHuqszI9P6nfhMeIG+rjJBlVE6OE= github.com/lightningnetwork/lnd/tor v1.1.6/go.mod h1:qSRB8llhAK+a6kaTPWOLLXSZc6Hg8ZC0mq1sUQ/8JfI= github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 h1:sjOGyegMIhvgfq5oaue6Td+hxZuf3tDC8lAPrFldqFw= diff --git a/tlv/internal/gen/gen_tlv_types.go b/tlv/internal/gen/gen_tlv_types.go index ab9bbbbd6..9f15e3b69 100644 --- a/tlv/internal/gen/gen_tlv_types.go +++ b/tlv/internal/gen/gen_tlv_types.go @@ -22,6 +22,10 @@ const ( // second defined unsigned TLV range used in pure TLV messages. pureTLVSecondUnsignedTypeRangeStart uint32 = 3000000000 + // inboundFeeType defines the TLV type used within the channel_update + // message to indicate the inbound fee for a channel. + inboundFeeType = 55555 + defaultOutputFile = "tlv_types_generated.go" ) @@ -29,6 +33,7 @@ const ( var typeMarkers = map[uint32]struct{}{ pureTLVSecondSignedTypeRangeStart: {}, pureTLVSecondUnsignedTypeRangeStart: {}, + inboundFeeType: {}, } const typeCodeTemplate = `// Code generated by tlv/internal/gen; DO NOT EDIT. diff --git a/tlv/tlv_types_generated.go b/tlv/tlv_types_generated.go index 9a80f9364..f0eefc4a5 100644 --- a/tlv/tlv_types_generated.go +++ b/tlv/tlv_types_generated.go @@ -3012,6 +3012,16 @@ func (t *tlvType300) tlv() {} type TlvType300 = *tlvType300 +type tlvType55555 struct{} + +func (t *tlvType55555) TypeVal() Type { + return 55555 +} + +func (t *tlvType55555) tlv() {} + +type TlvType55555 = *tlvType55555 + type tlvType65536 struct{} func (t *tlvType65536) TypeVal() Type { From 420001a98cf210fa36db9e8ce5a579bad16794a4 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:08:19 +0200 Subject: [PATCH 09/11] lnwire: add InboundFee TLV record to ChannelUpdate --- lnwire/channel_update.go | 34 +++++++++++++++++++++++++++++-- lnwire/test_message.go | 43 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/lnwire/channel_update.go b/lnwire/channel_update.go index 55d9d3181..558d81a9e 100644 --- a/lnwire/channel_update.go +++ b/lnwire/channel_update.go @@ -6,6 +6,7 @@ import ( "io" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/lightningnetwork/lnd/tlv" ) // ChanUpdateMsgFlags is a bitfield that signals whether optional fields are @@ -114,6 +115,10 @@ type ChannelUpdate1 struct { // HtlcMaximumMsat is the maximum HTLC value which will be accepted. HtlcMaximumMsat MilliSatoshi + // InboundFee is an optional TLV record that contains the fee + // information for incoming HTLCs. + InboundFee tlv.OptionalRecordT[tlv.TlvType55555, Fee] + // ExtraData is the set of data that was appended to this message to // fill out the full maximum transport message size. These fields can // be used to specify optional data such as custom TLV fields. @@ -156,12 +161,27 @@ func (a *ChannelUpdate1) Decode(r io.Reader, _ uint32) error { } } - err = a.ExtraOpaqueData.Decode(r) + var tlvRecords ExtraOpaqueData + if err := ReadElements(r, &tlvRecords); err != nil { + return err + } + + var inboundFee = a.InboundFee.Zero() + typeMap, err := tlvRecords.ExtractRecords(&inboundFee) if err != nil { return err } - return a.ExtraOpaqueData.ValidateTLV() + val, ok := typeMap[a.InboundFee.TlvType()] + if ok && val == nil { + a.InboundFee = tlv.SomeRecordT(inboundFee) + } + + if len(tlvRecords) != 0 { + a.ExtraOpaqueData = tlvRecords + } + + return nil } // Encode serializes the target ChannelUpdate into the passed io.Writer @@ -218,6 +238,16 @@ func (a *ChannelUpdate1) Encode(w *bytes.Buffer, pver uint32) error { } } + recordProducers := make([]tlv.RecordProducer, 0, 1) + a.InboundFee.WhenSome(func(fee tlv.RecordT[tlv.TlvType55555, Fee]) { + recordProducers = append(recordProducers, &fee) + }) + + err := EncodeMessageExtraData(&a.ExtraOpaqueData, recordProducers...) + if err != nil { + return err + } + // Finally, append any extra opaque data. return WriteBytes(w, a.ExtraOpaqueData) } diff --git a/lnwire/test_message.go b/lnwire/test_message.go index 8b3d98400..9d086c868 100644 --- a/lnwire/test_message.go +++ b/lnwire/test_message.go @@ -12,6 +12,7 @@ import ( "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/tlv" + "github.com/stretchr/testify/require" "pgregory.net/rapid" ) @@ -406,6 +407,45 @@ func (a *ChannelUpdate1) RandTestMessage(t *rapid.T) Message { maxHtlc = 0 } + // Randomly decide if an inbound fee should be included. + // By default, our extra opaque data will just be random TLV but if we + // include an inbound fee, then we will also set the record in the + // extra opaque data. + var ( + customRecords, _ = RandCustomRecords(t, nil, false) + inboundFee tlv.OptionalRecordT[tlv.TlvType55555, Fee] + ) + includeInboundFee := rapid.Bool().Draw(t, "includeInboundFee") + if includeInboundFee { + if customRecords == nil { + customRecords = make(CustomRecords) + } + + inFeeBase := int32( + rapid.IntRange(-1000, 1000).Draw(t, "inFeeBase"), + ) + inFeeProp := int32( + rapid.IntRange(-1000, 1000).Draw(t, "inFeeProp"), + ) + fee := Fee{ + BaseFee: inFeeBase, + FeeRate: inFeeProp, + } + inboundFee = tlv.SomeRecordT( + tlv.NewRecordT[tlv.TlvType55555, Fee](fee), + ) + + var b bytes.Buffer + feeRecord := fee.Record() + err := feeRecord.Encode(&b) + require.NoError(t, err) + + customRecords[uint64(FeeRecordType)] = b.Bytes() + } + + extraBytes, err := customRecords.Serialize() + require.NoError(t, err) + return &ChannelUpdate1{ Signature: RandSignature(t), ChainHash: hash, @@ -428,7 +468,8 @@ func (a *ChannelUpdate1) RandTestMessage(t *rapid.T) Message { t, "feeRate"), ), HtlcMaximumMsat: maxHtlc, - ExtraOpaqueData: RandExtraOpaqueData(t, nil), + InboundFee: inboundFee, + ExtraOpaqueData: extraBytes, } } From 6d8bc63ad6cc6235711bee739451898bb84686ab Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 12:56:55 +0200 Subject: [PATCH 10/11] discovery+graph: get inbound fee directly from ChannelUpdate Remove the previously added TODOs which would extract InboundFee info from the ExtraOpaqueData of a ChannelUpdate at the time of ChannelEdgePolicy construction. These can now be replaced by using the newly added InboundFee record on the ChannelUpdate message. --- discovery/gossiper.go | 21 +-------------------- graph/builder.go | 18 +----------------- netann/channel_update.go | 10 +++++++++- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index d79fe7a74..d6855ae45 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -3257,29 +3257,10 @@ func (d *AuthenticatedGossiper) handleChanUpdate(ctx context.Context, MaxHTLC: upd.HtlcMaximumMsat, FeeBaseMSat: lnwire.MilliSatoshi(upd.BaseFee), FeeProportionalMillionths: lnwire.MilliSatoshi(upd.FeeRate), + InboundFee: upd.InboundFee.ValOpt(), ExtraOpaqueData: upd.ExtraOpaqueData, } - // Extract the inbound fee from the ExtraOpaqueData, if present. - // - // TODO(elle): this can be removed once we define the optional TLV - // field on the lnwire.ChannelUpdate itself. - var inboundFee lnwire.Fee - typeMap, err := upd.ExtraOpaqueData.ExtractRecords(&inboundFee) - if err != nil { - rErr := fmt.Errorf("%w: %w", graphdb.ErrParsingExtraTLVBytes, - err) - - log.Error(rErr) - nMsg.err <- rErr - return nil, false - } - - val, ok := typeMap[lnwire.FeeRecordType] - if ok && val == nil { - update.InboundFee = fn.Some(inboundFee) - } - if err := d.cfg.Graph.UpdateEdge(update, ops...); err != nil { if graph.IsError( err, graph.ErrOutdated, diff --git a/graph/builder.go b/graph/builder.go index 32aac9434..99fc73c17 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -11,7 +11,6 @@ import ( "github.com/go-errors/errors" "github.com/lightningnetwork/lnd/batch" "github.com/lightningnetwork/lnd/chainntnfs" - "github.com/lightningnetwork/lnd/fn/v2" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnutils" @@ -955,25 +954,10 @@ func (b *Builder) ApplyChannelUpdate(msg *lnwire.ChannelUpdate1) bool { MaxHTLC: msg.HtlcMaximumMsat, FeeBaseMSat: lnwire.MilliSatoshi(msg.BaseFee), FeeProportionalMillionths: lnwire.MilliSatoshi(msg.FeeRate), + InboundFee: msg.InboundFee.ValOpt(), ExtraOpaqueData: msg.ExtraOpaqueData, } - // Extract the inbound fee from the ExtraOpaqueData, if present. - // - // TODO(elle): this can be removed once we define the optional TLV - // field on the lnwire.ChannelUpdate itself. - var inboundFee lnwire.Fee - typeMap, err := update.ExtraOpaqueData.ExtractRecords(&inboundFee) - if err != nil { - log.Errorf("%v: %v", graphdb.ErrParsingExtraTLVBytes, err) - return false - } - - val, ok := typeMap[lnwire.FeeRecordType] - if ok && val == nil { - update.InboundFee = fn.Some(inboundFee) - } - err = b.UpdateEdge(update) if err != nil && !IsError(err, ErrIgnored, ErrOutdated) { log.Errorf("Unable to apply channel update: %v", err) diff --git a/netann/channel_update.go b/netann/channel_update.go index efc5cf61e..1bb509b08 100644 --- a/netann/channel_update.go +++ b/netann/channel_update.go @@ -13,6 +13,7 @@ import ( "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/tlv" "github.com/pkg/errors" ) @@ -138,7 +139,7 @@ func ExtractChannelUpdate(ownerPubKey []byte, func UnsignedChannelUpdateFromEdge(info *models.ChannelEdgeInfo, policy *models.ChannelEdgePolicy) *lnwire.ChannelUpdate1 { - return &lnwire.ChannelUpdate1{ + update := &lnwire.ChannelUpdate1{ ChainHash: info.ChainHash, ShortChannelID: lnwire.NewShortChanIDFromInt(policy.ChannelID), Timestamp: uint32(policy.LastUpdate.Unix()), @@ -151,6 +152,13 @@ func UnsignedChannelUpdateFromEdge(info *models.ChannelEdgeInfo, FeeRate: uint32(policy.FeeProportionalMillionths), ExtraOpaqueData: policy.ExtraOpaqueData, } + policy.InboundFee.WhenSome(func(fee lnwire.Fee) { + update.InboundFee = tlv.SomeRecordT( + tlv.NewRecordT[tlv.TlvType55555, lnwire.Fee](fee), + ) + }) + + return update } // ChannelUpdateFromEdge reconstructs a signed ChannelUpdate from the given edge From 6141bcd851f3d46fc209d2c1741e450250e39b36 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 4 Jun 2025 13:33:41 +0200 Subject: [PATCH 11/11] docs: update release notes --- docs/release-notes/release-notes-0.20.0.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 7bc9d6e8e..451537870 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -88,6 +88,11 @@ circuit. The indices are only available for forwarding events saved after v0.20. # Technical and Architectural Updates ## BOLT Spec Updates +* Explicitly define the [inbound fee TLV + record](https://github.com/lightningnetwork/lnd/pull/9897) on the + `channel_update` message and handle it explicitly throughout the code base + instead of extracting it from the TLV stream at various call-sites. + ## Testing ## Database @@ -99,4 +104,5 @@ circuit. The indices are only available for forwarding events saved after v0.20. # Contributors (Alphabetical Order) * Abdulkbk -Pins +* Elle Mouton +* Pins