From b00080dc261564a98e73cc1ee1fea3765e13f7ff Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Fri, 17 Jun 2022 13:56:55 +0200
Subject: [PATCH 1/6] Use local alias in stored channel update

We now use either our local alias or the real scid in the channel update
that we store internally. When we send that channel update directly to
our peer, we override it to use the remote alias when it makes sense.
---
 .../fr/acinq/eclair/channel/Helpers.scala     |  36 ++++--
 .../fr/acinq/eclair/channel/fsm/Channel.scala |   2 +-
 .../acinq/eclair/router/Announcements.scala   |  17 +--
 .../c/WaitForChannelReadyStateSpec.scala      | 104 ++++++++++++------
 .../channel/states/e/NormalStateSpec.scala    |  46 ++++----
 .../router/ChannelRouterIntegrationSpec.scala |  13 ++-
 6 files changed, 137 insertions(+), 81 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 357f418a62..9e9f452d80 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -22,7 +22,6 @@ import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey, sha256}
 import fr.acinq.bitcoin.scalacompat.Script._
 import fr.acinq.bitcoin.scalacompat._
 import fr.acinq.eclair._
-import fr.acinq.eclair.blockchain.OnChainAddressGenerator
 import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets, FeeratePerKw}
 import fr.acinq.eclair.channel.fsm.Channel
 import fr.acinq.eclair.channel.fsm.Channel.{ChannelConf, REFRESH_CHANNEL_UPDATE_INTERVAL}
@@ -39,7 +38,6 @@ import fr.acinq.eclair.wire.protocol._
 import scodec.bits.ByteVector
 
 import scala.concurrent.duration._
-import scala.concurrent.{Await, ExecutionContext}
 import scala.util.{Failure, Success, Try}
 
 /**
@@ -186,28 +184,46 @@ object Helpers {
   }
 
   /**
-   * The general rule is that we use remote_alias for our channel_update until the channel is publicly announced, and
+   * The general rule is that we use our local alias for our channel_update until the channel is publicly announced, and
    * then we use the real scid.
    *
-   * Private channels are handled like public channels that have not yet been announced, there is no special case.
+   * Private channels are handled like public channels that have not yet been announced, unless our peer peer does not
+   * support scid_alias.
    *
    * Decision tree:
-   *  - received remote_alias from peer
-   *    - before channel announcement: use remote_alias
+   *  - received remote_alias from peer (which indicates that they support scid_alias)
+   *    - before channel announcement: use local alias
    *    - after channel announcement: use real scid
    *  - no remote_alias from peer
    *    - min_depth > 0: use real scid (may change if reorg between min_depth and 6 conf)
    *    - min_depth = 0 (zero-conf): spec violation, our peer MUST send an alias when using zero-conf
    */
   def scidForChannelUpdate(channelAnnouncement_opt: Option[ChannelAnnouncement], shortIds: ShortIds): ShortChannelId = {
-    channelAnnouncement_opt.map(_.shortChannelId) // we use the real "final" scid when it is publicly announced
-      .orElse(shortIds.remoteAlias_opt) // otherwise the remote alias
-      .orElse(shortIds.real.toOption) // if we don't have a remote alias, we use the real scid (which could change because the funding tx possibly has less than 6 confs here)
-      .getOrElse(throw new RuntimeException("this is a zero-conf channel and no alias was provided in channel_ready")) // if we don't have a real scid, it means this is a zero-conf channel and our peer must have sent an alias
+    channelAnnouncement_opt match {
+      case Some(ann) => ann.shortChannelId // we use the real "final" scid when it is publicly announced
+      case None => shortIds.remoteAlias_opt match {
+        case Some(_) => shortIds.localAlias // we use our local alias if the channel isn't announced and our peer supports it
+        case None => shortIds.real.toOption match {
+          case Some(real) => real // we use the real scid if our peer doesn't support scid_alias and the channel is confirmed
+          case None => shortIds.localAlias // spec violation: this is a 0-conf channel, our peer MUST send their alias, we just use ours
+        }
+      }
+    }
   }
 
   def scidForChannelUpdate(d: DATA_NORMAL): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds)
 
