From c1a925db11ee1aa141ad41cac7efe5760dfa0871 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Wed, 19 Oct 2022 11:57:20 +0200 Subject: [PATCH] Use 0-conf based on local features only (#2460) If we have activated 0-conf support for a given peer, we send our `channel_ready` early regardless of whether our peer has activated support for 0-conf. If they also immediately send their `channel_ready` it's great, if they don't it's ok, we'll just wait for confirmations, but it was worth trying. --- .../fr/acinq/eclair/channel/Helpers.scala | 14 +++++------ .../fr/acinq/eclair/channel/fsm/Channel.scala | 6 ++--- .../channel/fsm/ChannelOpenDualFunded.scala | 6 ++--- .../channel/fsm/ChannelOpenSingleFunded.scala | 6 ++--- .../fr/acinq/eclair/channel/HelpersSpec.scala | 18 ++++++------- .../zeroconf/ZeroConfActivationSpec.scala | 25 ++++++++++++++++++- 6 files changed, 49 insertions(+), 26 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala index 7dd02b9255..23d0f538bf 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala @@ -354,8 +354,8 @@ object Helpers { * wait for one conf, except if the channel has the zero-conf feature (because presumably the peer will send an * alias in that case). */ - def minDepthFunder(channelFeatures: ChannelFeatures): Option[Long] = { - if (channelFeatures.hasFeature(Features.ZeroConf)) { + def minDepthFunder(localFeatures: Features[InitFeature]): Option[Long] = { + if (localFeatures.hasFeature(Features.ZeroConf)) { None } else { Some(1) @@ -369,8 +369,8 @@ object Helpers { * @param fundingSatoshis funding amount of the channel * @return number of confirmations needed, if any */ - def minDepthFundee(channelConf: ChannelConf, channelFeatures: ChannelFeatures, fundingSatoshis: Satoshi): Option[Long] = fundingSatoshis match { - case _ if channelFeatures.hasFeature(Features.ZeroConf) => None // zero-conf stay zero-conf, whatever the funding amount is + def minDepthFundee(channelConf: ChannelConf, localFeatures: Features[InitFeature], fundingSatoshis: Satoshi): Option[Long] = fundingSatoshis match { + case _ if localFeatures.hasFeature(Features.ZeroConf) => None // zero-conf stay zero-conf, whatever the funding amount is case funding if funding <= Channel.MAX_FUNDING => Some(channelConf.minDepthBlocks) case funding => val blockReward = 6.25 // this is true as of ~May 2020, but will be too large after 2024 @@ -384,15 +384,15 @@ object Helpers { * - our peer may also contribute to the funding transaction * - even if they don't, we may RBF the transaction and don't want to handle reorgs */ - def minDepthDualFunding(channelConf: ChannelConf, channelFeatures: ChannelFeatures, fundingParams: InteractiveTxBuilder.InteractiveTxParams): Option[Long] = { + def minDepthDualFunding(channelConf: ChannelConf, localFeatures: Features[InitFeature], fundingParams: InteractiveTxBuilder.InteractiveTxParams): Option[Long] = { if (fundingParams.isInitiator && fundingParams.remoteAmount == 0.sat) { - if (channelFeatures.hasFeature(Features.ZeroConf)) { + if (localFeatures.hasFeature(Features.ZeroConf)) { None } else { Some(channelConf.minDepthBlocks) } } else { - minDepthFundee(channelConf, channelFeatures, fundingParams.fundingAmount) + minDepthFundee(channelConf, localFeatures, fundingParams.fundingAmount) } } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala index 8ee2fe1fac..bf46d36daf 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala @@ -1364,10 +1364,10 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val when(SYNCING)(handleExceptions { case Event(_: ChannelReestablish, d: DATA_WAIT_FOR_FUNDING_CONFIRMED) => val minDepth_opt = if (d.commitments.localParams.isInitiator) { - Helpers.Funding.minDepthFunder(d.commitments.channelFeatures) + Helpers.Funding.minDepthFunder(d.commitments.localParams.initFeatures) } else { // when we're not the channel initiator we scale the min_depth confirmations depending on the funding amount - Helpers.Funding.minDepthFundee(nodeParams.channelConf, d.commitments.channelFeatures, d.commitments.commitInput.txOut.amount) + Helpers.Funding.minDepthFundee(nodeParams.channelConf, d.commitments.localParams.initFeatures, d.commitments.commitInput.txOut.amount) } val minDepth = minDepth_opt.getOrElse { val defaultMinDepth = nodeParams.channelConf.minDepthBlocks @@ -1381,7 +1381,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val goto(WAIT_FOR_FUNDING_CONFIRMED) case Event(_: ChannelReestablish, d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED) => - val minDepth_opt = Helpers.Funding.minDepthDualFunding(nodeParams.channelConf, d.commitments.channelFeatures, d.fundingParams) + val minDepth_opt = Helpers.Funding.minDepthDualFunding(nodeParams.channelConf, d.commitments.localParams.initFeatures, d.fundingParams) val minDepth = minDepth_opt.getOrElse { val defaultMinDepth = nodeParams.channelConf.minDepthBlocks // If we are in state WAIT_FOR_DUAL_FUNDING_CONFIRMED, then the computed minDepth should be > 0, otherwise we would diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala index 23329eec02..5309401201 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala @@ -148,7 +148,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers { val localFundingPubkey = keyManager.fundingPublicKey(localParams.fundingKeyPath).publicKey val channelKeyPath = keyManager.keyPath(localParams, d.init.channelConfig) val totalFundingAmount = open.fundingAmount + d.init.fundingContribution_opt.getOrElse(0 sat) - val minimumDepth = Funding.minDepthFundee(nodeParams.channelConf, channelFeatures, totalFundingAmount) + val minimumDepth = Funding.minDepthFundee(nodeParams.channelConf, d.init.localParams.initFeatures, totalFundingAmount) val upfrontShutdownScript_opt = if (Features.canUseFeature(localParams.initFeatures, remoteInit.features, Features.UpfrontShutdownScript)) { Some(ChannelTlv.UpfrontShutdownScriptTlv(localParams.defaultFinalScriptPubKey)) } else { @@ -316,7 +316,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers { case InteractiveTxBuilder.SendMessage(msg) => stay() sending msg case InteractiveTxBuilder.Succeeded(fundingParams, fundingTx, commitments) => d.deferred.foreach(self ! _) - Funding.minDepthDualFunding(nodeParams.channelConf, commitments.channelFeatures, fundingParams) match { + Funding.minDepthDualFunding(nodeParams.channelConf, commitments.localParams.initFeatures, fundingParams) match { case Some(fundingMinDepth) => blockchain ! WatchFundingConfirmed(self, commitments.fundingTxId, fundingMinDepth) val d1 = DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED(commitments, fundingTx, fundingParams, d.localPushAmount, d.remotePushAmount, Nil, nodeParams.currentBlockHeight, nodeParams.currentBlockHeight, RbfStatus.NoRbf, None) @@ -525,7 +525,7 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers { case InteractiveTxBuilder.SendMessage(msg) => stay() sending msg case InteractiveTxBuilder.Succeeded(fundingParams, fundingTx, commitments) => // We now have more than one version of the funding tx, so we cannot use zero-conf. - val fundingMinDepth = Funding.minDepthDualFunding(nodeParams.channelConf, commitments.channelFeatures, fundingParams).getOrElse(nodeParams.channelConf.minDepthBlocks.toLong) + val fundingMinDepth = Funding.minDepthDualFunding(nodeParams.channelConf, commitments.localParams.initFeatures, fundingParams).getOrElse(nodeParams.channelConf.minDepthBlocks.toLong) blockchain ! WatchFundingConfirmed(self, commitments.fundingTxId, fundingMinDepth) val previousFundingTxs = DualFundingTx(d.fundingTx, d.commitments) +: d.previousFundingTxs val d1 = DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED(commitments, fundingTx, fundingParams, d.localPushAmount, d.remotePushAmount, previousFundingTxs, d.waitingSince, d.lastChecked, RbfStatus.NoRbf, d.deferred) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunded.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunded.scala index 3ebf66b091..2a70161c1f 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunded.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenSingleFunded.scala @@ -115,7 +115,7 @@ trait ChannelOpenSingleFunded extends SingleFundingHandlers with ErrorHandlers { context.system.eventStream.publish(ChannelCreated(self, peer, remoteNodeId, isInitiator = false, open.temporaryChannelId, open.feeratePerKw, None)) val fundingPubkey = keyManager.fundingPublicKey(d.initFundee.localParams.fundingKeyPath).publicKey val channelKeyPath = keyManager.keyPath(d.initFundee.localParams, d.initFundee.channelConfig) - val minimumDepth = Funding.minDepthFundee(nodeParams.channelConf, channelFeatures, open.fundingSatoshis) + val minimumDepth = Funding.minDepthFundee(nodeParams.channelConf, d.initFundee.localParams.initFeatures, open.fundingSatoshis) log.info("will use fundingMinDepth={}", minimumDepth) // In order to allow TLV extensions and keep backwards-compatibility, we include an empty upfront_shutdown_script if this feature is not used. // See https://github.com/lightningnetwork/lightning-rfc/pull/714. @@ -291,7 +291,7 @@ trait ChannelOpenSingleFunded extends SingleFundingHandlers with ErrorHandlers { // NB: we don't send a ChannelSignatureSent for the first commit log.info(s"waiting for them to publish the funding tx for channelId=$channelId fundingTxid=${commitments.fundingTxId}") watchFundingTx(commitments) - Funding.minDepthFundee(nodeParams.channelConf, commitments.channelFeatures, fundingAmount) match { + Funding.minDepthFundee(nodeParams.channelConf, commitments.localParams.initFeatures, fundingAmount) match { case Some(fundingMinDepth) => blockchain ! WatchFundingConfirmed(self, commitments.fundingTxId, fundingMinDepth) goto(WAIT_FOR_FUNDING_CONFIRMED) using DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments, None, nodeParams.currentBlockHeight, None, Right(fundingSigned)) storing() sending fundingSigned @@ -338,7 +338,7 @@ trait ChannelOpenSingleFunded extends SingleFundingHandlers with ErrorHandlers { watchFundingTx(commitments) // we will publish the funding tx only after the channel state has been written to disk because we want to // make sure we first persist the commitment that returns back the funds to us in case of problem - Funding.minDepthFunder(commitments.channelFeatures) match { + Funding.minDepthFunder(commitments.localParams.initFeatures) match { case Some(fundingMinDepth) => blockchain ! WatchFundingConfirmed(self, commitments.fundingTxId, fundingMinDepth) goto(WAIT_FOR_FUNDING_CONFIRMED) using DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments, Some(fundingTx), blockHeight, None, Left(fundingCreated)) storing() calling publishFundingTx(commitments, fundingTx, fundingTxFee) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala index c2736562ae..c2bb4ed61d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala @@ -27,7 +27,7 @@ import fr.acinq.eclair.channel.fsm.Channel import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.transactions.Transactions._ import fr.acinq.eclair.wire.protocol.UpdateAddHtlc -import fr.acinq.eclair.{BlockHeight, Features, MilliSatoshiLong, TestKitBaseClass, TimestampSecond, TimestampSecondLong} +import fr.acinq.eclair.{BlockHeight, FeatureSupport, Features, MilliSatoshiLong, TestKitBaseClass, TimestampSecond, TimestampSecondLong} import org.scalatest.Tag import org.scalatest.funsuite.AnyFunSuiteLike import scodec.bits.HexStringSyntax @@ -40,14 +40,14 @@ class HelpersSpec extends TestKitBaseClass with AnyFunSuiteLike with ChannelStat implicit val log: akka.event.LoggingAdapter = akka.event.NoLogging test("compute the funding tx min depth according to funding amount") { - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(1)).contains(4)) - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf.copy(minDepthBlocks = 6), ChannelFeatures(), Btc(1)).contains(6)) // 4 conf would be enough but we use min-depth=6 - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(6.25)).contains(16)) // we use scaling_factor=15 and a fixed block reward of 6.25BTC - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(12.50)).contains(31)) - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(12.60)).contains(32)) - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(30)).contains(73)) - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(), Btc(50)).contains(121)) - assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, ChannelFeatures(Features.ZeroConf), Btc(50)).isEmpty) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(1)).contains(4)) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf.copy(minDepthBlocks = 6), Features(), Btc(1)).contains(6)) // 4 conf would be enough but we use min-depth=6 + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(6.25)).contains(16)) // we use scaling_factor=15 and a fixed block reward of 6.25BTC + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(12.50)).contains(31)) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(12.60)).contains(32)) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(30)).contains(73)) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(), Btc(50)).contains(121)) + assert(Helpers.Funding.minDepthFundee(nodeParams.channelConf, Features(Features.ZeroConf -> FeatureSupport.Optional), Btc(50)).isEmpty) } test("compute refresh delay") { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/zeroconf/ZeroConfActivationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/zeroconf/ZeroConfActivationSpec.scala index 8b05ff0cc0..5b459c6711 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/zeroconf/ZeroConfActivationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/basic/zeroconf/ZeroConfActivationSpec.scala @@ -5,18 +5,22 @@ import fr.acinq.bitcoin.scalacompat.{ByteVector32, SatoshiLong} import fr.acinq.eclair.FeatureSupport.Optional import fr.acinq.eclair.Features.ZeroConf import fr.acinq.eclair.channel.ChannelTypes.AnchorOutputsZeroFeeHtlcTx -import fr.acinq.eclair.channel.PersistentChannelData +import fr.acinq.eclair.channel.{NORMAL, PersistentChannelData} import fr.acinq.eclair.integration.basic.fixtures.composite.TwoNodesFixture import fr.acinq.eclair.testutils.FixtureSpec import org.scalatest.concurrent.IntegrationPatience import org.scalatest.{Tag, TestData} import scodec.bits.HexStringSyntax +import scala.concurrent.duration.DurationInt + /** * Test the activation of zero-conf option, via features or channel type. */ class ZeroConfActivationSpec extends FixtureSpec with IntegrationPatience { + implicit val config: PatienceConfig = PatienceConfig(5 second, 50 milliseconds) + type FixtureParam = TwoNodesFixture val ZeroConfAlice = "zero_conf_alice" @@ -80,6 +84,25 @@ class ZeroConfActivationSpec extends FixtureSpec with IntegrationPatience { assert(getChannelData(bob, channelId).asInstanceOf[PersistentChannelData].commitments.channelFeatures.hasFeature(ZeroConf)) } + test("open a channel alice-bob (zero-conf enabled on bob, not requested via channel type by alice)", Tag(ZeroConfBob)) { f => + import f._ + + assert(!alice.nodeParams.features.activated.contains(ZeroConf)) + assert(bob.nodeParams.features.activated.contains(ZeroConf)) + + connect(alice, bob) + val channelType = AnchorOutputsZeroFeeHtlcTx(scidAlias = false, zeroConf = false) + val channelId = openChannel(alice, bob, 100_000 sat, channelType_opt = Some(channelType)).channelId + + // Bob has activated support for 0-conf with Alice, so he doesn't wait for the funding tx to confirm regardless of + // the channel type and activated feature bits. Since Alice has full control over the funding tx, she accepts Bob's + // early channel_ready and completes the channel opening flow without waiting for confirmations. + eventually { + assert(getChannelState(alice, channelId) == NORMAL) + assert(getChannelState(bob, channelId) == NORMAL) + } + } + test("open a channel alice-bob (zero-conf enabled on alice and bob, but not requested via channel type by alice)", Tag(ZeroConfAlice), Tag(ZeroConfBob)) { f => import f._