diff --git a/htlcswitch/hop/payload.go b/htlcswitch/hop/payload.go index dd595e11..075f9c7d 100644 --- a/htlcswitch/hop/payload.go +++ b/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 - // required was type 0. Otherwise, the failure must have - // occurred after type 6 and cid should contain an - // accurate value. + // NOTE: Sigh. If the sender included a next hop whose + // value is zero, this would be considered invalid by + // our validation rules below. It's not totally clear + // 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, diff --git a/htlcswitch/hop/payload_test.go b/htlcswitch/hop/payload_test.go index f11a6087..0e442c7f 100644 --- a/htlcswitch/hop/payload_test.go +++ b/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