+  /**
+   * If our peer sent us an alias, that's what we must use in the channel_update we send them to ensure they're able to
+   * match this update with the corresponding local channel.
+   */
+  def channelUpdateForDirectPeer(nodeParams: NodeParams, channelUpdate: ChannelUpdate, remoteAlias_opt: Option[Alias]): ChannelUpdate = {
+    remoteAlias_opt match {
+      case Some(remoteAlias) => Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = remoteAlias))
+      case None => channelUpdate
+    }
+  }
+
   /**
    * Compute the delay until we need to refresh the channel_update for our channel not to be considered stale by
    * other nodes.
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
index bab02b5eae..71a9618991 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
@@ -1640,7 +1640,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
           val lcu = LocalChannelUpdate(self, d.channelId, d.shortIds, d.commitments.remoteParams.nodeId, d.channelAnnouncement, d.channelUpdate, d.commitments)
           context.system.eventStream.publish(lcu)
           if (sendToPeer) {
-            send(d.channelUpdate)
+            send(Helpers.channelUpdateForDirectPeer(nodeParams, d.channelUpdate, d.shortIds.remoteAlias_opt))
           }
         case EmitLocalChannelDown(d) =>
           log.debug(s"emitting channel down event")
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
index da8381840c..9c000ea5d4 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
@@ -18,9 +18,8 @@ package fr.acinq.eclair.router
 
 import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey, sha256, verifySignature}
 import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, LexicographicalOrdering}
-import fr.acinq.eclair.RealShortChannelId
 import fr.acinq.eclair.wire.protocol._
-import fr.acinq.eclair.{CltvExpiryDelta, Feature, Features, MilliSatoshi, NodeFeature, ShortChannelId, TimestampSecond, TimestampSecondLong, serializationResult}
+import fr.acinq.eclair.{CltvExpiryDelta, Feature, Features, MilliSatoshi, NodeFeature, RealShortChannelId, ShortChannelId, TimestampSecond, TimestampSecondLong, serializationResult}
 import scodec.bits.ByteVector
 import shapeless.HNil
 
@@ -118,11 +117,8 @@ object Announcements {
   def makeChannelUpdate(chainHash: ByteVector32, nodeSecret: PrivateKey, remoteNodeId: PublicKey, shortChannelId: ShortChannelId, cltvExpiryDelta: CltvExpiryDelta, htlcMinimumMsat: MilliSatoshi, feeBaseMsat: MilliSatoshi, feeProportionalMillionths: Long, htlcMaximumMsat: MilliSatoshi, enable: Boolean = true, timestamp: TimestampSecond = TimestampSecond.now()): ChannelUpdate = {
     val channelFlags = ChannelUpdate.ChannelFlags(isNode1 = isNode1(nodeSecret.publicKey, remoteNodeId), isEnabled = enable)
     val htlcMaximumMsatOpt = Some(htlcMaximumMsat)
-
-    val witness = channelUpdateWitnessEncode(chainHash, shortChannelId, timestamp, channelFlags, cltvExpiryDelta, htlcMinimumMsat, feeBaseMsat, feeProportionalMillionths, htlcMaximumMsatOpt, TlvStream.empty)
-    val sig = Crypto.sign(witness, nodeSecret)
-    ChannelUpdate(
-      signature = sig,
+    val u = ChannelUpdate(
+      signature = ByteVector64.Zeroes,
       chainHash = chainHash,
       shortChannelId = shortChannelId,
       timestamp = timestamp,
@@ -133,6 +129,13 @@ object Announcements {
       feeProportionalMillionths = feeProportionalMillionths,
       htlcMaximumMsat = htlcMaximumMsatOpt
     )
+    signChannelUpdate(nodeSecret, u)
+  }
+
+  def signChannelUpdate(nodeSecret: PrivateKey, u: ChannelUpdate): ChannelUpdate = {
+    val witness = channelUpdateWitnessEncode(u.chainHash, u.shortChannelId, u.timestamp, u.channelFlags, u.cltvExpiryDelta, u.htlcMinimumMsat, u.feeBaseMsat, u.feeProportionalMillionths, u.htlcMaximumMsat, u.tlvStream)
+    val sig = Crypto.sign(witness, nodeSecret)
+    u.copy(signature = sig)
   }
 
   def checkSigs(ann: ChannelAnnouncement): Boolean = {
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
index 167c75563d..ecc4dfba3c 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
@@ -25,6 +25,7 @@ import fr.acinq.eclair.channel.fsm.Channel
 import fr.acinq.eclair.channel.publish.TxPublisher
 import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags}
 import fr.acinq.eclair.payment.relay.Relayer.RelayFees
+import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.wire.protocol._
 import fr.acinq.eclair.{BlockHeight, MilliSatoshiLong, TestConstants, TestKitBaseClass}
 import org.scalatest.funsuite.FixtureAnyFunSuiteLike
@@ -91,16 +92,22 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
   test("recv ChannelReady") { f =>
     import f._
     // we have a real scid at this stage, because this isn't a zero-conf channel
-    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary])
-    assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary])
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(aliceIds.real.isInstanceOf[RealScidStatus.Temporary])
+    val bobIds = bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(bobIds.real.isInstanceOf[RealScidStatus.Temporary])
     val channelReady = bob2alice.expectMsgType[ChannelReady]
+    assert(channelReady.alias_opt.contains(bobIds.localAlias))
     bob2alice.forward(alice)
-    val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate]
-    assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate)
-    // we have a real scid, but the channel is not announced so alice uses bob's alias
-    assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get)
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    // we have a real scid, but the channel is not announced so alice uses bob's alias
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    assert(channelUpdateSentToPeer.shortChannelId == bobIds.localAlias)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    assert(Announcements.checkSig(channelUpdateSentToPeer, alice.underlyingActor.nodeParams.nodeId))
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
@@ -109,16 +116,18 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
   test("recv ChannelReady (no alias)") { f =>
     import f._
     // we have a real scid at this stage, because this isn't a zero-conf channel
-    val realScid = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    val realScid = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
     val channelReady = bob2alice.expectMsgType[ChannelReady]
     val channelReadyNoAlias = channelReady.modify(_.tlvStream.records).using(_.filterNot(_.isInstanceOf[ChannelReadyTlv.ShortChannelIdTlv]))
     bob2alice.forward(alice, channelReadyNoAlias)
-    val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate]
-    assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate)
     // the channel is not announced but bob didn't send an alias so we use the real scid
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
     assert(initialChannelUpdate.shortChannelId == realScid)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    assert(channelUpdateSentToPeer == initialChannelUpdate)
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
@@ -127,16 +136,22 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
   test("recv ChannelReady (zero-conf)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f =>
     import f._
     // zero-conf channel: we don't have a real scid
-    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
-    assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(aliceIds.real == RealScidStatus.Unknown)
+    val bobIds = bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(bobIds.real == RealScidStatus.Unknown)
     val channelReady = bob2alice.expectMsgType[ChannelReady]
+    assert(channelReady.alias_opt.contains(bobIds.localAlias))
     bob2alice.forward(alice)
-    val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate]
-    assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate)
-    // the channel is not announced so alice uses bob's alias (we have a no real scid anyway)
-    assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get)
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    // the channel is not announced so alice uses bob's alias (we have a no real scid anyway)
+    assert(channelUpdateSentToPeer.shortChannelId == bobIds.localAlias)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    assert(Announcements.checkSig(channelUpdateSentToPeer, alice.underlyingActor.nodeParams.nodeId))
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
@@ -145,33 +160,48 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
   test("recv ChannelReady (zero-conf, no alias)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f =>
     import f._
     // zero-conf channel: we don't have a real scid
-    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
-    assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(aliceIds.real == RealScidStatus.Unknown)
+    val bobIds = bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(bobIds.real == RealScidStatus.Unknown)
     val channelReady = bob2alice.expectMsgType[ChannelReady]
     val channelReadyNoAlias = channelReady.modify(_.tlvStream.records).using(_.filterNot(_.isInstanceOf[ChannelReadyTlv.ShortChannelIdTlv]))
     bob2alice.forward(alice, channelReadyNoAlias)
-    awaitCond(alice.stateName == CLOSING)
-    assert(alice2blockchain.expectMsgType[TxPublisher.PublishFinalTx].desc == "commit-tx")
-    assert(alice2blockchain.expectMsgType[TxPublisher.PublishTx].desc == "local-anchor")
-    assert(alice2blockchain.expectMsgType[TxPublisher.PublishFinalTx].desc == "local-main-delayed")
-    alice2blockchain.expectMsgType[WatchTxConfirmed]
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
+    assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
+    assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    // the channel is 0-conf but bob didn't provide an alias: it's a spec violation, so we use our local alias and if
+    // they can't understand it, too bad for them
+    assert(channelUpdateSentToPeer.shortChannelId == aliceIds.localAlias)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
+    bob2alice.expectNoMessage(100 millis)
+    awaitCond(alice.stateName == NORMAL)
   }
 
   test("recv ChannelReady (public)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>
     import f._
     // we have a real scid at this stage, because this isn't a zero-conf channel
-    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary])
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(aliceIds.real.isInstanceOf[RealScidStatus.Temporary])
     assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].commitments.channelFlags.announceChannel)
-    assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary])
+    val bobIds = bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(bobIds.real.isInstanceOf[RealScidStatus.Temporary])
     assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].commitments.channelFlags.announceChannel)
     val channelReady = bob2alice.expectMsgType[ChannelReady]
+    assert(channelReady.alias_opt.contains(bobIds.localAlias))
     bob2alice.forward(alice)
-    val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate]
-    assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate)
-    // we have a real scid, but it is not the final one (less than 6 confirmations) so alice uses bob's alias
-    assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get)
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    // we have a real scid, but it is not the final one (less than 6 confirmations) so alice uses bob's alias
+    assert(channelUpdateSentToPeer.shortChannelId == bobIds.localAlias)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    assert(Announcements.checkSig(channelUpdateSentToPeer, alice.underlyingActor.nodeParams.nodeId))
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
@@ -180,16 +210,22 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
   test("recv ChannelReady (public, zero-conf)", Tag(ChannelStateTestsTags.ChannelsPublic), Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f =>
     import f._
     // zero-conf channel: we don't have a real scid
-    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
-    assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown)
+    val aliceIds = alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(aliceIds.real == RealScidStatus.Unknown)
+    val bobIds = bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds
+    assert(bobIds.real == RealScidStatus.Unknown)
     val channelReady = bob2alice.expectMsgType[ChannelReady]
+    assert(channelReady.alias_opt.contains(bobIds.localAlias))
     bob2alice.forward(alice)
-    val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate]
-    assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate)
-    // the channel is not announced, so alice uses bob's alias (we have a no real scid anyway)
-    assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get)
+    val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
+    // the channel is not announced, so alice uses bob's alias (we have a no real scid anyway)
+    assert(channelUpdateSentToPeer.shortChannelId == bobIds.localAlias)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    assert(Announcements.checkSig(channelUpdateSentToPeer, alice.underlyingActor.nodeParams.nodeId))
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
index 8ff235ff85..0456fd499a 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
@@ -844,7 +844,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     alice2bob.expectMsgType[CommitSig]
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.isLeft)
     val waitForRevocation = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.left.toOption.get
-    assert(waitForRevocation.reSignAsap == false)
+    assert(!waitForRevocation.reSignAsap)
 
     // actual test starts here
     alice ! CMD_SIGN()
@@ -861,7 +861,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     alice2bob.expectMsgType[CommitSig]
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.isLeft)
     val waitForRevocation = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.left.toOption.get
-    assert(waitForRevocation.reSignAsap == false)
+    assert(!waitForRevocation.reSignAsap)
 
     // actual test starts here
     addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
@@ -1221,7 +1221,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
     alice ! CMD_SIGN()
     sender.expectNoMessage(300 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.left.toOption.get.reSignAsap == true)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteNextCommitInfo.left.toOption.get.reSignAsap)
 
     // actual test starts here
     bob2alice.expectMsgType[RevokeAndAck]
@@ -1364,7 +1364,6 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
 
   test("recv RevokeAndAck (over max dust htlc exposure in local commit only with pending local changes)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.HighDustLimitDifferenceAliceBob)) { f =>
     import f._
-    val sender = TestProbe()
     assert(alice.underlyingActor.nodeParams.channelConf.dustLimit == 5000.sat)
     assert(bob.underlyingActor.nodeParams.channelConf.dustLimit == 1000.sat)
     testRevokeAndAckDustOverflowSingleCommit(f)
@@ -1372,7 +1371,6 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
 
   test("recv RevokeAndAck (over max dust htlc exposure in remote commit only with pending local changes)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.HighDustLimitDifferenceBobAlice)) { f =>
     import f._
-    val sender = TestProbe()
     assert(alice.underlyingActor.nodeParams.channelConf.dustLimit == 1000.sat)
     assert(bob.underlyingActor.nodeParams.channelConf.dustLimit == 5000.sat)
     testRevokeAndAckDustOverflowSingleCommit(f)
@@ -3402,8 +3400,8 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
 
   test("recv WatchFundingDeeplyBuriedTriggered (public channel)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>
     import f._
-    val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
+    val realShortChannelId = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
     // existing funding tx coordinates
     val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
     alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null)
@@ -3414,14 +3412,14 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
   }
 
   test("recv WatchFundingDeeplyBuriedTriggered (public channel, zero-conf)", Tag(ChannelStateTestsTags.ChannelsPublic), Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f =>
     import f._
     // in zero-conf channel we don't have a real short channel id when going to NORMAL state
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Unknown)
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
+    assert(aliceIds.real == RealScidStatus.Unknown)
     // funding tx coordinates (unknown before)
     val (blockHeight, txIndex) = (BlockHeight(400000), 42)
     alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null)
@@ -3433,13 +3431,13 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
   }
 
   test("recv WatchFundingDeeplyBuriedTriggered (public channel, short channel id changed)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>
     import f._
-    val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
+    val realShortChannelId = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
     // existing funding tx coordinates
     val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
     // new funding tx coordinates (there was a reorg)
@@ -3453,13 +3451,13 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
   }
 
   test("recv WatchFundingDeeplyBuriedTriggered (private channel)") { f =>
     import f._
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
     val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
     // existing funding tx coordinates
     val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
     alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null)
@@ -3468,7 +3466,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
   }
 
   test("recv WatchFundingDeeplyBuriedTriggered (private channel, zero-conf)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f =>
@@ -3477,8 +3475,8 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     val listener = TestProbe()
     alice.underlying.system.eventStream.subscribe(listener.ref, classOf[TransactionConfirmed])
     // zero-conf channel: the funding tx isn't confirmed
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Unknown)
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
+    assert(aliceIds.real == RealScidStatus.Unknown)
     alice ! WatchFundingDeeplyBuriedTriggered(BlockHeight(42000), 42, null)
     val realShortChannelId = RealShortChannelId(BlockHeight(42000), 42, 0)
     // update data with real short channel id
@@ -3486,15 +3484,15 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
     // this is the first time we know the funding tx has been confirmed
     listener.expectMsgType[TransactionConfirmed]
   }
 
   test("recv WatchFundingDeeplyBuriedTriggered (private channel, short channel id changed)") { f =>
     import f._
-    val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
-    val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias
+    val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
+    val realShortChannelId = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
     // existing funding tx coordinates
     val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
     // new funding tx coordinates (there was a reorg)
@@ -3506,7 +3504,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
   }
 
   test("recv AnnouncementSignatures", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>
@@ -3579,14 +3577,16 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     import f._
     alice ! WatchFundingDeeplyBuriedTriggered(BlockHeight(400000), 42, null)
     bob ! WatchFundingDeeplyBuriedTriggered(BlockHeight(400000), 42, null)
-    bob2alice.expectMsgType[AnnouncementSignatures]
+    val realScid = bob2alice.expectMsgType[AnnouncementSignatures].shortChannelId
     bob2alice.forward(alice)
     val update1 = channelUpdateListener.expectMsgType[LocalChannelUpdate]
+    assert(update1.channelUpdate.shortChannelId == realScid)
 
     // actual test starts here
     Thread.sleep(1100)
     alice ! BroadcastChannelUpdate(PeriodicRefresh)
     val update2 = channelUpdateListener.expectMsgType[LocalChannelUpdate]
+    assert(update2.channelUpdate.shortChannelId == realScid)
     assert(update1.channelUpdate.timestamp < update2.channelUpdate.timestamp)
   }
 
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala
index ebe90ec8f5..643e682d5c 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala
@@ -10,6 +10,7 @@ import fr.acinq.eclair.router.Graph.GraphStructure.GraphEdge
 import fr.acinq.eclair.router.Router._
 import fr.acinq.eclair.wire.protocol.{AnnouncementSignatures, ChannelUpdate, Shutdown}
 import fr.acinq.eclair.{BlockHeight, TestKitBaseClass}
+import org.scalatest.OptionValues.convertOptionToValuable
 import org.scalatest.funsuite.FixtureAnyFunSuiteLike
 import org.scalatest.{Outcome, Tag}
 
@@ -57,9 +58,9 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu
     // alice will only have a real scid if this is not a zeroconf channel
     assert(channels.alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.toOption.isEmpty == f.testTags.contains(ChannelStateTestsTags.ZeroConf))
     assert(channels.alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.remoteAlias_opt.isDefined)
-    // alice uses bob's alias for her channel update
-    assert(privateChannel.update_1_opt.get.shortChannelId != privateChannel.shortIds.localAlias)
-    assert(privateChannel.update_1_opt.get.shortChannelId == channels.alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.remoteAlias_opt.get)
+    // alice uses her alias for her internal channel update
+    val aliceInitialChannelUpdate = privateChannel.update_1_opt.value
+    assert(aliceInitialChannelUpdate.shortChannelId == privateChannel.shortIds.localAlias)
 
     // alice and bob send their channel_updates using remote alias when they go to NORMAL state
     val aliceChannelUpdate1 = channels.alice2bob.expectMsgType[ChannelUpdate]
@@ -73,7 +74,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu
 
     // router processes bob's channel_update and now knows both channel updates
     awaitCond {
-      privateChannel.update_1_opt.contains(aliceChannelUpdate1) && privateChannel.update_2_opt.contains(bobChannelUpdate1)
+      privateChannel.update_1_opt.contains(aliceInitialChannelUpdate) && privateChannel.update_2_opt.contains(bobChannelUpdate1)
     }
 
     // there is nothing for the router to rebroadcast, channel is not announced
@@ -81,7 +82,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu
 
     // router graph contains a single channel
     assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(aliceNodeId, bobNodeId))
-    assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceChannelUpdate1, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel)))
+    assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceInitialChannelUpdate, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel)))
 
     if (testTags.contains(ChannelStateTestsTags.ChannelsPublic)) {
       // this is a public channel
@@ -161,7 +162,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu
 
       // router graph contains a single channel
       assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(aliceNodeId, bobNodeId))
-      assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceChannelUpdate1, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel)))
+      assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceInitialChannelUpdate, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel)))
     }
     // channel closes
     channels.alice ! CMD_CLOSE(TestProbe().ref, scriptPubKey = None, feerates = None)

From 18ff97136b52f31f0483443afd64a6582460066f Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Fri, 17 Jun 2022 15:52:06 +0200
Subject: [PATCH 2/6] Return the incoming scid in onion failures

There are multiple valid scids for public channels depending on the
confirmation status of the funding tx. A payer may use either of these
scids, and we must return the one they used in case of error.
---
 docs/release-notes/eclair-vnext.md            | 13 ++--
 .../eclair/payment/relay/ChannelRelay.scala   | 35 ++++++++---
 .../eclair/payment/relay/ChannelRelayer.scala |  2 +-
 .../acinq/eclair/payment/relay/Relayer.scala  | 11 ++--
 .../payment/relay/ChannelRelayerSpec.scala    | 59 ++++++++++++-------
 .../src/test/resources/api/channelbalances    |  2 +-
 .../src/test/resources/api/usablebalances     |  2 +-
 .../fr/acinq/eclair/api/ApiServiceSpec.scala  |  8 +--
 8 files changed, 86 insertions(+), 46 deletions(-)

diff --git a/docs/release-notes/eclair-vnext.md b/docs/release-notes/eclair-vnext.md
index 33cdb33d0b..e956f945ea 100644
--- a/docs/release-notes/eclair-vnext.md
+++ b/docs/release-notes/eclair-vnext.md
@@ -89,8 +89,9 @@ upgrading to this release.
 
 ### API changes
 
-- `channelbalances` Retrieves information about the balances of all local channels. (#2196)
-- `stop` Stops eclair. Please note that the recommended way of stopping eclair is simply to kill its process (#2233)
+- `channelbalances`: retrieves information about the balances of all local channels (#2196)
+- `stop`: stops eclair. Please note that the recommended way of stopping eclair is simply to kill its process (#2233)
+- `channelbalances` and `usablebalances` return a `shortIds` object instead of a single `shortChannelId` (#2323)
 
 ### Miscellaneous improvements and bug fixes
 
@@ -112,8 +113,8 @@ to your node.
 The `eclair.channel.min-funding-satoshis` setting has been deprecated and replaced with the following two new settings
 and defaults:
 
-* `eclair.channel.min-public-funding-satoshis = 100000`
-* `eclair.channel.min-private-funding-satoshis = 100000`
+- `eclair.channel.min-public-funding-satoshis = 100000`
+- `eclair.channel.min-private-funding-satoshis = 100000`
 
 If your configuration file changes `eclair.channel.min-funding-satoshis` then you should replace it with both of these
 new settings.
@@ -124,8 +125,8 @@ Expired incoming invoices that are unpaid will be searched for and purged from t
 Thereafter searches for expired unpaid invoices to purge will run once every 24 hours. You can disable this feature, or
 change the search interval with two new settings:
 
-* `eclair.purge-expired-invoices.enabled = true
-* `eclair.purge-expired-invoices.interval = 24 hours`
+- `eclair.purge-expired-invoices.enabled = true
+- `eclair.purge-expired-invoices.interval = 24 hours`
 
 ## Verifying signatures
 
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
index c811d03a84..d9dd9dbe4d 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
@@ -27,6 +27,7 @@ import fr.acinq.eclair.db.PendingCommandsDb
 import fr.acinq.eclair.payment.Monitoring.{Metrics, Tags}
 import fr.acinq.eclair.payment.relay.Relayer.{OutgoingChannel, OutgoingChannelParams}
 import fr.acinq.eclair.payment.{ChannelPaymentRelayed, IncomingPaymentPacket}
+import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.wire.protocol._
 import fr.acinq.eclair.{Logs, NodeParams, TimestampSecond, channel, nodeFee}
 
@@ -175,13 +176,13 @@ class ChannelRelay private(nodeParams: NodeParams,
     selectPreferredChannel(alreadyTried) match {
       case None if previousFailures.nonEmpty =>
         // no more channels to try
-        val error = previousFailures
+        val selected = previousFailures
           // we return the error for the initially requested channel if it exists
           .find(failure => requestedChannelId_opt.contains(failure.channelId))
           // otherwise we return the error for the first channel tried
           .getOrElse(previousFailures.head)
-          .failure
-        RelayFailure(CMD_FAIL_HTLC(r.add.id, Right(translateLocalError(error.t, error.channelUpdate)), commit = true))
+        val channelUpdate = selected.failure.channelUpdate.map(u => channelUpdateForFailure(selected.channelId, u))
+        RelayFailure(CMD_FAIL_HTLC(r.add.id, Right(translateLocalError(selected.failure.t, channelUpdate)), commit = true))
       case outgoingChannel_opt =>
         relayOrFail(outgoingChannel_opt)
     }
@@ -191,7 +192,10 @@ class ChannelRelay private(nodeParams: NodeParams,
   private val nextNodeId_opt = channels.headOption.map(_._2.nextNodeId)
 
   /** channel id explicitly requested in the onion payload */
