Refactor Sphinx failures

docs/Guides.md

@@ -7,7 +7,6 @@ This section contains how-to guides for more advanced scenarios:
 * [Manage Bitcoin Core's private keys](./ManagingBitcoinCoreKeys.md)
 * [Use Tor with Eclair](./Tor.md)
 * [Multipart Payments](./MultipartPayments.md)
-* [Trampoline Payments](./TrampolinePayments.md)
 * [Monitoring Eclair](./Monitoring.md)
 * [PostgreSQL Configuration](./PostgreSQL.md)
 * [Perform Circular Rebalancing](./CircularRebalancing.md)

eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala

@@ -26,7 +26,7 @@ import fr.acinq.eclair.channel.fund.{InteractiveTxBuilder, InteractiveTxSigningS
 import fr.acinq.eclair.io.Peer
 import fr.acinq.eclair.transactions.CommitmentSpec
 import fr.acinq.eclair.transactions.Transactions._
-import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelReady, ChannelReestablish, ChannelUpdate, ClosingSigned, CommitSig, FailureMessage, FundingCreated, FundingSigned, Init, LiquidityAds, OnionRoutingPacket, OpenChannel, OpenDualFundedChannel, Shutdown, SpliceInit, Stfu, TxInitRbf, TxSignatures, UpdateAddHtlc, UpdateFailHtlc, UpdateFailMalformedHtlc, UpdateFulfillHtlc}
+import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelReady, ChannelReestablish, ChannelUpdate, ClosingSigned, CommitSig, FailureReason, FundingCreated, FundingSigned, Init, LiquidityAds, OnionRoutingPacket, OpenChannel, OpenDualFundedChannel, Shutdown, SpliceInit, Stfu, TxInitRbf, TxSignatures, UpdateAddHtlc, UpdateFailHtlc, UpdateFailMalformedHtlc, UpdateFulfillHtlc}
 import fr.acinq.eclair.{Alias, BlockHeight, CltvExpiry, CltvExpiryDelta, Features, InitFeature, MilliSatoshi, MilliSatoshiLong, RealShortChannelId, TimestampMilli, UInt64}
 import scodec.bits.ByteVector
 
