fixup! multi: decode zero-length onion message payloads

Author: gijswijs

payload.go

@@ -101,18 +101,22 @@ func (hp *HopPayload) Decode(r io.Reader) error {
 
 	var payloadSize uint16
 
-	// If the HopPayload isn't guaranteed to be a TLV payload, we check the
-	// first byte to see if it is a legacy payload.
-	if hp.Type != PayloadTLV && isLegacyPayloadByte(peekByte[0]) {
+	// If the HopPayload might be a legacy payload (indicated by the Type
+	// being equal to the zero-value PayloadLegacy), we check the first
+	// byte to see if it is a legacy payload.
+	if hp.Type == PayloadLegacy && isLegacyPayloadByte(peekByte[0]) {
 		payloadSize = legacyPayloadSize()
 	} else {
-		// If the first byte doesn't indicate a legacy payload, then it
-		// *must* be a TLV payload.
+		// Either this is already known to be a TLV payload, or the
+		// first byte indicates that this is a TLV payload (i.e., the
+		// first byte is not 0x00).
 		payloadSize, err = tlvPayloadSize(bufReader)
 		if err != nil {
 			return err
 		}
 
+		// We still need to set the payload type in case it was the
+		// zero-value, PayloadLegacy, AND the first byte was not 0x00.
 		hp.Type = PayloadTLV
 	}
 
@@ -140,11 +144,15 @@ func readPayloadAndHMAC(hp *HopPayload, r io.Reader, payloadSize uint16) error {
 	// Now that we know the payload size, we'll create a new buffer to read
 	// it out in full.
 	hp.Payload = make([]byte, payloadSize)
-	if _, err := io.ReadFull(r, hp.Payload[:]); err != nil {
-		return err
+
+	_, err := io.ReadFull(r, hp.Payload)
+	if err != nil {
+		return fmt.Errorf("%w : %w", ErrIOReadFull, err)
 	}
-	if _, err := io.ReadFull(r, hp.HMAC[:]); err != nil {
-		return err
+
+	_, err = io.ReadFull(r, hp.HMAC[:])
+	if err != nil {
+		return fmt.Errorf("%w : %w", ErrIOReadFull, err)
 	}
 
 	return nil

sphinx.go

@@ -579,7 +579,7 @@ func WithBlindingPoint(point *btcec.PublicKey) ProcessOnionOpt {
 }
 
 // WithTLVPayloadOnly is a functional option that signals that the onion packet
-// being processed is from onion message.
+// being processed is an onion_message_packet.
 func WithTLVPayloadOnly() ProcessOnionOpt {
 	return func(cfg *processOnionCfg) {
 		cfg.tlvPayloadOnly = true

sphinx_test.go

@@ -430,8 +430,8 @@ func TestTLVPayloadMessagePacket(t *testing.T) {
 		hex.EncodeToString(finalPacket), hex.EncodeToString(b.Bytes()))
 }
 
-// TestProcessOnionMessageZeroLengthPayload tests that we can properly process an
-// onion message that has a zero-length payload.
+// TestProcessOnionMessageZeroLengthPayload tests that we can properly process
+// an onion message that has a zero-length payload.
 func TestProcessOnionMessageZeroLengthPayload(t *testing.T) {
 	t.Parallel()
 
@@ -443,6 +443,7 @@ func TestProcessOnionMessageZeroLengthPayload(t *testing.T) {
 	router := NewRouter(&PrivKeyECDH{privKey}, NewMemoryReplayLog())
 	err = router.Start()
 	require.NoError(t, err)
+
 	defer router.Stop()
 
 	// Next, create a session key for the onion packet.
@@ -459,6 +460,7 @@ func TestProcessOnionMessageZeroLengthPayload(t *testing.T) {
 	// The hop payload will be an empty TLV payload.
 	payload, err := NewTLVHopPayload(nil)
 	require.NoError(t, err)
+	require.Empty(t, payload.Payload)
 	path[0].HopPayload = payload
 
 	// Now, create the onion packet.