-  private val requestedChannelId_opt = channels.find(_._2.channelUpdate.shortChannelId == r.payload.outgoingChannelId).map(_._1)
+  private val requestedChannelId_opt = channels.collectFirst {
+    case (channelId, channel) if channel.shortIds.localAlias == r.payload.outgoingChannelId => channelId
+    case (channelId, channel) if channel.shortIds.real.toOption.contains(r.payload.outgoingChannelId) => channelId
+  }
 
   /**
    * Select a channel to the same node to relay the payment to, that has the lowest capacity and balance and is
@@ -262,20 +266,35 @@ class ChannelRelay private(nodeParams: NodeParams,
       case None =>
         RelayFailure(CMD_FAIL_HTLC(add.id, Right(UnknownNextPeer), commit = true))
       case Some(c) if !c.channelUpdate.channelFlags.isEnabled =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(ChannelDisabled(c.channelUpdate.messageFlags, c.channelUpdate.channelFlags, c.channelUpdate)), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(ChannelDisabled(c.channelUpdate.messageFlags, c.channelUpdate.channelFlags, channelUpdateForFailure(c))), commit = true))
       case Some(c) if payload.amountToForward < c.channelUpdate.htlcMinimumMsat =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(AmountBelowMinimum(payload.amountToForward, c.channelUpdate)), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(AmountBelowMinimum(payload.amountToForward, channelUpdateForFailure(c))), commit = true))
       case Some(c) if r.expiryDelta < c.channelUpdate.cltvExpiryDelta =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(IncorrectCltvExpiry(payload.outgoingCltv, c.channelUpdate)), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(IncorrectCltvExpiry(payload.outgoingCltv, channelUpdateForFailure(c))), commit = true))
       case Some(c) if r.relayFeeMsat < nodeFee(c.channelUpdate.relayFees, payload.amountToForward) &&
         // fees also do not satisfy the previous channel update for `enforcementDelay` seconds after current update
         (TimestampSecond.now() - c.channelUpdate.timestamp > nodeParams.relayParams.enforcementDelay ||
           outgoingChannel_opt.flatMap(_.prevChannelUpdate).forall(c => r.relayFeeMsat < nodeFee(c.relayFees, payload.amountToForward))) =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(FeeInsufficient(add.amountMsat, c.channelUpdate)), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(FeeInsufficient(add.amountMsat, channelUpdateForFailure(c))), commit = true))
       case Some(c: OutgoingChannel) =>
         val origin = Origin.ChannelRelayedHot(addResponseAdapter.toClassic, add, payload.amountToForward)
         RelaySuccess(c.channelId, CMD_ADD_HTLC(addResponseAdapter.toClassic, payload.amountToForward, add.paymentHash, payload.outgoingCltv, nextPacket, origin, commit = true))
     }
   }
 
+  /**
+   * Our channel updates may contain either the real scid or our local alias. The incoming payment may use either for
+   * public channels (note that the [[ChannelRelayer]] already rejected requests using the real scid when scid_alias is
+   * active), so we must ensure that the channel update we return matches what they sent us.
+   */
+  def channelUpdateForFailure(channelId: ByteVector32, channelUpdate: ChannelUpdate): ChannelUpdate = {
+    if (requestedChannelId_opt.contains(channelId) && channelUpdate.shortChannelId != r.payload.outgoingChannelId) {
+      Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = r.payload.outgoingChannelId))
+    } else {
+      channelUpdate
+    }
+  }
+
+  def channelUpdateForFailure(c: OutgoingChannelParams): ChannelUpdate = channelUpdateForFailure(c.channelId, c.channelUpdate)
+
 }
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelayer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelayer.scala
index 1a9bfc2fc1..59eb7b58b0 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelayer.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelayer.scala
@@ -95,7 +95,7 @@ object ChannelRelayer {
           case WrappedLocalChannelUpdate(lcu@LocalChannelUpdate(_, channelId, shortIds, remoteNodeId, _, channelUpdate, commitments)) =>
             context.log.debug(s"updating local channel info for channelId=$channelId realScid=${shortIds.real} localAlias=${shortIds.localAlias} remoteNodeId=$remoteNodeId channelUpdate={} commitments={}", channelUpdate, commitments)
             val prevChannelUpdate = channels.get(channelId).map(_.channelUpdate)
-            val channel = Relayer.OutgoingChannel(remoteNodeId, channelUpdate, prevChannelUpdate, commitments)
+            val channel = Relayer.OutgoingChannel(shortIds, remoteNodeId, channelUpdate, prevChannelUpdate, commitments)
             val channels1 = channels + (channelId -> channel)
             val mappings = lcu.scidsForRouting.map(_ -> channelId).toMap
             context.log.debug("adding mappings={} to channelId={}", mappings.keys.mkString(","), channelId)
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/Relayer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/Relayer.scala
index 1996927e8c..c7341e7fab 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/Relayer.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/Relayer.scala
@@ -29,7 +29,7 @@ import fr.acinq.eclair.channel._
 import fr.acinq.eclair.db.PendingCommandsDb
 import fr.acinq.eclair.payment._
 import fr.acinq.eclair.wire.protocol._
-import fr.acinq.eclair.{Logs, MilliSatoshi, NodeParams, ShortChannelId}
+import fr.acinq.eclair.{Logs, MilliSatoshi, NodeParams}
 import grizzled.slf4j.Logging
 
 import scala.concurrent.Promise
@@ -133,9 +133,10 @@ object Relayer extends Logging {
   }
 
   case class RelayForward(add: UpdateAddHtlc)
-  case class ChannelBalance(remoteNodeId: PublicKey, shortChannelId: ShortChannelId, canSend: MilliSatoshi, canReceive: MilliSatoshi, isPublic: Boolean, isEnabled: Boolean)
+  case class ChannelBalance(remoteNodeId: PublicKey, shortIds: ShortIds, canSend: MilliSatoshi, canReceive: MilliSatoshi, isPublic: Boolean, isEnabled: Boolean)
 
   sealed trait OutgoingChannelParams {
+    def channelId: ByteVector32
     def channelUpdate: ChannelUpdate
     def prevChannelUpdate: Option[ChannelUpdate]
   }
@@ -146,11 +147,11 @@ object Relayer extends Logging {
    * @param enabledOnly if true, filter out disabled channels.
    */
   case class GetOutgoingChannels(enabledOnly: Boolean = true)
-  case class OutgoingChannel(nextNodeId: PublicKey, channelUpdate: ChannelUpdate, prevChannelUpdate: Option[ChannelUpdate], commitments: AbstractCommitments) extends OutgoingChannelParams {
-    val channelId: ByteVector32 = commitments.channelId
+  case class OutgoingChannel(shortIds: ShortIds, nextNodeId: PublicKey, channelUpdate: ChannelUpdate, prevChannelUpdate: Option[ChannelUpdate], commitments: AbstractCommitments) extends OutgoingChannelParams {
+    override val channelId: ByteVector32 = commitments.channelId
     def toChannelBalance: ChannelBalance = ChannelBalance(
       remoteNodeId = nextNodeId,
-      shortChannelId = channelUpdate.shortChannelId,
+      shortIds = shortIds,
       canSend = commitments.availableBalanceForSend,
       canReceive = commitments.availableBalanceForReceive,
       isPublic = commitments.announceChannel,
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
index 912569ce3c..600c816eb0 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
@@ -78,7 +78,7 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     fwd
   }
 
-  def basicRelaytest(f: FixtureParam)(relayPayloadScid: ShortChannelId, lcu: LocalChannelUpdate, success: Boolean): Unit = {
+  def basicRelayTest(f: FixtureParam)(relayPayloadScid: ShortChannelId, lcu: LocalChannelUpdate, success: Boolean): Unit = {
     import f._
 
     val payload = RelayLegacyPayload(relayPayloadScid, outgoingAmount, outgoingExpiry)
@@ -95,37 +95,35 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
   }
 
   test("relay with real scid (channel update uses real scid)") { f =>
-    basicRelaytest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1), success = true)
+    basicRelayTest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1), success = true)
   }
 
-  test("relay with real scid (channel update uses remote alias)") { f =>
-    basicRelaytest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(remoteAlias1)), success = true)
+  test("relay with real scid (channel update uses local alias)") { f =>
+    basicRelayTest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(localAlias1)), success = true)
   }
 
   test("relay with local alias (channel update uses real scid)") { f =>
-    basicRelaytest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1), success = true)
+    basicRelayTest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1), success = true)
   }
 
