Add failed node ID field to FailureSummary (#2042)

The output of `getsentinfo` didn't include the `nodeId` of the failing node.
This PR adds it, as it can be used by external apps when they build routes
themselves instead of relying on eclair's internals (e.g. channel rebalancing).

docs/release-notes/eclair-vnext.md

@@ -67,6 +67,7 @@ This release contains many other API updates:
 - `findroute`, `findroutetonode` and `findroutebetweennodes` now accept `--ignoreNodeIds` to specify nodes you want to be ignored in path-finding (#1969)
 - `findroute`, `findroutetonode` and `findroutebetweennodes` now accept `--ignoreShortChannelIds` to specify channels you want to be ignored in path-finding (#1969)
 - `findroute`, `findroutetonode` and `findroutebetweennodes` now accept `--maxFeeMsat` to specify an upper bound of fees (#1969)
+- `getsentinfo` output includes `failedNode` field for all failed routes
 
 Have a look at our [API documentation](https://acinq.github.io/eclair) for more details.
 

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

@@ -207,7 +207,9 @@ object HopSummary {
 }
 
 /** A minimal representation of a payment failure (suitable to store in a database). */
-case class FailureSummary(failureType: FailureType.Value, failureMessage: String, failedRoute: List[HopSummary])
+trait GenericFailureSummary
+
+case class FailureSummary(failureType: FailureType.Value, failureMessage: String, failedRoute: List[HopSummary], failedNode: Option[PublicKey]) extends GenericFailureSummary
 
 object FailureType extends Enumeration {
   type FailureType = Value
@@ -218,9 +220,9 @@ object FailureType extends Enumeration {
 
 object FailureSummary {
   def apply(f: PaymentFailure): FailureSummary = f match {
-    case LocalFailure(_, route, t) => FailureSummary(FailureType.LOCAL, t.getMessage, route.map(h => HopSummary(h)).toList)
-    case RemoteFailure(_, route, e) => FailureSummary(FailureType.REMOTE, e.failureMessage.message, route.map(h => HopSummary(h)).toList)
-    case UnreadableRemoteFailure(_, route) => FailureSummary(FailureType.UNREADABLE_REMOTE, "could not decrypt failure onion", route.map(h => HopSummary(h)).toList)
+    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)
   }
 }
 
@@ -262,4 +264,50 @@ case class PlainOutgoingPayment(parentId: Option[UUID],
                                 paymentRequest: Option[String],
                                 status: OutgoingPaymentStatus,
                                 createdAt: TimestampMilli,
-                                completedAt: Option[TimestampMilli]) extends PlainPayment
+                                completedAt: Option[TimestampMilli]) extends PlainPayment
+
+object PaymentsDb {
+
+  import fr.acinq.eclair.wire.protocol.CommonCodecs
+  import scodec.Attempt
+  import scodec.bits.BitVector
+  import scodec.codecs._
+
+  private case class LegacyFailureSummary(failureType: FailureType.Value, failureMessage: String, failedRoute: List[HopSummary]) extends GenericFailureSummary {
+    def toFailureSummary: FailureSummary = FailureSummary(failureType, failureMessage, failedRoute, None)
+  }
+
+  private val hopSummaryCodec = (("node_id" | CommonCodecs.publicKey) :: ("next_node_id" | CommonCodecs.publicKey) :: ("short_channel_id" | optional(bool, CommonCodecs.shortchannelid))).as[HopSummary]
+  val paymentRouteCodec = discriminated[List[HopSummary]].by(byte)
+    .typecase(0x01, listOfN(uint8, hopSummaryCodec))
+  private val legacyFailureSummaryCodec = (("type" | enumerated(uint8, FailureType)) :: ("message" | ascii32) :: paymentRouteCodec).as[LegacyFailureSummary]
+  private val failureSummaryCodec = (("type" | enumerated(uint8, FailureType)) :: ("message" | ascii32) :: paymentRouteCodec :: ("node_id" | optional(bool, CommonCodecs.publicKey))).as[FailureSummary]
+  private val paymentFailuresCodec = discriminated[List[GenericFailureSummary]].by(byte)
+    .typecase(0x02, listOfN(uint8, failureSummaryCodec))
+    .typecase(0x01, listOfN(uint8, legacyFailureSummaryCodec).decodeOnly)
+
+  def encodeRoute(route: List[HopSummary]): Array[Byte] = {
+    paymentRouteCodec.encode(route).require.toByteArray
+  }
+
+  def decodeRoute(b: BitVector): List[HopSummary] = {
+    paymentRouteCodec.decode(b) match {
+      case Attempt.Successful(route) => route.value
+      case Attempt.Failure(_) => Nil
+    }
+  }
+
+  def encodeFailures(failures: List[FailureSummary]): Array[Byte] = {
+    paymentFailuresCodec.encode(failures).require.toByteArray
+  }
+
+  def decodeFailures(b: BitVector): List[FailureSummary] = {
+    paymentFailuresCodec.decode(b) match {
+      case Attempt.Successful(f) => f.value.collect {
+        case failure: FailureSummary => failure
+        case legacy: LegacyFailureSummary => legacy.toFailureSummary
+      }
+      case Attempt.Failure(_) => Nil
+    }
+  }
+}

eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgPaymentsDb.scala

@@ -20,15 +20,13 @@ import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.bitcoin.Crypto.PublicKey
 import fr.acinq.eclair.db.Monitoring.Metrics.withMetrics
 import fr.acinq.eclair.db.Monitoring.Tags.DbBackends
+import fr.acinq.eclair.db.PaymentsDb.{decodeFailures, decodeRoute, encodeFailures, encodeRoute}
 import fr.acinq.eclair.db._
 import fr.acinq.eclair.db.pg.PgUtils.PgLock
 import fr.acinq.eclair.payment.{PaymentFailed, PaymentRequest, PaymentSent}
-import fr.acinq.eclair.wire.protocol.CommonCodecs
 import fr.acinq.eclair.{MilliSatoshi, TimestampMilli, TimestampMilliLong}
 import grizzled.slf4j.Logging
-import scodec.Attempt
 import scodec.bits.BitVector
-import scodec.codecs._
 
 import java.sql.{Connection, ResultSet, Statement, Timestamp}
 import java.time.Instant
@@ -49,13 +47,6 @@ class PgPaymentsDb(implicit ds: DataSource, lock: PgLock) extends PaymentsDb wit
   import PgUtils._
   import lock._
 
-  private val hopSummaryCodec = (("node_id" | CommonCodecs.publicKey) :: ("next_node_id" | CommonCodecs.publicKey) :: ("short_channel_id" | optional(bool, CommonCodecs.shortchannelid))).as[HopSummary]
-  private val paymentRouteCodec = discriminated[List[HopSummary]].by(byte)
-    .typecase(0x01, listOfN(uint8, hopSummaryCodec))
-  private val failureSummaryCodec = (("type" | enumerated(uint8, FailureType)) :: ("message" | ascii32) :: paymentRouteCodec).as[FailureSummary]
-  private val paymentFailuresCodec = discriminated[List[FailureSummary]].by(byte)
-    .typecase(0x01, listOfN(uint8, failureSummaryCodec))
-
   inTransaction { pg =>
     using(pg.createStatement()) { statement =>
 
@@ -128,7 +119,7 @@ class PgPaymentsDb(implicit ds: DataSource, lock: PgLock) extends PaymentsDb wit
           statement.setTimestamp(1, p.timestamp.toSqlTimestamp)
           statement.setString(2, paymentResult.paymentPreimage.toHex)
           statement.setLong(3, p.feesPaid.toLong)
-          statement.setBytes(4, paymentRouteCodec.encode(p.route.getOrElse(Nil).map(h => HopSummary(h)).toList).require.toByteArray)
+          statement.setBytes(4, encodeRoute(p.route.getOrElse(Nil).map(h => HopSummary(h)).toList))
           statement.setString(5, p.id.toString)
           statement.addBatch()
         })
@@ -141,7 +132,7 @@ class PgPaymentsDb(implicit ds: DataSource, lock: PgLock) extends PaymentsDb wit
     withLock { pg =>
       using(pg.prepareStatement("UPDATE payments.sent SET (completed_at, failures) = (?, ?) WHERE id = ? AND completed_at IS NULL")) { statement =>
         statement.setTimestamp(1, paymentResult.timestamp.toSqlTimestamp)
-        statement.setBytes(2, paymentFailuresCodec.encode(paymentResult.failures.map(f => FailureSummary(f)).toList).require.toByteArray)
+        statement.setBytes(2, encodeFailures(paymentResult.failures.map(f => FailureSummary(f)).toList))
         statement.setString(3, paymentResult.id.toString)
         if (statement.executeUpdate() == 0) throw new IllegalArgumentException(s"Tried to mark an outgoing payment as failed but already in final status (id=${paymentResult.id})")
       }
@@ -175,19 +166,13 @@ class PgPaymentsDb(implicit ds: DataSource, lock: PgLock) extends PaymentsDb wit
     preimage_opt match {
       // If we have a pre-image, the payment succeeded.
       case Some(preimage) => OutgoingPaymentStatus.Succeeded(
-        preimage, fees_opt.getOrElse(MilliSatoshi(0)), paymentRoute_opt.map(b => paymentRouteCodec.decode(b) match {
-          case Attempt.Successful(route) => route.value
-          case Attempt.Failure(_) => Nil
-        }).getOrElse(Nil),
+        preimage, fees_opt.getOrElse(MilliSatoshi(0)), paymentRoute_opt.map(decodeRoute).getOrElse(Nil),
         completedAt_opt.getOrElse(0 unixms)
       )
       case None => completedAt_opt match {
         // Otherwise if the payment was marked completed, it's a failure.
         case Some(completedAt) => OutgoingPaymentStatus.Failed(
-          failures.map(b => paymentFailuresCodec.decode(b) match {
-            case Attempt.Successful(f) => f.value
-            case Attempt.Failure(_) => Nil
-          }).getOrElse(Nil),
+          failures.map(decodeFailures).getOrElse(Nil),
           completedAt
         )
         // Else it's still pending.

eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala

@@ -20,15 +20,13 @@ import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey}
 import fr.acinq.eclair.db.Monitoring.Metrics.withMetrics
 import fr.acinq.eclair.db.Monitoring.Tags.DbBackends
+import fr.acinq.eclair.db.PaymentsDb.{decodeFailures, decodeRoute, encodeFailures, encodeRoute}
 import fr.acinq.eclair.db._
 import fr.acinq.eclair.db.sqlite.SqliteUtils._
 import fr.acinq.eclair.payment.{PaymentFailed, PaymentRequest, PaymentSent}
-import fr.acinq.eclair.wire.protocol.CommonCodecs
 import fr.acinq.eclair.{MilliSatoshi, TimestampMilli, TimestampMilliLong}
 import grizzled.slf4j.Logging
-import scodec.Attempt
 import scodec.bits.BitVector
-import scodec.codecs._
 
 import java.sql.{Connection, ResultSet, Statement}
 import java.util.UUID
@@ -145,7 +143,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging {
         statement.setLong(1, p.timestamp.toLong)
         statement.setBytes(2, paymentResult.paymentPreimage.toArray)
         statement.setLong(3, p.feesPaid.toLong)
-        statement.setBytes(4, paymentRouteCodec.encode(p.route.getOrElse(Nil).map(h => HopSummary(h)).toList).require.toByteArray)
+        statement.setBytes(4, encodeRoute(p.route.getOrElse(Nil).map(h => HopSummary(h)).toList))
         statement.setString(5, p.id.toString)
         statement.addBatch()
       })
@@ -156,7 +154,7 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging {
   override def updateOutgoingPayment(paymentResult: PaymentFailed): Unit = withMetrics("payments/update-outgoing-failed", DbBackends.Sqlite) {
     using(sqlite.prepareStatement("UPDATE sent_payments SET (completed_at, failures) = (?, ?) WHERE id = ? AND completed_at IS NULL")) { statement =>
       statement.setLong(1, paymentResult.timestamp.toLong)
-      statement.setBytes(2, paymentFailuresCodec.encode(paymentResult.failures.map(f => FailureSummary(f)).toList).require.toByteArray)
+      statement.setBytes(2, encodeFailures(paymentResult.failures.map(f => FailureSummary(f)).toList))
       statement.setString(3, paymentResult.id.toString)
       if (statement.executeUpdate() == 0) throw new IllegalArgumentException(s"Tried to mark an outgoing payment as failed but already in final status (id=${paymentResult.id})")
     }
@@ -189,19 +187,13 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging {
     preimage_opt match {
       // If we have a pre-image, the payment succeeded.
       case Some(preimage) => OutgoingPaymentStatus.Succeeded(
-        preimage, fees_opt.getOrElse(MilliSatoshi(0)), paymentRoute_opt.map(b => paymentRouteCodec.decode(b) match {
-          case Attempt.Successful(route) => route.value
-          case Attempt.Failure(_) => Nil
-        }).getOrElse(Nil),
+        preimage, fees_opt.getOrElse(MilliSatoshi(0)), paymentRoute_opt.map(decodeRoute).getOrElse(Nil),
         completedAt_opt.getOrElse(0 unixms)
       )
       case None => completedAt_opt match {
         // Otherwise if the payment was marked completed, it's a failure.
         case Some(completedAt) => OutgoingPaymentStatus.Failed(
-          failures.map(b => paymentFailuresCodec.decode(b) match {
-            case Attempt.Successful(f) => f.value
-            case Attempt.Failure(_) => Nil
-          }).getOrElse(Nil),
+          failures.map(decodeFailures).getOrElse(Nil),
           completedAt
         )
         // Else it's still pending.
@@ -413,12 +405,4 @@ class SqlitePaymentsDb(sqlite: Connection) extends PaymentsDb with Logging {
 object SqlitePaymentsDb {
   val DB_NAME = "payments"
   val CURRENT_VERSION = 4
-
-  private val hopSummaryCodec = (("node_id" | CommonCodecs.publicKey) :: ("next_node_id" | CommonCodecs.publicKey) :: ("short_channel_id" | optional(bool, CommonCodecs.shortchannelid))).as[HopSummary]
-  val paymentRouteCodec = discriminated[List[HopSummary]].by(byte)
-    .typecase(0x01, listOfN(uint8, hopSummaryCodec))
-  private val failureSummaryCodec = (("type" | enumerated(uint8, FailureType)) :: ("message" | ascii32) :: paymentRouteCodec).as[FailureSummary]
-  val paymentFailuresCodec = discriminated[List[FailureSummary]].by(byte)
-    .typecase(0x01, listOfN(uint8, failureSummaryCodec))
-
 }

eclair-core/src/test/scala/fr/acinq/eclair/db/PaymentsDbSpec.scala

@@ -214,7 +214,7 @@ class PaymentsDbSpec extends AnyFunSuite {
     val (id1, id2, id3) = (UUID.randomUUID(), UUID.randomUUID(), UUID.randomUUID())
     val parentId = UUID.randomUUID()
     val invoice1 = PaymentRequest(Block.TestnetGenesisBlock.hash, Some(2834 msat), paymentHash1, bobPriv, Left("invoice #1"), CltvExpiryDelta(18), expirySeconds = Some(30))
-    val ps1 = OutgoingPayment(id1, id1, Some("42"), randomBytes32(), PaymentType.Standard, 561 msat, 561 msat, alice, 1000 unixms, None, OutgoingPaymentStatus.Failed(Seq(FailureSummary(FailureType.REMOTE, "no candy for you", List(HopSummary(hop_ab), HopSummary(hop_bc)))), 1020 unixms))
+    val ps1 = OutgoingPayment(id1, id1, Some("42"), randomBytes32(), PaymentType.Standard, 561 msat, 561 msat, alice, 1000 unixms, None, OutgoingPaymentStatus.Failed(Seq(FailureSummary(FailureType.REMOTE, "no candy for you", List(HopSummary(hop_ab), HopSummary(hop_bc)), Some(bob))), 1020 unixms))
     val ps2 = OutgoingPayment(id2, parentId, Some("42"), paymentHash1, PaymentType.Standard, 1105 msat, 1105 msat, bob, 1010 unixms, Some(invoice1), OutgoingPaymentStatus.Pending)
     val ps3 = OutgoingPayment(id3, parentId, None, paymentHash1, PaymentType.Standard, 1729 msat, 1729 msat, bob, 1040 unixms, None, OutgoingPaymentStatus.Succeeded(preimage1, 10 msat, Seq(HopSummary(hop_ab), HopSummary(hop_bc)), 1060 unixms))
 
@@ -244,7 +244,7 @@ class PaymentsDbSpec extends AnyFunSuite {
           statement.setBytes(6, ps1.recipientNodeId.value.toArray)
           statement.setLong(7, ps1.createdAt.toLong)
           statement.setLong(8, ps1.status.asInstanceOf[OutgoingPaymentStatus.Failed].completedAt.toLong)
-          statement.setBytes(9, SqlitePaymentsDb.paymentFailuresCodec.encode(ps1.status.asInstanceOf[OutgoingPaymentStatus.Failed].failures.toList).require.toByteArray)
+          statement.setBytes(9, PaymentsDb.encodeFailures(ps1.status.asInstanceOf[OutgoingPaymentStatus.Failed].failures.toList))
           statement.executeUpdate()
         }
 
@@ -270,7 +270,7 @@ class PaymentsDbSpec extends AnyFunSuite {
           statement.setLong(7, ps3.status.asInstanceOf[OutgoingPaymentStatus.Succeeded].completedAt.toLong)
           statement.setBytes(8, ps3.status.asInstanceOf[OutgoingPaymentStatus.Succeeded].paymentPreimage.toArray)
           statement.setLong(9, ps3.status.asInstanceOf[OutgoingPaymentStatus.Succeeded].feesPaid.toLong)
-          statement.setBytes(10, SqlitePaymentsDb.paymentRouteCodec.encode(ps3.status.asInstanceOf[OutgoingPaymentStatus.Succeeded].route.toList).require.toByteArray)
+          statement.setBytes(10, PaymentsDb.encodeRoute(ps3.status.asInstanceOf[OutgoingPaymentStatus.Succeeded].route.toList))
           statement.executeUpdate()
         }
 
@@ -492,7 +492,7 @@ class PaymentsDbSpec extends AnyFunSuite {
       val ss3 = s3.copy(status = OutgoingPaymentStatus.Failed(Nil, 310 unixms))
       assert(db.getOutgoingPayment(s3.id) === Some(ss3))
       db.updateOutgoingPayment(PaymentFailed(s4.id, s4.paymentHash, Seq(LocalFailure(s4.amount, Seq(hop_ab), new RuntimeException("woops")), RemoteFailure(s4.amount, Seq(hop_ab, hop_bc), Sphinx.DecryptedFailurePacket(carol, UnknownNextPeer))), 320 unixms))
-      val ss4 = s4.copy(status = OutgoingPaymentStatus.Failed(Seq(FailureSummary(FailureType.LOCAL, "woops", List(HopSummary(alice, bob, Some(ShortChannelId(42))))), FailureSummary(FailureType.REMOTE, "processing node does not know the next peer in the route", List(HopSummary(alice, bob, Some(ShortChannelId(42))), HopSummary(bob, carol, None)))), 320 unixms))
+      val ss4 = s4.copy(status = OutgoingPaymentStatus.Failed(Seq(FailureSummary(FailureType.LOCAL, "woops", List(HopSummary(alice, bob, Some(ShortChannelId(42)))), Some(alice)), FailureSummary(FailureType.REMOTE, "processing node does not know the next peer in the route", List(HopSummary(alice, bob, Some(ShortChannelId(42))), HopSummary(bob, carol, None)), Some(carol))), 320 unixms))
       assert(db.getOutgoingPayment(s4.id) === Some(ss4))
 
       // can't update again once it's in a final state

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

@@ -440,7 +440,7 @@ class MultiPartPaymentLifecycleSpec extends TestKitBaseClass with FixtureAnyFunS
 
     val Some(outgoing) = nodeParams.db.payments.getOutgoingPayment(cfg.id)
     assert(outgoing.status.isInstanceOf[OutgoingPaymentStatus.Failed])
-    assert(outgoing.status.asInstanceOf[OutgoingPaymentStatus.Failed].failures === Seq(FailureSummary(FailureType.LOCAL, RouteNotFound.getMessage, Nil)))
+    assert(outgoing.status.asInstanceOf[OutgoingPaymentStatus.Failed].failures === Seq(FailureSummary(FailureType.LOCAL, RouteNotFound.getMessage, Nil, None)))
 
     sender.expectTerminated(payFsm)
     sender.expectNoMessage(100 millis)