From 2c36ff7398e271eee6c8302562554fd134b9c152 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 14 Jun 2022 10:53:18 +0200 Subject: [PATCH 1/2] Test improvements --- .../eclair/wire/protocol/CommonCodecs.scala | 6 +- .../scala/fr/acinq/eclair/TestUtils.scala | 8 ++ .../fr/acinq/eclair/channel/RestoreSpec.scala | 10 +- .../c/WaitForChannelReadyStateSpec.scala | 36 +++++-- .../channel/states/e/NormalStateSpec.scala | 46 ++++---- .../channel/states/e/OfflineStateSpec.scala | 8 +- .../basic/ZeroConfAliasIntegrationSpec.scala | 100 ++++++++++-------- .../basic/fixtures/MinimalNodeFixture.scala | 24 ++--- .../router/ChannelRouterIntegrationSpec.scala | 31 +++--- .../protocol/LightningMessageCodecsSpec.scala | 5 +- .../acinq/eclair/api/handlers/Channel.scala | 7 +- 11 files changed, 156 insertions(+), 125 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala index 4b3e402f54..314fbaf4bf 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala @@ -21,7 +21,7 @@ import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Satoshi, Transa import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel.{ChannelFlags, RealScidStatus, ShortIds} import fr.acinq.eclair.crypto.Mac32 -import fr.acinq.eclair.{BlockHeight, CltvExpiry, CltvExpiryDelta, UnspecifiedShortChannelId, Alias, MilliSatoshi, RealShortChannelId, ShortChannelId, TimestampSecond, UInt64, channel} +import fr.acinq.eclair.{Alias, BlockHeight, CltvExpiry, CltvExpiryDelta, MilliSatoshi, RealShortChannelId, ShortChannelId, TimestampSecond, UInt64, UnspecifiedShortChannelId} import org.apache.commons.codec.binary.Base32 import scodec.bits.{BitVector, ByteVector} import scodec.codecs._ @@ -138,8 +138,8 @@ object CommonCodecs { val realShortChannelIdStatus: Codec[RealScidStatus] = discriminated[RealScidStatus].by(uint8) .typecase(0, provide(RealScidStatus.Unknown)) - .typecase(1, realshortchannelid.as[channel.RealScidStatus.Temporary]) - .typecase(2, realshortchannelid.as[channel.RealScidStatus.Final]) + .typecase(1, realshortchannelid.as[RealScidStatus.Temporary]) + .typecase(2, realshortchannelid.as[RealScidStatus.Final]) val shortids: Codec[ShortIds] = ( ("real" | realShortChannelIdStatus) :: diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/TestUtils.scala b/eclair-core/src/test/scala/fr/acinq/eclair/TestUtils.scala index 82825f9cee..a6482c1646 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/TestUtils.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/TestUtils.scala @@ -23,11 +23,13 @@ import fr.acinq.eclair.channel.fsm.Channel import fr.acinq.eclair.io.Peer import fr.acinq.eclair.wire.protocol.LightningMessage import org.scalatest.concurrent.Eventually.eventually +import org.scalatest.concurrent.PatienceConfiguration import java.io.File import java.net.ServerSocket import java.nio.file.Files import java.util.UUID +import scala.concurrent.duration.{DurationInt, FiniteDuration} object TestUtils { @@ -104,7 +106,13 @@ object TestUtils { eventStream.publish(DummyEvent()) assert(listener.msgAvailable) } + } + def waitFor(duration: FiniteDuration): Unit = { + val now = TimestampMilli.now() + eventually(PatienceConfiguration.Timeout(duration * 2), PatienceConfiguration.Interval(50 millis)) { + assert(TimestampMilli.now() - now > duration) + } } } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/RestoreSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/RestoreSpec.scala index 65f4343ce0..256d4b90df 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/RestoreSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/RestoreSpec.scala @@ -213,11 +213,11 @@ class RestoreSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with Chan .modify(_.relayParams.privateChannelFees.feeBase).setTo(765 msat), Alice.nodeParams .modify(_.relayParams.privateChannelFees.feeProportionalMillionths).setTo(2345), - // Alice.nodeParams - // .modify(_.channelConf.expiryDelta).setTo(CltvExpiryDelta(147)), - // Alice.nodeParams - // .modify(_.relayParams.privateChannelFees.feeProportionalMillionths).setTo(2345) - // .modify(_.channelConf.expiryDelta).setTo(CltvExpiryDelta(147)), + Alice.nodeParams + .modify(_.channelConf.expiryDelta).setTo(CltvExpiryDelta(147)), + Alice.nodeParams + .modify(_.relayParams.privateChannelFees.feeProportionalMillionths).setTo(2345) + .modify(_.channelConf.expiryDelta).setTo(CltvExpiryDelta(147)), ) foreach { newConfig => val newAlice: TestFSMRef[ChannelState, ChannelData, Channel] = TestFSMRef(new Channel(newConfig, wallet, Bob.nodeParams.nodeId, alice2blockchain.ref, alice2relayer.ref, FakeTxPublisherFactory(alice2blockchain)), alicePeer.ref) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala index c43a8f7ea5..167c75563d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForChannelReadyStateSpec.scala @@ -92,14 +92,17 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu import f._ // we have a real scid at this stage, because this isn't a zero-conf channel assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary]) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary]) val channelReady = bob2alice.expectMsgType[ChannelReady] bob2alice.forward(alice) - val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate + val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate] + assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate) // we have a real scid, but the channel is not announced so alice uses bob's alias assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get) assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase) assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths) - bob2alice.expectNoMessage(200 millis) + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2alice.expectNoMessage(100 millis) awaitCond(alice.stateName == NORMAL) } @@ -110,12 +113,14 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu val channelReady = bob2alice.expectMsgType[ChannelReady] val channelReadyNoAlias = channelReady.modify(_.tlvStream.records).using(_.filterNot(_.isInstanceOf[ChannelReadyTlv.ShortChannelIdTlv])) bob2alice.forward(alice, channelReadyNoAlias) - val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate + val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate] + assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate) // the channel is not announced but bob didn't send an alias so we use the real scid assert(initialChannelUpdate.shortChannelId == realScid) assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase) assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths) - bob2alice.expectNoMessage(200 millis) + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2alice.expectNoMessage(100 millis) awaitCond(alice.stateName == NORMAL) } @@ -123,14 +128,17 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu import f._ // zero-conf channel: we don't have a real scid assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) val channelReady = bob2alice.expectMsgType[ChannelReady] bob2alice.forward(alice) - val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate + val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate] + assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate) // the channel is not announced so alice uses bob's alias (we have a no real scid anyway) assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get) assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase) assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths) - bob2alice.expectNoMessage(200 millis) + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2alice.expectNoMessage(100 millis) awaitCond(alice.stateName == NORMAL) } @@ -138,6 +146,7 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu import f._ // zero-conf channel: we don't have a real scid assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) val channelReady = bob2alice.expectMsgType[ChannelReady] val channelReadyNoAlias = channelReady.modify(_.tlvStream.records).using(_.filterNot(_.isInstanceOf[ChannelReadyTlv.ShortChannelIdTlv])) bob2alice.forward(alice, channelReadyNoAlias) @@ -153,14 +162,18 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu // we have a real scid at this stage, because this isn't a zero-conf channel assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary]) assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].commitments.channelFlags.announceChannel) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real.isInstanceOf[RealScidStatus.Temporary]) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].commitments.channelFlags.announceChannel) val channelReady = bob2alice.expectMsgType[ChannelReady] bob2alice.forward(alice) - val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate + val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate] + assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate) // we have a real scid, but it is not the final one (less than 6 confirmations) so alice uses bob's alias assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get) assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase) assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths) - bob2alice.expectNoMessage(200 millis) + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2alice.expectNoMessage(100 millis) awaitCond(alice.stateName == NORMAL) } @@ -168,14 +181,17 @@ class WaitForChannelReadyStateSpec extends TestKitBaseClass with FixtureAnyFunSu import f._ // zero-conf channel: we don't have a real scid assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) + assert(bob.stateData.asInstanceOf[DATA_WAIT_FOR_CHANNEL_READY].shortIds.real == RealScidStatus.Unknown) val channelReady = bob2alice.expectMsgType[ChannelReady] bob2alice.forward(alice) - val initialChannelUpdate = alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate + val initialChannelUpdate = alice2bob.expectMsgType[ChannelUpdate] + assert(initialChannelUpdate == alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate) // the channel is not announced, so alice uses bob's alias (we have a no real scid anyway) assert(initialChannelUpdate.shortChannelId == channelReady.alias_opt.get) assert(initialChannelUpdate.feeBaseMsat == relayFees.feeBase) assert(initialChannelUpdate.feeProportionalMillionths == relayFees.feeProportionalMillionths) - bob2alice.expectNoMessage(200 millis) + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2alice.expectNoMessage(100 millis) awaitCond(alice.stateName == NORMAL) } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index cd0854c31a..8ff235ff85 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -3403,6 +3403,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with test("recv WatchFundingDeeplyBuriedTriggered (public channel)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f => import f._ val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias // existing funding tx coordinates val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId) alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null) @@ -3410,16 +3411,17 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with assert(annSigs.shortChannelId == realShortChannelId) // alice updates her internal state awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId)) - // public channel: we prefer the real scid alias and it hasn't changed, so we don't send a new channel_update - alice2bob.expectNoMessage(1 second) - // we don't re-publish the same channel_update if there was no change - channelUpdateListener.expectNoMessage(1 second) + // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) } 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 assert(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Unknown) + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias // funding tx coordinates (unknown before) val (blockHeight, txIndex) = (BlockHeight(400000), 42) alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null) @@ -3428,13 +3430,16 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with assert(annSigs.shortChannelId == realShortChannelId) // alice updates her internal state awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId)) - // we don't send out a new channel_update with the real scid just yet, we wait for the peer's announcement_signatures - channelUpdateListener.expectNoMessage(1 second) + // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) } test("recv WatchFundingDeeplyBuriedTriggered (public channel, short channel id changed)", Tag(ChannelStateTestsTags.ChannelsPublic)) { f => import f._ val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias // existing funding tx coordinates val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId) // new funding tx coordinates (there was a reorg) @@ -3445,22 +3450,25 @@ 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)) - // we don't send out a new channel_update with the real scid just yet, we wait for the peer's announcement_signatures - channelUpdateListener.expectNoMessage(1 second) + // public channel: alice will update the channel update to use the real scid when she receives her peer's announcement_signatures + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) } test("recv WatchFundingDeeplyBuriedTriggered (private channel)") { f => import f._ val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias // existing funding tx coordinates val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId) alice ! WatchFundingDeeplyBuriedTriggered(blockHeight, txIndex, null) // update data with real short channel id awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(realShortChannelId)) // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one - alice2bob.expectNoMessage() - // we don't re-publish the same channel_update if there was no change - channelUpdateListener.expectNoMessage(1 second) + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) } test("recv WatchFundingDeeplyBuriedTriggered (private channel, zero-conf)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs), Tag(ChannelStateTestsTags.ZeroConf)) { f => @@ -3470,21 +3478,23 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice.underlying.system.eventStream.subscribe(listener.ref, classOf[TransactionConfirmed]) // zero-conf channel: the funding tx isn't confirmed assert(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Unknown) + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias 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 prefer the remote alias, so there is no change in the channel_update, and we don't send a new one - alice2bob.expectNoMessage() + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) // this is the first time we know the funding tx has been confirmed listener.expectMsgType[TransactionConfirmed] - // we don't re-publish the same channel_update if there was no change - channelUpdateListener.expectNoMessage(1 second) } test("recv WatchFundingDeeplyBuriedTriggered (private channel, short channel id changed)") { f => import f._ val realShortChannelId = alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real.asInstanceOf[RealScidStatus.Temporary].realScid + val bobAlias = bob.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias // existing funding tx coordinates val TxCoordinates(blockHeight, txIndex, _) = ShortChannelId.coordinates(realShortChannelId) // new funding tx coordinates (there was a reorg) @@ -3494,9 +3504,9 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with // update data with real short channel id awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.real == RealScidStatus.Final(newRealShortChannelId)) // private channel: we prefer the remote alias, so there is no change in the channel_update, and we don't send a new one - alice2bob.expectNoMessage() - // we don't re-publish the same channel_update if there was no change - channelUpdateListener.expectNoMessage(1 second) + alice2bob.expectNoMessage(100 millis) + channelUpdateListener.expectNoMessage(100 millis) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate.shortChannelId == bobAlias) } test("recv AnnouncementSignatures", Tag(ChannelStateTestsTags.ChannelsPublic)) { f => @@ -3513,7 +3523,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with import initialState.commitments.{localParams, remoteParams} val channelAnn = Announcements.makeChannelAnnouncement(Alice.nodeParams.chainHash, annSigsA.shortChannelId, Alice.nodeParams.nodeId, remoteParams.nodeId, Alice.channelKeyManager.fundingPublicKey(localParams.fundingKeyPath).publicKey, remoteParams.fundingPubKey, annSigsA.nodeSignature, annSigsB.nodeSignature, annSigsA.bitcoinSignature, annSigsB.bitcoinSignature) // actual test starts here - bob2alice.forward(alice) + bob2alice.forward(alice, annSigsB) awaitAssert { val normal = alice.stateData.asInstanceOf[DATA_NORMAL] assert(normal.shortIds.real == RealScidStatus.Final(annSigsA.shortChannelId) && normal.channelAnnouncement.contains(channelAnn) && normal.channelUpdate.shortChannelId == annSigsA.shortChannelId) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala index d66d57c927..6a66d8a789 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala @@ -32,7 +32,7 @@ import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishTx} import fr.acinq.eclair.channel.states.ChannelStateTestsBase import fr.acinq.eclair.transactions.Transactions.HtlcSuccessTx import fr.acinq.eclair.wire.protocol._ -import fr.acinq.eclair.{BlockHeight, CltvExpiry, CltvExpiryDelta, MilliSatoshiLong, TestConstants, TestFeeEstimator, TestKitBaseClass, randomBytes32} +import fr.acinq.eclair.{BlockHeight, CltvExpiry, CltvExpiryDelta, MilliSatoshiLong, TestConstants, TestFeeEstimator, TestKitBaseClass, TestUtils, TimestampMilli, randomBytes32} import org.scalatest.funsuite.FixtureAnyFunSuiteLike import org.scalatest.{Outcome, Tag} @@ -737,7 +737,7 @@ class OfflineStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with // we simulate a disconnection / reconnection disconnect(alice, bob) // we wait 1s so the new channel_update doesn't have the same timestamp - Thread.sleep(1_000) + TestUtils.waitFor(1 second) reconnect(alice, bob, alice2bob, bob2alice) alice2bob.expectMsgType[ChannelReestablish] bob2alice.expectMsgType[ChannelReestablish] @@ -762,7 +762,7 @@ class OfflineStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with // we simulate a disconnection / reconnection disconnect(alice, bob) // we wait 1s so the new channel_update doesn't have the same timestamp - Thread.sleep(1_000) + TestUtils.waitFor(1 second) reconnect(alice, bob, alice2bob, bob2alice) alice2bob.expectMsgType[ChannelReestablish] bob2alice.expectMsgType[ChannelReestablish] @@ -785,7 +785,7 @@ class OfflineStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with // we get disconnected again disconnect(alice, bob) // we wait 1s so the new channel_update doesn't have the same timestamp - Thread.sleep(1_000) + TestUtils.waitFor(1 second) reconnect(alice, bob, alice2bob, bob2alice) alice2bob.expectMsgType[ChannelReestablish] bob2alice.expectMsgType[ChannelReestablish] diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala index ebc2823a4c..5e3bad9ab6 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala @@ -8,7 +8,6 @@ import fr.acinq.eclair.channel.{DATA_NORMAL, RealScidStatus} import fr.acinq.eclair.integration.basic.fixtures.ThreeNodesFixture import fr.acinq.eclair.payment.PaymentSent import fr.acinq.eclair.testutils.FixtureSpec -import fr.acinq.eclair.wire.protocol.{ChannelReady, ChannelReadyTlv} import fr.acinq.eclair.{MilliSatoshiLong, RealShortChannelId} import org.scalatest.concurrent.IntegrationPatience import org.scalatest.{Tag, TestData} @@ -76,11 +75,10 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience bcScidAlias: Boolean, paymentWorksWithoutHint: Boolean, paymentWorksWithHint_opt: Option[Boolean], - paymentWorksWithRealScidHint_opt: Option[Boolean], - ): Unit = { + paymentWorksWithRealScidHint_opt: Option[Boolean]): Unit = { import f._ - val (channelId_ab, channelId_bc) = createChannels(f)(deepConfirm = deepConfirm) + val (_, channelId_bc) = createChannels(f)(deepConfirm = deepConfirm) eventually { assert(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].commitments.channelFeatures.features.contains(ZeroConf) == bcZeroConf) @@ -96,7 +94,7 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience } if (bcPublic && deepConfirm) { - // if channel b-c is public, we wait for alice to learn about it + // if channel bob->carol is public, we wait for alice to learn about it eventually { val data = getRouterData(alice) assert(data.channels.size == 2) @@ -112,111 +110,123 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience } } - if (paymentWorksWithHint_opt.contains(true)) { - sendPaymentAliceToCarol(f, useHint = true) - } else if (paymentWorksWithHint_opt.contains(false)) { - intercept[AssertionError] { + paymentWorksWithHint_opt match { + case Some(true) => sendPaymentAliceToCarol(f, useHint = true) + case Some(false) => intercept[AssertionError] { sendPaymentAliceToCarol(f, useHint = true) } - } else { - // skipped + case None => // skipped } - if (paymentWorksWithRealScidHint_opt.contains(true)) { - // if alice uses the real scid instead of the b-c alias, it still works - sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.get)) - } else if (paymentWorksWithRealScidHint_opt.contains(false)) { - intercept[AssertionError] { + paymentWorksWithRealScidHint_opt match { + // if alice uses the real scid instead of the bob->carol alias, it still works + case Some(true) => sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.get)) + case Some(false) => intercept[AssertionError] { sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.get)) } - } else { - // skipped + case None => // skipped } } - test("a->b->c (b-c private)") { f => + test("a->b->c (b->c private)") { f => + import f._ + internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = false, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = Some(true) // if alice uses the real scid instead of the b-c alias, it still works + paymentWorksWithRealScidHint_opt = Some(true) // if alice uses the real scid instead of the bob->carol alias, it still works ) - //TODO - // NB: the default hints use bob's alias, even id scid alias isn't enabled, because eclair always sends and understands aliases - //assert(getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get.shortChannelId == - // getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].localAlias) - + // Bob and Carol understand scid aliases even when the feature isn't enabled. + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get + val bobAlias = getRouterData(bob).privateChannels.values.head.shortIds.localAlias + assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b-c scid-alias private)", Tag(ScidAliasBobCarol)) { f => + test("a->b->c (b->c scid-alias private)", Tag(ScidAliasBobCarol)) { f => + import f._ + internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = false, bcScidAlias = true, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the b-c alias, it doesn't work due to option_scid_alias + paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the bob->carol alias, it doesn't work due to option_scid_alias ) + + // Carol must use Bob's scid alias in her routing hints. + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get + val bobAlias = getRouterData(bob).privateChannels.values.head.shortIds.localAlias + assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b-c zero-conf unconfirmed private)", Tag(ZeroConfBobCarol)) { f => + test("a->b->c (b->c zero-conf unconfirmed private)", Tag(ZeroConfBobCarol)) { f => + import f._ + internalTest(f, deepConfirm = false, bcPublic = false, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = None // there is no real scid for b-c yet + paymentWorksWithRealScidHint_opt = None // there is no real scid for bob->carol yet ) + + // Carol uses Bob's scid alias until the channel confirms. + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get + val bobAlias = getRouterData(bob).privateChannels.values.collectFirst { case pc if pc.remoteNodeId == carol.nodeParams.nodeId => pc }.get.shortIds.localAlias + assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b-c zero-conf deeply confirmed private)", Tag(ZeroConfBobCarol)) { f => + test("a->b->c (b->c zero-conf deeply confirmed private)", Tag(ZeroConfBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = None // skipped, see below - // TODO This actually doesn't work, because the ChannelRelayer relies on the the LocalChannelUpdate event to maintain - // TODO its scid resolution map, and the channel doesn't emit a new one when a real scid is assigned, because we use the - // TODO remote alias for the channel_update, not the real scid. So the channel_update remains the same. We used to - // TODO have the ChannelRelayer also listen to ShortChannelIdAssigned event, but it's doesn't seem worth it here. + // TODO: we should be able to send payments with the real scid in the routing hint, but this currently doesn't work, + // because the ChannelRelayer relies on the the LocalChannelUpdate event to maintain its scid resolution map, and + // the channel doesn't emit a new one when a real scid is assigned, because we use the remote alias for the + // channel_update, not the real scid. So the channel_update remains the same. We used to have the ChannelRelayer + // also listen to ShortChannelIdAssigned event, but it's doesn't seem worth it here. + paymentWorksWithRealScidHint_opt = None ) } - test("a->b->c (b-c zero-conf scid-alias deeply confirmed private)", Tag(ZeroConfBobCarol), Tag(ScidAliasBobCarol)) { f => + test("a->b->c (b->c zero-conf scid-alias deeply confirmed private)", Tag(ZeroConfBobCarol), Tag(ScidAliasBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = true, bcScidAlias = true, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the b-c alias, it doesn't work due to option_scid_alias ) } - test("a->b->c (b-c zero-conf unconfirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => + test("a->b->c (b->c zero-conf unconfirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => internalTest(f, deepConfirm = false, bcPublic = true, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because b-c isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced yet paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = None // there is no real scid for b-c yet + paymentWorksWithRealScidHint_opt = None // there is no real scid for bob->carol yet ) } - test("a->b->c (b-c zero-conf deeply confirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => + test("a->b->c (b->c zero-conf deeply confirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = true, diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala index 4f4164552a..52a19c3bbf 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/fixtures/MinimalNodeFixture.scala @@ -24,7 +24,7 @@ import fr.acinq.eclair.payment.send.PaymentInitiator import fr.acinq.eclair.payment.{Bolt11Invoice, PaymentSent} import fr.acinq.eclair.router.Router import fr.acinq.eclair.wire.protocol.IPAddress -import fr.acinq.eclair.{BlockHeight, MilliSatoshi, MilliSatoshiLong, NodeParams, RealShortChannelId, ShortChannelId, TestBitcoinCoreClient, TestDatabases, TestFeeEstimator} +import fr.acinq.eclair.{BlockHeight, MilliSatoshi, MilliSatoshiLong, NodeParams, RealShortChannelId, TestBitcoinCoreClient, TestDatabases, TestFeeEstimator} import org.scalatest.Assertions import org.scalatest.concurrent.Eventually.eventually @@ -178,17 +178,10 @@ object MinimalNodeFixture extends Assertions { val data1After = getChannelData(node1, channelId).asInstanceOf[DATA_NORMAL] val data2After = getChannelData(node2, channelId).asInstanceOf[DATA_NORMAL] - - if ((blockHeight, txIndex) == (BlockHeight(0), 0)) { - // special case for zero-conf channels, the watcher answers with a special 0/0 coordinates when the funding tx - // hasn't been confirmed yet and doesn't have a real scid - None - } else { - val realScid1 = data1After.shortIds.real.asInstanceOf[RealScidStatus.Temporary] - val realScid2 = data2After.shortIds.real.asInstanceOf[RealScidStatus.Temporary] - assert(realScid1 == realScid2) - Some(realScid1) - } + val realScid1 = data1After.shortIds.real.asInstanceOf[RealScidStatus.Temporary] + val realScid2 = data2After.shortIds.real.asInstanceOf[RealScidStatus.Temporary] + assert(realScid1 == realScid2) + Some(realScid1) } def confirmChannelDeep(node1: MinimalNodeFixture, node2: MinimalNodeFixture, channelId: ByteVector32, blockHeight: BlockHeight, txIndex: Int)(implicit system: ActorSystem): RealScidStatus.Final = { @@ -257,12 +250,7 @@ object MinimalNodeFixture extends Assertions { */ def watcherAutopilot(knownFundingTxs: () => Iterable[Transaction], deepConfirm: Boolean = true): TestActor.AutoPilot = (_, msg) => msg match { case watch: ZmqWatcher.WatchFundingConfirmed => - val realScid = if (watch.minDepth == 0) { - // special case for zero-conf channels, this is what the real watcher does - RealShortChannelId(BlockHeight(0), 0, 0) - } else { - deterministicShortId(watch.txId) - } + val realScid = deterministicShortId(watch.txId) val fundingTx = knownFundingTxs().find(_.txid == watch.txId) .getOrElse(throw new RuntimeException(s"unknown fundingTxId=${watch.txId}, known=${knownFundingTxs().map(_.txid).mkString(",")}")) watch.replyTo ! ZmqWatcher.WatchFundingConfirmedTriggered(realScid.blockHeight, txIndex(realScid), fundingTx) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala index 55c5f8b522..ebe90ec8f5 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/router/ChannelRouterIntegrationSpec.scala @@ -45,6 +45,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu private def internalTest(f: FixtureParam): Unit = { import f._ + val (aliceNodeId, bobNodeId) = (channels.alice.underlyingActor.nodeParams.nodeId, channels.bob.underlyingActor.nodeParams.nodeId) reachNormal(channels, testTags, interceptChannelUpdates = false) // the router learns about the local, still unannounced, channel @@ -68,7 +69,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu assert(bobChannelUpdate1.shortChannelId == channels.alice.stateData.asInstanceOf[DATA_NORMAL].shortIds.localAlias) // channel_updates are handled by the peer connection and sent to the router val peerConnection = TestProbe() - router ! PeerRoutingMessage(peerConnection.ref, channels.bob.underlyingActor.nodeParams.nodeId, bobChannelUpdate1) + router ! PeerRoutingMessage(peerConnection.ref, bobNodeId, bobChannelUpdate1) // router processes bob's channel_update and now knows both channel updates awaitCond { @@ -79,7 +80,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu assert(router.stateData.rebroadcast == Rebroadcast(Map.empty, Map.empty, Map.empty)) // router graph contains a single channel - assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(channels.alice.underlyingActor.nodeParams.nodeId, channels.bob.underlyingActor.nodeParams.nodeId)) + assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(aliceNodeId, bobNodeId)) assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceChannelUpdate1, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel))) if (testTags.contains(ChannelStateTestsTags.ChannelsPublic)) { @@ -110,21 +111,24 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu val aliceChannelUpdate2 = channels.alice.stateData.asInstanceOf[DATA_NORMAL].channelUpdate val bobChannelUpdate2 = channels.bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate // this time, they use the real scid - assert(aliceChannelUpdate2.shortChannelId == channels.alice.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get.shortChannelId) - assert(bobChannelUpdate2.shortChannelId == channels.bob.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get.shortChannelId) + val aliceAnn = channels.alice.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get + val bobAnn = channels.bob.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get + assert(aliceAnn == bobAnn) + assert(aliceChannelUpdate2.shortChannelId == aliceAnn.shortChannelId) + assert(bobChannelUpdate2.shortChannelId == bobAnn.shortChannelId) // the router has already processed the new local channel update from alice which uses the real scid, and keeps bob's previous channel update assert(publicChannel.update_1_opt.contains(aliceChannelUpdate2) && publicChannel.update_2_opt.contains(bobChannelUpdate1)) - // the router prepares to rebroadcast the channel announcement, the local update which use the real scid, and the first node announcement + // the router prepares to rebroadcast the channel announcement, the local update which uses the real scid, and the first node announcement assert(router.stateData.rebroadcast == Rebroadcast( - channels = Map(channels.alice.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get -> Set[GossipOrigin](LocalGossip)), + channels = Map(aliceAnn -> Set[GossipOrigin](LocalGossip)), updates = Map(aliceChannelUpdate2 -> Set[GossipOrigin](LocalGossip)), nodes = Map(router.stateData.nodes.values.head -> Set[GossipOrigin](LocalGossip))) ) // bob's channel_update reaches the router - router ! PeerRoutingMessage(peerConnection.ref, channels.bob.underlyingActor.nodeParams.nodeId, bobChannelUpdate2) + router ! PeerRoutingMessage(peerConnection.ref, bobNodeId, bobChannelUpdate2) // router processes bob's channel_update and now knows both channel updates with real scids awaitCond { @@ -133,20 +137,20 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu // router is now ready to rebroadcast both channel updates assert(router.stateData.rebroadcast == Rebroadcast( - channels = Map(channels.alice.stateData.asInstanceOf[DATA_NORMAL].channelAnnouncement.get -> Set[GossipOrigin](LocalGossip)), + channels = Map(aliceAnn -> Set[GossipOrigin](LocalGossip)), updates = Map( aliceChannelUpdate2 -> Set[GossipOrigin](LocalGossip), - bobChannelUpdate2 -> Set[GossipOrigin](RemoteGossip(peerConnection.ref, nodeId = channels.bob.underlyingActor.nodeParams.nodeId))), + bobChannelUpdate2 -> Set[GossipOrigin](RemoteGossip(peerConnection.ref, bobNodeId)) + ), nodes = Map(router.stateData.nodes.values.head -> Set[GossipOrigin](LocalGossip))) ) // router graph contains a single channel - assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(channels.alice.underlyingActor.nodeParams.nodeId, channels.bob.underlyingActor.nodeParams.nodeId)) + assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(aliceNodeId, bobNodeId)) assert(router.stateData.graphWithBalances.graph.edgeSet().size == 2) assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceChannelUpdate2, publicChannel), GraphEdge(bobChannelUpdate2, publicChannel))) } else { // this is a private channel - // funding tx reaches 6 blocks, no announcements are exchanged because the channel is private channels.alice ! WatchFundingDeeplyBuriedTriggered(BlockHeight(400000), 42, null) channels.bob ! WatchFundingDeeplyBuriedTriggered(BlockHeight(400000), 42, null) @@ -156,7 +160,7 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu channels.bob2alice.expectNoMessage(100 millis) // router graph contains a single channel - assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(channels.alice.underlyingActor.nodeParams.nodeId, channels.bob.underlyingActor.nodeParams.nodeId)) + assert(router.stateData.graphWithBalances.graph.vertexSet() == Set(aliceNodeId, bobNodeId)) assert(router.stateData.graphWithBalances.graph.edgeSet().toSet == Set(GraphEdge(aliceChannelUpdate1, privateChannel), GraphEdge(bobChannelUpdate1, privateChannel))) } // channel closes @@ -177,7 +181,8 @@ class ChannelRouterIntegrationSpec extends TestKitBaseClass with FixtureAnyFunSu assert(router.stateData.privateChannels == Map.empty) assert(router.stateData.scid2PrivateChannels == Map.empty) assert(router.stateData.graphWithBalances.graph.edgeSet().isEmpty) - //assert(router.stateData.graphWithBalances.graph.vertexSet().isEmpty) // TODO: bug! + // TODO: we're not currently pruning nodes without channels from the graph, but we should! + // assert(router.stateData.graphWithBalances.graph.vertexSet().isEmpty) } } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala index 44d3e5308a..064dfbf758 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala @@ -21,8 +21,6 @@ import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, ByteVector64, SatoshiLong, ScriptWitness, Transaction} import fr.acinq.eclair.FeatureSupport.Optional import fr.acinq.eclair.Features.DataLossProtect -import fr.acinq.eclair.RealShortChannelId -import fr.acinq.eclair.RealShortChannelId import fr.acinq.eclair._ import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel.{ChannelFlags, ChannelTypes} @@ -378,7 +376,7 @@ class LightningMessageCodecsSpec extends AnyFunSuite { TlvStream(QueryChannelRangeTlv.QueryFlags(QueryChannelRangeTlv.QueryFlags.WANT_ALL) :: Nil, unknownTlv :: Nil)), ReplyChannelRange(Block.RegtestGenesisBlock.blockId, BlockHeight(100000), 1500, 1, - EncodedShortChannelIds(EncodingType.UNCOMPRESSED, List(RealShortChannelId(142), RealShortChannelId(15465), RealShortChannelId(4564676))), + EncodedShortChannelIds(EncodingType.UNCOMPRESSED, List(RealShortChannelId(142), RealShortChannelId(15465), RealShortChannelId(4564676))), TlvStream( EncodedTimestamps(EncodingType.UNCOMPRESSED, List(Timestamps(1 unixsec, 1 unixsec), Timestamps(2 unixsec, 2 unixsec), Timestamps(3 unixsec, 3 unixsec))) :: EncodedChecksums(List(Checksums(1, 1), Checksums(2, 2), Checksums(3, 3))) :: Nil, unknownTlv :: Nil) @@ -575,5 +573,4 @@ class LightningMessageCodecsSpec extends AnyFunSuite { } } - } diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala index 0948988816..f7a04b0263 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/Channel.scala @@ -40,13 +40,11 @@ trait Channel { ChannelTypes.AnchorOutputsZeroFeeHtlcTx(scidAlias = false, zeroConf = true), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(scidAlias = true, zeroConf = false), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(scidAlias = true, zeroConf = true) - ).map(ct => ct.toString -> ct) // we use the toString method as name in the api - .toMap + ).map(ct => ct.toString -> ct).toMap // we use the toString method as name in the api val open: Route = postRequest("open") { implicit t => formFields(nodeIdFormParam, "fundingSatoshis".as[Satoshi], "pushMsat".as[MilliSatoshi].?, "channelType".?, "fundingFeerateSatByte".as[FeeratePerByte].?, "announceChannel".as[Boolean].?, "openTimeoutSeconds".as[Timeout].?) { (nodeId, fundingSatoshis, pushMsat, channelTypeName_opt, fundingFeerateSatByte, announceChannel_opt, openTimeout_opt) => - val (channelTypeOk, channelType_opt) = channelTypeName_opt match { case Some(channelTypeName) => supportedChannelTypes.get(channelTypeName) match { case Some(channelType) => (true, Some(channelType)) @@ -54,9 +52,8 @@ trait Channel { } case None => (true, None) } - if (!channelTypeOk) { - reject(MalformedFormFieldRejection("channelType", s"Channel type not supported: must be ${supportedChannelTypes.keys.mkString(",")}")) + reject(MalformedFormFieldRejection("channelType", s"Channel type not supported: must be one of ${supportedChannelTypes.keys.mkString(",")}")) } else { complete { eclairApi.open(nodeId, fundingSatoshis, pushMsat, channelType_opt, fundingFeerateSatByte, announceChannel_opt, openTimeout_opt) From c439a564863ea5ec34b2e4d1615916bc211824ca Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 14 Jun 2022 12:00:53 +0200 Subject: [PATCH 2/2] fixup! Test improvements --- .../basic/ZeroConfAliasIntegrationSpec.scala | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala index 5e3bad9ab6..fe8759f4af 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/ZeroConfAliasIntegrationSpec.scala @@ -9,6 +9,7 @@ import fr.acinq.eclair.integration.basic.fixtures.ThreeNodesFixture import fr.acinq.eclair.payment.PaymentSent import fr.acinq.eclair.testutils.FixtureSpec import fr.acinq.eclair.{MilliSatoshiLong, RealShortChannelId} +import org.scalatest.OptionValues.convertOptionToValuable import org.scalatest.concurrent.IntegrationPatience import org.scalatest.{Tag, TestData} import scodec.bits.HexStringSyntax @@ -94,7 +95,7 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience } if (bcPublic && deepConfirm) { - // if channel bob->carol is public, we wait for alice to learn about it + // if channel bob-carol is public, we wait for alice to learn about it eventually { val data = getRouterData(alice) assert(data.channels.size == 2) @@ -119,16 +120,16 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience } paymentWorksWithRealScidHint_opt match { - // if alice uses the real scid instead of the bob->carol alias, it still works - case Some(true) => sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.get)) + // if alice uses the real scid instead of the bob-carol alias, it still works + case Some(true) => sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value)) case Some(false) => intercept[AssertionError] { - sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.get)) + sendPaymentAliceToCarol(f, useHint = true, overrideHintScid_opt = Some(getChannelData(bob, channelId_bc).asInstanceOf[DATA_NORMAL].shortIds.real.toOption.value)) } case None => // skipped } } - test("a->b->c (b->c private)") { f => + test("a->b->c (b-c private)") { f => import f._ internalTest(f, @@ -136,18 +137,18 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience bcPublic = false, bcZeroConf = false, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = Some(true) // if alice uses the real scid instead of the bob->carol alias, it still works + paymentWorksWithRealScidHint_opt = Some(true) // if alice uses the real scid instead of the bob-carol alias, it still works ) // Bob and Carol understand scid aliases even when the feature isn't enabled. - val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.value val bobAlias = getRouterData(bob).privateChannels.values.head.shortIds.localAlias assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b->c scid-alias private)", Tag(ScidAliasBobCarol)) { f => + test("a->b->c (b-c scid-alias private)", Tag(ScidAliasBobCarol)) { f => import f._ internalTest(f, @@ -155,18 +156,18 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience bcPublic = false, bcZeroConf = false, bcScidAlias = true, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the bob->carol alias, it doesn't work due to option_scid_alias + paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the bob-carol alias, it doesn't work due to option_scid_alias ) // Carol must use Bob's scid alias in her routing hints. - val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.value val bobAlias = getRouterData(bob).privateChannels.values.head.shortIds.localAlias assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b->c zero-conf unconfirmed private)", Tag(ZeroConfBobCarol)) { f => + test("a->b->c (b-c zero-conf unconfirmed private)", Tag(ZeroConfBobCarol)) { f => import f._ internalTest(f, @@ -174,24 +175,24 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience bcPublic = false, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = None // there is no real scid for bob->carol yet + paymentWorksWithRealScidHint_opt = None // there is no real scid for bob-carol yet ) // Carol uses Bob's scid alias until the channel confirms. - val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.get - val bobAlias = getRouterData(bob).privateChannels.values.collectFirst { case pc if pc.remoteNodeId == carol.nodeParams.nodeId => pc }.get.shortIds.localAlias + val carolHint = getRouterData(carol).privateChannels.values.head.toIncomingExtraHop.value + val bobAlias = getRouterData(bob).privateChannels.values.collectFirst { case pc if pc.remoteNodeId == carol.nodeParams.nodeId => pc.shortIds.localAlias }.value assert(carolHint.shortChannelId == bobAlias) } - test("a->b->c (b->c zero-conf deeply confirmed private)", Tag(ZeroConfBobCarol)) { f => + test("a->b->c (b-c zero-conf deeply confirmed private)", Tag(ZeroConfBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works // TODO: we should be able to send payments with the real scid in the routing hint, but this currently doesn't work, // because the ChannelRelayer relies on the the LocalChannelUpdate event to maintain its scid resolution map, and @@ -202,31 +203,31 @@ class ZeroConfAliasIntegrationSpec extends FixtureSpec with IntegrationPatience ) } - test("a->b->c (b->c zero-conf scid-alias deeply confirmed private)", Tag(ZeroConfBobCarol), Tag(ScidAliasBobCarol)) { f => + test("a->b->c (b-c zero-conf scid-alias deeply confirmed private)", Tag(ZeroConfBobCarol), Tag(ScidAliasBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = false, bcZeroConf = true, bcScidAlias = true, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works paymentWorksWithRealScidHint_opt = Some(false) // if alice uses the real scid instead of the b-c alias, it doesn't work due to option_scid_alias ) } - test("a->b->c (b->c zero-conf unconfirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => + test("a->b->c (b-c zero-conf unconfirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => internalTest(f, deepConfirm = false, bcPublic = true, bcZeroConf = true, bcScidAlias = false, - paymentWorksWithoutHint = false, // alice can't find a route to carol because bob->carol isn't announced yet + paymentWorksWithoutHint = false, // alice can't find a route to carol because bob-carol isn't announced yet paymentWorksWithHint_opt = Some(true), // with a routing hint the payment works - paymentWorksWithRealScidHint_opt = None // there is no real scid for bob->carol yet + paymentWorksWithRealScidHint_opt = None // there is no real scid for bob-carol yet ) } - test("a->b->c (b->c zero-conf deeply confirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => + test("a->b->c (b-c zero-conf deeply confirmed public)", Tag(ZeroConfBobCarol), Tag(PublicBobCarol)) { f => internalTest(f, deepConfirm = true, bcPublic = true,