-  test("relay with local alias (channel update uses remote alias)") { f =>
-    // we use a random value to simulate a remote alias
-    basicRelaytest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(remoteAlias1)), success = true)
+  test("relay with local alias (channel update uses local alias)") { f =>
+    basicRelayTest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(localAlias1)), success = true)
   }
 
   test("fail to relay with real scid when option_scid_alias is enabled (channel update uses real scid)") { f =>
-    basicRelaytest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, optionScidAlias = true), success = false)
+    basicRelayTest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, optionScidAlias = true), success = false)
   }
 
-  test("fail to relay with real scid when option_scid_alias is enabled (channel update uses remote alias)") { f =>
-    basicRelaytest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, optionScidAlias = true, channelUpdateScid_opt = Some(remoteAlias1)), success = false)
+  test("fail to relay with real scid when option_scid_alias is enabled (channel update uses local alias)") { f =>
+    basicRelayTest(f)(relayPayloadScid = realScid1, lcu = createLocalUpdate(channelId1, optionScidAlias = true, channelUpdateScid_opt = Some(localAlias1)), success = false)
   }
 
   test("relay with local alias when option_scid_alias is enabled (channel update uses real scid)") { f =>
-    basicRelaytest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, optionScidAlias = true), success = true)
+    basicRelayTest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, optionScidAlias = true), success = true)
   }
 
-  test("relay with local alias when option_scid_alias is enabled (channel update uses remote alias)") { f =>
-    // we use a random value to simulate a remote alias
-    basicRelaytest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, optionScidAlias = true, channelUpdateScid_opt = Some(remoteAlias1)), success = true)
+  test("relay with local alias when option_scid_alias is enabled (channel update uses local alias)") { f =>
+    basicRelayTest(f)(relayPayloadScid = localAlias1, lcu = createLocalUpdate(channelId1, optionScidAlias = true, channelUpdateScid_opt = Some(localAlias1)), success = true)
   }
 
   test("relay with new real scid after reorg") { f =>
@@ -154,7 +152,6 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     expectFwdAdd(register, lcu2.channelId, outgoingAmount, outgoingExpiry)
   }
 
-
   test("relay with onion tlv payload") { f =>
     import f._
     import fr.acinq.eclair.wire.protocol.OnionPaymentPayloadTlv._
@@ -365,6 +362,30 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     }
   }
 
