From cfd65141111fdfa9872bb68eeff718408eda9332 Mon Sep 17 00:00:00 2001 From: t-bast Date: Fri, 10 Jun 2022 08:38:46 +0200 Subject: [PATCH] Various nits --- .../fr/acinq/eclair/channel/ChannelData.scala | 6 +++++- .../acinq/eclair/channel/ChannelEvents.scala | 19 +++++++++++-------- .../eclair/channel/ChannelFeatures.scala | 2 +- .../fr/acinq/eclair/channel/Helpers.scala | 10 +++++----- .../fr/acinq/eclair/channel/Register.scala | 2 +- .../channel/version3/ChannelCodecs3.scala | 2 +- .../channel/version3/ChannelCodecs3Spec.scala | 2 +- 7 files changed, 25 insertions(+), 18 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala index 095d377be3..8a7ba9b978 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala @@ -429,10 +429,14 @@ final case class DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments: Commitments, final case class DATA_WAIT_FOR_CHANNEL_READY(commitments: Commitments, shortIds: ShortIds, lastSent: ChannelReady) extends PersistentChannelData + sealed trait RealScidStatus { def toOption: Option[RealShortChannelId] } object RealScidStatus { + /** The funding transaction has been confirmed but hasn't reached min_depth, we must be ready for a reorg. */ case class Temporary(realScid: RealShortChannelId) extends RealScidStatus { override def toOption: Option[RealShortChannelId] = Some(realScid) } + /** The funding transaction has been deeply confirmed. */ case class Final(realScid: RealShortChannelId) extends RealScidStatus { override def toOption: Option[RealShortChannelId] = Some(realScid) } + /** The funding transaction is not confirmed. */ case object Unknown extends RealScidStatus { override def toOption: Option[RealShortChannelId] = None } } @@ -441,7 +445,7 @@ object RealScidStatus { * * @param real the real scid, it may change if a reorg happens before the channel reaches 6 conf * @param localAlias we must remember the alias that we sent to our peer because we use it to: - * - identify incoming [[ChannelUpdate]] + * - identify incoming [[ChannelUpdate]] at the connection level * - route outgoing payments to that channel * @param remoteAlias_opt we only remember the last alias received from our peer, we use this to generate * routing hints in [[fr.acinq.eclair.payment.Bolt11Invoice]] diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelEvents.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelEvents.scala index 910a731407..b41cffaa9c 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelEvents.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelEvents.scala @@ -19,10 +19,10 @@ package fr.acinq.eclair.channel import akka.actor.ActorRef import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey import fr.acinq.bitcoin.scalacompat.{ByteVector32, Satoshi, Transaction} -import fr.acinq.eclair.{BlockHeight, Features, Alias, RealShortChannelId, ShortChannelId} import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel.Helpers.Closing.ClosingType import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelUpdate} +import fr.acinq.eclair.{BlockHeight, Features, ShortChannelId} /** * Created by PM on 17/08/2016. @@ -51,15 +51,18 @@ case class ShortChannelIdAssigned(channel: ActorRef, channelId: ByteVector32, sh case class LocalChannelUpdate(channel: ActorRef, channelId: ByteVector32, shortIds: ShortIds, remoteNodeId: PublicKey, channelAnnouncement_opt: Option[ChannelAnnouncement], channelUpdate: ChannelUpdate, commitments: AbstractCommitments) extends ChannelEvent { /** - * We always map the local alias because we must always be able to route based on it - * However we only map the real scid if option_scid_alias (TODO: rename to option_scid_privacy) is disabled + * We always include the local alias because we must always be able to route based on it. + * However we only include the real scid if option_scid_alias is disabled, because we otherwise want to hide it. */ def scidsForRouting: Seq[ShortChannelId] = { - commitments match { - case c: Commitments => - val realScid_opt = if (c.channelFeatures.hasFeature(Features.ScidAlias)) None else shortIds.real.toOption - realScid_opt.toSeq :+ shortIds.localAlias - case _ => Seq(shortIds.localAlias) // TODO: ugly + val canUseRealScid = commitments match { + case c: Commitments => !c.channelFeatures.hasFeature(Features.ScidAlias) + case _ => false + } + if (canUseRealScid) { + shortIds.real.toOption.toSeq :+ shortIds.localAlias + } else { + Seq(shortIds.localAlias) } } } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala index 7bd79bb98e..77b1b15561 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala @@ -115,7 +115,7 @@ object ChannelTypes { ).flatten override def paysDirectlyToWallet: Boolean = false override def commitmentFormat: CommitmentFormat = ZeroFeeHtlcTxAnchorOutputsCommitmentFormat - override def toString: String = s"anchor_outputs_zero_fee_htlc_tx${if (scidAlias) "+scid_alias" else ""}${if (zeroConf) "+zeroconf" else ""}" + override def toString: String = s"anchor_outputs_zero_fee_htlc_tx${if (scidAlias) "+scid_alias" else ""}${if (zeroConf) "+zero_conf" else ""}" } case class UnsupportedChannelType(featureBits: Features[InitFeature]) extends ChannelType { override def features: Set[InitFeature] = featureBits.activated.keySet 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 a2649ef69e..cbf6d4b4bc 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 @@ -21,6 +21,7 @@ import fr.acinq.bitcoin.ScriptFlags 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 @@ -35,7 +36,6 @@ import fr.acinq.eclair.transactions.Scripts._ import fr.acinq.eclair.transactions.Transactions._ import fr.acinq.eclair.transactions._ import fr.acinq.eclair.wire.protocol._ -import fr.acinq.eclair.{RealShortChannelId, _} import scodec.bits.ByteVector import scala.concurrent.duration._ @@ -196,17 +196,17 @@ object Helpers { * - before channel announcement: use remote_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) : unsupported + * - 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)(implicit log: DiagnosticLoggingAdapter): ShortChannelId = { + 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 } - def scidForChannelUpdate(d: DATA_NORMAL)(implicit log: DiagnosticLoggingAdapter): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds) + def scidForChannelUpdate(d: DATA_NORMAL): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds) /** * Compute the delay until we need to refresh the channel_update for our channel not to be considered stale by diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Register.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Register.scala index c385f352ac..7622bd13f1 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Register.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Register.scala @@ -49,7 +49,7 @@ class Register extends Actor with ActorLogging { case scidAssigned: ShortChannelIdAssigned => // We map all known scids (real or alias) to the channel_id. The relayer is in charge of deciding whether a real - // scid can be used or not for routing (see option_scid_privacy), but the register is neutral. + // scid can be used or not for routing (see option_scid_alias), but the register is neutral. val m = (scidAssigned.shortIds.real.toOption.toSeq :+ scidAssigned.shortIds.localAlias).map(_ -> scidAssigned.channelId).toMap context become main(channels, shortIds ++ m, channelsTo) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3.scala index 8d239dd94b..8dfb7410b8 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3.scala @@ -410,7 +410,7 @@ private[channel] object ChannelCodecs3 { // Order matters! val channelDataCodec: Codec[PersistentChannelData] = discriminated[PersistentChannelData].by(uint16) - .typecase(0x10, Codecs.DATA_WAIT_FOR_CHANNEL_READY_Codec) + .typecase(0x0a, Codecs.DATA_WAIT_FOR_CHANNEL_READY_Codec) .typecase(0x09, Codecs.DATA_NORMAL_Codec) .typecase(0x08, Codecs.DATA_SHUTDOWN_Codec) .typecase(0x07, Codecs.DATA_NORMAL_COMPAT_07_Codec) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3Spec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3Spec.scala index 8c93c02966..ff76462c35 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3Spec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/wire/internal/channel/version3/ChannelCodecs3Spec.scala @@ -257,7 +257,7 @@ class ChannelCodecs3Spec extends AnyFunSuite { assert(data.shortIds.localAlias == ShortChannelId(123456789L)) assert(data.shortIds.real == RealScidStatus.Temporary(RealShortChannelId(123456789L))) val binMigrated = channelDataCodec.encode(data).require.toHex - assert(binMigrated.startsWith("0010")) // NB: 01 -> 10 + assert(binMigrated.startsWith("000a")) // NB: 01 -> 0a } {