From de984e945acb41c1d30f1661a146ae350c216f16 Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 28 Jun 2022 17:55:08 +0200 Subject: [PATCH 1/4] random value for alias Turns out the value space is large enough to not have to worry about duplicates. --- .../fr/acinq/eclair/ShortChannelId.scala | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala index 8a4e18490b..09c01dea20 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala @@ -58,7 +58,24 @@ object ShortChannelId { def toShortId(blockHeight: Int, txIndex: Int, outputIndex: Int): Long = ((blockHeight & 0xFFFFFFL) << 40) | ((txIndex & 0xFFFFFFL) << 16) | (outputIndex & 0xFFFFL) - def generateLocalAlias(): Alias = Alias(System.nanoTime()) // TODO: fixme (duplicate, etc.) + /** + * At block height 350 000 LN didn't exist, so all real scids less than that will never be used. This results in + * `384_829_069_721_665_536` possible values, greater than `2^58`. + * + * The expected number of values before we have a collision can be approximated by (*): + * `Q(H) ~= sqrt( Pi * H / 2) = sqrt(Pi * 2^57) = 673 000 000` + * + * We don't expect to have anywhere close to that many channels at any given time on our node, so we don't need to + * check for duplicate values. + * + * (*) https://en.wikipedia.org/wiki/Birthday_attack + */ + private val aliasUpperBound = RealShortChannelId.apply(BlockHeight(350_000),1,0).toLong + + def generateLocalAlias(): Alias = { + val l = randomLong() % aliasUpperBound + Alias(l) + } @inline def blockHeight(shortChannelId: ShortChannelId): BlockHeight = BlockHeight((shortChannelId.toLong >> 40) & 0xFFFFFF) From 3ff48316a7f8ea81160a129de810a6d98934de93 Mon Sep 17 00:00:00 2001 From: pm47 Date: Wed, 29 Jun 2022 11:16:21 +0200 Subject: [PATCH 2/4] add collision check in register --- .../src/main/scala/fr/acinq/eclair/channel/Register.scala | 6 ++++++ 1 file changed, 6 insertions(+) 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 f7d3e6c2cc..df053325c1 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 @@ -52,6 +52,12 @@ class Register extends Actor with ActorLogging { // 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_alias), but the register is neutral. val m = (scidAssigned.shortIds.real.toOption.toSeq :+ scidAssigned.shortIds.localAlias).map(_ -> scidAssigned.channelId).toMap + // duplicate check for aliases (we use a random value in a large enough space that there should never be collisions) + shortIds.get(scidAssigned.shortIds.localAlias) match { + case Some(channelId) if channelId != scidAssigned.channelId => + log.error("duplicate alias={} for channelIds={},{} this should never happen!", scidAssigned.shortIds.localAlias, channelId, scidAssigned.channelId) + case _ => () + } context become main(channels, shortIds ++ m, channelsTo) case Terminated(actor) if channels.values.toSet.contains(actor) => From 09d32d8567d212a9290355fe80f57479ff3065c6 Mon Sep 17 00:00:00 2001 From: pm47 Date: Wed, 29 Jun 2022 11:27:17 +0200 Subject: [PATCH 3/4] use a loop instead of modulo to find random value --- .../src/main/scala/fr/acinq/eclair/ShortChannelId.scala | 7 +++++-- .../test/scala/fr/acinq/eclair/ShortChannelIdSpec.scala | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala index 09c01dea20..56f8dac351 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala @@ -70,10 +70,13 @@ object ShortChannelId { * * (*) https://en.wikipedia.org/wiki/Birthday_attack */ - private val aliasUpperBound = RealShortChannelId.apply(BlockHeight(350_000),1,0).toLong + private val aliasUpperBound = RealShortChannelId.apply(BlockHeight(350_000), 1, 0).toLong def generateLocalAlias(): Alias = { - val l = randomLong() % aliasUpperBound + var l = -1L + do { + l = randomLong() + } while (l < 0 || l > aliasUpperBound) Alias(l) } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/ShortChannelIdSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/ShortChannelIdSpec.scala index fc472f2054..094f4dfa4b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/ShortChannelIdSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/ShortChannelIdSpec.scala @@ -70,4 +70,11 @@ class ShortChannelIdSpec extends AnyFunSuite { Seq(-561L, 0xffffffffffffffffL, 0x2affffffffffffffL).foreach(id => assert(Alias(id) == UnspecifiedShortChannelId(id))) } + test("basic check on random alias generation") { + for(_ <- 0 until 100) { + val alias = ShortChannelId.generateLocalAlias() + assert(alias.toLong >= 0 && alias.toLong <= 384_829_069_721_665_536L) + } + } + } From 88cbc77b6c40808af59a8984972d56a6a0293f97 Mon Sep 17 00:00:00 2001 From: pm47 Date: Wed, 29 Jun 2022 11:53:32 +0200 Subject: [PATCH 4/4] back to modulo --- .../main/scala/fr/acinq/eclair/ShortChannelId.scala | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala index 56f8dac351..3e33ff2118 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala @@ -60,7 +60,7 @@ object ShortChannelId { /** * At block height 350 000 LN didn't exist, so all real scids less than that will never be used. This results in - * `384_829_069_721_665_536` possible values, greater than `2^58`. + * more than `2^58` values. * * The expected number of values before we have a collision can be approximated by (*): * `Q(H) ~= sqrt( Pi * H / 2) = sqrt(Pi * 2^57) = 673 000 000` @@ -70,13 +70,11 @@ object ShortChannelId { * * (*) https://en.wikipedia.org/wiki/Birthday_attack */ - private val aliasUpperBound = RealShortChannelId.apply(BlockHeight(350_000), 1, 0).toLong + private val aliasUpperBound = 2^58 def generateLocalAlias(): Alias = { - var l = -1L - do { - l = randomLong() - } while (l < 0 || l > aliasUpperBound) + // modulo won't skew the distribution because 2^64 is a multiple of 2^58 + val l = Math.abs(randomLong() % aliasUpperBound) Alias(l) }