+  test("use incoming scid when returning a channel update") { f =>
+    import f._
+
+    // Our channel updates use an alias before the channel confirms.
+    val u1 = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(localAlias1))
+    channelRelayer ! WrappedLocalChannelUpdate(u1)
+    // We switch to using the real scid once the channel confirms.
+    val u2 = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(realScid1), feeBaseMsat = 10 msat, feeProportionalMillionths = 250)
+    channelRelayer ! WrappedLocalChannelUpdate(u2)
+    // But payers may still use the alias.
+    val payload = RelayLegacyPayload(localAlias1, outgoingAmount, outgoingExpiry)
+    val r = createValidIncomingPacket(payload)
+    channelRelayer ! Relay(r)
+    val fwd = expectFwdAdd(register, channelId1, outgoingAmount, outgoingExpiry)
+    fwd.message.replyTo ! RES_ADD_FAILED(fwd.message, InsufficientFunds(channelId1, payload.amountToForward, 100 sat, 0 sat, 0 sat), Some(u2.channelUpdate))
+    // The relayer ensures that the channel update uses the sender's scid.
+    val failure = register.expectMessageType[Register.Forward[channel.Command]]
+    assert(failure.channelId == r.add.channelId)
+    val Right(TemporaryChannelFailure(u3)) = failure.message.asInstanceOf[CMD_FAIL_HTLC].reason
+    assert(u3.shortChannelId == localAlias1)
+    assert(Announcements.areSameIgnoreFlags(u2.channelUpdate, u3))
+    assert(Announcements.checkSig(u3, nodeParams.nodeId))
+  }
+
   test("select preferred channels") { f =>
     import f._
 
@@ -535,9 +556,9 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     val channels1 = getOutgoingChannels(true)
     assert(channels1.size == 2)
     assert(channels1.head.channelUpdate == channelUpdate_ab)
-    assert(channels1.head.toChannelBalance == Relayer.ChannelBalance(a, channelUpdate_ab.shortChannelId, 0 msat, 300000 msat, isPublic = false, isEnabled = true))
+    assert(channels1.head.toChannelBalance == Relayer.ChannelBalance(a, shortIds_ab, 0 msat, 300000 msat, isPublic = false, isEnabled = true))
     assert(channels1.last.channelUpdate == channelUpdate_bc)
-    assert(channels1.last.toChannelBalance == Relayer.ChannelBalance(c, channelUpdate_bc.shortChannelId, 400000 msat, 0 msat, isPublic = false, isEnabled = true))
+    assert(channels1.last.toChannelBalance == Relayer.ChannelBalance(c, shortIds_bc, 400000 msat, 0 msat, isPublic = false, isEnabled = true))
 
     channelRelayer ! WrappedAvailableBalanceChanged(AvailableBalanceChanged(null, channelId_bc, shortIds_ab, makeCommitments(channelId_bc, 200000 msat, 500000 msat)))
     val channels2 = getOutgoingChannels(true)
@@ -575,8 +596,6 @@ object ChannelRelayerSpec {
   val localAlias1: Alias = Alias(111000)
   val localAlias2: Alias = Alias(222000)
 
-  val remoteAlias1: ShortChannelId = ShortChannelId(111999)
-
   val channelId1: ByteVector32 = randomBytes32()
   val channelId2: ByteVector32 = randomBytes32()
 
diff --git a/eclair-node/src/test/resources/api/channelbalances b/eclair-node/src/test/resources/api/channelbalances
index 962b4c89aa..bbfc7619b3 100644
--- a/eclair-node/src/test/resources/api/channelbalances
+++ b/eclair-node/src/test/resources/api/channelbalances
@@ -1 +1 @@
-[{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortChannelId":"0x0x1","canSend":100000000,"canReceive":20000000,"isPublic":true,"isEnabled":true},{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortChannelId":"0x0x2","canSend":0,"canReceive":30000000,"isPublic":false,"isEnabled":false}]
\ No newline at end of file
+[{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortIds":{"real":{"status":"temporary","realScid":"0x0x1"},"localAlias":"0x2"},"canSend":100000000,"canReceive":20000000,"isPublic":true,"isEnabled":true},{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortIds":{"real":{"status":"final","realScid":"0x0x3"},"localAlias":"0x3","remoteAlias":"0x3"},"canSend":0,"canReceive":30000000,"isPublic":false,"isEnabled":false}]
\ No newline at end of file
diff --git a/eclair-node/src/test/resources/api/usablebalances b/eclair-node/src/test/resources/api/usablebalances
index 5613aaf151..2876388234 100644
--- a/eclair-node/src/test/resources/api/usablebalances
+++ b/eclair-node/src/test/resources/api/usablebalances
@@ -1 +1 @@
-[{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortChannelId":"0x0x1","canSend":100000000,"canReceive":20000000,"isPublic":true,"isEnabled":true},{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortChannelId":"0x0x2","canSend":400000000,"canReceive":30000000,"isPublic":false,"isEnabled":true}]
\ No newline at end of file
+[{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortIds":{"real":{"status":"unknown"},"localAlias":"0x1"},"canSend":100000000,"canReceive":20000000,"isPublic":true,"isEnabled":true},{"remoteNodeId":"03af0ed6052cf28d670665549bc86f4b721c9fdb309d40c58f5811f63966e005d0","shortIds":{"real":{"status":"final","realScid":"0x0x2"},"localAlias":"0x3","remoteAlias":"0x4"},"canSend":400000000,"canReceive":30000000,"isPublic":false,"isEnabled":true}]
\ No newline at end of file
diff --git a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala
index 36f3d7c37f..dba2e0834a 100644
--- a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala
+++ b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala
@@ -205,8 +205,8 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM
   test("'usablebalances' returns expected balance json only for enabled channels") {
     val eclair = mock[Eclair]
     eclair.usableBalances()(any[Timeout]) returns Future.successful(List(
-      ChannelBalance(aliceNodeId, ShortChannelId(1), 100000000 msat, 20000000 msat, isPublic = true, isEnabled = true),
-      ChannelBalance(aliceNodeId, ShortChannelId(2), 400000000 msat, 30000000 msat, isPublic = false, isEnabled = true)
+      ChannelBalance(aliceNodeId, ShortIds(RealScidStatus.Unknown, Alias(1), None), 100000000 msat, 20000000 msat, isPublic = true, isEnabled = true),
+      ChannelBalance(aliceNodeId, ShortIds(RealScidStatus.Final(RealShortChannelId(2)), Alias(3), Some(Alias(4))), 400000000 msat, 30000000 msat, isPublic = false, isEnabled = true)
     ))
 
     val mockService = mockApi(eclair)
@@ -225,8 +225,8 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM
   test("'channelbalances' returns expected balance json for all channels") {
     val eclair = mock[Eclair]
     eclair.channelBalances()(any[Timeout]) returns Future.successful(List(
-      ChannelBalance(aliceNodeId, ShortChannelId(1), 100000000 msat, 20000000 msat, isPublic = true, isEnabled = true),
-      ChannelBalance(aliceNodeId, ShortChannelId(2), 0 msat, 30000000 msat, isPublic = false, isEnabled = false)
+      ChannelBalance(aliceNodeId, ShortIds(RealScidStatus.Temporary(RealShortChannelId(1)), Alias(2), None), 100000000 msat, 20000000 msat, isPublic = true, isEnabled = true),
+      ChannelBalance(aliceNodeId, ShortIds(RealScidStatus.Final(RealShortChannelId(3)), Alias(3), Some(Alias(3))), 0 msat, 30000000 msat, isPublic = false, isEnabled = false)
     ))
 
     val mockService = mockApi(eclair)

From 794a287b3b9c46ad3ec147cc48a1ad69c1721a36 Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Thu, 30 Jun 2022 16:38:04 +0200
Subject: [PATCH 3/6] Simplify choice of which scid to use

---
 .../fr/acinq/eclair/channel/Helpers.scala     | 43 ++++++-------------
 .../fr/acinq/eclair/channel/fsm/Channel.scala |  6 +--
 .../channel/fsm/ChannelOpenSingleFunder.scala |  2 +-
 .../ChannelStateTestsHelperMethods.scala      |  2 +-
 .../c/WaitForChannelReadyStateSpec.scala      |  8 ++--
 .../channel/states/e/NormalStateSpec.scala    |  9 ++--
 6 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 9e9f452d80..61ba293e6c 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -184,43 +184,28 @@ object Helpers {
   }
 
   /**
-   * The general rule is that we use our local alias for our channel_update until the channel is publicly announced, and
-   * then we use the real scid.
-   *
-   * Private channels are handled like public channels that have not yet been announced, unless our peer peer does not
-   * support scid_alias.
-   *
-   * Decision tree:
-   *  - received remote_alias from peer (which indicates that they support scid_alias)
-   *    - before channel announcement: use local alias
-   *    - after channel announcement: use real scid
-   *  - no remote_alias from peer
-   *    - min_depth > 0: use real scid (may change if reorg between min_depth and 6 conf)
-   *    - min_depth = 0 (zero-conf): spec violation, our peer MUST send an alias when using zero-conf
+   * We use the real scid if the channel has been announced, otherwise we use our local alias.
    */
-  def scidForChannelUpdate(channelAnnouncement_opt: Option[ChannelAnnouncement], shortIds: ShortIds): ShortChannelId = {
-    channelAnnouncement_opt match {
-      case Some(ann) => ann.shortChannelId // we use the real "final" scid when it is publicly announced
-      case None => shortIds.remoteAlias_opt match {
-        case Some(_) => shortIds.localAlias // we use our local alias if the channel isn't announced and our peer supports it
-        case None => shortIds.real.toOption match {
-          case Some(real) => real // we use the real scid if our peer doesn't support scid_alias and the channel is confirmed
-          case None => shortIds.localAlias // spec violation: this is a 0-conf channel, our peer MUST send their alias, we just use ours
-        }
-      }
-    }
+  def scidForChannelUpdate(channelAnnouncement_opt: Option[ChannelAnnouncement], localAlias: Alias): ShortChannelId = {
+    channelAnnouncement_opt.map(_.shortChannelId).getOrElse(localAlias)
   }
 
-  def scidForChannelUpdate(d: DATA_NORMAL): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds)
+  def scidForChannelUpdate(d: DATA_NORMAL): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds.localAlias)
 
   /**
    * If our peer sent us an alias, that's what we must use in the channel_update we send them to ensure they're able to
-   * match this update with the corresponding local channel.
+   * match this update with the corresponding local channel. If they didn't send us an alias, it means we're not using
+   * 0-conf and we'll use the real scid.
    */
-  def channelUpdateForDirectPeer(nodeParams: NodeParams, channelUpdate: ChannelUpdate, remoteAlias_opt: Option[Alias]): ChannelUpdate = {
-    remoteAlias_opt match {
+  def channelUpdateForDirectPeer(nodeParams: NodeParams, channelUpdate: ChannelUpdate, shortIds: ShortIds): ChannelUpdate = {
+    shortIds.remoteAlias_opt match {
       case Some(remoteAlias) => Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = remoteAlias))
-      case None => channelUpdate
+      case None => shortIds.real.toOption match {
+        case Some(realScid) => Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = realScid))
+        // This case is a spec violation: this is a 0-conf channel, so our peer MUST send their alias.
+        // They won't be able to match our channel_update with their local channel, too bad for them.
+        case None => channelUpdate
+      }
     }
   }
 
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
index 71a9618991..9decc58b0d 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
@@ -622,7 +622,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
         // this is a zero-conf channel and it is the first time we know for sure that the funding tx has been confirmed
         context.system.eventStream.publish(TransactionConfirmed(d.channelId, remoteNodeId, fundingTx))
       }
-      val scidForChannelUpdate = Helpers.scidForChannelUpdate(d.channelAnnouncement, shortIds1)
+      val scidForChannelUpdate = Helpers.scidForChannelUpdate(d.channelAnnouncement, shortIds1.localAlias)
       // if the shortChannelId is different from the one we had before, we need to re-announce it
       val channelUpdate1 = if (d.channelUpdate.shortChannelId != scidForChannelUpdate) {
         log.info(s"using new scid in channel_update: old=${d.channelUpdate.shortChannelId} new=$scidForChannelUpdate")
@@ -659,7 +659,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
                 handleLocalError(InvalidAnnouncementSignatures(d.channelId, remoteAnnSigs), d, Some(remoteAnnSigs))
               } else {
                 // we generate a new channel_update because the scid used may change if we were previously using an alias
-                val scidForChannelUpdate = Helpers.scidForChannelUpdate(Some(channelAnn), d.shortIds)
+                val scidForChannelUpdate = Helpers.scidForChannelUpdate(Some(channelAnn), d.shortIds.localAlias)
                 val channelUpdate = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, scidForChannelUpdate, d.channelUpdate.cltvExpiryDelta, d.channelUpdate.htlcMinimumMsat, d.channelUpdate.feeBaseMsat, d.channelUpdate.feeProportionalMillionths, d.commitments.capacity.toMilliSatoshi, enable = Helpers.aboveReserve(d.commitments))
                 // we use goto() instead of stay() because we want to fire transitions
                 goto(NORMAL) using d.copy(channelAnnouncement = Some(channelAnn), channelUpdate = channelUpdate) storing()
@@ -1640,7 +1640,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
           val lcu = LocalChannelUpdate(self, d.channelId, d.shortIds, d.commitments.remoteParams.nodeId, d.channelAnnouncement, d.channelUpdate, d.commitments)
           context.system.eventStream.publish(lcu)
           if (sendToPeer) {
-            send(Helpers.channelUpdateForDirectPeer(nodeParams, d.channelUpdate, d.shortIds.remoteAlias_opt))
+            send(Helpers.channelUpdateForDirectPeer(nodeParams, d.channelUpdate, d.shortIds))
           }
         case EmitLocalChannelDown(d) =>
           log.debug(s"emitting channel down event")
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunder.scala
index 8966b25364..14722ddfe6 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunder.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunder.scala
@@ -421,7 +421,7 @@ trait ChannelOpenSingleFunder extends FundingHandlers with ErrorHandlers {
       }
       log.info("shortIds: real={} localAlias={} remoteAlias={}", shortIds1.real.toOption.getOrElse("none"), shortIds1.localAlias, shortIds1.remoteAlias_opt.getOrElse("none"))
       // we create a channel_update early so that we can use it to send payments through this channel, but it won't be propagated to other nodes since the channel is not yet announced
-      val scidForChannelUpdate = Helpers.scidForChannelUpdate(channelAnnouncement_opt = None, shortIds1)
+      val scidForChannelUpdate = Helpers.scidForChannelUpdate(channelAnnouncement_opt = None, shortIds1.localAlias)
       log.info("using shortChannelId={} for initial channel_update", scidForChannelUpdate)
       val relayFees = getRelayFees(nodeParams, remoteNodeId, d.commitments)
       val initialChannelUpdate = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, scidForChannelUpdate, nodeParams.channelConf.expiryDelta, d.commitments.remoteParams.htlcMinimum, relayFees.feeBase, relayFees.feeProportionalMillionths, d.commitments.capacity.toMilliSatoshi, enable = Helpers.aboveReserve(d.commitments))
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala
index 48b031fd52..6386271f94 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala
@@ -181,7 +181,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
 
     // those features can only be enabled with AnchorOutputsZeroFeeHtlcTxs, this is to prevent incompatible test configurations
     if (tags.contains(ChannelStateTestsTags.ZeroConf)) assert(tags.contains(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), "invalid test configuration")
-    if (tags.contains(ChannelStateTestsTags.ScidAlias)) assert(channelType.features.contains(Features.ScidAlias), "invalid test configuration")
+    if (tags.contains(ChannelStateTestsTags.ScidAlias)) assert(tags.contains(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), "invalid test configuration")
 
     implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
     val aliceParams = Alice.channelParams
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
index ecc4dfba3c..e08f1c3879 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala
@@ -121,13 +121,15 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu
     val channelReady = bob2alice.expectMsgType[ChannelReady]
     val channelReadyNoAlias = channelReady.modify(_.tlvStream.records).using(_.filterNot(_.isInstanceOf[ChannelReadyTlv.ShortChannelIdTlv]))
     bob2alice.forward(alice, channelReadyNoAlias)
-    // the channel is not announced but bob didn't send an alias so we use the real scid
     val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate
-    assert(initialChannelUpdate.shortChannelId == realScid)
+    assert(initialChannelUpdate.shortChannelId == aliceIds.localAlias)
     assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase)
     assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths)
+    // the channel is not announced but bob didn't send an alias so we use the real scid
     val channelUpdateSentToPeer = alice2bob.expectMsgType[ChannelUpdate]
-    assert(channelUpdateSentToPeer == initialChannelUpdate)
+    assert(channelUpdateSentToPeer.shortChannelId == realScid)
+    assert(Announcements.areSameIgnoreFlags(initialChannelUpdate, channelUpdateSentToPeer))
+    assert(Announcements.checkSig(channelUpdateSentToPeer, alice.underlyingActor.nodeParams.nodeId))
     alice2blockchain.expectMsgType[WatchFundingDeeplyBuried]
     bob2alice.expectNoMessage(100 millis)
     awaitCond(alice.stateName == NORMAL)
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
index 0456fd499a..5ec60cd941 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
@@ -3409,7 +3409,6 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     assert(annSigs.shortChannelId == realShortChannelId)
     // alice updates her internal state
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId))
-    // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
@@ -3428,7 +3427,6 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     assert(annSigs.shortChannelId == realShortChannelId)
     // alice updates her internal state
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId))
-    // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
@@ -3448,7 +3446,6 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     assert(annSigs.shortChannelId == newRealShortChannelId)
     // update data with real short channel id
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(newRealShortChannelId))
-    // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
@@ -3463,7 +3460,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null)
     // update data with real short channel id
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId))
-    // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
+    // private channel: we'll use the remote alias in the channel_update we sent to our peer, so there is no change and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
@@ -3481,7 +3478,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     val realShortChannelId = RealShortChannelId(BlockHeight(42000), 42, 0)
     // update data with real short channel id
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId))
-    // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
+    // private channel: we'll use the remote alias in the channel_update we sent to our peer, so there is no change and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
@@ -3501,7 +3498,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     val newRealShortChannelId = RealShortChannelId(blockHeight1, txIndex1, alice.stateData.asInstanceOf[DATA_NORMAL].commitments.commitInput.outPoint.index.toInt)
     // update data with real short channel id
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(newRealShortChannelId))
-    // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one
+    // private channel: we'll use the remote alias in the channel_update we sent to our peer, so there is no change and we don't send a new one
     alice2bob.expectNoMessage(100 millis)
     channelUpdateListener.expectNoMessage(100 millis)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)

From 1d4d1e182088138c9e46ef2f8ba638bbfff1d12e Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Thu, 30 Jun 2022 18:07:39 +0200
Subject: [PATCH 4/6] fixup! Return the incoming scid in onion failures

---
 .../scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala     | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
