From 868a5425c1b9b80598c020360230498e2042e317 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 16 Dec 2019 13:06:30 -0800 Subject: [PATCH] feature/deps: validate feature dependencies --- feature/deps.go | 124 ++++++++++++++++++++++++++++++++ feature/deps_test.go | 167 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 291 insertions(+) create mode 100644 feature/deps.go create mode 100644 feature/deps_test.go diff --git a/feature/deps.go b/feature/deps.go new file mode 100644 index 00000000..173318d5 --- /dev/null +++ b/feature/deps.go @@ -0,0 +1,124 @@ +package feature + +import ( + "fmt" + + "github.com/lightningnetwork/lnd/lnwire" +) + +type ( + // featureSet contains a set of feature bits. + featureSet map[lnwire.FeatureBit]struct{} + + // supportedFeatures maps the feature bit from a feature vector to a + // boolean indicating if this features dependencies have already been + // verified. This allows us to short circuit verification if multiple + // features have common dependencies, or map traversal starts verifying + // from the bottom up. + supportedFeatures map[lnwire.FeatureBit]bool + + // depDesc maps a features to its set of dependent features, which must + // also be present for the vector to be valid. This can be used to + // recursively check the dependency chain for features in a feature + // vector. + depDesc map[lnwire.FeatureBit]featureSet +) + +// ErrMissingFeatureDep is an error signaling that a transitive dependency in a +// feature vector is not set properly. +type ErrMissingFeatureDep struct { + dep lnwire.FeatureBit +} + +// Error returns a human-readable description of the missing dep error. +func (e ErrMissingFeatureDep) Error() string { + return fmt.Sprintf("missing feature dependency: %v", e.dep) +} + +// deps is the default set of dependencies for assigned feature bits. If a +// feature is not present in the depDesc it is assumed to have no dependencies. +// +// NOTE: For proper functioning, only the optional variant of feature bits +// should be used in the following descriptor. In the future it may be necessary +// to distinguish the dependencies for optional and required bits, but for now +// the validation code maps required bits to optional ones since it simplifies +// the number of constraints. +var deps = depDesc{ + lnwire.PaymentAddrOptional: { + lnwire.TLVOnionPayloadOptional: {}, + }, + lnwire.MPPOptional: { + lnwire.PaymentAddrOptional: {}, + }, +} + +// ValidateDeps asserts that a feature vector sets all features and their +// transitive dependencies properly. It assumes that the dependencies between +// optional and required features are identical, e.g. if a feature is required +// but its dependency is optional, that is sufficient. +func ValidateDeps(fv *lnwire.FeatureVector) error { + features := fv.Features() + supported := initSupported(features) + + return validateDeps(features, supported) +} + +// validateDeps is a subroutine that recursively checks that the passed features +// have all of their associated dependencies in the supported map. +func validateDeps(features featureSet, supported supportedFeatures) error { + for bit := range features { + // Convert any required bits to optional. + bit = mapToOptional(bit) + + // If the supported features doesn't contain the dependency, this + // vector is invalid. + checked, ok := supported[bit] + if !ok { + return ErrMissingFeatureDep{bit} + } + + // Alternatively, if we know that this depdendency is valid, we + // can short circuit and continue verifying other bits. + if checked { + continue + } + + // Recursively validate dependencies, since this method ranges + // over the subDeps. This method will return true even if + // subDeps is nil. + subDeps := deps[bit] + if err := validateDeps(subDeps, supported); err != nil { + return err + } + + // Once we've confirmed that this feature's dependencies, if + // any, are sound, we record this so other paths taken through + // `bit` return early when inspecting the supported map. + supported[bit] = true + } + + return nil +} + +// initSupported sets all bits from the feature vector as supported but not +// checked. This signals that the validity of their dependencies has not been +// verified. All required bits are mapped to optional to simplify the DAG. +func initSupported(features featureSet) supportedFeatures { + supported := make(supportedFeatures) + for bit := range features { + bit = mapToOptional(bit) + supported[bit] = false + } + + return supported +} + +// mapToOptional returns the optional variant of a given feature bit pair. Our +// dependendency graph is described using only optional feature bits, which +// reduces the number of constraints we need to express in the descriptor. +func mapToOptional(bit lnwire.FeatureBit) lnwire.FeatureBit { + if bit.IsRequired() { + bit ^= 0x01 + } + return bit +} diff --git a/feature/deps_test.go b/feature/deps_test.go new file mode 100644 index 00000000..f1d950df --- /dev/null +++ b/feature/deps_test.go @@ -0,0 +1,167 @@ +package feature + +import ( + "reflect" + "testing" + + "github.com/lightningnetwork/lnd/lnwire" +) + +type depTest struct { + name string + raw *lnwire.RawFeatureVector + expErr error +} + +var depTests = []depTest{ + { + name: "empty", + raw: lnwire.NewRawFeatureVector(), + }, + { + name: "no deps optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + ), + }, + { + name: "no deps required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + ), + }, + { + name: "one dep optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + ), + }, + { + name: "one dep required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + ), + }, + { + name: "one missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "one missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + }, + { + name: "two dep required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + }, + { + name: "two dep last missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep last missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep first missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "two dep first missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "forest optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + }, + { + name: "forest required", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesRequired, + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + }, + { + name: "broken forest optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + lnwire.TLVOnionPayloadOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "broken forest required", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesRequired, + lnwire.TLVOnionPayloadRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, +} + +// TestValidateDeps tests that ValidateDeps correctly asserts whether or not the +// set features constitute a valid feature chain when accounting for transititve +// dependencies. +func TestValidateDeps(t *testing.T) { + for _, test := range depTests { + test := test + t.Run(test.name, func(t *testing.T) { + testValidateDeps(t, test) + }) + } +} + +func testValidateDeps(t *testing.T, test depTest) { + fv := lnwire.NewFeatureVector(test.raw, lnwire.Features) + err := ValidateDeps(fv) + if !reflect.DeepEqual(err, test.expErr) { + t.Fatalf("validation mismatch, want: %v, got: %v", + test.expErr, err) + + } +}