mirrors/eclair
1
0
Fork 0
mirror of https://github.com/ACINQ/eclair.git synced 2025-02-23 14:40:34 +01:00
Code Issues Projects Releases Wiki Activity

Fix flaky zeroconf test (#2352)

Browse source 
The flaky test was `ZeroConfAliasIntegrationSpec`.`a->b->c (b-c private)`.

Due to a race between node watchers, we were sometimes entering into
this scenario between bob and carol:

1) carol's watcher wins the race and sends her `channel_ready` to bob
  before bob has received `WatchFundingConfirmedTriggered` from his own
  watcher
2) the channel isn't officially zeroconf, but bob gladly accepts carol's
  early `channel_ready` and moves directly to `WAIT_FOR_CHANNEL_READY` and
  then `NORMAL`, *without a real scid*.
3) bob ignores the `WatchFundingConfirmedTriggered` that it receives in
  state `WAIT_FOR_CHANNEL_READY` or `NORMAL`
4) bob later receives the `WatchFundingDeeplyBuriedTriggered`, discovers
  his own real scid and emits a `ShortChannelIdAssigned` event, but the
  channel relayer only listens to `LocalChannelUpdate`.
5) subsequent a->b->c payment using a real scid for b-c as routing hint
  fails because b's relayer doesn't know it.

There are two ways of fixing this:

a) make the relayer listen to `ShortChannelIdAssigned` (like it did before)
b) emit a `LocalChannelUpdate` event containing the new real scid and
  the same `channel_update`.

In this commit, we chose to implement (b).

Co-authored-by: t-bast <bastuc@hotmail.fr>
This commit is contained in:
Pierre-Marie Padiou 2022-07-26 14:40:42 +02:00 • committed by GitHub
parent 214873e4a7
commit 23de6c4efd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
View file

