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.

Author: ellemouton

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
 }
 

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