diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 4e2f562af..7c09cb0ed 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -223,6 +223,9 @@ reader of a payment request. 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. +* [Don't error out](https://github.com/lightningnetwork/lnd/pull/9884) if an + invoice's feature vector contain both the required and optional versions of a + feature bit. In those cases, just treat the feature as mandatory. * [Require invoices to include a payment address or blinded paths](https://github.com/lightningnetwork/lnd/pull/9752) to comply with updated BOLT 11 specifications before sending payments. diff --git a/lnrpc/routerrpc/router_backend.go b/lnrpc/routerrpc/router_backend.go index 022f4619a..377d0be3e 100644 --- a/lnrpc/routerrpc/router_backend.go +++ b/lnrpc/routerrpc/router_backend.go @@ -365,10 +365,7 @@ func (r *RouterBackend) parseQueryRoutesRequest(in *lnrpc.QueryRoutesRequest) ( } // Parse destination feature bits. - destinationFeatures, err = UnmarshalFeatures(in.DestFeatures) - if err != nil { - return nil, err - } + destinationFeatures = UnmarshalFeatures(in.DestFeatures) } // We need to subtract the final delta before passing it into path @@ -510,10 +507,7 @@ func unmarshalBlindedPayment(rpcPayment *lnrpc.BlindedPaymentPath) ( return nil, err } - features, err := UnmarshalFeatures(rpcPayment.Features) - if err != nil { - return nil, err - } + features := UnmarshalFeatures(rpcPayment.Features) return &routing.BlindedPayment{ BlindedPath: path, @@ -1148,10 +1142,7 @@ func (r *RouterBackend) extractIntentFromSendRequest( payIntent.Amount = reqAmt // Parse destination feature bits. - features, err := UnmarshalFeatures(rpcPayReq.DestFeatures) - if err != nil { - return nil, err - } + features := UnmarshalFeatures(rpcPayReq.DestFeatures) // Validate the features if any was specified. if features != nil { @@ -1173,10 +1164,7 @@ func (r *RouterBackend) extractIntentFromSendRequest( lnrpc.FeatureBit_AMP_OPT, } - features, err = UnmarshalFeatures(ampFeatures) - if err != nil { - return nil, err - } + features = UnmarshalFeatures(ampFeatures) } // First make sure the destination supports AMP. @@ -1372,24 +1360,26 @@ func MarshalFeatures(feats *lnwire.FeatureVector) []lnrpc.FeatureBit { // UnmarshalFeatures converts a list of uint32's into a valid feature vector. // This method checks that feature bit pairs aren't assigned together, and // validates transitive dependencies. -func UnmarshalFeatures( - rpcFeatures []lnrpc.FeatureBit) (*lnwire.FeatureVector, error) { - +func UnmarshalFeatures(rpcFeatures []lnrpc.FeatureBit) *lnwire.FeatureVector { // If no destination features are specified we'll return nil to signal // that the router should try to use the graph as a fallback. if rpcFeatures == nil { - return nil, nil + return nil } raw := lnwire.NewRawFeatureVector() for _, bit := range rpcFeatures { - err := raw.SafeSet(lnwire.FeatureBit(bit)) - if err != nil { - return nil, err - } + // Even though the spec says that the writer of a feature vector + // should never set both the required and optional bits of a + // feature, it also says that if we receive a vector with both + // bits set, then we should just treat the feature as required. + // Therefore, we don't use SafeSet here when parsing a peer's + // feature bits and just set the feature no matter what so that + // if both are set then IsRequired returns true. + raw.Set(lnwire.FeatureBit(bit)) } - return lnwire.NewFeatureVector(raw, lnwire.Features), nil + return lnwire.NewFeatureVector(raw, lnwire.Features) } // ValidatePayReqExpiry checks if the passed payment request has expired. In diff --git a/lnrpc/routerrpc/router_backend_test.go b/lnrpc/routerrpc/router_backend_test.go index b94e7a00b..a1095a36c 100644 --- a/lnrpc/routerrpc/router_backend_test.go +++ b/lnrpc/routerrpc/router_backend_test.go @@ -881,6 +881,27 @@ func TestExtractIntentFromSendRequest(t *testing.T) { valid: false, expectedErrorMsg: "self-payments not allowed", }, + { + name: "Required and optional feature bits set", + backend: &RouterBackend{ + MaxTotalTimelock: 1000, + ShouldSetExpEndorsement: func() bool { + return false + }, + }, + sendReq: &SendPaymentRequest{ + Dest: destNodeBytes, + Amt: int64(paymentAmount), + PaymentHash: make([]byte, 32), + MaxParts: 10, + MaxShardSizeMsat: 30_000_000, + DestFeatures: []lnrpc.FeatureBit{ + lnrpc.FeatureBit_GOSSIP_QUERIES_OPT, + lnrpc.FeatureBit_GOSSIP_QUERIES_REQ, + }, + }, + valid: true, + }, { name: "Valid send req parameters, payment settled", backend: &RouterBackend{ diff --git a/lnwire/features.go b/lnwire/features.go index 31e750055..107828ee2 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -446,7 +446,7 @@ func (fv RawFeatureVector) Equals(other *RawFeatureVector) bool { return true } -// Merges sets all feature bits in other on the receiver's feature vector. +// Merge sets all feature bits in other on the receiver's feature vector. func (fv *RawFeatureVector) Merge(other *RawFeatureVector) error { for bit := range other.features { err := fv.SafeSet(bit) diff --git a/rpcserver.go b/rpcserver.go index 938b3c341..364f49a02 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -5827,12 +5827,9 @@ func (r *rpcServer) extractPaymentIntent(rpcPayReq *rpcPaymentRequest) (rpcPayme } // Unmarshal any custom destination features. - payIntent.destFeatures, err = routerrpc.UnmarshalFeatures( + payIntent.destFeatures = routerrpc.UnmarshalFeatures( rpcPayReq.DestFeatures, ) - if err != nil { - return payIntent, err - } return payIntent, nil }