@ -1627,7 +1627,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
case (WAIT_FOR_INIT_INTERNAL, OFFLINE, _, d: DATA_NORMAL) => Some(EmitLocalChannelUpdate("restore", d, sendToPeer = false))
case (WAIT_FOR_FUNDING_CONFIRMED, NORMAL, _, d: DATA_NORMAL) => Some(EmitLocalChannelUpdate("initial", d, sendToPeer = true))
case (WAIT_FOR_CHANNEL_READY, NORMAL, _, d: DATA_NORMAL) => Some(EmitLocalChannelUpdate("initial", d, sendToPeer = true))
case (NORMAL, NORMAL, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate != d2.channelUpdate || d1.channelAnnouncement != d2.channelAnnouncement => Some(EmitLocalChannelUpdate("normal->normal", d2, sendToPeer = d2.channelAnnouncement.isEmpty))
case (NORMAL, NORMAL, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.shortIds.real.toOption != d2.shortIds.real.toOption || d1.channelUpdate != d2.channelUpdate || d1.channelAnnouncement != d2.channelAnnouncement => Some(EmitLocalChannelUpdate("normal->normal", d2, sendToPeer = d2.channelAnnouncement.isEmpty && d1.channelUpdate != d2.channelUpdate))
case (SYNCING, NORMAL, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate != d2.channelUpdate || d1.channelAnnouncement != d2.channelAnnouncement => Some(EmitLocalChannelUpdate("syncing->normal", d2, sendToPeer = d2.channelAnnouncement.isEmpty))
case (NORMAL, OFFLINE, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate != d2.channelUpdate || d1.channelAnnouncement != d2.channelAnnouncement => Some(EmitLocalChannelUpdate("normal->offline", d2, sendToPeer = false))
case (OFFLINE, OFFLINE, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate != d2.channelUpdate || d1.channelAnnouncement != d2.channelAnnouncement => Some(EmitLocalChannelUpdate("offline->offline", d2, sendToPeer = false))

2
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunder.scala
View file

@ -356,7 +356,7 @@ trait ChannelOpenSingleFunder extends FundingHandlers with ErrorHandlers {
when(WAIT_FOR_FUNDING_CONFIRMED)(handleExceptions {
case Event(remoteChannelReady: ChannelReady, d: DATA_WAIT_FOR_FUNDING_CONFIRMED) =>
if (remoteChannelReady.alias_opt.isDefined && d.commitments.localParams.isInitiator) {
log.info("this chanel isn't zero-conf, but we are funder and they sent an early channel_ready with an alias: no need to wait for confirmations")
log.info("this channel isn't zero-conf, but we are funder and they sent an early channel_ready with an alias: no need to wait for confirmations")
// No need to emit ShortChannelIdAssigned: we will emit it when handling their channel_ready in WAIT_FOR_CHANNEL_READY
val (shortIds, localChannelReady) = acceptFundingTx(d.commitments, RealScidStatus.Unknown)
self ! remoteChannelReady

46
eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
View file

@ -28,6 +28,7 @@ import fr.acinq.eclair._
import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._
import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw, FeeratesPerKw}
import fr.acinq.eclair.blockchain.{CurrentBlockHeight, CurrentFeerates}
import fr.acinq.eclair.channel.RealScidStatus.Final
import fr.acinq.eclair.channel._
import fr.acinq.eclair.channel.fsm.Channel
import fr.acinq.eclair.channel.fsm.Channel._
@ -3441,8 +3442,8 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
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
val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
assert(aliceIds.real == RealScidStatus.Unknown)
val aliceState = alice.stateData.asInstanceOf[DATA_NORMAL]
assert(aliceState.shortIds.real == RealScidStatus.Unknown)
// funding tx coordinates (unknown before)
val (blockHeight, txIndex) = (BlockHeight(400000), 42)
alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null)
@ -3452,14 +3453,17 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
// alice updates her internal state
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId))
alice2bob.expectNoMessage(100 millis)
channelUpdateListener.expectNoMessage(100 millis)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
// we emit a new local channel update containing the same channel_update, but also the new real scid
val lcu = channelUpdateListener.expectMsgType[LocalChannelUpdate]
assert(lcu.shortIds.real == Final(realShortChannelId))
assert(lcu.channelUpdate == aliceState.channelUpdate)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceState.shortIds.localAlias)
}
test("recv WatchFundingDeeplyBuriedTriggered (public channel, short channel id changed)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>
import f._
val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
val realShortChannelId = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
val aliceState = alice.stateData.asInstanceOf[DATA_NORMAL]
val realShortChannelId = aliceState.shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
// existing funding tx coordinates
val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
// new funding tx coordinates (there was a reorg)
@ -3470,9 +3474,12 @@ 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))
alice2bob.expectNoMessage(100 millis)
// we emit a new local channel update containing the same channel_update, but also the new real scid
val lcu = channelUpdateListener.expectMsgType[LocalChannelUpdate]
assert(lcu.shortIds.real == Final(newRealShortChannelId))
assert(lcu.channelUpdate == aliceState.channelUpdate)
channelUpdateListener.expectNoMessage(100 millis)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceIds.localAlias)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceState.shortIds.localAlias)
}
test("recv WatchFundingDeeplyBuriedTriggered (private channel)") { f =>
@ -3496,24 +3503,27 @@ 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
val aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
assert(aliceIds.real == RealScidStatus.Unknown)
val aliceState = alice.stateData.asInstanceOf[DATA_NORMAL]
assert(aliceState.shortIds.real == RealScidStatus.Unknown)
alice ! WatchFundingDeeplyBuriedTriggered(BlockHeight(42000), 42, null)
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'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)
// we emit a new local channel update containing the same channel_update, but also the new real scid
val lcu = channelUpdateListener.expectMsgType[LocalChannelUpdate]
assert(lcu.shortIds.real == Final(realShortChannelId))
assert(lcu.channelUpdate == aliceState.channelUpdate)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceState.shortIds.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 aliceIds = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds
val realShortChannelId = aliceIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
val aliceState = alice.stateData.asInstanceOf[DATA_NORMAL]
val realShortChannelId = aliceState.shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid
// existing funding tx coordinates
val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId)
// new funding tx coordinates (there was a reorg)
@ -3524,8 +3534,12 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(newRealShortChannelId))
// 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)
// we emit a new local channel update containing the same channel_update, but also the new real scid
val lcu = channelUpdateListener.expectMsgType[LocalChannelUpdate]
assert(lcu.shortIds.real == Final(newRealShortChannelId))
assert(lcu.channelUpdate == aliceState.channelUpdate)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceState.shortIds.localAlias)
assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == aliceState.shortIds.localAlias)
}
test("recv AnnouncementSignatures", Tag(ChannelStateTestsTags.ChannelsPublic)) { f =>