Skip to content

Generate local alias before spawning channel #2316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pm47 wants to merge 4 commits into from
Closed

Generate local alias before spawning channel #2316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f3fa19
generate local alias before spawning channel
pm47 Jun 15, 2022
d900265
use last 7B of channel_id for alias
pm47 Jun 17, 2022
4ee1025
cleanup
pm47 Jun 17, 2022
60f581c
random local alias with collision protection
pm47 Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package fr.acinq.eclair

import fr.acinq.eclair.ShortChannelId.toShortId

import scala.util.Random

// @formatter:off
sealed trait ShortChannelId extends Ordered[ShortChannelId] {
def toLong: Long
Expand Down Expand Up @@ -61,7 +63,11 @@ 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.)
def generateLocalAlias(): Alias = {
// at block height 1000 LN didn't exist, so all real scids less than that will never be used
val upperBound = RealShortChannelId.apply(BlockHeight(1000),1,0).toLong
Comment on lines +67 to +68
Copy link
Copy Markdown
Member

@t-bast t-bast Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use such a small range? We can go up to at least block 350 000, can't we?

Copy link
Copy Markdown
Member Author

@pm47 pm47 Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The space is already huge : 1_099_511_627_841_536 (*) and we are checking for duplicates. I figure leaving some values available could prove handy later?

(*) So huge in fact that I believe our previous computation was invalid. If we indeed go up to block height 350 000 it gives us a ceiling of 384_829_069_721_665_536 which is about 2^59. The probability of a collision for 250 000 channels would be 0.000011 %, not 0.8 % ?

Alias(Random.nextLong(upperBound))
Copy link
Copy Markdown
Member

@t-bast t-bast Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the randomLong from package.scala here.

}

@inline
def blockHeight(shortChannelId: ShortChannelId): BlockHeight = BlockHeight((shortChannelId.toLong >> 40) & 0xFFFFFF)
Expand Down
10 changes: 8 additions & 2 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,17 @@ case class INPUT_INIT_FUNDER(temporaryChannelId: ByteVector32,
remoteInit: Init,
channelFlags: ChannelFlags,
channelConfig: ChannelConfig,
channelType: SupportedChannelType) {
channelType: SupportedChannelType,
localAlias: Alias) {
require(!(channelType.features.contains(Features.ScidAlias) && channelFlags.announceChannel), "option_scid_alias is not compatible with public channels")
}
case class INPUT_INIT_FUNDEE(temporaryChannelId: ByteVector32,
localParams: LocalParams,
remote: ActorRef,
remoteInit: Init,
channelConfig: ChannelConfig,
channelType: SupportedChannelType)
channelType: SupportedChannelType,
localAlias: Alias)
case object INPUT_CLOSE_COMPLETE_TIMEOUT // when requesting a mutual close, we wait for as much as this timeout, then unilateral close
case object INPUT_DISCONNECTED
case class INPUT_RECONNECTED(remote: ActorRef, localInit: Init, remoteInit: Init)
Expand Down Expand Up @@ -393,6 +395,7 @@ final case class DATA_WAIT_FOR_FUNDING_INTERNAL(temporaryChannelId: ByteVector32
remoteFirstPerCommitmentPoint: PublicKey,
channelConfig: ChannelConfig,
channelFeatures: ChannelFeatures,
localAlias: Alias,
lastSent: OpenChannel) extends TransientChannelData {
val channelId: ByteVector32 = temporaryChannelId
}
Expand All @@ -406,6 +409,7 @@ final case class DATA_WAIT_FOR_FUNDING_CREATED(temporaryChannelId: ByteVector32,
channelFlags: ChannelFlags,
channelConfig: ChannelConfig,
channelFeatures: ChannelFeatures,
localAlias: Alias,
lastSent: AcceptChannel) extends TransientChannelData {
val channelId: ByteVector32 = temporaryChannelId
}
Expand All @@ -420,10 +424,12 @@ final case class DATA_WAIT_FOR_FUNDING_SIGNED(channelId: ByteVector32,
channelFlags: ChannelFlags,
channelConfig: ChannelConfig,
channelFeatures: ChannelFeatures,
localAlias: Alias,
lastSent: FundingCreated) extends TransientChannelData
final case class DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments: Commitments,
fundingTx: Option[Transaction],
waitingSince: BlockHeight, // how long have we been waiting for the funding tx to confirm
localAlias: Alias,
deferred: Option[ChannelReady],
lastSent: Either[FundingCreated, FundingSigned]) extends PersistentChannelData
final case class DATA_WAIT_FOR_CHANNEL_READY(commitments: Commitments,
Expand Down
14 changes: 13 additions & 1 deletion eclair-core/src/main/scala/fr/acinq/eclair/channel/Register.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package fr.acinq.eclair.channel
import akka.actor.{Actor, ActorLogging, ActorRef, Terminated}
import fr.acinq.bitcoin.scalacompat.ByteVector32
import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
import fr.acinq.eclair.ShortChannelId
import fr.acinq.eclair.{Alias, ShortChannelId}
import fr.acinq.eclair.channel.Register._

import scala.concurrent.Promise

/**
* Created by PM on 26/01/2016.
*/
Expand All @@ -32,6 +34,7 @@ class Register extends Actor with ActorLogging {
context.system.eventStream.subscribe(self, classOf[AbstractChannelRestored])
context.system.eventStream.subscribe(self, classOf[ChannelIdAssigned])
context.system.eventStream.subscribe(self, classOf[ShortChannelIdAssigned])
context.system.eventStream.subscribe(self, classOf[GenerateLocalAlias])

override def receive: Receive = main(Map.empty, Map.empty, Map.empty)

Expand Down Expand Up @@ -79,6 +82,13 @@ class Register extends Actor with ActorLogging {
case Some(channel) => channel.tell(msg, compatReplyTo)
case None => compatReplyTo ! ForwardShortIdFailure(fwd)
}

case GenerateLocalAlias(promise) =>
Copy link
Copy Markdown
Member

@t-bast t-bast Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we re-populate the shortIds map asynchronously as channels are restored and send ShortChannelIdAssigned, we may receive that message before we've restored all channels and recovered a complete shortIds map.

I believe it won't be an issue in practice as long as the range we use for ShortChannelId.generateLocalAlias() is big enough to make collisions unlikely.

var alias: Alias = null
do {
alias = ShortChannelId.generateLocalAlias()
} while (shortIds.contains(alias)) // we make sure that there is no collision with an existing scid
promise.trySuccess(alias)
}
}