index d9dd9dbe4d..a675716139 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
@@ -288,6 +288,9 @@ class ChannelRelay private(nodeParams: NodeParams,
    * active), so we must ensure that the channel update we return matches what they sent us.
    */
   def channelUpdateForFailure(channelId: ByteVector32, channelUpdate: ChannelUpdate): ChannelUpdate = {
+    // NB: we must ensure that the channel update is for the channel they requested before updating it, otherwise we
+    // would replace the scid of another channel which would be incorrect. When we return an error for a different
+    // channel, we should use the scid of that other channel.
     if (requestedChannelId_opt.contains(channelId) && channelUpdate.shortChannelId != r.payload.outgoingChannelId) {
       Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = r.payload.outgoingChannelId))
     } else {

From 15cddbbe3710ee01686383e5054a6e9f4b1477dc Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Thu, 30 Jun 2022 19:27:14 +0200
Subject: [PATCH 5/6] Remove channel relayer changes

We shouldn't need them since we don't add the real scid to the routing
table when scid_alias is negotiated.
---
 .../eclair/payment/relay/ChannelRelay.scala   | 33 ++------
 .../basic/ThreeNodesIntegrationSpec.scala     |  2 +-
 .../basic/TwoNodesIntegrationSpec.scala       |  2 +-
 .../basic/ZeroConfAliasIntegrationSpec.scala  | 83 +++++++++++++++----
 .../basic/fixtures/MinimalNodeFixture.scala   | 16 +++-
 .../payment/relay/ChannelRelayerSpec.scala    | 24 ------
 6 files changed, 88 insertions(+), 72 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
index a675716139..3ba8c507f4 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala
@@ -27,7 +27,6 @@ import fr.acinq.eclair.db.PendingCommandsDb
 import fr.acinq.eclair.payment.Monitoring.{Metrics, Tags}
 import fr.acinq.eclair.payment.relay.Relayer.{OutgoingChannel, OutgoingChannelParams}
 import fr.acinq.eclair.payment.{ChannelPaymentRelayed, IncomingPaymentPacket}
-import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.wire.protocol._
 import fr.acinq.eclair.{Logs, NodeParams, TimestampSecond, channel, nodeFee}
 
@@ -176,13 +175,13 @@ class ChannelRelay private(nodeParams: NodeParams,
     selectPreferredChannel(alreadyTried) match {
       case None if previousFailures.nonEmpty =>
         // no more channels to try
-        val selected = previousFailures
+        val error = previousFailures
           // we return the error for the initially requested channel if it exists
           .find(failure => requestedChannelId_opt.contains(failure.channelId))
           // otherwise we return the error for the first channel tried
           .getOrElse(previousFailures.head)
-        val channelUpdate = selected.failure.channelUpdate.map(u => channelUpdateForFailure(selected.channelId, u))
-        RelayFailure(CMD_FAIL_HTLC(r.add.id, Right(translateLocalError(selected.failure.t, channelUpdate)), commit = true))
+          .failure
+        RelayFailure(CMD_FAIL_HTLC(r.add.id, Right(translateLocalError(error.t, error.channelUpdate)), commit = true))
       case outgoingChannel_opt =>
         relayOrFail(outgoingChannel_opt)
     }
@@ -266,38 +265,20 @@ class ChannelRelay private(nodeParams: NodeParams,
       case None =>
         RelayFailure(CMD_FAIL_HTLC(add.id, Right(UnknownNextPeer), commit = true))
       case Some(c) if !c.channelUpdate.channelFlags.isEnabled =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(ChannelDisabled(c.channelUpdate.messageFlags, c.channelUpdate.channelFlags, channelUpdateForFailure(c))), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(ChannelDisabled(c.channelUpdate.messageFlags, c.channelUpdate.channelFlags, c.channelUpdate)), commit = true))
       case Some(c) if payload.amountToForward < c.channelUpdate.htlcMinimumMsat =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(AmountBelowMinimum(payload.amountToForward, channelUpdateForFailure(c))), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(AmountBelowMinimum(payload.amountToForward, c.channelUpdate)), commit = true))
       case Some(c) if r.expiryDelta < c.channelUpdate.cltvExpiryDelta =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(IncorrectCltvExpiry(payload.outgoingCltv, channelUpdateForFailure(c))), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(IncorrectCltvExpiry(payload.outgoingCltv, c.channelUpdate)), commit = true))
       case Some(c) if r.relayFeeMsat < nodeFee(c.channelUpdate.relayFees, payload.amountToForward) &&
         // fees also do not satisfy the previous channel update for `enforcementDelay` seconds after current update
         (TimestampSecond.now() - c.channelUpdate.timestamp > nodeParams.relayParams.enforcementDelay ||
           outgoingChannel_opt.flatMap(_.prevChannelUpdate).forall(c => r.relayFeeMsat < nodeFee(c.relayFees, payload.amountToForward))) =>
-        RelayFailure(CMD_FAIL_HTLC(add.id, Right(FeeInsufficient(add.amountMsat, channelUpdateForFailure(c))), commit = true))
+        RelayFailure(CMD_FAIL_HTLC(add.id, Right(FeeInsufficient(add.amountMsat, c.channelUpdate)), commit = true))
       case Some(c: OutgoingChannel) =>
         val origin = Origin.ChannelRelayedHot(addResponseAdapter.toClassic, add, payload.amountToForward)
         RelaySuccess(c.channelId, CMD_ADD_HTLC(addResponseAdapter.toClassic, payload.amountToForward, add.paymentHash, payload.outgoingCltv, nextPacket, origin, commit = true))
     }
   }
 
-  /**
-   * Our channel updates may contain either the real scid or our local alias. The incoming payment may use either for
-   * public channels (note that the [[ChannelRelayer]] already rejected requests using the real scid when scid_alias is
-   * active), so we must ensure that the channel update we return matches what they sent us.
-   */
-  def channelUpdateForFailure(channelId: ByteVector32, channelUpdate: ChannelUpdate): ChannelUpdate = {
-    // NB: we must ensure that the channel update is for the channel they requested before updating it, otherwise we
-    // would replace the scid of another channel which would be incorrect. When we return an error for a different
-    // channel, we should use the scid of that other channel.
-    if (requestedChannelId_opt.contains(channelId) && channelUpdate.shortChannelId != r.payload.outgoingChannelId) {
-      Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = r.payload.outgoingChannelId))
-    } else {
-      channelUpdate
-    }
-  }
-
-  def channelUpdateForFailure(c: OutgoingChannelParams): ChannelUpdate = channelUpdateForFailure(c.channelId, c.channelUpdate)
-
 }
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ThreeNodesIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ThreeNodesIntegrationSpec.scala
index 72da991d06..c0cc5482f8 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ThreeNodesIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ThreeNodesIntegrationSpec.scala
@@ -51,7 +51,7 @@ class ThreeNodesIntegrationSpec extends FixtureSpec with IntegrationPatience {
       assert(routerData.channels.values.flatMap(c => c.update_1_opt.toSeq ++ c.update_2_opt.toSeq).size == 4) // 2 channel_updates per channel
     }
 
-    sendPayment(alice, carol, 100_000 msat)
+    sendSuccessfulPayment(alice, carol, 100_000 msat)
   }
 
 }
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/TwoNodesIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/TwoNodesIntegrationSpec.scala
index 89598db9e5..1e8a87c32f 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/TwoNodesIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/TwoNodesIntegrationSpec.scala
@@ -87,7 +87,7 @@ class TwoNodesIntegrationSpec extends FixtureSpec with IntegrationPatience {
       getRouterData(alice).privateChannels.size == 1
     }
 
-    sendPayment(alice, bob, 10_000 msat)
+    sendSuccessfulPayment(alice, bob, 10_000 msat)
   }
 
 }
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
index 05af0ee034..d611719bc5 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
@@ -5,9 +5,14 @@ import fr.acinq.bitcoin.scalacompat.{ByteVector32, SatoshiLong}
 import fr.acinq.eclair.FeatureSupport.{Mandatory, Optional}
 import fr.acinq.eclair.Features.{ScidAlias, ZeroConf}
 import fr.acinq.eclair.channel.{DATA_NORMAL, RealScidStatus}
+import fr.acinq.eclair.crypto.Sphinx
 import fr.acinq.eclair.integration.basic.fixtures.ThreeNodesFixture
-import fr.acinq.eclair.payment.PaymentSent
+import fr.acinq.eclair.payment.Bolt11Invoice.ExtraHop
+import fr.acinq.eclair.payment._
+import fr.acinq.eclair.payment.send.PaymentError.RetryExhausted
+import fr.acinq.eclair.router.RouteNotFound
 import fr.acinq.eclair.testutils.FixtureSpec
+import fr.acinq.eclair.wire.protocol.{UnknownNextPeer, Update}
 import fr.acinq.eclair.{MilliSatoshiLong, RealShortChannelId}
 import org.scalatest.OptionValues.convertOptionToValuable
 import org.scalatest.concurrent.IntegrationPatience
@@ -60,17 +65,28 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
     (channelId_ab, channelId_bc)
   }
 
-  private def sendPaymentAliceToCarol(f: FixtureParam, useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): PaymentSent = {
+  private def createBobToCarolTestHint(f: FixtureParam, useHint: Boolean, overrideHintScid_opt: Option[RealShortChannelId]): Seq[ExtraHop] = {
     import f._
-    val hint = if (useHint) {
+    if (useHint) {
       val Some(carolHint) = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop
       // due to how node ids are built, bob < carol so carol is always the node 2
       val bobAlias = getRouterData(bob).privateChannels.values.find(_.nodeId2 == carol.nodeParams.nodeId).value.shortIds.localAlias
       // the hint is always using the alias
       assert(carolHint.shortChannelId == bobAlias)
       Seq(carolHint.modify(_.shortChannelId).setToIfDefined(overrideHintScid_opt))
-    } else Seq.empty
-    sendPayment(alice, carol, 100_000 msat, hints = Seq(hint))
+    } else {
+      Seq.empty
+    }
+  }
+
+  private def sendSuccessfulPaymentAliceToCarol(f: FixtureParam, useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): PaymentSent = {
+    import f._
+    sendSuccessfulPayment(alice, carol, 100_000 msat, hints = Seq(createBobToCarolTestHint(f, useHint, overrideHintScid_opt)))
+  }
+
+  private def sendFailingPaymentAliceToCarol(f: FixtureParam, useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): PaymentFailed = {
+    import f._
+    sendFailingPayment(alice, carol, 100_000 msat, hints = Seq(createBobToCarolTestHint(f, useHint, overrideHintScid_opt)))
   }
 
   private def internalTest(f: FixtureParam,
@@ -107,22 +123,29 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       }
     }
 
+    def validateFailure(failure: PaymentFailed): Unit = {
+      failure.failures.foreach {
+        case LocalFailure(_, _, t) => assert(t == RouteNotFound || t == RetryExhausted)
+        case RemoteFailure(_, _, e) => assert(e.failureMessage == UnknownNextPeer)
+        case _: UnreadableRemoteFailure => fail("received unreadable remote failure")
+      }
+    }
+
     eventually {
       if (paymentWorksWithoutHint) {
-        sendPaymentAliceToCarol(f)
+        sendSuccessfulPaymentAliceToCarol(f)
       } else {
-        intercept[AssertionError] {
-          sendPaymentAliceToCarol(f)
-        }
+        val failure = sendFailingPaymentAliceToCarol(f)
+        validateFailure(failure)
       }
     }
 
     eventually {
       paymentWorksWithHint_opt match {
-        case Some(true) => sendPaymentAliceToCarol(f, useHint = true)
-        case Some(false) => intercept[AssertionError] {
-          sendPaymentAliceToCarol(f, useHint = true)
-        }
+        case Some(true) => sendSuccessfulPaymentAliceToCarol(f, useHint = true)
+        case Some(false) =>
+          val failure = sendFailingPaymentAliceToCarol(f, useHint = true)
+          validateFailure(failure)
         case None => // skipped
       }
     }
@@ -130,10 +153,10 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
     eventually {
       paymentWorksWithRealScidHint_opt match {
         // if alice uses the real scid instead of the bob-carol alias, it still works
-        case Some(true) => sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
-        case Some(false) => intercept[AssertionError] {
-          sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
-        }
+        case Some(true) => sendSuccessfulPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
+        case Some(false) =>
+          val failure = sendFailingPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
+          validateFailure(failure)
         case None => // skipped
       }
     }
@@ -228,4 +251,30 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
     )
   }
 
