Implement InvalidOnionPayload final spec decision.

We don't provide the offset hint though.

eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala

@@ -28,7 +28,7 @@ import fr.acinq.eclair.db.OutgoingPaymentStatus
 import fr.acinq.eclair.payment.PaymentLifecycle.{PaymentFailed, PaymentSucceeded}
 import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.wire._
-import fr.acinq.eclair.{CltvExpiryDelta, Features, LongToBtcAmount, MilliSatoshi, NodeParams, ShortChannelId, nodeFee}
+import fr.acinq.eclair.{CltvExpiryDelta, Features, LongToBtcAmount, MilliSatoshi, NodeParams, ShortChannelId, UInt64, nodeFee}
 import grizzled.slf4j.Logging
 import scodec.bits.ByteVector
 import scodec.{Attempt, DecodeResult}
@@ -245,18 +245,19 @@ object Relayer extends Logging {
             }
             if (p.isLastPacket) {
               perHopPayload.paymentInfo match {
-                case Some(_) => Right(FinalPayload(add, perHopPayload))
-                case None => Left(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add.onionRoutingPacket)))
+                case Right(_) => Right(FinalPayload(add, perHopPayload))
+                case Left(err) => Left(err)
               }
             } else {
               perHopPayload.forwardInfo match {
-                case Some(forwardInfo) => Right(RelayPayload(add, forwardInfo, nextPacket))
-                case None => Left(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add.onionRoutingPacket)))
+                case Right(forwardInfo) => Right(RelayPayload(add, forwardInfo, nextPacket))
+                case Left(err) => Left(err)
               }
             }
           case Attempt.Failure(_) =>
             // Onion is correctly encrypted but the content of the per-hop payload couldn't be parsed.
-            Left(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add.onionRoutingPacket)))
+            // It's hard to provide tag and offset information from scodec failures, so we currently don't do it.
+            Left(InvalidOnionPayload(UInt64(0), 0))
         }
       case Left(badOnion) => Left(badOnion)
     }
@@ -272,12 +273,12 @@ object Relayer extends Logging {
   def handleFinal(finalPayload: FinalPayload): Either[CMD_FAIL_HTLC, UpdateAddHtlc] = {
     import finalPayload.add
     finalPayload.payload.paymentInfo match {
-      case Some(OnionPaymentInfo(amountMsat, _)) if amountMsat > add.amountMsat =>
+      case Right(OnionPaymentInfo(amountMsat, _)) if amountMsat > add.amountMsat =>
         Left(CMD_FAIL_HTLC(add.id, Right(FinalIncorrectHtlcAmount(add.amountMsat)), commit = true))
-      case Some(OnionPaymentInfo(_, cltvExpiry)) if cltvExpiry != add.cltvExpiry =>
+      case Right(OnionPaymentInfo(_, cltvExpiry)) if cltvExpiry != add.cltvExpiry =>
         Left(CMD_FAIL_HTLC(add.id, Right(FinalIncorrectCltvExpiry(add.cltvExpiry)), commit = true))
-      case None =>
-        Left(CMD_FAIL_HTLC(add.id, Right(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add.onionRoutingPacket))), commit = true))
+      case Left(err) =>
+        Left(CMD_FAIL_HTLC(add.id, Right(err), commit = true))
       case _ =>
         Right(add)
     }

eclair-core/src/main/scala/fr/acinq/eclair/wire/FailureMessage.scala

@@ -18,10 +18,10 @@ package fr.acinq.eclair.wire
 
 import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.eclair.crypto.Mac32
-import fr.acinq.eclair.wire.CommonCodecs.{cltvExpiry, discriminatorWithDefault, millisatoshi, sha256}
+import fr.acinq.eclair.wire.CommonCodecs._
 import fr.acinq.eclair.wire.FailureMessageCodecs.failureMessageCodec
 import fr.acinq.eclair.wire.LightningMessageCodecs.{channelUpdateCodec, lightningMessageCodec}
-import fr.acinq.eclair.{CltvExpiry, LongToBtcAmount, MilliSatoshi}
+import fr.acinq.eclair.{CltvExpiry, LongToBtcAmount, MilliSatoshi, UInt64}
 import scodec.codecs._
 import scodec.{Attempt, Codec}
 
