Bolt4: remove final_expiry_too_soon error message

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

@@ -62,7 +62,7 @@ class Autoprobe(nodeParams: NodeParams, router: ActorRef, paymentInitiator: Acto
 
     case paymentResult: PaymentResult =>
       paymentResult match {
-        case PaymentFailed(_, _, _ :+ RemoteFailure(_, DecryptedFailurePacket(targetNodeId, IncorrectOrUnknownPaymentDetails(_)))) =>
+        case PaymentFailed(_, _, _ :+ RemoteFailure(_, DecryptedFailurePacket(targetNodeId, IncorrectOrUnknownPaymentDetails(_, _)))) =>
           log.info(s"payment probe successful to node=$targetNodeId")
         case _ =>
           log.info(s"payment probe failed with paymentResult=$paymentResult")

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

@@ -22,7 +22,7 @@ import fr.acinq.eclair.channel.{CMD_FAIL_HTLC, CMD_FULFILL_HTLC, Channel}
 import fr.acinq.eclair.db.IncomingPayment
 import fr.acinq.eclair.payment.PaymentLifecycle.ReceivePayment
 import fr.acinq.eclair.wire._
-import fr.acinq.eclair.{NodeParams, randomBytes32}
+import fr.acinq.eclair.{Globals, NodeParams, randomBytes32}
 
 import scala.compat.Platform
 import scala.concurrent.ExecutionContext
@@ -65,15 +65,15 @@ class LocalPaymentHandler(nodeParams: NodeParams) extends Actor with ActorLoggin
           // see https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#failure-messages
           paymentRequest.amount match {
             case _ if paymentRequest.isExpired =>
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case _ if htlc.cltvExpiry < minFinalExpiry =>
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(FinalExpiryTooSoon), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case Some(amount) if htlc.amountMsat < amount =>
               log.warning(s"received payment with amount too small for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case Some(amount) if htlc.amountMsat > amount * 2 =>
               log.warning(s"received payment with amount too large for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case _ =>
               log.info(s"received payment for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
               // amount is correct or was not specified in the payment request
@@ -82,7 +82,7 @@ class LocalPaymentHandler(nodeParams: NodeParams) extends Actor with ActorLoggin
               context.system.eventStream.publish(PaymentReceived(htlc.amountMsat, htlc.paymentHash, htlc.channelId))
           }
         case None =>
-          sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+          sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
       }
   }
 

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

@@ -120,7 +120,7 @@ class Relayer(nodeParams: NodeParams, register: ActorRef, paymentHandler: ActorR
           }
         case Left(badOnion) =>
           log.warning(s"couldn't parse onion: reason=${badOnion.message}")
-          val cmdFail = CMD_FAIL_MALFORMED_HTLC(add.id, badOnion.onionHash, FailureMessageCodecs.failureCode(badOnion), commit = true)
+          val cmdFail = CMD_FAIL_MALFORMED_HTLC(add.id, badOnion.onionHash, badOnion.code, commit = true)
           log.info(s"rejecting htlc #${add.id} paymentHash=${add.paymentHash} from channelId=${add.channelId} reason=malformed onionHash=${cmdFail.onionHash} failureCode=${cmdFail.failureCode}")
           commandBuffer ! CommandBuffer.CommandSend(add.channelId, add.id, cmdFail)
       }

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

@@ -18,7 +18,8 @@ package fr.acinq.eclair.wire
 
 import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.eclair.crypto.Mac32
-import fr.acinq.eclair.wire.CommonCodecs.{cltvExpiry, millisatoshi, sha256}
+import fr.acinq.eclair.wire.CommonCodecs.{cltvExpiry, discriminatorWithDefault, millisatoshi, sha256}
+import fr.acinq.eclair.wire.FailureMessageCodecs.failureMessageCodec
 import fr.acinq.eclair.wire.LightningMessageCodecs.{channelUpdateCodec, lightningMessageCodec}
 import fr.acinq.eclair.{CltvExpiry, LongToBtcAmount, MilliSatoshi}
 import scodec.codecs._
@@ -30,7 +31,12 @@ import scodec.{Attempt, Codec}
  */
 
 // @formatter:off
-sealed trait FailureMessage { def message: String }
+sealed trait FailureMessage {
+  def message: String
+  // We actually encode the failure message, which is a bit clunky and not particularly efficient.
+  // It would be nice to be able to get that value from the discriminated codec directly.
+  lazy val code: Int = failureMessageCodec.encode(this).flatMap(uint16.decode).require.value
+}
 sealed trait BadOnion extends FailureMessage { def onionHash: ByteVector32 }
 sealed trait Perm extends FailureMessage
 sealed trait Node extends FailureMessage
@@ -52,13 +58,25 @@ case class AmountBelowMinimum(amount: MilliSatoshi, update: ChannelUpdate) exten
 case class FeeInsufficient(amount: MilliSatoshi, update: ChannelUpdate) extends Update { def message = s"payment fee was below the minimum required by the channel" }
 case class ChannelDisabled(messageFlags: Byte, channelFlags: Byte, update: ChannelUpdate) extends Update { def message = "channel is currently disabled" }
 case class IncorrectCltvExpiry(expiry: CltvExpiry, update: ChannelUpdate) extends Update { def message = "payment expiry doesn't match the value in the onion" }
-case class IncorrectOrUnknownPaymentDetails(amount: MilliSatoshi) extends Perm { def message = "incorrect payment amount or unknown payment hash" }
-case object IncorrectPaymentAmount extends Perm { def message = "payment amount is incorrect" }
+case class IncorrectOrUnknownPaymentDetails(amount: MilliSatoshi, height: Long) extends Perm { def message = "incorrect payment details or unknown payment hash" }
 case class ExpiryTooSoon(update: ChannelUpdate) extends Update { def message = "payment expiry is too close to the current block height for safe handling by the relaying node" }
-case object FinalExpiryTooSoon extends FailureMessage { def message = "payment expiry is too close to the current block height for safe handling by the final node" }
 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" }
+
+/**
+ * We allow remote nodes to send us unknown failure codes (e.g. deprecated failure codes).
+ * By reading the PERM and NODE bits we can still extract useful information for payment retry even without knowing how
+ * to decode the failure payload (but we can't extract a channel update or onion hash).
+ */
+sealed trait UnknownFailureMessage extends FailureMessage {
+  def message = "unknown failure message"
+  override def toString = s"$message (${code.toHexString})"
+  override def equals(obj: Any): Boolean = obj match {
+    case f: UnknownFailureMessage => f.code == code
+    case _ => false
+  }
+}
 // @formatter:on
 
 object FailureMessageCodecs {
@@ -73,36 +91,43 @@ object FailureMessageCodecs {
   // this codec supports both versions for decoding, and will encode with the message type
   val channelUpdateWithLengthCodec = variableSizeBytes(uint16, choice(channelUpdateCodecWithType, channelUpdateCodec))
 
-  val failureMessageCodec = discriminated[FailureMessage].by(uint16)
-    .typecase(PERM | 1, provide(InvalidRealm))
-    .typecase(NODE | 2, provide(TemporaryNodeFailure))
-    .typecase(PERM | 2, provide(PermanentNodeFailure))
-    .typecase(PERM | NODE | 3, provide(RequiredNodeFeatureMissing))
-    .typecase(BADONION | PERM, sha256.as[InvalidOnionPayload])
-    .typecase(BADONION | PERM | 4, sha256.as[InvalidOnionVersion])
-    .typecase(BADONION | PERM | 5, sha256.as[InvalidOnionHmac])
-    .typecase(BADONION | PERM | 6, sha256.as[InvalidOnionKey])
-    .typecase(UPDATE | 7, ("channelUpdate" | channelUpdateWithLengthCodec).as[TemporaryChannelFailure])
-    .typecase(PERM | 8, provide(PermanentChannelFailure))
-    .typecase(PERM | 9, provide(RequiredChannelFeatureMissing))
-    .typecase(PERM | 10, provide(UnknownNextPeer))
-    .typecase(UPDATE | 11, (("amountMsat" | millisatoshi) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[AmountBelowMinimum])
-    .typecase(UPDATE | 12, (("amountMsat" | millisatoshi) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[FeeInsufficient])
-    .typecase(UPDATE | 13, (("expiry" | cltvExpiry) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[IncorrectCltvExpiry])
-    .typecase(UPDATE | 14, ("channelUpdate" | channelUpdateWithLengthCodec).as[ExpiryTooSoon])
-    .typecase(UPDATE | 20, (("messageFlags" | byte) :: ("channelFlags" | byte) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[ChannelDisabled])
-    .typecase(PERM | 15, ("amountMsat" | withDefaultValue(optional(bitsRemaining, millisatoshi), 0 msat)).as[IncorrectOrUnknownPaymentDetails])
-    .typecase(PERM | 16, provide(IncorrectPaymentAmount))
-    .typecase(17, provide(FinalExpiryTooSoon))
-    .typecase(18, ("expiry" | cltvExpiry).as[FinalIncorrectCltvExpiry])
-    .typecase(19, ("amountMsat" | millisatoshi).as[FinalIncorrectHtlcAmount])
-    .typecase(21, provide(ExpiryTooFar))
-
-  /**
-   * Return the failure code for a given failure message. This method actually encodes the failure message, which is a
-   * bit clunky and not particularly efficient. It shouldn't be used on the application's hot path.
-   */
-  def failureCode(failure: FailureMessage): Int = failureMessageCodec.encode(failure).flatMap(uint16.decode).require.value
+  val failureMessageCodec = discriminatorWithDefault(
+    discriminated[FailureMessage].by(uint16)
+      .typecase(PERM | 1, provide(InvalidRealm))
+      .typecase(NODE | 2, provide(TemporaryNodeFailure))
+      .typecase(PERM | NODE | 2, provide(PermanentNodeFailure))
+      .typecase(PERM | NODE | 3, provide(RequiredNodeFeatureMissing))
+      .typecase(BADONION | PERM, sha256.as[InvalidOnionPayload])
+      .typecase(BADONION | PERM | 4, sha256.as[InvalidOnionVersion])
+      .typecase(BADONION | PERM | 5, sha256.as[InvalidOnionHmac])
+      .typecase(BADONION | PERM | 6, sha256.as[InvalidOnionKey])
+      .typecase(UPDATE | 7, ("channelUpdate" | channelUpdateWithLengthCodec).as[TemporaryChannelFailure])
+      .typecase(PERM | 8, provide(PermanentChannelFailure))
+      .typecase(PERM | 9, provide(RequiredChannelFeatureMissing))
+      .typecase(PERM | 10, provide(UnknownNextPeer))
+      .typecase(UPDATE | 11, (("amountMsat" | millisatoshi) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[AmountBelowMinimum])
+      .typecase(UPDATE | 12, (("amountMsat" | millisatoshi) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[FeeInsufficient])
+      .typecase(UPDATE | 13, (("expiry" | cltvExpiry) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[IncorrectCltvExpiry])
+      .typecase(UPDATE | 14, ("channelUpdate" | channelUpdateWithLengthCodec).as[ExpiryTooSoon])
+      .typecase(UPDATE | 20, (("messageFlags" | byte) :: ("channelFlags" | byte) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[ChannelDisabled])
+      .typecase(PERM | 15, (("amountMsat" | withDefaultValue(optional(bitsRemaining, millisatoshi), 0 msat)) :: ("height" | withDefaultValue(optional(bitsRemaining, uint32), 0L))).as[IncorrectOrUnknownPaymentDetails])
+      // PERM | 16 (incorrect_payment_amount) has been deprecated because it allowed probing attacks: IncorrectOrUnknownPaymentDetails should be used instead.
+      // PERM | 17 (final_expiry_too_soon) has been deprecated because it allowed probing attacks: IncorrectOrUnknownPaymentDetails should be used instead.
+      .typecase(18, ("expiry" | cltvExpiry).as[FinalIncorrectCltvExpiry])
+      .typecase(19, ("amountMsat" | millisatoshi).as[FinalIncorrectHtlcAmount])
+      .typecase(21, provide(ExpiryTooFar)),
+    uint16.xmap(code => {
+      val failureMessage = code match {
+        // @formatter:off
+        case fc if (fc & PERM) != 0 && (fc & NODE) != 0 => new UnknownFailureMessage with Perm with Node { override lazy val code = fc }
+        case fc if (fc & NODE) != 0 => new UnknownFailureMessage with Node { override lazy val code = fc }
+        case fc if (fc & PERM) != 0 => new UnknownFailureMessage with Perm { override lazy val code = fc }
+        case fc => new UnknownFailureMessage { override lazy val code  = fc }
+        // @formatter:on
+      }
+      failureMessage.asInstanceOf[FailureMessage]
+    }, (_: FailureMessage).code)
+  )
 
   /**
    * An onion-encrypted failure from an intermediate node:

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala

@@ -452,7 +452,7 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
 
     // We simulate a pending failure on that HTLC.
     // Even if we get close to expiring upstream we shouldn't close the channel, because we have nothing to lose.
-    sender.send(commandBuffer, CommandSend(htlc.channelId, htlc.id, CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(0 msat)))))
+    sender.send(commandBuffer, CommandSend(htlc.channelId, htlc.id, CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(0 msat, 0)))))
     sender.send(bob, CurrentBlockCount((htlc.cltvExpiry - bob.underlyingActor.nodeParams.fulfillSafetyBeforeTimeoutBlocks).toLong))
 
     bob2blockchain.expectNoMsg(250 millis)

eclair-core/src/test/scala/fr/acinq/eclair/integration/IntegrationSpec.scala

@@ -345,7 +345,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(failed.id == paymentId)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(100000000 msat)))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(100000000 msat, Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with a lower amount than requested") {
@@ -365,7 +365,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(failed.id == paymentId)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(100000000 msat)))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(100000000 msat, Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with too much overpayment") {
@@ -385,7 +385,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(paymentId == failed.id)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(600000000 msat)))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(600000000 msat, Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with a reasonable overpayment") {

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

@@ -24,8 +24,8 @@ import fr.acinq.eclair.TestConstants.Alice
 import fr.acinq.eclair.channel.{CMD_FAIL_HTLC, CMD_FULFILL_HTLC}
 import fr.acinq.eclair.payment.PaymentLifecycle.ReceivePayment
 import fr.acinq.eclair.payment.PaymentRequest.ExtraHop
-import fr.acinq.eclair.wire.{FinalExpiryTooSoon, UpdateAddHtlc}
-import fr.acinq.eclair.{CltvExpiryDelta, LongToBtcAmount, ShortChannelId, TestConstants, randomKey}
+import fr.acinq.eclair.wire.{IncorrectOrUnknownPaymentDetails, UpdateAddHtlc}
+import fr.acinq.eclair.{CltvExpiryDelta, Globals, LongToBtcAmount, ShortChannelId, TestConstants, randomKey}
 import org.scalatest.FunSuiteLike
 import scodec.bits.ByteVector
 
@@ -83,7 +83,7 @@ class PaymentHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike
 
       val add = UpdateAddHtlc(ByteVector32(ByteVector.fill(32)(1)), 0, amountMsat, pr.paymentHash, cltvExpiry = CltvExpiryDelta(3).toCltvExpiry, TestConstants.emptyOnionPacket)
       sender.send(handler, add)
-      assert(sender.expectMsgType[CMD_FAIL_HTLC].reason == Right(FinalExpiryTooSoon))
+      assert(sender.expectMsgType[CMD_FAIL_HTLC].reason == Right(IncorrectOrUnknownPaymentDetails(amountMsat, Globals.blockCount.get())))
       eventListener.expectNoMsg(300 milliseconds)
       assert(nodeParams.db.payments.getIncomingPayment(pr.paymentHash).isEmpty)
     }

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

@@ -327,7 +327,7 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
     awaitCond(paymentDb.getOutgoingPayment(id).exists(_.status == OutgoingPaymentStatus.FAILED))
   }
 
-  test("payment failed (PermanentChannelFailure)") { fixture =>
+  def testPermanentFailure(fixture: FixtureParam, failure: FailureMessage): Unit = {
     import fixture._
     val nodeParams = TestConstants.Alice.nodeParams.copy(keyManager = testKeyManager)
     val paymentDb = nodeParams.db.payments
@@ -351,8 +351,6 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
     awaitCond(paymentFSM.stateName == WAITING_FOR_PAYMENT_COMPLETE)
     val WaitingForComplete(_, _, cmd1, Nil, sharedSecrets1, _, _, hops) = paymentFSM.stateData
 
-    val failure = PermanentChannelFailure
-
     relayer.expectMsg(ForwardShortId(channelId_ab, cmd1))
     sender.send(paymentFSM, UpdateFailHtlc(ByteVector32.Zeroes, 0, Sphinx.FailurePacket.create(sharedSecrets1.head._1, failure)))
 
@@ -366,6 +364,16 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
     awaitCond(paymentDb.getOutgoingPayment(id).exists(_.status == OutgoingPaymentStatus.FAILED))
   }
 
+  test("payment failed (PermanentChannelFailure)") { fixture =>
+    testPermanentFailure(fixture, PermanentChannelFailure)
+  }
+
+  test("payment failed (deprecated permanent failure)") { fixture =>
+    import scodec.bits.HexStringSyntax
+    // PERM | 17 (final_expiry_too_soon) has been deprecated but older nodes might still use it.
+    testPermanentFailure(fixture, FailureMessageCodecs.failureMessageCodec.decode(hex"4011".bits).require.value)
+  }
+
   test("payment succeeded") { fixture =>
     import fixture._
     val defaultPaymentHash = randomBytes32