+  test("temporary channel failures don't leak the real scid", Tag(ScidAliasBobCarol), Tag(ZeroConfBobCarol)) { f =>
+    import f._
+
+    val (_, channelId_bc) = createChannels(f)(deepConfirm = false)
+
+    eventually {
+      assert(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].commitments.channelFeatures.features.contains(ZeroConf))
+      assert(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].commitments.channelFeatures.features.contains(ScidAlias))
+      assert(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Unknown)
+      assert(getRouterData(bob).privateChannels.values.exists(_.nodeId2 == carol.nodeParams.nodeId))
+    }
+
+    val Some(carolHint) = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop
+    val bobAlias = getRouterData(bob).privateChannels.values.find(_.nodeId2 == carol.nodeParams.nodeId).value.shortIds.localAlias
+    assert(carolHint.shortChannelId == bobAlias)
+
+    // We make sure Bob won't have enough liquidity to relay another payment.
+    sendSuccessfulPayment(bob, carol, 60_000_000 msat)
+
+    // The channel update returned in failures doesn't leak the real scid.
+    val failure = sendFailingPayment(alice, carol, 50_000_000 msat, hints = Seq(Seq(carolHint)))
+    val failureWithChannelUpdate = failure.failures.collect { case RemoteFailure(_, _, Sphinx.DecryptedFailurePacket(_, f: Update)) => f }
+    assert(failureWithChannelUpdate.length == 1)
+    assert(failureWithChannelUpdate.head.update.shortChannelId == bobAlias)
+  }
+
 }
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
index cd6ba696e9..82348d64bb 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
@@ -21,7 +21,7 @@ import fr.acinq.eclair.payment.Bolt11Invoice.ExtraHop
 import fr.acinq.eclair.payment.receive.{MultiPartHandler, PaymentHandler}
 import fr.acinq.eclair.payment.relay.{ChannelRelayer, Relayer}
 import fr.acinq.eclair.payment.send.PaymentInitiator
-import fr.acinq.eclair.payment.{Bolt11Invoice, PaymentSent}
+import fr.acinq.eclair.payment.{Bolt11Invoice, PaymentFailed, PaymentSent}
 import fr.acinq.eclair.router.Router
 import fr.acinq.eclair.wire.protocol.IPAddress
 import fr.acinq.eclair.{BlockHeight, MilliSatoshi, MilliSatoshiLong, NodeParams, RealShortChannelId, SubscriptionsComplete, TestBitcoinCoreClient, TestDatabases, TestFeeEstimator}
@@ -278,16 +278,26 @@ object MinimalNodeFixture extends Assertions {
     case _ => TestActor.KeepRunning
   }
 
-  def sendPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): PaymentSent = {
-    val sender = TestProbe("sender")
+  private def sendPayment(sender: TestProbe, node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): Unit = {
     sender.send(node2.paymentHandler, MultiPartHandler.ReceivePayment(Some(amount), Left("test payment")))
     val invoice = sender.expectMsgType[Bolt11Invoice]
 
     val routeParams = node1.nodeParams.routerConf.pathFindingExperimentConf.experiments.values.head.getDefaultRouteParams
     sender.send(node1.paymentInitiator, PaymentInitiator.SendPaymentToNode(amount, invoice, maxAttempts = 1, routeParams = routeParams, extraEdges = hints.flatMap(Bolt11Invoice.toExtraEdges(_, node2.nodeParams.nodeId)), blockUntilComplete = true))
+  }
+
+  def sendSuccessfulPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): PaymentSent = {
+    val sender = TestProbe("sender")
+    sendPayment(sender, node1, node2, amount, hints)
     sender.expectMsgType[PaymentSent]
   }
 
+  def sendFailingPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): PaymentFailed = {
+    val sender = TestProbe("sender")
+    sendPayment(sender, node1, node2, amount, hints)
+    sender.expectMsgType[PaymentFailed]
+  }
+
   def prettyPrint(routerData: Router.Data, nodes: MinimalNodeFixture*): Unit = {
     val nodeId2Alias = nodes.map(n => n.nodeParams.nodeId -> n.nodeParams.alias).toMap
       .withDefault(nodeId => throw new RuntimeException(s"cannot resolve nodeId=$nodeId, make sure you have provided all node fixtures"))
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
index 600c816eb0..695a9cf96b 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala
@@ -362,30 +362,6 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     }
   }
 
-  test("use incoming scid when returning a channel update") { f =>
-    import f._
-
-    // Our channel updates use an alias before the channel confirms.
-    val u1 = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(localAlias1))
-    channelRelayer ! WrappedLocalChannelUpdate(u1)
-    // We switch to using the real scid once the channel confirms.
-    val u2 = createLocalUpdate(channelId1, channelUpdateScid_opt = Some(realScid1), feeBaseMsat = 10 msat, feeProportionalMillionths = 250)
-    channelRelayer ! WrappedLocalChannelUpdate(u2)
-    // But payers may still use the alias.
-    val payload = RelayLegacyPayload(localAlias1, outgoingAmount, outgoingExpiry)
-    val r = createValidIncomingPacket(payload)
-    channelRelayer ! Relay(r)
-    val fwd = expectFwdAdd(register, channelId1, outgoingAmount, outgoingExpiry)
-    fwd.message.replyTo ! RES_ADD_FAILED(fwd.message, InsufficientFunds(channelId1, payload.amountToForward, 100 sat, 0 sat, 0 sat), Some(u2.channelUpdate))
-    // The relayer ensures that the channel update uses the sender's scid.
-    val failure = register.expectMessageType[Register.Forward[channel.Command]]
-    assert(failure.channelId == r.add.channelId)
-    val Right(TemporaryChannelFailure(u3)) = failure.message.asInstanceOf[CMD_FAIL_HTLC].reason
-    assert(u3.shortChannelId == localAlias1)
-    assert(Announcements.areSameIgnoreFlags(u2.channelUpdate, u3))
-    assert(Announcements.checkSig(u3, nodeParams.nodeId))
-  }
-
   test("select preferred channels") { f =>
     import f._
 

From 8687bfa4e6168dac65fc6d47f4106ce5d6e7f05d Mon Sep 17 00:00:00 2001
From: t-bast <bastuc@hotmail.fr>
Date: Fri, 1 Jul 2022 14:10:30 +0200
Subject: [PATCH 6/6] Address PR comments

* Revert change to makeChannelUpdate
* Improve zero-conf tests
---
 .../fr/acinq/eclair/channel/Helpers.scala     |   4 +-
 .../acinq/eclair/payment/PaymentEvents.scala  |   2 +
 .../acinq/eclair/router/Announcements.scala   |  15 ++-
 .../basic/ZeroConfAliasIntegrationSpec.scala  | 106 ++++++++----------
 .../basic/fixtures/MinimalNodeFixture.scala   |  22 ++--
 5 files changed, 70 insertions(+), 79 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 61ba293e6c..8cdb82327a 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -199,9 +199,9 @@ object Helpers {
    */
   def channelUpdateForDirectPeer(nodeParams: NodeParams, channelUpdate: ChannelUpdate, shortIds: ShortIds): ChannelUpdate = {
     shortIds.remoteAlias_opt match {
-      case Some(remoteAlias) => Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = remoteAlias))
+      case Some(remoteAlias) => Announcements.updateScid(nodeParams.privateKey, channelUpdate, remoteAlias)
       case None => shortIds.real.toOption match {
-        case Some(realScid) => Announcements.signChannelUpdate(nodeParams.privateKey, channelUpdate.copy(shortChannelId = realScid))
+        case Some(realScid) => Announcements.updateScid(nodeParams.privateKey, channelUpdate, realScid)
         // This case is a spec violation: this is a 0-conf channel, so our peer MUST send their alias.
         // They won't be able to match our channel_update with their local channel, too bad for them.
         case None => channelUpdate
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
index 621e6b62a5..226ad8544c 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
@@ -20,6 +20,7 @@ import fr.acinq.bitcoin.scalacompat.ByteVector32
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.eclair.crypto.Sphinx
 import fr.acinq.eclair.payment.Invoice.{BasicEdge, ExtraEdge}
+import fr.acinq.eclair.payment.send.PaymentError.RetryExhausted
 import fr.acinq.eclair.payment.send.PaymentInitiator.SendPaymentConfig
 import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.router.Router.{ChannelDesc, ChannelHop, Hop, Ignore}
@@ -154,6 +155,7 @@ object PaymentFailure {
   def transformForUser(failures: Seq[PaymentFailure]): Seq[PaymentFailure] = {
     failures match {
       case previousFailures :+ LocalFailure(_, _, RouteNotFound) if previousFailures.nonEmpty => previousFailures
+      case previousFailures :+ LocalFailure(_, _, RetryExhausted) if previousFailures.nonEmpty => previousFailures
       case other => other
     }
   }
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
index 9c000ea5d4..5441a3407f 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala
@@ -117,8 +117,10 @@ object Announcements {
   def makeChannelUpdate(chainHash: ByteVector32, nodeSecret: PrivateKey, remoteNodeId: PublicKey, shortChannelId: ShortChannelId, cltvExpiryDelta: CltvExpiryDelta, htlcMinimumMsat: MilliSatoshi, feeBaseMsat: MilliSatoshi, feeProportionalMillionths: Long, htlcMaximumMsat: MilliSatoshi, enable: Boolean = true, timestamp: TimestampSecond = TimestampSecond.now()): ChannelUpdate = {
     val channelFlags = ChannelUpdate.ChannelFlags(isNode1 = isNode1(nodeSecret.publicKey, remoteNodeId), isEnabled = enable)
     val htlcMaximumMsatOpt = Some(htlcMaximumMsat)
-    val u = ChannelUpdate(
-      signature = ByteVector64.Zeroes,
+    val witness = channelUpdateWitnessEncode(chainHash, shortChannelId, timestamp, channelFlags, cltvExpiryDelta, htlcMinimumMsat, feeBaseMsat, feeProportionalMillionths, htlcMaximumMsatOpt, TlvStream.empty)
+    val sig = Crypto.sign(witness, nodeSecret)
+    ChannelUpdate(
+      signature = sig,
       chainHash = chainHash,
       shortChannelId = shortChannelId,
       timestamp = timestamp,
@@ -129,13 +131,14 @@ object Announcements {
       feeProportionalMillionths = feeProportionalMillionths,
       htlcMaximumMsat = htlcMaximumMsatOpt
     )
-    signChannelUpdate(nodeSecret, u)
   }
 
-  def signChannelUpdate(nodeSecret: PrivateKey, u: ChannelUpdate): ChannelUpdate = {
-    val witness = channelUpdateWitnessEncode(u.chainHash, u.shortChannelId, u.timestamp, u.channelFlags, u.cltvExpiryDelta, u.htlcMinimumMsat, u.feeBaseMsat, u.feeProportionalMillionths, u.htlcMaximumMsat, u.tlvStream)
+  def updateScid(nodeSecret: PrivateKey, u: ChannelUpdate, scid: ShortChannelId): ChannelUpdate = {
+    // NB: we don't update the timestamp as we're not changing any parameter.
+    val u1 = u.copy(shortChannelId = scid)
+    val witness = channelUpdateWitnessEncode(u.chainHash, scid, u.timestamp, u.channelFlags, u.cltvExpiryDelta, u.htlcMinimumMsat, u.feeBaseMsat, u.feeProportionalMillionths, u.htlcMaximumMsat, u.tlvStream)
     val sig = Crypto.sign(witness, nodeSecret)
-    u.copy(signature = sig)
+    u1.copy(signature = sig)
   }
 
   def checkSigs(ann: ChannelAnnouncement): Boolean = {
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
index d611719bc5..33bdf0d4c9 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala
@@ -9,10 +9,9 @@ import fr.acinq.eclair.crypto.Sphinx
 import fr.acinq.eclair.integration.basic.fixtures.ThreeNodesFixture
 import fr.acinq.eclair.payment.Bolt11Invoice.ExtraHop
 import fr.acinq.eclair.payment._
-import fr.acinq.eclair.payment.send.PaymentError.RetryExhausted
 import fr.acinq.eclair.router.RouteNotFound
 import fr.acinq.eclair.testutils.FixtureSpec
-import fr.acinq.eclair.wire.protocol.{UnknownNextPeer, Update}
+import fr.acinq.eclair.wire.protocol.{FailureMessage, UnknownNextPeer, Update}
 import fr.acinq.eclair.{MilliSatoshiLong, RealShortChannelId}
 import org.scalatest.OptionValues.convertOptionToValuable
 import org.scalatest.concurrent.IntegrationPatience
@@ -79,14 +78,21 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
     }
   }
 
-  private def sendSuccessfulPaymentAliceToCarol(f: FixtureParam, useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): PaymentSent = {
-    import f._
-    sendSuccessfulPayment(alice, carol, 100_000 msat, hints = Seq(createBobToCarolTestHint(f, useHint, overrideHintScid_opt)))
-  }
+  case object Ok
 
-  private def sendFailingPaymentAliceToCarol(f: FixtureParam, useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): PaymentFailed = {
+  private def sendPaymentAliceToCarol(f: FixtureParam, expected: Either[Either[Throwable, FailureMessage], Ok.type], useHint: Boolean = false, overrideHintScid_opt: Option[RealShortChannelId] = None): Unit = {
     import f._
-    sendFailingPayment(alice, carol, 100_000 msat, hints = Seq(createBobToCarolTestHint(f, useHint, overrideHintScid_opt)))
+    val result = sendPayment(alice, carol, 100_000 msat, hints = Seq(createBobToCarolTestHint(f, useHint, overrideHintScid_opt))) match {
+      case Left(paymentFailed) =>
+        Left(PaymentFailure.transformForUser(paymentFailed.failures).last match {
+          case LocalFailure(_, _, t) => Left(t)
+          case RemoteFailure(_, _, e) => Right(e.failureMessage)
+          case _: UnreadableRemoteFailure => fail("received unreadable remote failure")
+        })
+      case Right(_) => Right(Ok)
+    }
+
+    assert(result == expected)
   }
 
   private def internalTest(f: FixtureParam,
@@ -94,9 +100,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
                            bcPublic: Boolean,
                            bcZeroConf: Boolean,
                            bcScidAlias: Boolean,
-                           paymentWorksWithoutHint: Boolean,
-                           paymentWorksWithHint_opt: Option[Boolean],
-                           paymentWorksWithRealScidHint_opt: Option[Boolean]): Unit = {
+                           paymentWithoutHint: Either[Either[Throwable, FailureMessage], Ok.type],
+                           paymentWithHint_opt: Option[Either[Either[Throwable, FailureMessage], Ok.type]],
+                           paymentWithRealScidHint_opt: Option[Either[Either[Throwable, FailureMessage], Ok.type]]): Unit = {
     import f._
 
     val (_, channelId_bc) = createChannels(f)(deepConfirm = deepConfirm)
@@ -123,41 +129,19 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       }
     }
 
-    def validateFailure(failure: PaymentFailed): Unit = {
-      failure.failures.foreach {
-        case LocalFailure(_, _, t) => assert(t == RouteNotFound || t == RetryExhausted)
-        case RemoteFailure(_, _, e) => assert(e.failureMessage == UnknownNextPeer)
-        case _: UnreadableRemoteFailure => fail("received unreadable remote failure")
-      }
-    }
-
     eventually {
-      if (paymentWorksWithoutHint) {
-        sendSuccessfulPaymentAliceToCarol(f)
-      } else {
-        val failure = sendFailingPaymentAliceToCarol(f)
-        validateFailure(failure)
-      }
+      sendPaymentAliceToCarol(f, paymentWithoutHint)
     }
 
-    eventually {
-      paymentWorksWithHint_opt match {
-        case Some(true) => sendSuccessfulPaymentAliceToCarol(f, useHint = true)
-        case Some(false) =>
-          val failure = sendFailingPaymentAliceToCarol(f, useHint = true)
-          validateFailure(failure)
-        case None => // skipped
+    paymentWithHint_opt.foreach { paymentWithHint =>
+      eventually {
+        sendPaymentAliceToCarol(f, paymentWithHint, useHint = true)
       }
     }
 
-    eventually {
-      paymentWorksWithRealScidHint_opt match {
-        // if alice uses the real scid instead of the bob-carol alias, it still works
-        case Some(true) => sendSuccessfulPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
-        case Some(false) =>
-          val failure = sendFailingPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
-          validateFailure(failure)
-        case None => // skipped
+    paymentWithRealScidHint_opt.foreach { paymentWithRealScidHint =>
+      eventually {
+        sendPaymentAliceToCarol(f, paymentWithRealScidHint, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value))
       }
     }
   }
@@ -168,9 +152,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = false,
       bcZeroConf = false,
       bcScidAlias = false,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works (and it will use the alias, even if the feature isn't enabled)
-      paymentWorksWithRealScidHint_opt = Some(true) // if alice uses the real scid instead of the bob-carol alias, it still works
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works (and it will use the alias, even if the feature isn't enabled)
+      paymentWithRealScidHint_opt = Some(Right(Ok)) // if alice uses the real scid instead of the bob-carol alias, it still works
     )
   }
 
@@ -180,9 +164,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = false,
       bcZeroConf = false,
       bcScidAlias = true,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works
-      paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the bob-carol alias, it doesn't work due to option_scid_alias
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works
+      paymentWithRealScidHint_opt = Some(Left(Right(UnknownNextPeer))) // if alice uses the real scid instead of the bob-carol alias, it doesn't work due to option_scid_alias
     )
   }
 
@@ -192,9 +176,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = false,
       bcZeroConf = true,
       bcScidAlias = false,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works
-      paymentWorksWithRealScidHint_opt = None // there is no real scid for bob-carol yet
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works
+      paymentWithRealScidHint_opt = None // there is no real scid for bob-carol yet
     )
   }
 
@@ -204,14 +188,14 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = false,
       bcZeroConf = true,
       bcScidAlias = false,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works
       // TODO: we should be able to send payments with the real scid in the routing hint, but this currently doesn't work,
       //  because the ChannelRelayer relies on the the LocalChannelUpdate event to maintain its scid resolution map, and
       //  the channel doesn't emit a new one when a real scid is assigned, because we use the remote alias for the
       //  channel_update, not the real scid. So the channel_update remains the same. We used to have the ChannelRelayer
       //  also listen to ShortChannelIdAssigned event, but it's doesn't seem worth it here.
-      paymentWorksWithRealScidHint_opt = None
+      paymentWithRealScidHint_opt = None
     )
   }
 
@@ -221,9 +205,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = false,
       bcZeroConf = true,
       bcScidAlias = true,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works
-      paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the b-c alias, it doesn't work due to option_scid_alias
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works
+      paymentWithRealScidHint_opt = Some(Left(Right(UnknownNextPeer))) // if alice uses the real scid instead of the b-c alias, it doesn't work due to option_scid_alias
     )
   }
 
@@ -233,9 +217,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = true,
       bcZeroConf = true,
       bcScidAlias = false,
-      paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced yet
-      paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works
-      paymentWorksWithRealScidHint_opt = None // there is no real scid for bob-carol yet
+      paymentWithoutHint = Left(Left(RouteNotFound)), // alice can't find a route to carol because bob-carol isn't announced yet
+      paymentWithHint_opt = Some(Right(Ok)), // with a routing hint the payment works
+      paymentWithRealScidHint_opt = None // there is no real scid for bob-carol yet
     )
   }
 