Expand All @@ -90,5 +100,7 @@ object Register {

case class ForwardFailure[T](fwd: Forward[T])
case class ForwardShortIdFailure[T](fwd: ForwardShortId[T])

case class GenerateLocalAlias(promise: Promise[Alias])
// @formatter:on
}
4 changes: 2 additions & 2 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
startWith(WAIT_FOR_INIT_INTERNAL, Nothing)

when(WAIT_FOR_INIT_INTERNAL)(handleExceptions {
case Event(initFunder@INPUT_INIT_FUNDER(temporaryChannelId, fundingSatoshis, pushMsat, commitTxFeerate, fundingTxFeerate, localParams, remote, remoteInit, channelFlags, channelConfig, channelType), Nothing) =>
case Event(initFunder@INPUT_INIT_FUNDER(temporaryChannelId, fundingSatoshis, pushMsat, commitTxFeerate, fundingTxFeerate, localParams, remote, remoteInit, channelFlags, channelConfig, channelType, localAlias), Nothing) =>
context.system.eventStream.publish(ChannelCreated(self, peer, remoteNodeId, isInitiator = true, temporaryChannelId, commitTxFeerate, Some(fundingTxFeerate)))
activeConnection = remote
txPublisher ! SetChannelId(remoteNodeId, temporaryChannelId)
Expand Down Expand Up @@ -241,7 +241,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
))
goto(WAIT_FOR_ACCEPT_CHANNEL) using DATA_WAIT_FOR_ACCEPT_CHANNEL(initFunder, open) sending open

case Event(inputFundee@INPUT_INIT_FUNDEE(_, localParams, remote, _, _, _), Nothing) if !localParams.isInitiator =>
case Event(inputFundee@INPUT_INIT_FUNDEE(_, localParams, remote, _, _, _, _), Nothing) if !localParams.isInitiator =>
activeConnection = remote
txPublisher ! SetChannelId(remoteNodeId, inputFundee.temporaryChannelId)
goto(WAIT_FOR_OPEN_CHANNEL) using DATA_WAIT_FOR_OPEN_CHANNEL(inputFundee)
Expand Down
Loading