@@ -62,7 +62,7 @@ case class ExpiryTooSoon(update: ChannelUpdate) extends Update { def message = "
 case class FinalIncorrectCltvExpiry(expiry: CltvExpiry) extends FailureMessage { def message = "payment expiry doesn't match the value in the onion" }
 case class FinalIncorrectHtlcAmount(amount: MilliSatoshi) extends FailureMessage { def message = "payment amount is incorrect in the final htlc" }
 case object ExpiryTooFar extends FailureMessage { def message = "payment expiry is too far in the future" }
-case class InvalidOnionPayload(onionHash: ByteVector32) extends Perm { def message = "onion per-hop payload is invalid" }
+case class InvalidOnionPayload(tag: UInt64, offset: Int) extends Perm { def message = "onion per-hop payload is invalid" }
 
 /**
  * We allow remote nodes to send us unknown failure codes (e.g. deprecated failure codes).
@@ -115,7 +115,7 @@ object FailureMessageCodecs {
       .typecase(18, ("expiry" | cltvExpiry).as[FinalIncorrectCltvExpiry])
       .typecase(19, ("amountMsat" | millisatoshi).as[FinalIncorrectHtlcAmount])
       .typecase(21, provide(ExpiryTooFar))
-      .typecase(PERM, sha256.as[InvalidOnionPayload]),
+      .typecase(PERM | 22, (("tag" | varint) :: ("offset" | uint16)).as[InvalidOnionPayload]),
     uint16.xmap(code => {
       val failureMessage = code match {
         // @formatter:off

eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala

@@ -48,21 +48,35 @@ case class OnionPaymentInfo(amount: MilliSatoshi, cltvExpiry: CltvExpiry)
 
 case class OnionPerHopPayload(payload: Either[TlvStream[OnionTlv], OnionForwardInfo]) {
 
-  lazy val paymentInfo: Option[OnionPaymentInfo] = payload match {
-    case Right(OnionForwardInfo(_, amount, cltv)) => Some(OnionPaymentInfo(amount, cltv))
-    case Left(tlv) => for {
-      amount <- tlv.get[AmountToForward].map(_.amount)
-      cltv <- tlv.get[OutgoingCltv].map(_.cltv)
-    } yield OnionPaymentInfo(amount, cltv)
+  lazy val paymentInfo: Either[InvalidOnionPayload, OnionPaymentInfo] = payload match {
+    case Right(OnionForwardInfo(_, amount, cltv)) => Right(OnionPaymentInfo(amount, cltv))
+    case Left(tlv) =>
+      val amount = tlv.get[AmountToForward].map(_.amount)
+      val cltv = tlv.get[OutgoingCltv].map(_.cltv)
+      if (amount.isEmpty) {
+        Left(InvalidOnionPayload(UInt64(2), 0))
+      } else if (cltv.isEmpty) {
+        Left(InvalidOnionPayload(UInt64(4), 0))
+      } else {
+        Right(OnionPaymentInfo(amount.get, cltv.get))
+      }
   }
 
-  lazy val forwardInfo: Option[OnionForwardInfo] = payload match {
-    case Right(onionForwardInfo) => Some(onionForwardInfo)
-    case Left(tlv) => for {
-      shortChannelId <- tlv.get[OutgoingChannelId].map(_.shortChannelId)
-      amount <- tlv.get[AmountToForward].map(_.amount)
-      cltv <- tlv.get[OutgoingCltv].map(_.cltv)
-    } yield OnionForwardInfo(shortChannelId, amount, cltv)
+  lazy val forwardInfo: Either[InvalidOnionPayload, OnionForwardInfo] = payload match {
+    case Right(onionForwardInfo) => Right(onionForwardInfo)
+    case Left(tlv) =>
+      val shortChannelId = tlv.get[OutgoingChannelId].map(_.shortChannelId)
+      val amount = tlv.get[AmountToForward].map(_.amount)
+      val cltv = tlv.get[OutgoingCltv].map(_.cltv)
+      if (amount.isEmpty) {
+        Left(InvalidOnionPayload(UInt64(2), 0))
+      } else if (cltv.isEmpty) {
+        Left(InvalidOnionPayload(UInt64(4), 0))
+      } else if (shortChannelId.isEmpty) {
+        Left(InvalidOnionPayload(UInt64(6), 0))
+      } else {
+        Right(OnionForwardInfo(shortChannelId.get, amount.get, cltv.get))
+      }
   }
 
 }

eclair-core/src/test/scala/fr/acinq/eclair/crypto/SphinxSpec.scala

@@ -18,7 +18,7 @@ package fr.acinq.eclair.crypto
 
 import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey}
-import fr.acinq.eclair.wire
+import fr.acinq.eclair.{UInt64, wire}
 import fr.acinq.eclair.wire._
 import org.scalatest.FunSuite
 import scodec.bits._
@@ -249,7 +249,7 @@ class SphinxSpec extends FunSuite {
 
     val packet = FailurePacket.wrap(
       FailurePacket.wrap(
-        FailurePacket.create(sharedSecrets.head, InvalidOnionPayload(ByteVector32.Zeroes)),
+        FailurePacket.create(sharedSecrets.head, InvalidOnionPayload(UInt64(0), 0)),
         sharedSecrets(1)),
       sharedSecrets(2))
 

eclair-core/src/test/scala/fr/acinq/eclair/payment/HtlcGenerationSpec.scala

@@ -123,7 +123,7 @@ class HtlcGenerationSpec extends FunSuite with BeforeAndAfterAll {
     val payload_e = OnionCodecs.tlvPerHopPayloadCodec.decode(bin_e.toBitVector).require.value
     val paymentInfo = OnionPerHopPayload(Left(payload_e)).paymentInfo
     assert(packet_random.payload.length === Sphinx.PaymentPacket.PayloadLength)
-    assert(paymentInfo === Some(OnionPaymentInfo(finalAmountMsat, finalExpiry)))
+    assert(paymentInfo === Right(OnionPaymentInfo(finalAmountMsat, finalExpiry)))
   }
 
   test("build a command including the onion") {

eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala

@@ -295,20 +295,20 @@ class RelayerSpec extends TestkitBaseClass {
 
     // B is not the last hop and receives an onion missing some routing information.
     val invalidPayloads_bc = Seq(
-      TlvStream[OnionTlv](OutgoingChannelId(channelUpdate_bc.shortChannelId), AmountToForward(amount_bc)), // Missing cltv expiry.
-      TlvStream[OnionTlv](OutgoingChannelId(channelUpdate_bc.shortChannelId), OutgoingCltv(expiry_bc)), // Missing forwarding amount.
-      TlvStream[OnionTlv](AmountToForward(amount_bc), OutgoingCltv(expiry_bc))) // Missing channel id.
+      (InvalidOnionPayload(UInt64(2), 0), TlvStream[OnionTlv](OutgoingChannelId(channelUpdate_bc.shortChannelId), OutgoingCltv(expiry_bc))), // Missing forwarding amount.
+      (InvalidOnionPayload(UInt64(4), 0), TlvStream[OnionTlv](OutgoingChannelId(channelUpdate_bc.shortChannelId), AmountToForward(amount_bc))), // Missing cltv expiry.
+      (InvalidOnionPayload(UInt64(6), 0), TlvStream[OnionTlv](AmountToForward(amount_bc), OutgoingCltv(expiry_bc)))) // Missing channel id.
     val payload_cd = TlvStream[OnionTlv](OutgoingChannelId(channelUpdate_cd.shortChannelId), AmountToForward(amount_cd), OutgoingCltv(expiry_cd))
 
     val sender = TestProbe()
     relayer ! LocalChannelUpdate(null, channelId_bc, channelUpdate_bc.shortChannelId, c, None, channelUpdate_bc, makeCommitments(channelId_bc))
 
-    for (invalidPayload_bc <- invalidPayloads_bc) {
+    for ((expectedErr, invalidPayload_bc) <- invalidPayloads_bc) {
       val Sphinx.PacketAndSecrets(onion, _) = buildOnion(Seq(b, c), Seq(invalidPayload_bc, payload_cd), paymentHash)
       val add_ab = UpdateAddHtlc(channelId_ab, 123456, amount_ab, paymentHash, expiry_ab, onion)
       sender.send(relayer, ForwardAdd(add_ab))
 
-      register.expectMsg(Register.Forward(channelId_ab, CMD_FAIL_HTLC(add_ab.id, Right(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add_ab.onionRoutingPacket))), commit = true)))
+      register.expectMsg(Register.Forward(channelId_ab, CMD_FAIL_HTLC(add_ab.id, Right(expectedErr), commit = true)))
       register.expectNoMsg(100 millis)
       paymentHandler.expectNoMsg(100 millis)
     }
@@ -444,17 +444,17 @@ class RelayerSpec extends TestkitBaseClass {
 
     // B is the last hop and receives an onion missing some payment information.
     val invalidFinalPayloads = Seq(
-      TlvStream[OnionTlv](AmountToForward(amount_bc)), // Missing cltv expiry.
-      TlvStream[OnionTlv](OutgoingCltv(expiry_bc))) // Missing forwarding amount.
+      (InvalidOnionPayload(UInt64(2), 0), TlvStream[OnionTlv](OutgoingCltv(expiry_bc))), // Missing forwarding amount.
+      (InvalidOnionPayload(UInt64(4), 0), TlvStream[OnionTlv](AmountToForward(amount_bc)))) // Missing cltv expiry.
 
     val sender = TestProbe()
 
-    for (invalidFinalPayload <- invalidFinalPayloads) {
+    for ((expectedErr, invalidFinalPayload) <- invalidFinalPayloads) {
       val Sphinx.PacketAndSecrets(onion, _) = buildOnion(Seq(b), Seq(invalidFinalPayload), paymentHash)
       val add_ab = UpdateAddHtlc(channelId_ab, 123456, amount_ab, paymentHash, expiry_ab, onion)
       sender.send(relayer, ForwardAdd(add_ab))
 
-      register.expectMsg(Register.Forward(channelId_ab, CMD_FAIL_HTLC(add_ab.id, Right(InvalidOnionPayload(Sphinx.PaymentPacket.hash(add_ab.onionRoutingPacket))), commit = true)))
+      register.expectMsg(Register.Forward(channelId_ab, CMD_FAIL_HTLC(add_ab.id, Right(expectedErr), commit = true)))
       register.expectNoMsg(100 millis)
       paymentHandler.expectNoMsg(100 millis)
     }

eclair-core/src/test/scala/fr/acinq/eclair/wire/FailureMessageCodecsSpec.scala

@@ -19,7 +19,7 @@ package fr.acinq.eclair.wire
 import fr.acinq.bitcoin.{Block, ByteVector32, ByteVector64}
 import fr.acinq.eclair.crypto.Hmac256
 import fr.acinq.eclair.wire.FailureMessageCodecs._
-import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, LongToBtcAmount, MilliSatoshi, ShortChannelId, randomBytes32, randomBytes64}
+import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, LongToBtcAmount, MilliSatoshi, ShortChannelId, UInt64, randomBytes32, randomBytes64}
 import org.scalatest.FunSuite
 import scodec.bits._
 
@@ -47,7 +47,7 @@ class FailureMessageCodecsSpec extends FunSuite {
         InvalidOnionVersion(randomBytes32) :: InvalidOnionHmac(randomBytes32) :: InvalidOnionKey(randomBytes32) ::
         TemporaryChannelFailure(channelUpdate) :: PermanentChannelFailure :: RequiredChannelFeatureMissing :: UnknownNextPeer ::
         AmountBelowMinimum(123456 msat, channelUpdate) :: FeeInsufficient(546463 msat, channelUpdate) :: IncorrectCltvExpiry(CltvExpiry(1211), channelUpdate) :: ExpiryTooSoon(channelUpdate) ::
-        IncorrectOrUnknownPaymentDetails(123456 msat, 1105) :: FinalIncorrectCltvExpiry(CltvExpiry(1234)) :: ChannelDisabled(0, 1, channelUpdate) :: ExpiryTooFar :: InvalidOnionPayload(randomBytes32) :: Nil
+        IncorrectOrUnknownPaymentDetails(123456 msat, 1105) :: FinalIncorrectCltvExpiry(CltvExpiry(1234)) :: ChannelDisabled(0, 1, channelUpdate) :: ExpiryTooFar :: InvalidOnionPayload(UInt64(561), 1105) :: Nil
 
     msgs.foreach {
       msg => {

eclair-core/src/test/scala/fr/acinq/eclair/wire/OnionCodecsSpec.scala

@@ -21,7 +21,7 @@ import fr.acinq.eclair.UInt64.Conversions._
 import fr.acinq.eclair.wire.OnionCodecs._
 import fr.acinq.eclair.wire.OnionPerHopPayload._
 import fr.acinq.eclair.wire.OnionTlv._
-import fr.acinq.eclair.{CltvExpiry, LongToBtcAmount, ShortChannelId}
+import fr.acinq.eclair.{CltvExpiry, LongToBtcAmount, ShortChannelId, UInt64}
 import org.scalatest.FunSuite
 import scodec.bits.HexStringSyntax
 
@@ -110,39 +110,39 @@ class OnionCodecsSpec extends FunSuite {
 
   test("get payment info") {
     val legacyPayload: OnionPerHopPayload = OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))
-    assert(legacyPayload.paymentInfo === Some(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
+    assert(legacyPayload.paymentInfo === Right(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
 
     val tlvPayload: OnionPerHopPayload = TlvStream[OnionTlv](AmountToForward(561 msat), OutgoingCltv(CltvExpiry(1105)))
-    assert(tlvPayload.paymentInfo === Some(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
+    assert(tlvPayload.paymentInfo === Right(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
 
     val tlvPayloadUnknown: OnionPerHopPayload = TlvStream[OnionTlv](Seq(AmountToForward(561 msat), OutgoingCltv(CltvExpiry(1105))), Seq(GenericTlv(13, hex"2a")))
-    assert(tlvPayloadUnknown.paymentInfo === Some(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
+    assert(tlvPayloadUnknown.paymentInfo === Right(OnionPaymentInfo(561 msat, CltvExpiry(1105))))
 
     val tlvPayloadNoCltv: OnionPerHopPayload = TlvStream[OnionTlv](AmountToForward(561 msat))
-    assert(tlvPayloadNoCltv.paymentInfo === None)
+    assert(tlvPayloadNoCltv.paymentInfo === Left(InvalidOnionPayload(UInt64(4), 0)))
 
     val tlvPayloadNoAmount: OnionPerHopPayload = TlvStream[OnionTlv](OutgoingCltv(CltvExpiry(1105)))
-    assert(tlvPayloadNoAmount.paymentInfo === None)
+    assert(tlvPayloadNoAmount.paymentInfo === Left(InvalidOnionPayload(UInt64(2), 0)))
   }
 
   test("get forward info") {
     val legacyPayload: OnionPerHopPayload = OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))
-    assert(legacyPayload.forwardInfo === Some(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
+    assert(legacyPayload.forwardInfo === Right(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
 
     val tlvPayload: OnionPerHopPayload = TlvStream[OnionTlv](AmountToForward(561 msat), OutgoingCltv(CltvExpiry(1105)), OutgoingChannelId(ShortChannelId(550)))
-    assert(tlvPayload.forwardInfo === Some(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
+    assert(tlvPayload.forwardInfo === Right(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
 
     val tlvPayloadUnknown: OnionPerHopPayload = TlvStream[OnionTlv](Seq(AmountToForward(561 msat), OutgoingCltv(CltvExpiry(1105)), OutgoingChannelId(ShortChannelId(550))), Seq(GenericTlv(13, hex"2a")))
-    assert(tlvPayloadUnknown.forwardInfo === Some(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
+    assert(tlvPayloadUnknown.forwardInfo === Right(OnionForwardInfo(ShortChannelId(550), 561 msat, CltvExpiry(1105))))
 
     val tlvPayloadNoCltv: OnionPerHopPayload = TlvStream[OnionTlv](AmountToForward(561 msat), OutgoingChannelId(ShortChannelId(550)))
-    assert(tlvPayloadNoCltv.forwardInfo === None)
+    assert(tlvPayloadNoCltv.forwardInfo === Left(InvalidOnionPayload(UInt64(4), 0)))
 
     val tlvPayloadNoAmount: OnionPerHopPayload = TlvStream[OnionTlv](OutgoingCltv(CltvExpiry(1105)), OutgoingChannelId(ShortChannelId(550)))
-    assert(tlvPayloadNoAmount.forwardInfo === None)
+    assert(tlvPayloadNoAmount.forwardInfo === Left(InvalidOnionPayload(UInt64(2), 0)))
 
     val tlvPayloadNoChannelId: OnionPerHopPayload = TlvStream[OnionTlv](AmountToForward(561 msat), OutgoingCltv(CltvExpiry(1105)))
-    assert(tlvPayloadNoChannelId.forwardInfo === None)
+    assert(tlvPayloadNoChannelId.forwardInfo === Left(InvalidOnionPayload(UInt64(6), 0)))
   }
 
 }