@@ -215,7 +215,7 @@ final case class CMD_ADD_HTLC(replyTo: ActorRef,
 
 sealed trait HtlcSettlementCommand extends HasOptionalReplyToCommand with ForbiddenCommandDuringQuiescenceNegotiation with ForbiddenCommandWhenQuiescent { def id: Long }
 final case class CMD_FULFILL_HTLC(id: Long, r: ByteVector32, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand
-final case class CMD_FAIL_HTLC(id: Long, reason: Either[ByteVector, FailureMessage], delay_opt: Option[FiniteDuration] = None, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand
+final case class CMD_FAIL_HTLC(id: Long, reason: FailureReason, delay_opt: Option[FiniteDuration] = None, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand
 final case class CMD_FAIL_MALFORMED_HTLC(id: Long, onionHash: ByteVector32, failureCode: Int, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand
 final case class CMD_UPDATE_FEE(feeratePerKw: FeeratePerKw, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HasOptionalReplyToCommand with ForbiddenCommandDuringQuiescenceNegotiation with ForbiddenCommandWhenQuiescent
 final case class CMD_SIGN(replyTo_opt: Option[ActorRef] = None) extends HasOptionalReplyToCommand with ForbiddenCommandWhenQuiescent

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala

@@ -643,7 +643,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
             case PostRevocationAction.RejectHtlc(add) =>
               log.debug("rejecting incoming htlc {}", add)
               // NB: we don't set commit = true, we will sign all updates at once afterwards.
-              self ! CMD_FAIL_HTLC(add.id, Right(TemporaryChannelFailure(Some(d.channelUpdate))), commit = true)
+              self ! CMD_FAIL_HTLC(add.id, FailureReason.LocalFailure(TemporaryChannelFailure(Some(d.channelUpdate))), commit = true)
             case PostRevocationAction.RelayFailure(result) =>
               log.debug("forwarding {} to relayer", result)
               relayer ! result
@@ -1544,11 +1544,11 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
             case PostRevocationAction.RelayHtlc(add) =>
               // BOLT 2: A sending node SHOULD fail to route any HTLC added after it sent shutdown.
               log.debug("closing in progress: failing {}", add)
-              self ! CMD_FAIL_HTLC(add.id, Right(PermanentChannelFailure()), commit = true)
+              self ! CMD_FAIL_HTLC(add.id, FailureReason.LocalFailure(PermanentChannelFailure()), commit = true)
             case PostRevocationAction.RejectHtlc(add) =>
               // BOLT 2: A sending node SHOULD fail to route any HTLC added after it sent shutdown.
               log.debug("closing in progress: rejecting {}", add)
-              self ! CMD_FAIL_HTLC(add.id, Right(PermanentChannelFailure()), commit = true)
+              self ! CMD_FAIL_HTLC(add.id, FailureReason.LocalFailure(PermanentChannelFailure()), commit = true)
             case PostRevocationAction.RelayFailure(result) =>
               log.debug("forwarding {} to relayer", result)
               relayer ! result

eclair-core/src/main/scala/fr/acinq/eclair/crypto/Sphinx.scala

@@ -272,6 +272,13 @@ object Sphinx extends Logging {
    */
   case class DecryptedFailurePacket(originNode: PublicKey, failureMessage: FailureMessage)
 
+  /**
+   * The downstream failure could not be decrypted.
+   *
+   * @param unwrapped encrypted failure packet after unwrapping using our shared secrets.
+   */
+  case class CannotDecryptFailurePacket(unwrapped: ByteVector)
+
   object FailurePacket {
 
     /**
@@ -314,18 +321,18 @@ object Sphinx extends Logging {
      *
      * @param packet        failure packet.
      * @param sharedSecrets nodes shared secrets.
-     * @return Success(secret, failure message) if the origin of the packet could be identified and the packet
-     *         decrypted, Failure otherwise.
+     * @return failure message if the origin of the packet could be identified and the packet decrypted, the unwrapped
+     *         failure packet otherwise.
      */
-    def decrypt(packet: ByteVector, sharedSecrets: Seq[(ByteVector32, PublicKey)]): Try[DecryptedFailurePacket] = Try {
+    def decrypt(packet: ByteVector, sharedSecrets: Seq[(ByteVector32, PublicKey)]): Either[CannotDecryptFailurePacket, DecryptedFailurePacket] = {
       @tailrec
-      def loop(packet: ByteVector, secrets: Seq[(ByteVector32, PublicKey)]): DecryptedFailurePacket = secrets match {
-        case Nil => throw new RuntimeException(s"couldn't parse error packet=$packet with sharedSecrets=$sharedSecrets")
+      def loop(packet: ByteVector, secrets: Seq[(ByteVector32, PublicKey)]): Either[CannotDecryptFailurePacket, DecryptedFailurePacket] = secrets match {
+        case Nil => Left(CannotDecryptFailurePacket(packet))
         case (secret, pubkey) :: tail =>
           val packet1 = wrap(packet, secret)
           val um = generateKey("um", secret)
           FailureMessageCodecs.failureOnionCodec(Hmac256(um)).decode(packet1.toBitVector) match {
-            case Attempt.Successful(value) => DecryptedFailurePacket(pubkey, value.value)
+            case Attempt.Successful(value) => Right(DecryptedFailurePacket(pubkey, value.value))
             case _ => loop(packet1, tail)
           }
       }
@@ -359,10 +366,10 @@ object Sphinx extends Logging {
     case class BlindedHop(blindedPublicKey: PublicKey, encryptedPayload: ByteVector)
 
     /**
-     * @param firstNodeId        the first node, not blinded so that the sender can locate it.
-     * @param firstPathKey       blinding tweak that can be used by the introduction node to derive the private key that
-     *                           matches the blinded public key.
-     * @param blindedHops       blinded nodes (including the introduction node).
+     * @param firstNodeId  the first node, not blinded so that the sender can locate it.
+     * @param firstPathKey blinding tweak that can be used by the introduction node to derive the private key that
+     *                     matches the blinded public key.
+     * @param blindedHops  blinded nodes (including the introduction node).
      */
     case class BlindedRoute(firstNodeId: EncodedNodeId, firstPathKey: PublicKey, blindedHops: Seq[BlindedHop]) {
       require(blindedHops.nonEmpty, "blinded route must not be empty")

eclair-core/src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala

@@ -250,7 +250,7 @@ object FailureSummary {
   def apply(f: PaymentFailure): FailureSummary = f match {
     case LocalFailure(_, route, t) => FailureSummary(FailureType.LOCAL, t.getMessage, route.map(h => HopSummary(h)).toList, route.headOption.map(_.nodeId))
     case RemoteFailure(_, route, e) => FailureSummary(FailureType.REMOTE, e.failureMessage.message, route.map(h => HopSummary(h)).toList, Some(e.originNode))
-    case UnreadableRemoteFailure(_, route) => FailureSummary(FailureType.UNREADABLE_REMOTE, "could not decrypt failure onion", route.map(h => HopSummary(h)).toList, None)
+    case UnreadableRemoteFailure(_, route, _) => FailureSummary(FailureType.UNREADABLE_REMOTE, "could not decrypt failure onion", route.map(h => HopSummary(h)).toList, None)
   }
 }
 

eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala

@@ -44,7 +44,7 @@ import fr.acinq.eclair.remote.EclairInternalsSerializer.RemoteTypes
 import fr.acinq.eclair.router.Router
 import fr.acinq.eclair.wire.protocol
 import fr.acinq.eclair.wire.protocol.FailureMessageCodecs.createBadOnionFailure
-import fr.acinq.eclair.wire.protocol.{AddFeeCredit, ChannelTlv, CurrentFeeCredit, Error, HasChannelId, HasTemporaryChannelId, LightningMessage, LiquidityAds, NodeAddress, OnTheFlyFundingFailureMessage, OnionMessage, OnionRoutingPacket, RecommendedFeerates, RoutingMessage, SpliceInit, TemporaryChannelFailure, TlvStream, TxAbort, UnknownMessage, Warning, WillAddHtlc, WillFailHtlc, WillFailMalformedHtlc}
+import fr.acinq.eclair.wire.protocol.{AddFeeCredit, ChannelTlv, CurrentFeeCredit, Error, FailureReason, HasChannelId, HasTemporaryChannelId, LightningMessage, LiquidityAds, NodeAddress, OnTheFlyFundingFailureMessage, OnionMessage, OnionRoutingPacket, RecommendedFeerates, RoutingMessage, SpliceInit, TemporaryChannelFailure, TlvStream, TxAbort, UnknownMessage, Warning, WillAddHtlc, WillFailHtlc, WillFailMalformedHtlc}
 
 /**
  * This actor represents a logical peer. There is one [[Peer]] per unique remote node id at all time.
@@ -300,8 +300,8 @@ class Peer(val nodeParams: NodeParams,
                 pending.proposed.find(_.htlc.id == msg.id) match {
                   case Some(htlc) =>
                     val failure = msg match {
-                      case msg: WillFailHtlc => Left(msg.reason)
-                      case msg: WillFailMalformedHtlc => Right(createBadOnionFailure(msg.onionHash, msg.failureCode))
+                      case msg: WillFailHtlc => FailureReason.EncryptedDownstreamFailure(msg.reason)
+                      case msg: WillFailMalformedHtlc => FailureReason.LocalFailure(createBadOnionFailure(msg.onionHash, msg.failureCode))
                     }
                     htlc.createFailureCommands(Some(failure)).foreach { case (channelId, cmd) => PendingCommandsDb.safeSend(register, nodeParams.db.pendingCommands, channelId, cmd) }
                     val proposed1 = pending.proposed.filterNot(_.htlc.id == msg.id)

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

@@ -19,6 +19,7 @@ package fr.acinq.eclair.payment
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.eclair.MilliSatoshi
 import fr.acinq.eclair.channel.CMD_FAIL_HTLC
+import fr.acinq.eclair.wire.protocol.FailureReason
 import kamon.Kamon
 
 object Monitoring {
@@ -127,14 +128,14 @@ object Monitoring {
       val Malformed = "MalformedHtlc"
 
       def apply(cmdFail: CMD_FAIL_HTLC): String = cmdFail.reason match {
-        case Left(_) => Remote
-        case Right(f) => f.getClass.getSimpleName
+        case _: FailureReason.EncryptedDownstreamFailure => Remote
+        case FailureReason.LocalFailure(f) => f.getClass.getSimpleName
       }
 
       def apply(pf: PaymentFailure): String = pf match {
         case LocalFailure(_, _, t) => t.getClass.getSimpleName
         case RemoteFailure(_, _, e) => e.failureMessage.getClass.getSimpleName
-        case UnreadableRemoteFailure(_, _) => "UnreadableRemoteFailure"
+        case _: UnreadableRemoteFailure => "UnreadableRemoteFailure"
       }
     }
 

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

@@ -150,7 +150,7 @@ case class LocalFailure(amount: MilliSatoshi, route: Seq[Hop], t: Throwable) ext
 case class RemoteFailure(amount: MilliSatoshi, route: Seq[Hop], e: Sphinx.DecryptedFailurePacket) extends PaymentFailure
 
 /** A remote node failed the payment but we couldn't decrypt the failure (e.g. a malicious node tampered with the message). */
-case class UnreadableRemoteFailure(amount: MilliSatoshi, route: Seq[Hop]) extends PaymentFailure
+case class UnreadableRemoteFailure(amount: MilliSatoshi, route: Seq[Hop], failurePacket: ByteVector) extends PaymentFailure
 
 object PaymentFailure {
 
@@ -235,7 +235,7 @@ object PaymentFailure {
       }
     case RemoteFailure(_, hops, Sphinx.DecryptedFailurePacket(nodeId, _)) =>
       ignoreNodeOutgoingEdge(nodeId, hops, ignore)
-    case UnreadableRemoteFailure(_, hops) =>
+    case UnreadableRemoteFailure(_, hops, _) =>
       // We don't know which node is sending garbage, let's blacklist all nodes except:
       //  - the one we are directly connected to: it would be too restrictive for retries
       //  - the final recipient: they have no incentive to send garbage since they want that payment

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

@@ -238,7 +238,8 @@ object IncomingPaymentPacket {
         case innerPayload =>
           // We merge contents from the outer and inner payloads.
           // We must use the inner payload's total amount and payment secret because the payment may be split between multiple trampoline payments (#reckless).
-          Right(FinalPacket(add, FinalPayload.Standard.createPayload(outerPayload.amount, innerPayload.totalAmount, innerPayload.expiry, innerPayload.paymentSecret, innerPayload.paymentMetadata)))
+          val trampolinePacket = outerPayload.records.get[OnionPaymentPayloadTlv.TrampolineOnion].map(_.packet)
+          Right(FinalPacket(add, FinalPayload.Standard.createPayload(outerPayload.amount, innerPayload.totalAmount, innerPayload.expiry, innerPayload.paymentSecret, innerPayload.paymentMetadata, trampolinePacket)))
       }
     }
   }
@@ -334,14 +335,24 @@ object OutgoingPaymentPacket {
     }
   }
 
-  private def buildHtlcFailure(nodeSecret: PrivateKey, reason: Either[ByteVector, FailureMessage], add: UpdateAddHtlc): Either[CannotExtractSharedSecret, ByteVector] = {
+  private def buildHtlcFailure(nodeSecret: PrivateKey, reason: FailureReason, add: UpdateAddHtlc): Either[CannotExtractSharedSecret, ByteVector] = {
+    extractSharedSecret(nodeSecret, add).map(sharedSecret => {
+      reason match {
+        case FailureReason.EncryptedDownstreamFailure(packet) => Sphinx.FailurePacket.wrap(packet, sharedSecret)
+        case FailureReason.LocalFailure(failure) => Sphinx.FailurePacket.create(sharedSecret, failure)
+      }
+    })
+  }
+
+  /**
+   * We decrypt the onion again to extract the shared secret used to encrypt onion failures.
+   * We could avoid this by storing the shared secret after the initial onion decryption, but we would have to store it
+   * in the database since we must be able to fail HTLCs after restarting our node.
+   * It's simpler to extract it again from the encrypted onion.
+   */
+  private def extractSharedSecret(nodeSecret: PrivateKey, add: UpdateAddHtlc): Either[CannotExtractSharedSecret, ByteVector32] = {
     Sphinx.peel(nodeSecret, Some(add.paymentHash), add.onionRoutingPacket) match {
-      case Right(Sphinx.DecryptedPacket(_, _, sharedSecret)) =>
-        val encryptedReason = reason match {
-          case Left(forwarded) => Sphinx.FailurePacket.wrap(forwarded, sharedSecret)
-          case Right(failure) => Sphinx.FailurePacket.create(sharedSecret, failure)
-        }
-        Right(encryptedReason)
+      case Right(Sphinx.DecryptedPacket(_, _, sharedSecret)) => Right(sharedSecret)
       case Left(_) => Left(CannotExtractSharedSecret(add.channelId, add))
     }
   }