@@ -245,9 +229,9 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience
       bcPublic = true,
       bcZeroConf = true,
       bcScidAlias = false,
-      paymentWorksWithoutHint = true,
-      paymentWorksWithHint_opt = None, // there is no routing hints for public channels
-      paymentWorksWithRealScidHint_opt = None // there is no routing hints for public channels
+      paymentWithoutHint = Right(Ok),
+      paymentWithHint_opt = None, // there is no routing hints for public channels
+      paymentWithRealScidHint_opt = None // there is no routing hints for public channels
     )
   }
 
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
index 82348d64bb..0667d515e1 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala
@@ -21,12 +21,12 @@ import fr.acinq.eclair.payment.Bolt11Invoice.ExtraHop
 import fr.acinq.eclair.payment.receive.{MultiPartHandler, PaymentHandler}
 import fr.acinq.eclair.payment.relay.{ChannelRelayer, Relayer}
 import fr.acinq.eclair.payment.send.PaymentInitiator
-import fr.acinq.eclair.payment.{Bolt11Invoice, PaymentFailed, PaymentSent}
+import fr.acinq.eclair.payment.{Bolt11Invoice, PaymentEvent, PaymentFailed, PaymentSent}
 import fr.acinq.eclair.router.Router
 import fr.acinq.eclair.wire.protocol.IPAddress
 import fr.acinq.eclair.{BlockHeight, MilliSatoshi, MilliSatoshiLong, NodeParams, RealShortChannelId, SubscriptionsComplete, TestBitcoinCoreClient, TestDatabases, TestFeeEstimator}
-import org.scalatest.Assertions
 import org.scalatest.concurrent.Eventually.eventually
+import org.scalatest.{Assertions, EitherValues}
 
 import java.net.InetAddress
 import java.util.UUID
@@ -51,7 +51,7 @@ case class MinimalNodeFixture private(nodeParams: NodeParams,
                                       watcher: TestProbe,
                                       wallet: DummyOnChainWallet)
 
-object MinimalNodeFixture extends Assertions {
+object MinimalNodeFixture extends Assertions with EitherValues {
 
   def nodeParamsFor(alias: String, seed: ByteVector32): NodeParams = {
     NodeParams.makeNodeParams(
@@ -278,24 +278,26 @@ object MinimalNodeFixture extends Assertions {
     case _ => TestActor.KeepRunning
   }
 
-  private def sendPayment(sender: TestProbe, node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): Unit = {
+  def sendPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): Either[PaymentFailed, PaymentSent] = {
+    val sender = TestProbe("sender")
     sender.send(node2.paymentHandler, MultiPartHandler.ReceivePayment(Some(amount), Left("test payment")))
     val invoice = sender.expectMsgType[Bolt11Invoice]
 
     val routeParams = node1.nodeParams.routerConf.pathFindingExperimentConf.experiments.values.head.getDefaultRouteParams
     sender.send(node1.paymentInitiator, PaymentInitiator.SendPaymentToNode(amount, invoice, maxAttempts = 1, routeParams = routeParams, extraEdges = hints.flatMap(Bolt11Invoice.toExtraEdges(_, node2.nodeParams.nodeId)), blockUntilComplete = true))
+    sender.expectMsgType[PaymentEvent] match {
+      case e: PaymentSent => Right(e)
+      case e: PaymentFailed => Left(e)
+      case e => fail(s"unexpected event $e")
+    }
   }
 
   def sendSuccessfulPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): PaymentSent = {
-    val sender = TestProbe("sender")
-    sendPayment(sender, node1, node2, amount, hints)
-    sender.expectMsgType[PaymentSent]
+    sendPayment(node1, node2, amount, hints).value
   }
 
   def sendFailingPayment(node1: MinimalNodeFixture, node2: MinimalNodeFixture, amount: MilliSatoshi, hints: Seq[Seq[ExtraHop]] = Seq.empty)(implicit system: ActorSystem): PaymentFailed = {
-    val sender = TestProbe("sender")
-    sendPayment(sender, node1, node2, amount, hints)
-    sender.expectMsgType[PaymentFailed]
+    sendPayment(node1, node2, amount, hints).left.value
   }
 
   def prettyPrint(routerData: Router.Data, nodes: MinimalNodeFixture*): Unit = {