Refactor Sphinx failures (#2955)

* Return unwrapped decryption failure onion

When we fail to decrypt an onion failure packet, we should return the
result of the unwrapping process. When using trampoline, this will let
us properly re-encrypt the failure and relay it upstream to the previous
trampoline node, until it reaches the sender.

* Refactor HTLC failure creation

We refactor the shared secret extraction to a dedicated function.

* Refactor HTLC failure reason

We previously used an `Either[ByteVector, FailureMessage]` to encode:

- a downstream error that we couldn't decrypt and must re-wrap (left)
- a local error that we must encrypt (right)

This won't be sufficient for trampoline, because we will need to handle
the following cases:

- a downstream error that we couldn't decrypt and must re-wrap
- a local error for the node who created the *outer* onion (which we
  encrypt with the sphinx shared secret of the outer onion)
- a local error for the node who created the *trampoline* onion (which
  we encrypt with the sphinx shared secret of the trampoline onion and
  then with the shared secret of the outer onion)

We thus introduce a trait, which currently only contains the first two
cases. We will extend this trait when adding support for trampoline
failures. This is a pure refactoring without any behavior changes so
far, which will simplify the future trampoline changes.

* Include trampoline onion in final trampoline payloads

When we receive a (non-blinded) payment that uses trampoline, we keep
the trampoline onion to be able to distinguish this payment from a
non-trampoline payment.

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))
     }
   }