← Watch feed
Reject `temporary_channel_id` duplicates early (#3324)
Public commit record
93/100 · Strong
What the developer wrote
Authored by Bastien Teinturier
Reject `temporary_channel_id` duplicates early (#3324)
We previously rejected duplicate `temporary_channel_id` immediately, but
we didn't immediately reject `temporary_channel_id`s that conflict with
a *final* `channel_id` with the same peer: the rejection would happen
later in the flow when transitioning from temporary to final IDs, which
made debugging harder.
Since the channel interceptor step isn't atomic, we must check for
duplicate channel IDs *after* rate-limiting, to ensure that we don't
try inserting an entry in the channels map that is already there.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This change tightens how Eclair handles incoming Lightning channel requests. It now rejects a new channel's temporary ID if it collides with an existing final channel ID for the same peer, and it re-checks for collisions after rate-limiting. Previously, some collisions were caught only later, which could make the node misbehave or make problems harder to diagnose. The patch is defensive hardening rather than a clear exploit fix.
Recommended action
Treat as a defensive hardening patch. Review whether any in-flight channel state could still bypass the new checks, and consider whether the same collision logic should be applied to other channel-opening code paths.
Security signals we found
01
duplicate identifier collision now rejected earlier
02
non-atomic interceptor step followed by re-check
03
final channel ID included in collision detection
04
new regression test for ID reuse against confirmed channel
Evidence from the diff
In Peer.scala, duplicate channel ID checks are expanded: lookups now also check FinalChannelId in addition to TemporaryChannelId when processing open_channel and open_channel2 messages. A second duplicate/conflict check is added inside SpawnChannelNonInitiator after the channel interceptor step, because that step is non-atomic. A new test verifies that a new open message whose temporaryChannelId equals an existing confirmed channel’s final channelId is ignored and does not spawn a new channel.
Changed components
eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scalaeclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scalaInspect captured patch +79 / −57
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala
index 73b319f..794a987 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala
@@ -239,7 +239,7 @@ class Peer(val nodeParams: NodeParams,
stay() using d.copy(channels = d.channels + (TemporaryChannelId(temporaryChannelId) -> channel))
case Event(open: protocol.OpenChannel, d: ConnectedData) =>
- d.channels.get(TemporaryChannelId(open.temporaryChannelId)) match {
+ d.channels.get(TemporaryChannelId(open.temporaryChannelId)).orElse(d.channels.get(FinalChannelId(open.temporaryChannelId))) match {
case None =>
openChannelInterceptor ! OpenChannelNonInitiator(remoteNodeId, Left(open), d.localFeatures, d.remoteFeatures, d.peerConnection.toTyped, d.address)
stay()
@@ -249,7 +249,7 @@ class Peer(val nodeParams: NodeParams,
}
case Event(open: protocol.OpenDualFundedChannel, d: ConnectedData) =>
- d.channels.get(TemporaryChannelId(open.temporaryChannelId)) match {
+ d.channels.get(TemporaryChannelId(open.temporaryChannelId)).orElse(d.channels.get(FinalChannelId(open.temporaryChannelId))) match {
case None if !Features.canUseFeature(d.localFeatures, d.remoteFeatures, Features.DualFunding) =>
log.info("rejecting open_channel2: dual funding is not supported")
self ! Peer.OutgoingMessage(Error(open.temporaryChannelId, "dual funding is not supported"), d.peerConnection)
@@ -268,61 +268,66 @@ class Peer(val nodeParams: NodeParams,
case Event(SpawnChannelNonInitiator(open, channelConfig, channelType, addFunding_opt, localParams, peerConnection), d: ConnectedData) =>
val temporaryChannelId = open.fold(_.temporaryChannelId, _.temporaryChannelId)
- if (peerConnection == d.peerConnection) {
- OnTheFlyFunding.validateOpen(nodeParams.onTheFlyFundingConfig, remoteNodeId, open, pendingOnTheFlyFunding, feeCredit.getOrElse(0 msat)) match {
- case reject: OnTheFlyFunding.ValidationResult.Reject =>
- log.warning("rejecting on-the-fly channel: {}", reject.cancel.toAscii)
- self ! Peer.OutgoingMessage(reject.cancel, d.peerConnection)
- cancelUnsignedOnTheFlyFunding(reject.paymentHashes)
- context.system.eventStream.publish(ChannelAborted(ActorRef.noSender, remoteNodeId, temporaryChannelId))
- stay()
- case accept: OnTheFlyFunding.ValidationResult.Accept =>
- val channelKeys = nodeParams.channelKeyManager.channelKeys(channelConfig, localParams.fundingKeyPath)
- val channel = spawnChannel(channelKeys)
- context.system.scheduler.scheduleOnce(nodeParams.channelConf.channelFundingTimeout, channel, Channel.TickChannelOpenTimeout)(context.dispatcher)
- log.info(s"accepting a new channel with type=$channelType temporaryChannelId=$temporaryChannelId localParams=$localParams")
- open match {
- case Left(open) =>
- val init = INPUT_INIT_CHANNEL_NON_INITIATOR(
- temporaryChannelId = open.temporaryChannelId,
- fundingContribution_opt = None,
- dualFunded = false,
- pushAmount_opt = None,
- requireConfirmedInputs = false,
- localChannelParams = localParams,
- proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingSatoshis, channelType, unlimitedMaxHtlcValueInFlight = false),
- remote = d.peerConnection,
- remoteInit = d.remoteInit,
- channelConfig = channelConfig,
- channelType = channelType)
- channel ! init
- channel ! open
- case Right(open) =>
- val init = INPUT_INIT_CHANNEL_NON_INITIATOR(
- temporaryChannelId = open.temporaryChannelId,
- fundingContribution_opt = addFunding_opt,
- dualFunded = true,
- pushAmount_opt = None,
- requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding,
- localChannelParams = localParams,
- proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingAmount + addFunding_opt.map(_.fundingAmount).getOrElse(0 sat), channelType, unlimitedMaxHtlcValueInFlight = false),
- remote = d.peerConnection,
- remoteInit = d.remoteInit,
- channelConfig = channelConfig,
- channelType = channelType)
- channel ! init
- accept.useFeeCredit_opt match {
- case Some(useFeeCredit) => channel ! open.copy(tlvStream = TlvStream(open.tlvStream.records + ChannelTlv.UseFeeCredit(useFeeCredit)))
- case None => channel ! open
- }
- }
- fulfillOnTheFlyFundingHtlcs(accept.preimages)
- stay() using d.copy(channels = d.channels + (TemporaryChannelId(temporaryChannelId) -> channel))
- }
- } else {
- log.warning("ignoring open_channel request that reconnected during channel intercept, temporaryChannelId={}", temporaryChannelId)
- context.system.eventStream.publish(ChannelAborted(ActorRef.noSender, remoteNodeId, temporaryChannelId))
- stay()
+ // Since the channel interceptor step isn't atomic, we must check again that there is no duplicate/conflict.
+ d.channels.get(TemporaryChannelId(temporaryChannelId)).orElse(d.channels.get(FinalChannelId(temporaryChannelId))) match {
+ case Some(_) =>
+ log.warning("ignoring open_channel with duplicate temporaryChannelId={}", temporaryChannelId)
+ stay()
+ case None if peerConnection != d.peerConnection =>
+ log.warning("ignoring open_channel request that reconnected during channel intercept, temporaryChannelId={}", temporaryChannelId)
+ context.system.eventStream.publish(ChannelAborted(ActorRef.noSender, remoteNodeId, temporaryChannelId))
+ stay()
+ case None =>
+ OnTheFlyFunding.validateOpen(nodeParams.onTheFlyFundingConfig, remoteNodeId, open, pendingOnTheFlyFunding, feeCredit.getOrElse(0 msat)) match {
+ case reject: OnTheFlyFunding.ValidationResult.Reject =>
+ log.warning("rejecting on-the-fly channel: {}", reject.cancel.toAscii)
+ self ! Peer.OutgoingMessage(reject.cancel, d.peerConnection)
+ cancelUnsignedOnTheFlyFunding(reject.paymentHashes)
+ context.system.eventStream.publish(ChannelAborted(ActorRef.noSender, remoteNodeId, temporaryChannelId))
+ stay()
+ case accept: OnTheFlyFunding.ValidationResult.Accept =>
+ val channelKeys = nodeParams.channelKeyManager.channelKeys(channelConfig, localParams.fundingKeyPath)
+ val channel = spawnChannel(channelKeys)
+ context.system.scheduler.scheduleOnce(nodeParams.channelConf.channelFundingTimeout, channel, Channel.TickChannelOpenTimeout)(context.dispatcher)
+ log.info(s"accepting a new channel with type=$channelType temporaryChannelId=$temporaryChannelId localParams=$localParams")
+ open match {
+ case Left(open) =>
+ val init = INPUT_INIT_CHANNEL_NON_INITIATOR(
+ temporaryChannelId = open.temporaryChannelId,
+ fundingContribution_opt = None,
+ dualFunded = false,
+ pushAmount_opt = None,
+ requireConfirmedInputs = false,
+ localChannelParams = localParams,
+ proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingSatoshis, channelType, unlimitedMaxHtlcValueInFlight = false),
+ remote = d.peerConnection,
+ remoteInit = d.remoteInit,
+ channelConfig = channelConfig,
+ channelType = channelType)
+ channel ! init
+ channel ! open
+ case Right(open) =>
+ val init = INPUT_INIT_CHANNEL_NON_INITIATOR(
+ temporaryChannelId = open.temporaryChannelId,
+ fundingContribution_opt = addFunding_opt,
+ dualFunded = true,
+ pushAmount_opt = None,
+ requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding,
+ localChannelParams = localParams,
+ proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingAmount + addFunding_opt.map(_.fundingAmount).getOrElse(0 sat), channelType, unlimitedMaxHtlcValueInFlight = false),
+ remote = d.peerConnection,
+ remoteInit = d.remoteInit,
+ channelConfig = channelConfig,
+ channelType = channelType)
+ channel ! init
+ accept.useFeeCredit_opt match {
+ case Some(useFeeCredit) => channel ! open.copy(tlvStream = TlvStream(open.tlvStream.records + ChannelTlv.UseFeeCredit(useFeeCredit)))
+ case None => channel ! open
+ }
+ }
+ fulfillOnTheFlyFundingHtlcs(accept.preimages)
+ stay() using d.copy(channels = d.channels + (TemporaryChannelId(temporaryChannelId) -> channel))
+ }
}
case Event(cmd: ProposeOnTheFlyFunding, d: ConnectedData) if !d.remoteFeatures.hasFeature(Features.OnTheFlyFunding) =>
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala
index f1bc5ab..ac7e9ed 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala
@@ -448,6 +448,23 @@ class PeerSpec extends FixtureSpec {
assert(peer.stateData.channels.size == 1)
}
+ test("don't spawn a channel that reuses an existing channel id") { f =>
+ import f._
+
+ val confirmedChannel = ChannelCodecsSpec.normal.withChannelKeys(nodeParams)
+ connect(remoteNodeId, peer, peerConnection, switchboard, channels = Set(confirmedChannel))
+ channel.expectMsg(INPUT_RESTORED(confirmedChannel.channelData))
+ channel.expectMsgType[INPUT_RECONNECTED]
+
+ val open = createOpenChannelMessage(ChannelTypes.AnchorOutputsZeroFeeHtlcTx()).copy(temporaryChannelId = confirmedChannel.channelId)
+ peerConnection.send(peer, open)
+ val open2 = createOpenDualFundedChannelMessage(ChannelTypes.AnchorOutputsZeroFeeHtlcTx()).copy(temporaryChannelId = confirmedChannel.channelId)
+ peerConnection.send(peer, open2)
+ channel.expectNoMessage(100 millis)
+ peerConnection.expectNoMessage(100 millis)
+ assert(peer.stateData.channels.size == 1)
+ }
+
test("send error when receiving message for unknown channel") { f =>
import f._
Risk score
Our methodology →Why this scored 37/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.