htlcswitch/hop/payload: extend tests to required type failures

htlcswitch/hop/payload.go

@@ -120,10 +120,21 @@ func NewPayloadFromReader(r io.Reader) (*Payload, error) {
 	if err != nil {
 		// Promote any required type failures into ErrInvalidPayload.
 		if e, required := err.(tlv.ErrUnknownRequiredType); required {
-			// NOTE: FinalHop will be incorrect if the unknown
+			// NOTE: Sigh. If the sender included a next hop whose
-			// required was type 0. Otherwise, the failure must have
+			// value is zero, this would be considered invalid by
-			// occurred after type 6 and cid should contain an
+			// our validation rules below. It's not totally clear
-			// accurate value.
+			// whether this required failure should take precedence
+			// over the constraints applied by known types.
+			// Unfortunately this is an artifact of the layering
+			// violation in placing the even/odd rule in the parsing
+			// logic and not at a higher level of validation like
+			// the other presence/omission checks.
+			//
+			// As a result, this may need to be revisted if it is
+			// decided that the checks below overrule an unknown
+			// required type failure, in which case an
+			// IncludedViolation should be returned instead of the
+			// RequiredViolation.
 			return nil, ErrInvalidPayload{
 				Type:      tlv.Type(e),
 				Violation: RequiredViolation,

htlcswitch/hop/payload_test.go

@@ -98,7 +98,7 @@ var decodePayloadTests = []decodePayloadTest{
 		},
 	},
 	{
-		name:    "required type zero",
+		name:    "required type zero final hop",
 		payload: []byte{0x00, 0x00},
 		expErr: hop.ErrInvalidPayload{
 			Type:      0,
@@ -106,6 +106,28 @@ var decodePayloadTests = []decodePayloadTest{
 			FinalHop:  true,
 		},
 	},
+	{
+		name: "required type zero final hop zero sid",
+		payload: []byte{0x00, 0x00, 0x06, 0x08, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00,
+		},
+		expErr: hop.ErrInvalidPayload{
+			Type:      0,
+			Violation: hop.RequiredViolation,
+			FinalHop:  true,
+		},
+	},
+	{
+		name: "required type zero intermediate hop",
+		payload: []byte{0x00, 0x00, 0x06, 0x08, 0x01, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00,
+		},
+		expErr: hop.ErrInvalidPayload{
+			Type:      0,
+			Violation: hop.RequiredViolation,
+			FinalHop:  false,
+		},
+	},
 }
 
 // TestDecodeHopPayloadRecordValidation asserts that parsing the payloads in the