From e7d08a73b88a8e01d3cf9145503c82d37b2cb93f Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Wed, 7 Jun 2023 10:07:18 +0200 Subject: [PATCH 01/25] Add quiescence protocol --- eclair-core/src/main/resources/reference.conf | 2 + .../main/scala/fr/acinq/eclair/Features.scala | 8 + .../scala/fr/acinq/eclair/NodeParams.scala | 3 +- .../fr/acinq/eclair/channel/ChannelData.scala | 16 +- .../eclair/channel/ChannelExceptions.scala | 1 + .../fr/acinq/eclair/channel/Commitments.scala | 5 +- .../fr/acinq/eclair/channel/fsm/Channel.scala | 245 +++++++--- .../channel/fsm/DualFundingHandlers.scala | 10 - .../protocol/LightningMessageCodecs.scala | 5 + .../wire/protocol/LightningMessageTypes.scala | 1 + .../scala/fr/acinq/eclair/TestConstants.scala | 6 +- .../ChannelStateTestsHelperMethods.scala | 4 + .../states/e/NormalQuiescentStateSpec.scala | 434 ++++++++++++++++++ 13 files changed, 667 insertions(+), 73 deletions(-) create mode 100644 eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index 926f566088..db0b11763f 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -154,6 +154,8 @@ eclair { max-total-pending-channels-private-nodes = 99 // maximum number of pending channels we will accept from all private nodes channel-opener-whitelist = [] // a list of public keys; we will ignore rate limits on pending channels from these peers } + + quiescence-timeout = 2 minutes // maximum time we will wait for quiescence to complete before disconnecting } balance-check-interval = 1 hour diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala index 625d6c6c5d..667a039804 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala @@ -300,6 +300,13 @@ object Features { val mandatory = 154 } + // TODO: @remyers reserve feature bits here: currently reserved here: https://github.com/lightning/bolts/issues/605 + // TODO: @remyers option_quiesce implementation, to be replaced once quiescence is spec-ed + case object QuiescePrototype extends Feature with InitFeature { + val rfcName = "option_quiesce_prototype" + val mandatory = 158 + } + val knownFeatures: Set[Feature] = Set( DataLossProtect, InitialRoutingSync, @@ -325,6 +332,7 @@ object Features { TrampolinePaymentPrototype, AsyncPaymentPrototype, SplicePrototype, + QuiescePrototype ) // Features may depend on other features, as specified in Bolt 9. diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala index 3eab339cec..9b7ce7535c 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala @@ -513,7 +513,8 @@ object NodeParams extends Logging { channelOpenerWhitelist = channelOpenerWhitelist, maxPendingChannelsPerPeer = maxPendingChannelsPerPeer, maxTotalPendingChannelsPrivateNodes = maxTotalPendingChannelsPrivateNodes, - remoteRbfLimits = Channel.RemoteRbfLimits(config.getInt("channel.funding.remote-rbf-limits.max-attempts"), config.getInt("channel.funding.remote-rbf-limits.attempt-delta-blocks")) + remoteRbfLimits = Channel.RemoteRbfLimits(config.getInt("channel.funding.remote-rbf-limits.max-attempts"), config.getInt("channel.funding.remote-rbf-limits.attempt-delta-blocks")), + quiescenceTimeout = FiniteDuration(config.getDuration("channel.quiescence-timeout").getSeconds, TimeUnit.SECONDS), ), onChainFeeConf = OnChainFeeConf( feeTargets = feeTargets, diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala index 59d6595716..808e70a70d 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala @@ -27,7 +27,7 @@ import fr.acinq.eclair.io.Peer import fr.acinq.eclair.payment.OutgoingPaymentPacket.Upstream import fr.acinq.eclair.transactions.CommitmentSpec import fr.acinq.eclair.transactions.Transactions._ -import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelReady, ChannelReestablish, ChannelUpdate, ClosingSigned, CommitSig, FailureMessage, FundingCreated, FundingSigned, Init, OnionRoutingPacket, OpenChannel, OpenDualFundedChannel, Shutdown, SpliceInit, UpdateAddHtlc, UpdateFailHtlc, UpdateFailMalformedHtlc, UpdateFulfillHtlc} +import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelReady, ChannelReestablish, ChannelUpdate, ClosingSigned, CommitSig, FailureMessage, FundingCreated, FundingSigned, Init, OnionRoutingPacket, OpenChannel, OpenDualFundedChannel, Shutdown, SpliceInit, Stfu, UpdateAddHtlc, UpdateFailHtlc, UpdateFailMalformedHtlc, UpdateFulfillHtlc} import fr.acinq.eclair.{Alias, BlockHeight, CltvExpiry, CltvExpiryDelta, Features, InitFeature, MilliSatoshi, MilliSatoshiLong, RealShortChannelId, UInt64} import scodec.bits.ByteVector @@ -187,13 +187,14 @@ sealed trait HasReplyToCommand extends Command { def replyTo: ActorRef } sealed trait HasOptionalReplyToCommand extends Command { def replyTo_opt: Option[ActorRef] } sealed trait ForbiddenCommandDuringSplice extends Command +sealed trait ForbiddenCommandDuringQuiescence extends Command -final case class CMD_ADD_HTLC(replyTo: ActorRef, amount: MilliSatoshi, paymentHash: ByteVector32, cltvExpiry: CltvExpiry, onion: OnionRoutingPacket, nextBlindingKey_opt: Option[PublicKey], origin: Origin.Hot, commit: Boolean = false) extends HasReplyToCommand with ForbiddenCommandDuringSplice -sealed trait HtlcSettlementCommand extends HasOptionalReplyToCommand with ForbiddenCommandDuringSplice { def id: Long } +final case class CMD_ADD_HTLC(replyTo: ActorRef, amount: MilliSatoshi, paymentHash: ByteVector32, cltvExpiry: CltvExpiry, onion: OnionRoutingPacket, nextBlindingKey_opt: Option[PublicKey], origin: Origin.Hot, commit: Boolean = false) extends HasReplyToCommand with ForbiddenCommandDuringSplice with ForbiddenCommandDuringQuiescence +sealed trait HtlcSettlementCommand extends HasOptionalReplyToCommand with ForbiddenCommandDuringSplice with ForbiddenCommandDuringQuiescence { def id: Long } final case class CMD_FULFILL_HTLC(id: Long, r: ByteVector32, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand final case class CMD_FAIL_HTLC(id: Long, reason: Either[ByteVector, FailureMessage], delay_opt: Option[FiniteDuration] = None, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand final case class CMD_FAIL_MALFORMED_HTLC(id: Long, onionHash: ByteVector32, failureCode: Int, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HtlcSettlementCommand -final case class CMD_UPDATE_FEE(feeratePerKw: FeeratePerKw, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HasOptionalReplyToCommand with ForbiddenCommandDuringSplice +final case class CMD_UPDATE_FEE(feeratePerKw: FeeratePerKw, commit: Boolean = false, replyTo_opt: Option[ActorRef] = None) extends HasOptionalReplyToCommand with ForbiddenCommandDuringSplice with ForbiddenCommandDuringQuiescence final case class CMD_SIGN(replyTo_opt: Option[ActorRef] = None) extends HasOptionalReplyToCommand with ForbiddenCommandDuringSplice final case class ClosingFees(preferred: Satoshi, min: Satoshi, max: Satoshi) @@ -202,7 +203,7 @@ final case class ClosingFeerates(preferred: FeeratePerKw, min: FeeratePerKw, max } sealed trait CloseCommand extends HasReplyToCommand -final case class CMD_CLOSE(replyTo: ActorRef, scriptPubKey: Option[ByteVector], feerates: Option[ClosingFeerates]) extends CloseCommand with ForbiddenCommandDuringSplice +final case class CMD_CLOSE(replyTo: ActorRef, scriptPubKey: Option[ByteVector], feerates: Option[ClosingFeerates]) extends CloseCommand with ForbiddenCommandDuringSplice with ForbiddenCommandDuringQuiescence final case class CMD_FORCECLOSE(replyTo: ActorRef) extends CloseCommand final case class CMD_BUMP_FUNDING_FEE(replyTo: akka.actor.typed.ActorRef[CommandResponse[CMD_BUMP_FUNDING_FEE]], targetFeerate: FeeratePerKw, lockTime: Long) extends Command @@ -450,8 +451,13 @@ object RbfStatus { } sealed trait SpliceStatus +sealed trait QuiescenceNegotiation extends SpliceStatus object SpliceStatus { case object NoSplice extends SpliceStatus + case class QuiescenceRequested(splice: CMD_SPLICE) extends QuiescenceNegotiation + case class InitiatorQuiescent(splice: CMD_SPLICE) extends QuiescenceNegotiation + case class ReceivedStfu(stfu: Stfu) extends QuiescenceNegotiation + case object NonInitiatorQuiescent extends QuiescenceNegotiation case class SpliceRequested(cmd: CMD_SPLICE, init: SpliceInit) extends SpliceStatus case class SpliceInProgress(cmd_opt: Option[CMD_SPLICE], splice: typed.ActorRef[InteractiveTxBuilder.Command], remoteCommitSig: Option[CommitSig]) extends SpliceStatus case class SpliceWaitingForSigs(signingSession: InteractiveTxSigningSession.WaitingForSigs) extends SpliceStatus diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala index b75cae3c79..5765275cc4 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala @@ -136,4 +136,5 @@ case class InvalidFailureCode (override val channelId: Byte case class PleasePublishYourCommitment (override val channelId: ByteVector32) extends ChannelException(channelId, "please publish your local commitment") case class CommandUnavailableInThisState (override val channelId: ByteVector32, command: String, state: ChannelState) extends ChannelException(channelId, s"cannot execute command=$command in state=$state") case class ForbiddenDuringSplice (override val channelId: ByteVector32, command: String) extends ChannelException(channelId, s"cannot process $command while splicing") +case class ForbiddenDuringQuiescence (override val channelId: ByteVector32, command: String) extends ChannelException(channelId, s"cannot process $command while quiescent") // @formatter:on \ No newline at end of file diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index f422aba91a..3e8aae7d37 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -4,6 +4,7 @@ import akka.event.LoggingAdapter import com.softwaremill.quicklens.ModifyPimp import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, Satoshi, SatoshiLong, Script, Transaction} +import fr.acinq.eclair.Features.QuiescePrototype import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw, FeeratesPerKw, OnChainFeeConf} import fr.acinq.eclair.channel.Helpers.Closing import fr.acinq.eclair.channel.Monitoring.{Metrics, Tags} @@ -359,7 +360,7 @@ case class Commitment(fundingTxIndex: Long, changes.localChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) || changes.remoteChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) - def isIdle(changes: CommitmentChanges): Boolean = hasNoPendingHtlcs && changes.localChanges.all.isEmpty && changes.remoteChanges.all.isEmpty + def isIdle(changes: CommitmentChanges, pendingHtlcsOk: Boolean): Boolean = (pendingHtlcsOk || hasNoPendingHtlcs) && changes.localChanges.all.isEmpty && changes.remoteChanges.all.isEmpty def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = { def expired(add: UpdateAddHtlc): Boolean = currentHeight >= add.cltvExpiry.blockHeight @@ -796,7 +797,7 @@ case class Commitments(params: ChannelParams, // @formatter:off // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. - def isIdle: Boolean = active.head.isIdle(changes) + def isIdle: Boolean = active.head.isIdle(changes, params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes) def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight) 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 f9e1882b2b..66e808976a 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 @@ -22,7 +22,7 @@ import akka.actor.{Actor, ActorContext, ActorRef, FSM, OneForOneStrategy, Possib import akka.event.Logging.MDC import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{ByteVector32, Satoshi, SatoshiLong, Transaction} -import fr.acinq.eclair.Features.SplicePrototype +import fr.acinq.eclair.Features.{QuiescePrototype, SplicePrototype} import fr.acinq.eclair.Logs.LogCategory import fr.acinq.eclair._ import fr.acinq.eclair.blockchain.OnChainWallet.MakeFundingTxResponse @@ -91,7 +91,8 @@ object Channel { channelOpenerWhitelist: Set[PublicKey], maxPendingChannelsPerPeer: Int, maxTotalPendingChannelsPrivateNodes: Int, - remoteRbfLimits: RemoteRbfLimits) { + remoteRbfLimits: RemoteRbfLimits, + quiescenceTimeout: FiniteDuration) { require(0 <= maxHtlcValueInFlightPercent && maxHtlcValueInFlightPercent <= 100, "max-htlc-value-in-flight-percent must be between 0 and 100") def minFundingSatoshis(announceChannel: Boolean): Satoshi = if (announceChannel) minFundingPublicSatoshis else minFundingPrivateSatoshis @@ -156,6 +157,9 @@ object Channel { // we will receive this message when we waited too long for a revocation for that commit number (NB: we explicitly specify the peer to allow for testing) case class RevocationTimeout(remoteCommitNumber: Long, peer: ActorRef) + // we will receive this message if we waited too long for peers to exchange stfu messages (NB: we explicitly specify the peer to allow for testing) + case class QuiescenceTimeout(peer: ActorRef) + /** We don't immediately process [[CurrentBlockHeight]] to avoid herd effects */ case class ProcessCurrentBlockHeight(c: CurrentBlockHeight) @@ -363,15 +367,37 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with */ when(NORMAL)(handleExceptions { - case Event(c: ForbiddenCommandDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice => + case Event(c: ForbiddenCommandDuringQuiescence, d: DATA_NORMAL) if d.spliceStatus.isInstanceOf[QuiescenceNegotiation] => + val error = ForbiddenDuringQuiescence(d.channelId, c.getClass.getSimpleName) + c match { + case c: CMD_ADD_HTLC => handleAddHtlcCommandError(c, error, Some(d.channelUpdate)) + case _: HtlcSettlementCommand => stay() // htlc settlement commands will be ignored and replayed when not quiescent + case _ => handleCommandError(error, c) + } + + case Event(c: ForbiddenCommandDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[QuiescenceNegotiation] => val error = ForbiddenDuringSplice(d.channelId, c.getClass.getSimpleName) c match { case c: CMD_ADD_HTLC => handleAddHtlcCommandError(c, error, Some(d.channelUpdate)) - // NB: the command cannot be an htlc settlement (fail/fulfill), because if we are splicing it means the channel is idle and has no htlcs + // NB: the command cannot be an htlc settlement (fail/fulfill), because if we are splicing without quiescence support it means the channel is idle and has no htlcs case _ => handleCommandError(error, c) } - case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.SpliceRequested] => + case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype) && d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.InitiatorQuiescent] => + val error = ForbiddenDuringSplice(d.channelId, msg.getClass.getSimpleName) + msg match { + case fulfill: UpdateFulfillHtlc => + d.commitments.receiveFulfill(fulfill) match { + case Right((commitments1, origin, htlc)) => + // we forward preimages as soon as possible to the upstream channel because it allows us to pull funds + relayer ! RES_ADD_SETTLED(origin, htlc, HtlcResult.RemoteFulfill(fulfill)) + handleLocalError(error, d.copy(commitments = commitments1), Some(msg)) + case Left(_) => handleLocalError(error, d, Some(fulfill)) + } + case _ => handleLocalError(error, d, Some(msg)) + } + + case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.SpliceRequested] && !d.spliceStatus.isInstanceOf[QuiescenceNegotiation] => // In case of a race between our splice_init and a forbidden message from our peer, we accept their message, because // we know they are going to reject our splice attempt val error = ForbiddenDuringSplice(d.channelId, msg.getClass.getSimpleName) @@ -536,8 +562,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with val minDepth_opt = d.commitments.params.minDepthDualFunding(nodeParams.channelConf.minDepthBlocks, signingSession1.fundingTx.sharedTx.tx) watchFundingConfirmed(signingSession.fundingTx.txId, minDepth_opt) val commitments1 = d.commitments.add(signingSession1.commitment) - val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) - stay() using d1 storing() sending signingSession1.localSigs + stay() using d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) storing() sending signingSession1.localSigs calling replayQuiescenceSettlements(d) } } case _ if d.commitments.params.channelFeatures.hasFeature(Features.DualFunding) && d.commitments.latest.localFundingStatus.signedTx_opt.isEmpty && commit.batchSize == 1 => @@ -562,7 +587,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with context.system.eventStream.publish(AvailableBalanceChanged(self, d.channelId, d.shortIds, commitments1)) } context.system.eventStream.publish(ChannelSignatureReceived(self, commitments1)) - stay() using d.copy(commitments = commitments1) storing() sending revocation + handleSendRevocation(revocation, d.copy(commitments = commitments1)) case Left(cause) => handleLocalError(cause, d, Some(commit)) } } @@ -606,6 +631,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(r: RevocationTimeout, d: DATA_NORMAL) => handleRevocationTimeout(r, d) + case Event(s: QuiescenceTimeout, d: DATA_NORMAL) => handleQuiescenceTimeout(s, d) + case Event(c: CMD_CLOSE, d: DATA_NORMAL) => if (d.localShutdown.isDefined) { handleCommandError(ClosingAlreadyInProgress(d.channelId), c) @@ -784,51 +811,62 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } case Event(cmd: CMD_SPLICE, d: DATA_NORMAL) => - d.spliceStatus match { - case SpliceStatus.NoSplice => - if (d.commitments.isIdle && d.commitments.params.remoteParams.initFeatures.hasFeature(SplicePrototype)) { - val parentCommitment = d.commitments.latest.commitment - val targetFeerate = nodeParams.onChainFeeConf.getFundingFeerate(nodeParams.currentFeerates) - val fundingContribution = InteractiveTxFunder.computeSpliceContribution( - isInitiator = true, - sharedInput = Multisig2of2Input(parentCommitment), - spliceInAmount = cmd.additionalLocalFunding, - spliceOut = cmd.spliceOutputs, - targetFeerate = targetFeerate) - if (parentCommitment.localCommit.spec.toLocal + fundingContribution < parentCommitment.localChannelReserve(d.commitments.params)) { - log.warning("cannot do splice: insufficient funds") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) - stay() - } else if (cmd.spliceOut_opt.map(_.scriptPubKey).exists(!MutualClose.isValidFinalScriptPubkey(_, allowAnySegwit = true))) { - log.warning("cannot do splice: invalid splice-out script") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) - stay() + if (d.commitments.params.remoteParams.initFeatures.hasFeature(SplicePrototype)) { + if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype) && d.spliceStatus == SpliceStatus.NoSplice) { + startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) + if (d.commitments.changes.localChanges.all.isEmpty) { + stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, 1) + } else { + stay() using d.copy(spliceStatus = SpliceStatus.QuiescenceRequested(cmd)) + } + } else { + handleNewSplice(cmd, d) + } + } else { + log.warning("cannot initiate splice, peer doesn't support splices") + cmd.replyTo ! RES_FAILURE(cmd, CommandUnavailableInThisState(d.channelId, "splice", NORMAL)) + stay() + } + + case Event(msg: Stfu, d: DATA_NORMAL) => + if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) { + d.spliceStatus match { + case SpliceStatus.NoSplice => + startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) + if (d.commitments.latest.changes.localChanges.all.isEmpty) { + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, 0) } else { - log.info(s"initiating splice with local.in.amount=${cmd.additionalLocalFunding} local.in.push=${cmd.pushAmount} local.out.amount=${cmd.spliceOut_opt.map(_.amount).sum}") - val spliceInit = SpliceInit(d.channelId, - fundingContribution = fundingContribution, - lockTime = nodeParams.currentBlockHeight.toLong, - feerate = targetFeerate, - fundingPubKey = keyManager.fundingPublicKey(d.commitments.params.localParams.fundingKeyPath, parentCommitment.fundingTxIndex + 1).publicKey, - pushAmount = cmd.pushAmount, - requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding - ) - stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) } - } else { - log.warning("cannot initiate splice, channel is not idle or peer doesn't support splices") - cmd.replyTo ! RES_FAILURE(cmd, CommandUnavailableInThisState(d.channelId, "splice", NORMAL)) + case SpliceStatus.QuiescenceRequested(_) => + stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) + case SpliceStatus.InitiatorQuiescent(splice) => + // if both sides send stfu at the same time, the quiescence initiator is the channel initiator + if (msg.initiator == 0 || d.commitments.params.localParams.isInitiator) { + handleNewSplice(splice, d) + } else { + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) + } + case SpliceStatus.ReceivedStfu(_) | SpliceStatus.NonInitiatorQuiescent => + cancelTimer(QuiescenceTimeout.toString) + val failure = new ChannelException(d.channelId, "received stfu twice") + log.info("quiesce attempt failed: {}", failure.getMessage) + // NB: we use a small delay to ensure we've sent our warning before disconnecting. + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, failure.toString) + case _ => + log.warning("ignoring stfu received during splice") stay() - } - case _ => - log.warning("cannot initiate splice, another one is already in progress") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceAlreadyInProgress(d.channelId)) - stay() + } + } else { + log.warning("ignoring stfu because peer doesn't support quiescence") + stay() } case Event(msg: SpliceInit, d: DATA_NORMAL) => d.spliceStatus match { - case SpliceStatus.NoSplice => + case SpliceStatus.NoSplice | SpliceStatus.NonInitiatorQuiescent => + cancelTimer(QuiescenceTimeout.toString) if (!d.commitments.isIdle) { log.info("rejecting splice request: channel not idle") stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceRequest(d.channelId).getMessage) @@ -873,6 +911,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case _: SpliceStatus.SpliceRequested | _: SpliceStatus.SpliceInProgress | _: SpliceStatus.SpliceWaitingForSigs => log.info("rejecting splice attempt: the current splice attempt must be completed or aborted first") stay() sending Warning(d.channelId, InvalidSpliceAlreadyInProgress(d.channelId).getMessage) + case _: SpliceStatus.QuiescenceRequested | _: SpliceStatus.InitiatorQuiescent | _: SpliceStatus.ReceivedStfu => + log.info("rejecting splice attempt: the current splice attempt must be completed or aborted first (quiescence)") + stay() sending Warning(d.channelId, InvalidSpliceAlreadyInProgress(d.channelId).getMessage) } case Event(msg: SpliceAck, d: DATA_NORMAL) => @@ -923,22 +964,28 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with log.info("our peer aborted the splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, SpliceAttemptAborted(d.channelId))) txBuilder ! InteractiveTxBuilder.Abort - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) case SpliceStatus.SpliceWaitingForSigs(signingSession) => log.info("our peer aborted the splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) rollbackFundingAttempt(signingSession.fundingTx.tx, previousTxs = Seq.empty) // no splice rbf yet - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) case SpliceStatus.SpliceRequested(cmd, _) => log.info("our peer rejected our splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) cmd.replyTo ! RES_FAILURE(cmd, new RuntimeException(s"splice attempt rejected by our peer: ${msg.toAscii}")) - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) case SpliceStatus.SpliceAborted => log.debug("our peer acked our previous tx_abort") - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) calling replayQuiescenceSettlements(d) case SpliceStatus.NoSplice => log.info("our peer wants to abort the splice, but we've already negotiated a splice transaction: ascii='{}' bin={}", msg.toAscii, msg.data) // We ack their tx_abort but we keep monitoring the funding transaction until it's confirmed or double-spent. stay() sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) + case _: SpliceStatus.QuiescenceRequested | _: SpliceStatus.InitiatorQuiescent | _: SpliceStatus.ReceivedStfu | SpliceStatus.NonInitiatorQuiescent => + log.info("our peer aborted the splice during quiescence negotiation, disconnecting: ascii='{}' bin={}", msg.toAscii, msg.data) + cancelTimer(QuiescenceTimeout.toString) + // NB: we use a small delay to ensure we've sent our warning before disconnecting. + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, "spec violation: you sent tx abort during quiescence negotiation") } case Event(msg: InteractiveTxBuilder.Response, d: DATA_NORMAL) => @@ -997,7 +1044,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with val commitments1 = d.commitments.add(signingSession1.commitment) val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) log.info("publishing funding tx for channelId={} fundingTxId={}", d.channelId, signingSession1.fundingTx.sharedTx.txId) - stay() using d1 storing() sending signingSession1.localSigs calling publishFundingTx(signingSession1.fundingTx) + stay() using d1 storing() sending signingSession1.localSigs calling publishFundingTx(signingSession1.fundingTx) calling replayQuiescenceSettlements(d1) } case _ => // We may receive an outdated tx_signatures if the transaction is already confirmed. @@ -1043,7 +1090,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with // we cancel the timer that would have made us send the enabled update after reconnection (flappy channel protection) cancelTimer(Reconnected.toString) // if we are splicing, we need to cancel it - reportSpliceFailure(d.spliceStatus, new RuntimeException("splice attempt failed: disconnected")) + reportSpliceFailure(d) val d1 = d.spliceStatus match { // We keep track of the RBF status: we should be able to complete the signature steps on reconnection. case _: SpliceStatus.SpliceWaitingForSigs => d @@ -1053,7 +1100,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with if (d.commitments.changes.localChanges.proposed.collectFirst { case add: UpdateAddHtlc => add }.isDefined) { log.debug("updating channel_update announcement (reason=disabled)") val channelUpdate1 = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, scidForChannelUpdate(d), d.channelUpdate.cltvExpiryDelta, d.channelUpdate.htlcMinimumMsat, d.channelUpdate.feeBaseMsat, d.channelUpdate.feeProportionalMillionths, d.commitments.params.maxHtlcAmount, isPrivate = !d.commitments.announceChannel, enable = false) - // NB: the htlcs stay() in the commitments.localChange, they will be cleaned up after reconnection + // NB: the htlcs stay in the commitments.localChange, they will be cleaned up after reconnection d.commitments.changes.localChanges.proposed.collect { case add: UpdateAddHtlc => relayer ! RES_ADD_SETTLED(d.commitments.originChannels(add.id), add, HtlcResult.DisconnectedBeforeSigned(channelUpdate1)) } @@ -2184,6 +2231,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with if (nextState == OFFLINE) { // we can cancel the timer, we are not expecting anything when disconnected cancelTimer(RevocationTimeout.toString) + cancelTimer(QuiescenceTimeout.toString) } sealed trait EmitLocalChannelEvent @@ -2525,6 +2573,97 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with proposedTx_opt.get.unsignedTx.copy(tx = tx) } + private def handleNewSplice(cmd: CMD_SPLICE, d: DATA_NORMAL): State = { + cancelTimer(QuiescenceTimeout.toString) + d.spliceStatus match { + case SpliceStatus.NoSplice | _: SpliceStatus.InitiatorQuiescent => + if (d.commitments.isIdle) { + val parentCommitment = d.commitments.latest.commitment + val targetFeerate = nodeParams.onChainFeeConf.getFundingFeerate(nodeParams.currentFeerates) + val fundingContribution = InteractiveTxFunder.computeSpliceContribution( + isInitiator = true, + sharedInput = Multisig2of2Input(parentCommitment), + spliceInAmount = cmd.additionalLocalFunding, + spliceOut = cmd.spliceOutputs, + targetFeerate = targetFeerate) + if (parentCommitment.localCommit.spec.toLocal + fundingContribution < parentCommitment.localChannelReserve(d.commitments.params)) { + log.warning("cannot do splice: insufficient funds") + cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) + stay() + } else if (cmd.spliceOut_opt.map(_.scriptPubKey).exists(!MutualClose.isValidFinalScriptPubkey(_, allowAnySegwit = true))) { + log.warning("cannot do splice: invalid splice-out script") + cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) + stay() + } else { + log.info(s"initiating splice with local.in.amount=${cmd.additionalLocalFunding} local.in.push=${cmd.pushAmount} local.out.amount=${cmd.spliceOut_opt.map(_.amount).sum}") + val spliceInit = SpliceInit(d.channelId, + fundingContribution = fundingContribution, + lockTime = nodeParams.currentBlockHeight.toLong, + feerate = targetFeerate, + fundingPubKey = keyManager.fundingPublicKey(d.commitments.params.localParams.fundingKeyPath, parentCommitment.fundingTxIndex + 1).publicKey, + pushAmount = cmd.pushAmount, + requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding + ) + stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + } + } else { + log.warning("cannot initiate splice, channel is not idle") + cmd.replyTo ! RES_FAILURE(cmd, CommandUnavailableInThisState(d.channelId, "splice", NORMAL)) + stay() + } + case _ => + log.warning("cannot initiate splice, another one is already in progress") + cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceAlreadyInProgress(d.channelId)) + stay() + } + } + + private def handleSendRevocation(revocation: RevokeAndAck, d: DATA_NORMAL): State = { + d.spliceStatus match { + case SpliceStatus.QuiescenceRequested(splice) if d.commitments.changes.localChanges.all.isEmpty => + stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(splice)) storing() sending Seq(revocation, Stfu(d.channelId, 1)) + case SpliceStatus.ReceivedStfu(_) if d.commitments.changes.localChanges.all.isEmpty => + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) storing() sending Seq(revocation, Stfu(d.channelId, 0)) + case _ => + stay() using d storing() sending revocation + } + } + + private def handleQuiescenceTimeout(spliceTimeout: QuiescenceTimeout, d: DATA_NORMAL): State = { + val msg = d.spliceStatus match { + case SpliceStatus.QuiescenceRequested(_) => Some("quiescence timed out waiting for pending local htlcs to finalize (initiator)") + case SpliceStatus.InitiatorQuiescent(_) => Some("quiescence timed out waiting for remote stfu") + case SpliceStatus.ReceivedStfu(_) => Some("quiescence timed out waiting for pending local htlcs to finalize (non-initiator)") + case SpliceStatus.NonInitiatorQuiescent => Some("quiescence timed out waiting to receive splice-init") + case _ => None + } + msg match { + case Some(msg) => + log.warning(s"$msg, closing connection") + context.system.scheduler.scheduleOnce(2 second, spliceTimeout.peer, Peer.Disconnect(remoteNodeId)) + stay() sending Warning(d.channelId, msg) + case None => stay() + } + } + + private def replayQuiescenceSettlements(d: DATA_NORMAL): Unit = + if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) { + PendingCommandsDb.getSettlementCommands(nodeParams.db.pendingCommands, d.channelId).foreach(self ! _) + } + + private def reportSpliceFailure(d: DATA_NORMAL): Unit = { + val cmd_opt = d.spliceStatus match { + case SpliceStatus.QuiescenceRequested(cmd) => Some(cmd) + case SpliceStatus.InitiatorQuiescent(cmd) => Some(cmd) + case SpliceStatus.SpliceRequested(cmd, _) => Some(cmd) + case SpliceStatus.SpliceInProgress(cmd_opt, txBuilder, _) => + txBuilder ! InteractiveTxBuilder.Abort + cmd_opt + case _ => None + } + cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, new RuntimeException("splice attempt failed: disconnected"))) + } + override def mdc(currentMessage: Any): MDC = { val category_opt = LogCategory(currentMessage) val id = currentMessage match { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala index 05f9f8473a..98c277bf6e 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala @@ -147,14 +147,4 @@ trait DualFundingHandlers extends CommonFundingHandlers { } } - def reportSpliceFailure(spliceStatus: SpliceStatus, f: Throwable): Unit = { - spliceStatus match { - case SpliceStatus.SpliceRequested(cmd, _) => cmd.replyTo ! RES_FAILURE(cmd, f) - case SpliceStatus.SpliceInProgress(cmd_opt, txBuilder, _) => - txBuilder ! InteractiveTxBuilder.Abort - cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, f)) - case _ => () - } - } - } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala index 2fdaa0bc8b..595ea3a6e3 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala @@ -420,6 +420,10 @@ object LightningMessageCodecs { ("channelId" | bytes32) :: ("fundingTxid" | bytes32) :: ("tlvStream" | SpliceLockedTlv.spliceLockedTlvCodec)).as[SpliceLocked] + + val stfuCodec: Codec[Stfu] = ( + ("channelId" | bytes32) :: + ("initiator" | byte) ).as[Stfu] // // @@ -477,6 +481,7 @@ object LightningMessageCodecs { .typecase(37000, spliceInitCodec) .typecase(37002, spliceAckCodec) .typecase(37004, spliceLockedCodec) + .typecase(37006, stfuCodec) // // diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala index 30ba59f7c8..72889ff8ee 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala @@ -562,6 +562,7 @@ case class GossipTimestampFilter(chainHash: ByteVector32, firstTimestamp: Timest case class OnionMessage(blindingKey: PublicKey, onionRoutingPacket: OnionRoutingPacket, tlvStream: TlvStream[OnionMessageTlv] = TlvStream.empty) extends LightningMessage +case class Stfu(channelId: ByteVector32, initiator: Byte) extends LightningMessage // NB: blank lines to minimize merge conflicts // diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala index 1ec95cb70e..21bd98694c 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala @@ -131,7 +131,8 @@ object TestConstants { channelOpenerWhitelist = Set.empty, maxPendingChannelsPerPeer = 3, maxTotalPendingChannelsPrivateNodes = 99, - remoteRbfLimits = RemoteRbfLimits(5, 0) + remoteRbfLimits = RemoteRbfLimits(5, 0), + quiescenceTimeout = 2 minutes ), onChainFeeConf = OnChainFeeConf( feeTargets = FeeTargets(funding = ConfirmationPriority.Medium, closing = ConfirmationPriority.Medium), @@ -290,7 +291,8 @@ object TestConstants { channelOpenerWhitelist = Set.empty, maxPendingChannelsPerPeer = 3, maxTotalPendingChannelsPrivateNodes = 99, - remoteRbfLimits = RemoteRbfLimits(5, 0) + remoteRbfLimits = RemoteRbfLimits(5, 0), + quiescenceTimeout = 2 minutes ), onChainFeeConf = OnChainFeeConf( feeTargets = FeeTargets(funding = ConfirmationPriority.Medium, closing = ConfirmationPriority.Medium), diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala index 44fe231984..5f71f4d4c7 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala @@ -90,6 +90,8 @@ object ChannelStateTestsTags { val RejectRbfAttempts = "reject_rbf_attempts" /** If set, the non-initiator will require a 1-block delay between RBF attempts. */ val DelayRbfAttempts = "delay_rbf_attempts" + /** If set, peers will support the quiesce protocol. */ + val Quiescence = "quiescence" } trait ChannelStateTestsBase extends Assertions with Eventually { @@ -180,6 +182,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually { .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional)) + .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.QuiescePrototype, FeatureSupport.Optional)) .initFeatures() val bobInitFeatures = Bob.nodeParams.features .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DisableWumbo))(_.removed(Features.Wumbo)) @@ -193,6 +196,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually { .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional)) + .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.QuiescePrototype, FeatureSupport.Optional)) .initFeatures() val channelType = ChannelTypes.defaultFromFeatures(aliceInitFeatures, bobInitFeatures, announceChannel = channelFlags.announceChannel) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala new file mode 100644 index 0000000000..8470d5cff7 --- /dev/null +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -0,0 +1,434 @@ +/* + * Copyright 2023 ACINQ SAS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package fr.acinq.eclair.channel.states.e + +import akka.actor.typed.scaladsl.adapter.actorRefAdapter +import akka.testkit.{TestFSMRef, TestProbe} +import fr.acinq.bitcoin.scalacompat.{ByteVector32, SatoshiLong, Script, Transaction} +import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ +import fr.acinq.eclair.blockchain.fee.FeeratePerKw +import fr.acinq.eclair.channel._ +import fr.acinq.eclair.channel.fsm.Channel +import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishReplaceableTx, PublishTx} +import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} +import fr.acinq.eclair.io.Peer +import fr.acinq.eclair.payment.relay.Relayer.RelayForward +import fr.acinq.eclair.wire.protocol._ +import fr.acinq.eclair.{channel, _} +import org.scalatest.Outcome +import org.scalatest.funsuite.FixtureAnyFunSuiteLike +import org.scalatest.time.SpanSugar.convertIntToGrainOfTime +import scodec.bits.HexStringSyntax + +class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { + + type FixtureParam = SetupFixture + + implicit val log: akka.event.LoggingAdapter = akka.event.NoLogging + + override def withFixture(test: OneArgTest): Outcome = { + val tags = test.tags + ChannelStateTestsTags.DualFunding + ChannelStateTestsTags.Splicing + ChannelStateTestsTags.Quiescence + val setup = init(tags = tags) + import setup._ + reachNormal(setup, tags) + awaitCond(alice.stateName == NORMAL) + awaitCond(bob.stateName == NORMAL) + withFixture(test.toNoArgTest(setup)) + } + + private def disconnect(f: FixtureParam): Unit = { + import f._ + alice ! INPUT_DISCONNECTED + bob ! INPUT_DISCONNECTED + awaitCond(alice.stateName == OFFLINE) + awaitCond(bob.stateName == OFFLINE) + } + + private def reconnect(f: FixtureParam): Unit = { + import f._ + + val aliceInit = Init(alice.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.localParams.initFeatures) + val bobInit = Init(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.localParams.initFeatures) + alice ! INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit) + bob ! INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit) + alice2bob.expectMsgType[ChannelReestablish] + alice2bob.forward(bob) + bob2alice.expectMsgType[ChannelReestablish] + bob2alice.forward(alice) + + alice2blockchain.expectMsgType[WatchFundingDeeplyBuried] + bob2blockchain.expectMsgType[WatchFundingDeeplyBuried] + awaitCond(alice.stateName == NORMAL) + awaitCond(bob.stateName == NORMAL) + } + + private def safeSend(r: TestFSMRef[ChannelState, ChannelData, Channel], cmds: Seq[channel.Command]): Unit = { + cmds.foreach { + case cmd: channel.HtlcSettlementCommand => + // this would be done automatically when the relayer calls safeSend + r.underlyingActor.nodeParams.db.pendingCommands.addSettlementCommand(r.stateData.channelId, cmd) + r ! cmd + case cmd: channel.Command => r ! cmd + } + } + + private def initiateQuiescence(f: FixtureParam, sendInitialStfu: Boolean = true): TestProbe = { + import f._ + + val sender = TestProbe() + val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + alice ! cmd + alice2bob.expectMsgType[Stfu] + if (!sendInitialStfu) { + // only alice is quiescent, we're holding the first stfu to pause the splice + } else { + alice2bob.forward(bob) + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + alice2bob.expectMsgType[SpliceInit] + // both alice and bob are quiescent, we're holding the splice-init to pause the splice + } + + sender + } + + private def assertPublished(probe: TestProbe, desc: String): Transaction = { + val p = probe.expectMsgType[PublishTx] + assert(desc == p.desc) + p match { + case p: PublishFinalTx => p.tx + case p: PublishReplaceableTx => p.txInfo.tx + } + } + + private def sendErrorAndClose(s: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, s2blockchain: TestProbe): Unit = { + s2r.expectMsgType[Error] + assertPublished(s2blockchain, "commit-tx") + assertPublished(s2blockchain, "local-main-delayed") + s2blockchain.expectMsgType[WatchTxConfirmed] + s2blockchain.expectMsgType[WatchTxConfirmed] + s2blockchain.expectNoMessage(100 millis) + awaitCond(s.stateName == CLOSING) + } + + sealed trait SettlementCommandEnum + case object FulfillHtlc extends SettlementCommandEnum + case object FailHtlc extends SettlementCommandEnum + case object FailMalformedHtlc extends SettlementCommandEnum + + private def receiveSettlementCommand(f: FixtureParam, c: SettlementCommandEnum, sendInitialStfu: Boolean, resetConnection: Boolean = false): Unit = { + import f._ + + val (preimage, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + val cmd = c match { + case FulfillHtlc => CMD_FULFILL_HTLC(add.id, preimage) + case FailHtlc => CMD_FAIL_HTLC(add.id, Left(randomBytes32())) + case FailMalformedHtlc => CMD_FAIL_MALFORMED_HTLC(add.id, randomBytes32(), FailureMessageCodecs.BADONION) + } + crossSign(bob, alice, bob2alice, alice2bob) + val sender = initiateQuiescence(f, sendInitialStfu) + safeSend(alice, Seq(cmd)) + // alice will not forward settlement msg to bob while quiescent + alice2bob.expectNoMessage(100 millis) + if (resetConnection) { + // both alice and bob leave quiescence when the channel is disconnected + disconnect(f) + } else if (sendInitialStfu) { + // alice sends splice-init to bob + alice2bob.forward(bob) + bob2alice.expectMsgType[SpliceAck] + // both alice and bob leave quiescence when the splice aborts + bob2alice.forward(alice, TxAbort(channelId(bob), hex"deadbeef")) + alice2bob.expectMsgType[TxAbort] + alice2bob.forward(bob) + bob2alice.expectMsgType[TxAbort] + } else { + // alice sends a warning and disconnects if an error occurs while waiting for stfu from bob + alice ! Channel.QuiescenceTimeout(alicePeer.ref) + alice2bob.expectMsgType[Warning] + alice2bob.forward(bob) + // we should disconnect after giving bob time to receive the warning + alicePeer.fishForMessage(3 seconds) { + case Peer.Disconnect(nodeId, _) if nodeId == alice.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case _ => false + } + disconnect(f) + } + if (resetConnection || !sendInitialStfu) { + // any failure during quiescence will cause alice to disconnect + sender.expectMsgType[RES_FAILURE[_, _]] + assert(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice) + reconnect(f) + } + + // alice sends pending settlement after quiescence ends + eventually { + c match { + case FulfillHtlc => alice2bob.expectMsgType[UpdateFulfillHtlc] + case FailHtlc => alice2bob.expectMsgType[UpdateFailHtlc] + case FailMalformedHtlc => alice2bob.expectMsgType[UpdateFailMalformedHtlc] + } + } + alice2bob.forward(bob) + } + + test("recv stfu when there are pending local changes") { f => + import f._ + initiateQuiescence(f, sendInitialStfu = false) + // we're holding the stfu from alice so that bob can add a pending local change + addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + // bob will not reply to alice's stfu until bob has no pending local commitment changes + alice2bob.forward(bob) + bob2alice.expectNoMessage(100 millis) + crossSign(bob, alice, bob2alice, alice2bob) + eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isIdle)) + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + // when both nodes are quiescent, alice will start the splice + alice2bob.expectMsgType[SpliceInit] + alice2bob.forward(bob) + bob2alice.expectMsgType[SpliceAck] + bob2alice.forward(alice) + } + + test("recv forbidden commands while quiescent") { f => + import f._ + // alice should reject commands that change the commitment while quiescent + val sender1, sender2, sender3 = TestProbe() + val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), + CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), + CMD_CLOSE(sender3.ref, None, None)) + initiateQuiescence(f) + safeSend(alice, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] + } + + test("recv fulfill htlc command while initiator awaiting stfu from remote") { f => + receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = false) + } + + test("recv fail htlc command while initiator awaiting stfu from remote") { f => + receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false) + } + + test("recv fail malformed htlc command while initiator awaiting stfu from remote") { f => + receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = false) + } + + test("recv fulfill htlc command while initiator awaiting stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = false, resetConnection = true) + } + + test("recv fail htlc command while initiator awaiting stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false, resetConnection = true) + } + + test("recv fail malformed htlc command while initiator awaiting stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = false, resetConnection = true) + } + + test("recv fulfill htlc command after initiator receives stfu from remote") { f => + receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true) + } + + test("recv fail htlc command after initiator receives stfu from remote") { f => + receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true) + } + + test("recv fail malformed htlc command after initiator receives stfu from remote") { f => + receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true) + } + + test("recv fulfill htlc command when initiator receives stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true, resetConnection = true) + } + + test("recv fail htlc command after initiator receives stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true, resetConnection = true) + } + + test("recv fail malformed htlc command after initiator receives stfu from remote and channel disconnects") { f => + receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true, resetConnection = true) + } + + test("recv settlement commands while initiator awaiting stfu from remote") { f => + import f._ + + // alice should reject commands that change the commitment until the splice is complete + val sender1, sender2, sender3 = TestProbe() + val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), + CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), + CMD_CLOSE(sender3.ref, None, None)) + initiateQuiescence(f, sendInitialStfu = false) + safeSend(alice, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] + } + + test("recv settlement commands after initiator receives stfu from remote") { f => + import f._ + + // alice should reject commands that change the commitment until the splice is complete + val sender1, sender2, sender3 = TestProbe() + val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), + CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), + CMD_CLOSE(sender3.ref, None, None)) + initiateQuiescence(f) + safeSend(alice, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] + } + + test("recv second stfu while non-initiator waiting for local commitment to be signed") { f => + import f._ + initiateQuiescence(f, sendInitialStfu = false) + val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + alice2bob.forward(bob) + // send a second stfu to bob + bob ! Stfu(channelId(bob), 1) + bob2alice.expectMsgType[Warning] + // we should disconnect after giving alice time to receive the warning + bobPeer.fishForMessage(3 seconds) { + case Peer.Disconnect(nodeId, _) if nodeId == bob.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case _ => false + } + } + + test("recv Shutdown message before initiator receives stfu from remote") { f => + import f._ + initiateQuiescence(f, sendInitialStfu = false) + val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] + alice ! Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + alice2bob.expectMsgType[Shutdown] + } + + test("recv (forbidden) Shutdown message after initiator receives stfu from remote") { f => + import f._ + initiateQuiescence(f) + val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] + alice ! Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv (forbidden) UpdateFulfillHtlc messages after initiator receives stfu from remote") { f => + import f._ + val (preimage, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + crossSign(bob, alice, bob2alice, alice2bob) + initiateQuiescence(f) + alice ! UpdateFulfillHtlc(channelId(bob), add.id, preimage) + alice2relayer.expectMsg(RelayForward(add)) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv (forbidden) UpdateFailHtlc messages after initiator receives stfu from remote") { f => + import f._ + val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + crossSign(bob, alice, bob2alice, alice2bob) + initiateQuiescence(f) + alice ! UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv (forbidden) UpdateFailMalformedHtlc messages after initiator receives stfu from remote") { f => + import f._ + val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + crossSign(bob, alice, bob2alice, alice2bob) + initiateQuiescence(f) + alice ! UpdateFailMalformedHtlc(channelId(bob), add.id, randomBytes32(), FailureMessageCodecs.BADONION) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv (forbidden) UpdateFee messages after initiator receives stfu from remote") { f => + import f._ + val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + crossSign(bob, alice, bob2alice, alice2bob) + initiateQuiescence(f) + alice ! UpdateFee(channelId(bob), FeeratePerKw(1 sat)) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv UpdateAddHtlc message after initiator receives stfu from remote") { f => + import f._ + initiateQuiescence(f) + + // have to build a htlc manually because eclair would refuse to accept this command as it's forbidden + val fakeHtlc = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) + bob2alice.forward(alice, fakeHtlc) + sendErrorAndClose(alice, alice2bob, alice2blockchain) + } + + test("recv (forbidden) Shutdown message after non-initiator receives stfu from initiator") { f => + import f._ + initiateQuiescence(f) + val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] + bob ! Shutdown(ByteVector32.Zeroes, bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + + test("recv (forbidden) UpdateFulfillHtlc messages after non-initiator receives stfu from initiator") { f => + import f._ + val (preimage, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + bob ! UpdateFulfillHtlc(channelId(bob), add.id, preimage) + bob2relayer.expectMsg(RelayForward(add)) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + + test("recv (forbidden) UpdateFailHtlc messages after non-initiator receives stfu from initiator") { f => + import f._ + val (_, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + bob ! UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + + test("recv (forbidden) UpdateFailMalformedHtlc messages after non-initiator receives stfu from initiator") { f => + import f._ + val (_, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + bob ! UpdateFailMalformedHtlc(channelId(bob), add.id, randomBytes32(), FailureMessageCodecs.BADONION) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + + test("recv (forbidden) UpdateFee messages after non-initiator receives stfu from initiator") { f => + import f._ + val (_, _) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + bob ! UpdateFee(channelId(bob), FeeratePerKw(1 sat)) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + + test("recv UpdateAddHtlc message after non-initiator receives stfu from initiator") { f => + import f._ + val (_, _) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + val fakeHtlc = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) + alice2bob.forward(bob, fakeHtlc) + sendErrorAndClose(bob, bob2alice, bob2blockchain) + } + +} From 9fa21350e0a18a88f37c8ffdf308afab8766b3e5 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 15 Jun 2023 13:50:54 +0200 Subject: [PATCH 02/25] Updated with suggested initial fixes - only use quiescence if both nodes signal the feature - reduce timeout to 1 min - use message type 2 for `stfu` - Use feature bits 34/35 for option_quiesce a per spec --- eclair-core/src/main/resources/reference.conf | 2 +- .../src/main/scala/fr/acinq/eclair/Features.scala | 4 +--- .../scala/fr/acinq/eclair/channel/Commitments.scala | 3 ++- .../scala/fr/acinq/eclair/channel/fsm/Channel.scala | 10 +++++----- .../eclair/wire/protocol/LightningMessageCodecs.scala | 2 +- .../eclair/wire/protocol/LightningMessageTypes.scala | 3 ++- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index db0b11763f..fac3a61140 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -155,7 +155,7 @@ eclair { channel-opener-whitelist = [] // a list of public keys; we will ignore rate limits on pending channels from these peers } - quiescence-timeout = 2 minutes // maximum time we will wait for quiescence to complete before disconnecting + quiescence-timeout = 1 minutes // maximum time we will wait for quiescence to complete before disconnecting } balance-check-interval = 1 hour diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala index 667a039804..718ee26cb2 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala @@ -300,11 +300,9 @@ object Features { val mandatory = 154 } - // TODO: @remyers reserve feature bits here: currently reserved here: https://github.com/lightning/bolts/issues/605 - // TODO: @remyers option_quiesce implementation, to be replaced once quiescence is spec-ed case object QuiescePrototype extends Feature with InitFeature { val rfcName = "option_quiesce_prototype" - val mandatory = 158 + val mandatory = 34 } val knownFeatures: Set[Feature] = Set( diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 3e8aae7d37..13a5aaec2b 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -145,6 +145,7 @@ case class ChannelParams(channelId: ByteVector32, else Right(remoteScriptPubKey) } + def useQuiescence: Boolean = localParams.initFeatures.hasFeature(Features.QuiescePrototype) && remoteParams.initFeatures.hasFeature(Features.QuiescePrototype) } object ChannelParams { @@ -797,7 +798,7 @@ case class Commitments(params: ChannelParams, // @formatter:off // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. - def isIdle: Boolean = active.head.isIdle(changes, params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) + def isIdle: Boolean = active.head.isIdle(changes, params.localParams.initFeatures.hasFeature(QuiescePrototype) && params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes) def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight) 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 66e808976a..110ec618bd 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 @@ -383,7 +383,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case _ => handleCommandError(error, c) } - case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype) && d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.InitiatorQuiescent] => + case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.commitments.params.useQuiescence && d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.InitiatorQuiescent] => val error = ForbiddenDuringSplice(d.channelId, msg.getClass.getSimpleName) msg match { case fulfill: UpdateFulfillHtlc => @@ -812,7 +812,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(cmd: CMD_SPLICE, d: DATA_NORMAL) => if (d.commitments.params.remoteParams.initFeatures.hasFeature(SplicePrototype)) { - if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype) && d.spliceStatus == SpliceStatus.NoSplice) { + if (d.commitments.params.useQuiescence && d.spliceStatus == SpliceStatus.NoSplice) { startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) if (d.commitments.changes.localChanges.all.isEmpty) { stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, 1) @@ -829,7 +829,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } case Event(msg: Stfu, d: DATA_NORMAL) => - if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) { + if (d.commitments.params.useQuiescence) { d.spliceStatus match { case SpliceStatus.NoSplice => startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) @@ -859,7 +859,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with stay() } } else { - log.warning("ignoring stfu because peer doesn't support quiescence") + log.warning("ignoring stfu because both peers do not advertise quiescence") stay() } @@ -2647,7 +2647,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } private def replayQuiescenceSettlements(d: DATA_NORMAL): Unit = - if (d.commitments.params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) { + if (d.commitments.params.useQuiescence) { PendingCommandsDb.getSettlementCommands(nodeParams.db.pendingCommands, d.channelId).foreach(self ! _) } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala index 595ea3a6e3..70e1350f6c 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala @@ -435,6 +435,7 @@ object LightningMessageCodecs { val lightningMessageCodec = discriminated[LightningMessage].by(uint16) .typecase(1, warningCodec) + .typecase(2, stfuCodec) .typecase(16, initCodec) .typecase(17, errorCodec) .typecase(18, pingCodec) @@ -481,7 +482,6 @@ object LightningMessageCodecs { .typecase(37000, spliceInitCodec) .typecase(37002, spliceAckCodec) .typecase(37004, spliceLockedCodec) - .typecase(37006, stfuCodec) // // diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala index 72889ff8ee..b09ddf3e3a 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala @@ -278,6 +278,8 @@ case class ChannelReady(channelId: ByteVector32, val alias_opt: Option[Alias] = tlvStream.get[ShortChannelIdTlv].map(_.alias) } +case class Stfu(channelId: ByteVector32, initiator: Byte) extends LightningMessage + case class SpliceInit(channelId: ByteVector32, fundingContribution: Satoshi, feerate: FeeratePerKw, @@ -562,7 +564,6 @@ case class GossipTimestampFilter(chainHash: ByteVector32, firstTimestamp: Timest case class OnionMessage(blindingKey: PublicKey, onionRoutingPacket: OnionRoutingPacket, tlvStream: TlvStream[OnionMessageTlv] = TlvStream.empty) extends LightningMessage -case class Stfu(channelId: ByteVector32, initiator: Byte) extends LightningMessage // NB: blank lines to minimize merge conflicts // From 98b3286e01701a20b50fd7e2d62210de86d4a0f9 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 21 Jun 2023 10:13:03 +0200 Subject: [PATCH 03/25] Clean up quiescence feature It's not a prototype, it matches the official (in-progress) specification. --- eclair-core/src/main/resources/reference.conf | 1 + .../src/main/scala/fr/acinq/eclair/Features.scala | 13 +++++++------ .../scala/fr/acinq/eclair/channel/Commitments.scala | 9 ++++----- .../scala/fr/acinq/eclair/channel/fsm/Channel.scala | 3 +-- .../states/ChannelStateTestsHelperMethods.scala | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index fac3a61140..7804a64e7b 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -69,6 +69,7 @@ eclair { option_route_blinding = disabled option_shutdown_anysegwit = optional option_dual_fund = disabled + option_quiesce = disabled option_onion_messages = optional option_channel_type = optional option_scid_alias = optional diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala index 718ee26cb2..20db6a6c87 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala @@ -247,6 +247,12 @@ object Features { val mandatory = 28 } + // TODO: this should also extend NodeFeature once the spec is finalized + case object Quiescence extends Feature with InitFeature { + val rfcName = "option_quiesce" + val mandatory = 34 + } + case object OnionMessages extends Feature with InitFeature with NodeFeature { val rfcName = "option_onion_messages" val mandatory = 38 @@ -300,11 +306,6 @@ object Features { val mandatory = 154 } - case object QuiescePrototype extends Feature with InitFeature { - val rfcName = "option_quiesce_prototype" - val mandatory = 34 - } - val knownFeatures: Set[Feature] = Set( DataLossProtect, InitialRoutingSync, @@ -321,6 +322,7 @@ object Features { RouteBlinding, ShutdownAnySegwit, DualFunding, + Quiescence, OnionMessages, ChannelType, ScidAlias, @@ -330,7 +332,6 @@ object Features { TrampolinePaymentPrototype, AsyncPaymentPrototype, SplicePrototype, - QuiescePrototype ) // Features may depend on other features, as specified in Bolt 9. diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 13a5aaec2b..f7b7b64170 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -4,7 +4,6 @@ import akka.event.LoggingAdapter import com.softwaremill.quicklens.ModifyPimp import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, Satoshi, SatoshiLong, Script, Transaction} -import fr.acinq.eclair.Features.QuiescePrototype import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw, FeeratesPerKw, OnChainFeeConf} import fr.acinq.eclair.channel.Helpers.Closing import fr.acinq.eclair.channel.Monitoring.{Metrics, Tags} @@ -118,7 +117,6 @@ case class ChannelParams(channelId: ByteVector32, } /** - * * @param localScriptPubKey local script pubkey (provided in CMD_CLOSE, as an upfront shutdown script, or set to the current final onchain script) * @return an exception if the provided script is not valid */ @@ -133,7 +131,6 @@ case class ChannelParams(channelId: ByteVector32, } /** - * * @param remoteScriptPubKey remote script included in a Shutdown message * @return an exception if the provided script is not valid */ @@ -145,7 +142,9 @@ case class ChannelParams(channelId: ByteVector32, else Right(remoteScriptPubKey) } - def useQuiescence: Boolean = localParams.initFeatures.hasFeature(Features.QuiescePrototype) && remoteParams.initFeatures.hasFeature(Features.QuiescePrototype) + /** If both peers support quiescence, we have to exchange stfu when splicing. */ + def useQuiescence: Boolean = Features.canUseFeature(localParams.initFeatures, remoteParams.initFeatures, Features.Quiescence) + } object ChannelParams { @@ -798,7 +797,7 @@ case class Commitments(params: ChannelParams, // @formatter:off // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. - def isIdle: Boolean = active.head.isIdle(changes, params.localParams.initFeatures.hasFeature(QuiescePrototype) && params.remoteParams.initFeatures.hasFeature(QuiescePrototype)) + def isIdle: Boolean = active.head.isIdle(changes, pendingHtlcsOk = Features.canUseFeature(params.localParams.initFeatures, params.remoteParams.initFeatures, Features.Quiescence)) def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes) def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight) 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 110ec618bd..b31f4e637c 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 @@ -22,7 +22,6 @@ import akka.actor.{Actor, ActorContext, ActorRef, FSM, OneForOneStrategy, Possib import akka.event.Logging.MDC import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{ByteVector32, Satoshi, SatoshiLong, Transaction} -import fr.acinq.eclair.Features.{QuiescePrototype, SplicePrototype} import fr.acinq.eclair.Logs.LogCategory import fr.acinq.eclair._ import fr.acinq.eclair.blockchain.OnChainWallet.MakeFundingTxResponse @@ -811,7 +810,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } case Event(cmd: CMD_SPLICE, d: DATA_NORMAL) => - if (d.commitments.params.remoteParams.initFeatures.hasFeature(SplicePrototype)) { + if (d.commitments.params.remoteParams.initFeatures.hasFeature(Features.SplicePrototype)) { if (d.commitments.params.useQuiescence && d.spliceStatus == SpliceStatus.NoSplice) { startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) if (d.commitments.changes.localChanges.all.isEmpty) { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala index 5f71f4d4c7..d2b16961a2 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala @@ -182,7 +182,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually { .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional)) - .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.QuiescePrototype, FeatureSupport.Optional)) + .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.Quiescence, FeatureSupport.Optional)) .initFeatures() val bobInitFeatures = Bob.nodeParams.features .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DisableWumbo))(_.removed(Features.Wumbo)) @@ -196,7 +196,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually { .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional)) .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional)) - .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.QuiescePrototype, FeatureSupport.Optional)) + .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.Quiescence, FeatureSupport.Optional)) .initFeatures() val channelType = ChannelTypes.defaultFromFeatures(aliceInitFeatures, bobInitFeatures, announceChannel = channelFlags.announceChannel) From 3e6e4e492a2822e999ee9b8d9b5567ad7e44c3b0 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 21 Jun 2023 10:16:00 +0200 Subject: [PATCH 04/25] Clean up `stfu` message Use a boolean for the `initiator` field. --- .../scala/fr/acinq/eclair/channel/fsm/Channel.scala | 10 +++++----- .../eclair/wire/protocol/LightningMessageCodecs.scala | 3 ++- .../eclair/wire/protocol/LightningMessageTypes.scala | 2 +- .../channel/states/e/NormalQuiescentStateSpec.scala | 2 +- .../wire/protocol/LightningMessageCodecsSpec.scala | 11 +++++++++++ 5 files changed, 20 insertions(+), 8 deletions(-) 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 b31f4e637c..1c60b92db7 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 @@ -814,7 +814,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with if (d.commitments.params.useQuiescence && d.spliceStatus == SpliceStatus.NoSplice) { startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) if (d.commitments.changes.localChanges.all.isEmpty) { - stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, 1) + stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, initiator = true) } else { stay() using d.copy(spliceStatus = SpliceStatus.QuiescenceRequested(cmd)) } @@ -833,7 +833,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case SpliceStatus.NoSplice => startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) if (d.commitments.latest.changes.localChanges.all.isEmpty) { - stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, 0) + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false) } else { stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) } @@ -841,7 +841,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) case SpliceStatus.InitiatorQuiescent(splice) => // if both sides send stfu at the same time, the quiescence initiator is the channel initiator - if (msg.initiator == 0 || d.commitments.params.localParams.isInitiator) { + if (!msg.initiator || d.commitments.params.localParams.isInitiator) { handleNewSplice(splice, d) } else { stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) @@ -2620,9 +2620,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with private def handleSendRevocation(revocation: RevokeAndAck, d: DATA_NORMAL): State = { d.spliceStatus match { case SpliceStatus.QuiescenceRequested(splice) if d.commitments.changes.localChanges.all.isEmpty => - stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(splice)) storing() sending Seq(revocation, Stfu(d.channelId, 1)) + stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(splice)) storing() sending Seq(revocation, Stfu(d.channelId, initiator = true)) case SpliceStatus.ReceivedStfu(_) if d.commitments.changes.localChanges.all.isEmpty => - stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) storing() sending Seq(revocation, Stfu(d.channelId, 0)) + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) storing() sending Seq(revocation, Stfu(d.channelId, initiator = false)) case _ => stay() using d storing() sending revocation } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala index 70e1350f6c..35f9269fcc 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecs.scala @@ -423,7 +423,8 @@ object LightningMessageCodecs { val stfuCodec: Codec[Stfu] = ( ("channelId" | bytes32) :: - ("initiator" | byte) ).as[Stfu] + ("initiator" | byte.xmap[Boolean](b => b != 0, b => if (b) 1 else 0))).as[Stfu] + // // diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala index b09ddf3e3a..6f6f932ba7 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala @@ -278,7 +278,7 @@ case class ChannelReady(channelId: ByteVector32, val alias_opt: Option[Alias] = tlvStream.get[ShortChannelIdTlv].map(_.alias) } -case class Stfu(channelId: ByteVector32, initiator: Byte) extends LightningMessage +case class Stfu(channelId: ByteVector32, initiator: Boolean) extends SetupMessage with HasChannelId case class SpliceInit(channelId: ByteVector32, fundingContribution: Satoshi, diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 8470d5cff7..1d4fb5ab14 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -304,7 +304,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) alice2bob.forward(bob) // send a second stfu to bob - bob ! Stfu(channelId(bob), 1) + bob ! Stfu(channelId(bob), initiator = true) bob2alice.expectMsgType[Warning] // we should disconnect after giving alice time to receive the warning bobPeer.fishForMessage(3 seconds) { 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 1a08fa6f22..cc634f6b37 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 @@ -107,6 +107,17 @@ class LightningMessageCodecsSpec extends AnyFunSuite { } } + test("encode/decode stfu") { + val testCases = Seq( + Stfu(ByteVector32.One, initiator = true) -> hex"0002 0100000000000000000000000000000000000000000000000000000000000000 01", + Stfu(ByteVector32.One, initiator = false) -> hex"0002 0100000000000000000000000000000000000000000000000000000000000000 00", + ) + testCases.foreach { case (expected, bin) => + assert(lightningMessageCodec.encode(expected).require.bytes == bin) + assert(lightningMessageCodec.decode(bin.bits).require.value == expected) + } + } + test("nonreg generic tlv") { val channelId = randomBytes32() val signature = randomBytes64() From 491487cd45162953605888bd089414b454874980 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 21 Jun 2023 16:36:22 +0200 Subject: [PATCH 05/25] Rework forbidden splice messages Add more traits to the `SpliceStatus` hierarchy, which lets us simplify some of the pattern matching used in the `Channel` actor. Send a `warning` and disconnect if we receive a forbidden message while we're splicing, which lets us gracefully deal with buggy peers without losing channels. --- .../fr/acinq/eclair/channel/ChannelData.scala | 31 +++++++++---- .../fr/acinq/eclair/channel/fsm/Channel.scala | 43 +++++++++++-------- 2 files changed, 47 insertions(+), 27 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala index 808e70a70d..7ae4ba4454 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala @@ -451,17 +451,32 @@ object RbfStatus { } sealed trait SpliceStatus +/** We're waiting for the channel to be quiescent. */ sealed trait QuiescenceNegotiation extends SpliceStatus +object QuiescenceNegotiation { + sealed trait Initiator extends QuiescenceNegotiation + sealed trait NonInitiator extends QuiescenceNegotiation +} +/** The channel is quiescent and a splice attempt was initiated. */ +sealed trait QuiescentSpliceStatus extends SpliceStatus object SpliceStatus { case object NoSplice extends SpliceStatus - case class QuiescenceRequested(splice: CMD_SPLICE) extends QuiescenceNegotiation - case class InitiatorQuiescent(splice: CMD_SPLICE) extends QuiescenceNegotiation - case class ReceivedStfu(stfu: Stfu) extends QuiescenceNegotiation - case object NonInitiatorQuiescent extends QuiescenceNegotiation - case class SpliceRequested(cmd: CMD_SPLICE, init: SpliceInit) extends SpliceStatus - case class SpliceInProgress(cmd_opt: Option[CMD_SPLICE], splice: typed.ActorRef[InteractiveTxBuilder.Command], remoteCommitSig: Option[CommitSig]) extends SpliceStatus - case class SpliceWaitingForSigs(signingSession: InteractiveTxSigningSession.WaitingForSigs) extends SpliceStatus - case object SpliceAborted extends SpliceStatus + /** We stop sending new updates and wait for our updates to be added to the local and remote commitments. */ + case class QuiescenceRequested(splice: CMD_SPLICE) extends QuiescenceNegotiation.Initiator + /** Our updates to be added to the local and remote commitments, we wait for our peer to do the same. */ + case class InitiatorQuiescent(splice: CMD_SPLICE) extends QuiescenceNegotiation.Initiator + /** Our peer is asked us to stop sending new updates and wait for our updates to be added to the local and remote commitments. */ + case class ReceivedStfu(stfu: Stfu) extends QuiescenceNegotiation.NonInitiator + /** Our updates to be added to the local and remote commitments, we wait for our peer to use the now quiescent channel. */ + case object NonInitiatorQuiescent extends QuiescenceNegotiation.NonInitiator + /** We told our peer we want to splice funds in the channel. */ + case class SpliceRequested(cmd: CMD_SPLICE, init: SpliceInit) extends QuiescentSpliceStatus + /** We both agreed to splice and are building the splice transaction. */ + case class SpliceInProgress(cmd_opt: Option[CMD_SPLICE], splice: typed.ActorRef[InteractiveTxBuilder.Command], remoteCommitSig: Option[CommitSig]) extends QuiescentSpliceStatus + /** The splice transaction has been negotiated, we're exchanging signatures. */ + case class SpliceWaitingForSigs(signingSession: InteractiveTxSigningSession.WaitingForSigs) extends QuiescentSpliceStatus + /** The splice attempt was aborted by us, we're waiting for our peer to ack. */ + case object SpliceAborted extends QuiescentSpliceStatus } sealed trait ChannelData extends PossiblyHarmful { 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 1c60b92db7..9adc1a9bfe 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 @@ -370,37 +370,42 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with val error = ForbiddenDuringQuiescence(d.channelId, c.getClass.getSimpleName) c match { case c: CMD_ADD_HTLC => handleAddHtlcCommandError(c, error, Some(d.channelUpdate)) - case _: HtlcSettlementCommand => stay() // htlc settlement commands will be ignored and replayed when not quiescent + // Htlc settlement commands are ignored and will be replayed when not quiescent. + // This could create issues if we're keeping htlcs that should be settled pending for too long, as they could timeout. + // That's why we have a timer to actively disconnect if we stay in quiescence for too long. + case _: HtlcSettlementCommand => stay() case _ => handleCommandError(error, c) } - case Event(c: ForbiddenCommandDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[QuiescenceNegotiation] => + case Event(c: ForbiddenCommandDuringSplice, d: DATA_NORMAL) if d.spliceStatus.isInstanceOf[QuiescentSpliceStatus] => val error = ForbiddenDuringSplice(d.channelId, c.getClass.getSimpleName) c match { case c: CMD_ADD_HTLC => handleAddHtlcCommandError(c, error, Some(d.channelUpdate)) - // NB: the command cannot be an htlc settlement (fail/fulfill), because if we are splicing without quiescence support it means the channel is idle and has no htlcs + // Htlc settlement commands are ignored and will be replayed when not quiescent. + // This could create issues if we're keeping htlcs that should be settled pending for too long, as they could timeout. + // That's why we have a timer to actively disconnect if we're splicing for too long. + case _: HtlcSettlementCommand => stay() case _ => handleCommandError(error, c) } - case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.commitments.params.useQuiescence && d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.InitiatorQuiescent] => + case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.spliceStatus.isInstanceOf[QuiescentSpliceStatus] => + log.warning("received forbidden message {} during splicing with status {}", msg.getClass.getSimpleName, d.spliceStatus.getClass.getSimpleName) val error = ForbiddenDuringSplice(d.channelId, msg.getClass.getSimpleName) + // We forward preimages as soon as possible to the upstream channel because it allows us to pull funds. msg match { - case fulfill: UpdateFulfillHtlc => - d.commitments.receiveFulfill(fulfill) match { - case Right((commitments1, origin, htlc)) => - // we forward preimages as soon as possible to the upstream channel because it allows us to pull funds - relayer ! RES_ADD_SETTLED(origin, htlc, HtlcResult.RemoteFulfill(fulfill)) - handleLocalError(error, d.copy(commitments = commitments1), Some(msg)) - case Left(_) => handleLocalError(error, d, Some(fulfill)) - } - case _ => handleLocalError(error, d, Some(msg)) + case fulfill: UpdateFulfillHtlc => d.commitments.receiveFulfill(fulfill) match { + case Right((_, origin, htlc)) => relayer ! RES_ADD_SETTLED(origin, htlc, HtlcResult.RemoteFulfill(fulfill)) + case _ => () + } + case _ => () } - - case Event(msg: ForbiddenMessageDuringSplice, d: DATA_NORMAL) if d.spliceStatus != SpliceStatus.NoSplice && !d.spliceStatus.isInstanceOf[SpliceStatus.SpliceRequested] && !d.spliceStatus.isInstanceOf[QuiescenceNegotiation] => - // In case of a race between our splice_init and a forbidden message from our peer, we accept their message, because - // we know they are going to reject our splice attempt - val error = ForbiddenDuringSplice(d.channelId, msg.getClass.getSimpleName) - handleLocalError(error, d, Some(msg)) + // Instead of force-closing (which would cost us on-chain fees), we try to resolve this issue by disconnecting. + // This will abort the splice attempt if it hasn't been signed yet, and restore the channel to a clean state. + // If the splice attempt was signed, it gives us an opportunity to re-exchange signatures on reconnection before + // the forbidden message. It also provides the opportunity for our peer to update their node to get rid of that + // bug and resume normal execution. + context.system.scheduler.scheduleOnce(1 second, peer, Peer.Disconnect(remoteNodeId)) + stay() sending Warning(d.channelId, error.getMessage) case Event(c: CMD_ADD_HTLC, d: DATA_NORMAL) if d.localShutdown.isDefined || d.remoteShutdown.isDefined => // note: spec would allow us to keep sending new htlcs after having received their shutdown (and not sent ours) From 26ad317074fae5f14dda6b2d681da00f4160dde4 Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 22 Jun 2023 10:15:26 +0200 Subject: [PATCH 06/25] Include splicing in the quiescence timeout Being quiescent for too long is dangerous, because HTLCs may timeout without giving the opportunity to our peer to send us a preimage. Splicing operations shouldn't take long to complete, so we include that in the quiescence timeout, and disconnect if the splice wasn't completed in time. This makes sure we replay pending commands and avoid getting the channel stuck if the interactive-tx isn't making progress. --- eclair-core/src/main/resources/reference.conf | 2 +- .../eclair/channel/ChannelExceptions.scala | 1 + .../fr/acinq/eclair/channel/fsm/Channel.scala | 53 +++++++++---------- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index 7804a64e7b..6b1622185b 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -156,7 +156,7 @@ eclair { channel-opener-whitelist = [] // a list of public keys; we will ignore rate limits on pending channels from these peers } - quiescence-timeout = 1 minutes // maximum time we will wait for quiescence to complete before disconnecting + quiescence-timeout = 1 minutes // maximum time we will stay quiescent (or wait to reach quiescence) before disconnecting } balance-check-interval = 1 hour diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala index 5765275cc4..db30daa649 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala @@ -71,6 +71,7 @@ case class InvalidCompleteInteractiveTx (override val channelId: Byte case class TooManyInteractiveTxRounds (override val channelId: ByteVector32) extends ChannelException(channelId, "too many messages exchanged during interactive tx construction") case class RbfAttemptAborted (override val channelId: ByteVector32) extends ChannelException(channelId, "rbf attempt aborted") case class SpliceAttemptAborted (override val channelId: ByteVector32) extends ChannelException(channelId, "splice attempt aborted") +case class SpliceAttemptTimedOut (override val channelId: ByteVector32) extends ChannelException(channelId, "splice attempt took too long, disconnecting") case class DualFundingAborted (override val channelId: ByteVector32) extends ChannelException(channelId, "dual funding aborted") case class UnexpectedInteractiveTxMessage (override val channelId: ByteVector32, msg: InteractiveTxMessage) extends ChannelException(channelId, s"unexpected interactive-tx message (${msg.getClass.getSimpleName})") case class UnexpectedFundingSignatures (override val channelId: ByteVector32) extends ChannelException(channelId, "unexpected funding signatures (tx_signatures)") 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 9adc1a9bfe..71b883b448 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 @@ -156,7 +156,7 @@ object Channel { // we will receive this message when we waited too long for a revocation for that commit number (NB: we explicitly specify the peer to allow for testing) case class RevocationTimeout(remoteCommitNumber: Long, peer: ActorRef) - // we will receive this message if we waited too long for peers to exchange stfu messages (NB: we explicitly specify the peer to allow for testing) + // we will receive this message if we waited too long to reach quiescence, or stayed quiescent for too long (NB: we explicitly specify the peer to allow for testing) case class QuiescenceTimeout(peer: ActorRef) /** We don't immediately process [[CurrentBlockHeight]] to avoid herd effects */ @@ -566,7 +566,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with val minDepth_opt = d.commitments.params.minDepthDualFunding(nodeParams.channelConf.minDepthBlocks, signingSession1.fundingTx.sharedTx.tx) watchFundingConfirmed(signingSession.fundingTx.txId, minDepth_opt) val commitments1 = d.commitments.add(signingSession1.commitment) - stay() using d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) storing() sending signingSession1.localSigs calling replayQuiescenceSettlements(d) + val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) + stay() using d1 storing() sending signingSession1.localSigs calling endQuiescence(d1) } } case _ if d.commitments.params.channelFeatures.hasFeature(Features.DualFunding) && d.commitments.latest.localFundingStatus.signedTx_opt.isEmpty && commit.batchSize == 1 => @@ -635,8 +636,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(r: RevocationTimeout, d: DATA_NORMAL) => handleRevocationTimeout(r, d) - case Event(s: QuiescenceTimeout, d: DATA_NORMAL) => handleQuiescenceTimeout(s, d) - case Event(c: CMD_CLOSE, d: DATA_NORMAL) => if (d.localShutdown.isDefined) { handleCommandError(ClosingAlreadyInProgress(d.channelId), c) @@ -852,7 +851,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) } case SpliceStatus.ReceivedStfu(_) | SpliceStatus.NonInitiatorQuiescent => - cancelTimer(QuiescenceTimeout.toString) val failure = new ChannelException(d.channelId, "received stfu twice") log.info("quiesce attempt failed: {}", failure.getMessage) // NB: we use a small delay to ensure we've sent our warning before disconnecting. @@ -867,10 +865,11 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with stay() } + case Event(_: QuiescenceTimeout, d: DATA_NORMAL) => handleQuiescenceTimeout(d) + case Event(msg: SpliceInit, d: DATA_NORMAL) => d.spliceStatus match { case SpliceStatus.NoSplice | SpliceStatus.NonInitiatorQuiescent => - cancelTimer(QuiescenceTimeout.toString) if (!d.commitments.isIdle) { log.info("rejecting splice request: channel not idle") stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceRequest(d.channelId).getMessage) @@ -968,25 +967,24 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with log.info("our peer aborted the splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, SpliceAttemptAborted(d.channelId))) txBuilder ! InteractiveTxBuilder.Abort - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling endQuiescence(d) case SpliceStatus.SpliceWaitingForSigs(signingSession) => log.info("our peer aborted the splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) rollbackFundingAttempt(signingSession.fundingTx.tx, previousTxs = Seq.empty) // no splice rbf yet - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling endQuiescence(d) case SpliceStatus.SpliceRequested(cmd, _) => log.info("our peer rejected our splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) cmd.replyTo ! RES_FAILURE(cmd, new RuntimeException(s"splice attempt rejected by our peer: ${msg.toAscii}")) - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling replayQuiescenceSettlements(d) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling endQuiescence(d) case SpliceStatus.SpliceAborted => log.debug("our peer acked our previous tx_abort") - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) calling replayQuiescenceSettlements(d) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) calling endQuiescence(d) case SpliceStatus.NoSplice => log.info("our peer wants to abort the splice, but we've already negotiated a splice transaction: ascii='{}' bin={}", msg.toAscii, msg.data) // We ack their tx_abort but we keep monitoring the funding transaction until it's confirmed or double-spent. stay() sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) case _: SpliceStatus.QuiescenceRequested | _: SpliceStatus.InitiatorQuiescent | _: SpliceStatus.ReceivedStfu | SpliceStatus.NonInitiatorQuiescent => log.info("our peer aborted the splice during quiescence negotiation, disconnecting: ascii='{}' bin={}", msg.toAscii, msg.data) - cancelTimer(QuiescenceTimeout.toString) // NB: we use a small delay to ensure we've sent our warning before disconnecting. context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, "spec violation: you sent tx abort during quiescence negotiation") @@ -1048,7 +1046,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with val commitments1 = d.commitments.add(signingSession1.commitment) val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice) log.info("publishing funding tx for channelId={} fundingTxId={}", d.channelId, signingSession1.fundingTx.sharedTx.txId) - stay() using d1 storing() sending signingSession1.localSigs calling publishFundingTx(signingSession1.fundingTx) calling replayQuiescenceSettlements(d1) + stay() using d1 storing() sending signingSession1.localSigs calling publishFundingTx(signingSession1.fundingTx) calling endQuiescence(d1) } case _ => // We may receive an outdated tx_signatures if the transaction is already confirmed. @@ -2578,7 +2576,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } private def handleNewSplice(cmd: CMD_SPLICE, d: DATA_NORMAL): State = { - cancelTimer(QuiescenceTimeout.toString) d.spliceStatus match { case SpliceStatus.NoSplice | _: SpliceStatus.InitiatorQuiescent => if (d.commitments.isIdle) { @@ -2633,27 +2630,25 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } } - private def handleQuiescenceTimeout(spliceTimeout: QuiescenceTimeout, d: DATA_NORMAL): State = { - val msg = d.spliceStatus match { - case SpliceStatus.QuiescenceRequested(_) => Some("quiescence timed out waiting for pending local htlcs to finalize (initiator)") - case SpliceStatus.InitiatorQuiescent(_) => Some("quiescence timed out waiting for remote stfu") - case SpliceStatus.ReceivedStfu(_) => Some("quiescence timed out waiting for pending local htlcs to finalize (non-initiator)") - case SpliceStatus.NonInitiatorQuiescent => Some("quiescence timed out waiting to receive splice-init") - case _ => None - } - msg match { - case Some(msg) => - log.warning(s"$msg, closing connection") - context.system.scheduler.scheduleOnce(2 second, spliceTimeout.peer, Peer.Disconnect(remoteNodeId)) - stay() sending Warning(d.channelId, msg) - case None => stay() + private def handleQuiescenceTimeout(d: DATA_NORMAL): State = { + if (d.spliceStatus == SpliceStatus.NoSplice) { + log.warning("quiescence timed out with no ongoing splice, did we forget to cancel the timer?") + stay() + } else { + log.warning("quiescence timed out in state {}, closing connection", d.spliceStatus.getClass.getSimpleName) + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() sending Warning(d.channelId, SpliceAttemptTimedOut(d.channelId).getMessage) } } - private def replayQuiescenceSettlements(d: DATA_NORMAL): Unit = - if (d.commitments.params.useQuiescence) { + /** Get out of a quiescent state: if there are HTLCs in flight, re-emit pending settlement commands. */ + private def endQuiescence(d: DATA_NORMAL): Unit = { + // We cancel the quiescence timeout timer, otherwise it may fire during the next quiescence session. + cancelTimer(QuiescenceTimeout.toString) + if (d.commitments.hasPendingOrProposedHtlcs) { PendingCommandsDb.getSettlementCommands(nodeParams.db.pendingCommands, d.channelId).foreach(self ! _) } + } private def reportSpliceFailure(d: DATA_NORMAL): Unit = { val cmd_opt = d.spliceStatus match { From 8b233f73e1086c5a9bf406512ef257edcfc7bbbb Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 22 Jun 2023 10:36:59 +0200 Subject: [PATCH 07/25] Ignore duplicate `stfu` The spec says we must not send duplicate `stfu`, but doesn't have any requirement on the receiver. If we receive a duplicate `stfu`, it's perfectly fine to just ignore it: the protocol will either correctly complete or will be canceled by the quiescence timeout. --- .../main/scala/fr/acinq/eclair/channel/fsm/Channel.scala | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) 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 71b883b448..e4e240e2fe 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 @@ -850,14 +850,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } else { stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) } - case SpliceStatus.ReceivedStfu(_) | SpliceStatus.NonInitiatorQuiescent => - val failure = new ChannelException(d.channelId, "received stfu twice") - log.info("quiesce attempt failed: {}", failure.getMessage) - // NB: we use a small delay to ensure we've sent our warning before disconnecting. - context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, failure.toString) case _ => - log.warning("ignoring stfu received during splice") + log.warning("ignoring duplicate stfu") stay() } } else { From b8cec0f122a33c77b525d9d090ed405b3666fa22 Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 22 Jun 2023 11:47:24 +0200 Subject: [PATCH 08/25] Remove `handleSendRevocation` This function was called in only one place, so it's not avoiding code duplication. It could make sense to isolate the logic, but in the case of event handlers in the highly critical channel FSM, we usually like to inline transition and state changes to make it easier to review the code linearly (no need to jump back and forth between function calls). I also renamed `isIdle` to `isQuiescent` to remove confusion. --- .../fr/acinq/eclair/channel/Commitments.scala | 4 +-- .../fr/acinq/eclair/channel/fsm/Channel.scala | 30 +++++++++---------- .../states/e/NormalQuiescentStateSpec.scala | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index f7b7b64170..a077f53a22 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -360,7 +360,7 @@ case class Commitment(fundingTxIndex: Long, changes.localChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) || changes.remoteChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) - def isIdle(changes: CommitmentChanges, pendingHtlcsOk: Boolean): Boolean = (pendingHtlcsOk || hasNoPendingHtlcs) && changes.localChanges.all.isEmpty && changes.remoteChanges.all.isEmpty + def isQuiescent(changes: CommitmentChanges, pendingHtlcsOk: Boolean): Boolean = (pendingHtlcsOk || hasNoPendingHtlcs) && changes.localChanges.all.isEmpty && changes.remoteChanges.all.isEmpty def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = { def expired(add: UpdateAddHtlc): Boolean = currentHeight >= add.cltvExpiry.blockHeight @@ -797,7 +797,7 @@ case class Commitments(params: ChannelParams, // @formatter:off // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. - def isIdle: Boolean = active.head.isIdle(changes, pendingHtlcsOk = Features.canUseFeature(params.localParams.initFeatures, params.remoteParams.initFeatures, Features.Quiescence)) + def isQuiescent: Boolean = active.head.isQuiescent(changes, pendingHtlcsOk = params.useQuiescence) def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes) def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight) 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 e4e240e2fe..86f86258e2 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 @@ -592,7 +592,18 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with context.system.eventStream.publish(AvailableBalanceChanged(self, d.channelId, d.shortIds, commitments1)) } context.system.eventStream.publish(ChannelSignatureReceived(self, commitments1)) - handleSendRevocation(revocation, d.copy(commitments = commitments1)) + // If we're now quiescent, we may send our stfu message. + val (d1, toSend) = d.spliceStatus match { + case SpliceStatus.QuiescenceRequested(cmd) if commitments1.changes.localChanges.all.isEmpty => + val stfu = Stfu(d.channelId, initiator = true) + (d.copy(commitments = commitments1, spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)), Seq(revocation, stfu)) + case _: SpliceStatus.ReceivedStfu if commitments1.changes.localChanges.all.isEmpty => + val stfu = Stfu(d.channelId, initiator = false) + (d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NonInitiatorQuiescent), Seq(revocation, stfu)) + case _ => + (d.copy(commitments = commitments1), Seq(revocation)) + } + stay() using d1 storing() sending toSend case Left(cause) => handleLocalError(cause, d, Some(commit)) } } @@ -864,8 +875,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(msg: SpliceInit, d: DATA_NORMAL) => d.spliceStatus match { case SpliceStatus.NoSplice | SpliceStatus.NonInitiatorQuiescent => - if (!d.commitments.isIdle) { - log.info("rejecting splice request: channel not idle") + if (!d.commitments.isQuiescent) { + log.info("rejecting splice request: channel not quiescent") stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceRequest(d.channelId).getMessage) } else if (msg.feerate < nodeParams.currentFeerates.minimum) { log.info("rejecting splice request: feerate too low") @@ -2572,7 +2583,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with private def handleNewSplice(cmd: CMD_SPLICE, d: DATA_NORMAL): State = { d.spliceStatus match { case SpliceStatus.NoSplice | _: SpliceStatus.InitiatorQuiescent => - if (d.commitments.isIdle) { + if (d.commitments.isQuiescent) { val parentCommitment = d.commitments.latest.commitment val targetFeerate = nodeParams.onChainFeeConf.getFundingFeerate(nodeParams.currentFeerates) val fundingContribution = InteractiveTxFunder.computeSpliceContribution( @@ -2613,17 +2624,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } } - private def handleSendRevocation(revocation: RevokeAndAck, d: DATA_NORMAL): State = { - d.spliceStatus match { - case SpliceStatus.QuiescenceRequested(splice) if d.commitments.changes.localChanges.all.isEmpty => - stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(splice)) storing() sending Seq(revocation, Stfu(d.channelId, initiator = true)) - case SpliceStatus.ReceivedStfu(_) if d.commitments.changes.localChanges.all.isEmpty => - stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) storing() sending Seq(revocation, Stfu(d.channelId, initiator = false)) - case _ => - stay() using d storing() sending revocation - } - } - private def handleQuiescenceTimeout(d: DATA_NORMAL): State = { if (d.spliceStatus == SpliceStatus.NoSplice) { log.warning("quiescence timed out with no ongoing splice, did we forget to cancel the timer?") diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 1d4fb5ab14..4c411b88bc 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -196,7 +196,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.forward(bob) bob2alice.expectNoMessage(100 millis) crossSign(bob, alice, bob2alice, alice2bob) - eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isIdle)) + eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent)) bob2alice.expectMsgType[Stfu] bob2alice.forward(alice) // when both nodes are quiescent, alice will start the splice From e7c16a9feeee23a8557255f461696c8907bdcd91 Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 22 Jun 2023 17:24:40 +0200 Subject: [PATCH 09/25] Refactor `handleNewSplice` and a few nits In channel event handlers, we try to leave all the logic that updates the current state or data directly in the event handler instead of delegating it to helper functions, otherwise it's a bit hard to see all the state transitions that can happen when receiving a given message. I changed the `handleNewSplice` function to be a pure function instead that just creates the `splice_init` message (or returns an error). I also refactored some nits. --- .../eclair/channel/ChannelExceptions.scala | 1 + .../fr/acinq/eclair/channel/Commitments.scala | 1 + .../fr/acinq/eclair/channel/fsm/Channel.scala | 137 +++++++++--------- 3 files changed, 73 insertions(+), 66 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala index db30daa649..90805842ac 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala @@ -85,6 +85,7 @@ case class InvalidRbfTxAbortNotAcked (override val channelId: Byte case class InvalidRbfAttemptsExhausted (override val channelId: ByteVector32, maxAttempts: Int) extends ChannelException(channelId, s"invalid rbf attempt: $maxAttempts/$maxAttempts attempts already published") case class InvalidRbfAttemptTooSoon (override val channelId: ByteVector32, previousAttempt: BlockHeight, nextAttempt: BlockHeight) extends ChannelException(channelId, s"invalid rbf attempt: last attempt made at block=$previousAttempt, next attempt available after block=$nextAttempt") case class InvalidSpliceTxAbortNotAcked (override val channelId: ByteVector32) extends ChannelException(channelId, "invalid splice attempt: our previous tx_abort has not been acked") +case class InvalidSpliceNotQuiescent (override val channelId: ByteVector32) extends ChannelException(channelId, "invalid splice attempt: the channel is not quiescent") case class InvalidRbfTxConfirmed (override val channelId: ByteVector32) extends ChannelException(channelId, "no need to rbf, transaction is already confirmed") case class InvalidRbfNonInitiator (override val channelId: ByteVector32) extends ChannelException(channelId, "cannot initiate rbf: we're not the initiator of this interactive-tx attempt") case class InvalidRbfZeroConf (override val channelId: ByteVector32) extends ChannelException(channelId, "cannot initiate rbf: we're using zero-conf for this interactive-tx attempt") diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index a077f53a22..7262774c0d 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -796,6 +796,7 @@ case class Commitments(params: ChannelParams, def add(commitment: Commitment): Commitments = copy(active = commitment +: active) // @formatter:off + def localIsQuiescent: Boolean = changes.localChanges.all.isEmpty // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. def isQuiescent: Boolean = active.head.isQuiescent(changes, pendingHtlcsOk = params.useQuiescence) def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) 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 86f86258e2..ffc6602117 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 @@ -594,10 +594,10 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with context.system.eventStream.publish(ChannelSignatureReceived(self, commitments1)) // If we're now quiescent, we may send our stfu message. val (d1, toSend) = d.spliceStatus match { - case SpliceStatus.QuiescenceRequested(cmd) if commitments1.changes.localChanges.all.isEmpty => + case SpliceStatus.QuiescenceRequested(cmd) if commitments1.localIsQuiescent => val stfu = Stfu(d.channelId, initiator = true) (d.copy(commitments = commitments1, spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)), Seq(revocation, stfu)) - case _: SpliceStatus.ReceivedStfu if commitments1.changes.localChanges.all.isEmpty => + case _: SpliceStatus.ReceivedStfu if commitments1.localIsQuiescent => val stfu = Stfu(d.channelId, initiator = false) (d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NonInitiatorQuiescent), Seq(revocation, stfu)) case _ => @@ -826,15 +826,26 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(cmd: CMD_SPLICE, d: DATA_NORMAL) => if (d.commitments.params.remoteParams.initFeatures.hasFeature(Features.SplicePrototype)) { - if (d.commitments.params.useQuiescence && d.spliceStatus == SpliceStatus.NoSplice) { - startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) - if (d.commitments.changes.localChanges.all.isEmpty) { - stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, initiator = true) - } else { - stay() using d.copy(spliceStatus = SpliceStatus.QuiescenceRequested(cmd)) - } - } else { - handleNewSplice(cmd, d) + d.spliceStatus match { + case SpliceStatus.NoSplice if d.commitments.params.useQuiescence => + startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.revocationTimeout) + if (d.commitments.localIsQuiescent) { + stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, initiator = true) + } else { + stay() using d.copy(spliceStatus = SpliceStatus.QuiescenceRequested(cmd)) + } + case SpliceStatus.NoSplice if !d.commitments.params.useQuiescence => + initiateSplice(cmd, d) match { + case Left(f) => + cmd.replyTo ! RES_FAILURE(cmd, f) + stay() + case Right(spliceInit) => + stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + } + case _ => + log.warning("cannot initiate splice, another one is already in progress") + cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceAlreadyInProgress(d.channelId)) + stay() } } else { log.warning("cannot initiate splice, peer doesn't support splices") @@ -847,17 +858,24 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with d.spliceStatus match { case SpliceStatus.NoSplice => startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) - if (d.commitments.latest.changes.localChanges.all.isEmpty) { + if (d.commitments.localIsQuiescent) { stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false) } else { stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) } case SpliceStatus.QuiescenceRequested(_) => stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) - case SpliceStatus.InitiatorQuiescent(splice) => + case SpliceStatus.InitiatorQuiescent(cmd) => // if both sides send stfu at the same time, the quiescence initiator is the channel initiator if (!msg.initiator || d.commitments.params.localParams.isInitiator) { - handleNewSplice(splice, d) + initiateSplice(cmd, d) match { + case Left(f) => + cmd.replyTo ! RES_FAILURE(cmd, f) + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, f.getMessage) + case Right(spliceInit) => + stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + } } else { stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) } @@ -877,7 +895,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case SpliceStatus.NoSplice | SpliceStatus.NonInitiatorQuiescent => if (!d.commitments.isQuiescent) { log.info("rejecting splice request: channel not quiescent") - stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceRequest(d.channelId).getMessage) + stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage) } else if (msg.feerate < nodeParams.currentFeerates.minimum) { log.info("rejecting splice request: feerate too low") stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceRequest(d.channelId).getMessage) @@ -916,12 +934,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case SpliceStatus.SpliceAborted => log.info("rejecting splice attempt: our previous tx_abort was not acked") stay() sending Warning(d.channelId, InvalidSpliceTxAbortNotAcked(d.channelId).getMessage) - case _: SpliceStatus.SpliceRequested | _: SpliceStatus.SpliceInProgress | _: SpliceStatus.SpliceWaitingForSigs => + case _ => log.info("rejecting splice attempt: the current splice attempt must be completed or aborted first") stay() sending Warning(d.channelId, InvalidSpliceAlreadyInProgress(d.channelId).getMessage) - case _: SpliceStatus.QuiescenceRequested | _: SpliceStatus.InitiatorQuiescent | _: SpliceStatus.ReceivedStfu => - log.info("rejecting splice attempt: the current splice attempt must be completed or aborted first (quiescence)") - stay() sending Warning(d.channelId, InvalidSpliceAlreadyInProgress(d.channelId).getMessage) } case Event(msg: SpliceAck, d: DATA_NORMAL) => @@ -988,11 +1003,11 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with log.info("our peer wants to abort the splice, but we've already negotiated a splice transaction: ascii='{}' bin={}", msg.toAscii, msg.data) // We ack their tx_abort but we keep monitoring the funding transaction until it's confirmed or double-spent. stay() sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) - case _: SpliceStatus.QuiescenceRequested | _: SpliceStatus.InitiatorQuiescent | _: SpliceStatus.ReceivedStfu | SpliceStatus.NonInitiatorQuiescent => + case _: QuiescenceNegotiation => log.info("our peer aborted the splice during quiescence negotiation, disconnecting: ascii='{}' bin={}", msg.toAscii, msg.data) // NB: we use a small delay to ensure we've sent our warning before disconnecting. context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, "spec violation: you sent tx abort during quiescence negotiation") + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, UnexpectedInteractiveTxMessage(d.channelId, msg).getMessage) } case Event(msg: InteractiveTxBuilder.Response, d: DATA_NORMAL) => @@ -1097,7 +1112,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with // we cancel the timer that would have made us send the enabled update after reconnection (flappy channel protection) cancelTimer(Reconnected.toString) // if we are splicing, we need to cancel it - reportSpliceFailure(d) + reportSpliceFailure(d.spliceStatus, new RuntimeException("splice attempt failed: disconnected")) val d1 = d.spliceStatus match { // We keep track of the RBF status: we should be able to complete the signature steps on reconnection. case _: SpliceStatus.SpliceWaitingForSigs => d @@ -2580,47 +2595,37 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with proposedTx_opt.get.unsignedTx.copy(tx = tx) } - private def handleNewSplice(cmd: CMD_SPLICE, d: DATA_NORMAL): State = { - d.spliceStatus match { - case SpliceStatus.NoSplice | _: SpliceStatus.InitiatorQuiescent => - if (d.commitments.isQuiescent) { - val parentCommitment = d.commitments.latest.commitment - val targetFeerate = nodeParams.onChainFeeConf.getFundingFeerate(nodeParams.currentFeerates) - val fundingContribution = InteractiveTxFunder.computeSpliceContribution( - isInitiator = true, - sharedInput = Multisig2of2Input(parentCommitment), - spliceInAmount = cmd.additionalLocalFunding, - spliceOut = cmd.spliceOutputs, - targetFeerate = targetFeerate) - if (parentCommitment.localCommit.spec.toLocal + fundingContribution < parentCommitment.localChannelReserve(d.commitments.params)) { - log.warning("cannot do splice: insufficient funds") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) - stay() - } else if (cmd.spliceOut_opt.map(_.scriptPubKey).exists(!MutualClose.isValidFinalScriptPubkey(_, allowAnySegwit = true))) { - log.warning("cannot do splice: invalid splice-out script") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceRequest(d.channelId)) - stay() - } else { - log.info(s"initiating splice with local.in.amount=${cmd.additionalLocalFunding} local.in.push=${cmd.pushAmount} local.out.amount=${cmd.spliceOut_opt.map(_.amount).sum}") - val spliceInit = SpliceInit(d.channelId, - fundingContribution = fundingContribution, - lockTime = nodeParams.currentBlockHeight.toLong, - feerate = targetFeerate, - fundingPubKey = keyManager.fundingPublicKey(d.commitments.params.localParams.fundingKeyPath, parentCommitment.fundingTxIndex + 1).publicKey, - pushAmount = cmd.pushAmount, - requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding - ) - stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit - } - } else { - log.warning("cannot initiate splice, channel is not idle") - cmd.replyTo ! RES_FAILURE(cmd, CommandUnavailableInThisState(d.channelId, "splice", NORMAL)) - stay() - } - case _ => - log.warning("cannot initiate splice, another one is already in progress") - cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceAlreadyInProgress(d.channelId)) - stay() + private def initiateSplice(cmd: CMD_SPLICE, d: DATA_NORMAL): Either[ChannelException, SpliceInit] = { + if (d.commitments.isQuiescent) { + val parentCommitment = d.commitments.latest.commitment + val targetFeerate = nodeParams.onChainFeeConf.getFundingFeerate(nodeParams.currentFeerates) + val fundingContribution = InteractiveTxFunder.computeSpliceContribution( + isInitiator = true, + sharedInput = Multisig2of2Input(parentCommitment), + spliceInAmount = cmd.additionalLocalFunding, + spliceOut = cmd.spliceOutputs, + targetFeerate = targetFeerate) + if (parentCommitment.localCommit.spec.toLocal + fundingContribution < parentCommitment.localChannelReserve(d.commitments.params)) { + log.warning("cannot do splice: insufficient funds") + Left(InvalidSpliceRequest(d.channelId)) + } else if (cmd.spliceOut_opt.map(_.scriptPubKey).exists(!MutualClose.isValidFinalScriptPubkey(_, allowAnySegwit = true))) { + log.warning("cannot do splice: invalid splice-out script") + Left(InvalidSpliceRequest(d.channelId)) + } else { + log.info(s"initiating splice with local.in.amount=${cmd.additionalLocalFunding} local.in.push=${cmd.pushAmount} local.out.amount=${cmd.spliceOut_opt.map(_.amount).sum}") + val spliceInit = SpliceInit(d.channelId, + fundingContribution = fundingContribution, + lockTime = nodeParams.currentBlockHeight.toLong, + feerate = targetFeerate, + fundingPubKey = keyManager.fundingPublicKey(d.commitments.params.localParams.fundingKeyPath, parentCommitment.fundingTxIndex + 1).publicKey, + pushAmount = cmd.pushAmount, + requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding + ) + Right(spliceInit) + } + } else { + log.warning("cannot initiate splice, channel is not quiescent") + Left(InvalidSpliceNotQuiescent(d.channelId)) } } @@ -2644,8 +2649,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } } - private def reportSpliceFailure(d: DATA_NORMAL): Unit = { - val cmd_opt = d.spliceStatus match { + private def reportSpliceFailure(spliceStatus: SpliceStatus, f: Throwable): Unit = { + val cmd_opt = spliceStatus match { case SpliceStatus.QuiescenceRequested(cmd) => Some(cmd) case SpliceStatus.InitiatorQuiescent(cmd) => Some(cmd) case SpliceStatus.SpliceRequested(cmd, _) => Some(cmd) @@ -2654,7 +2659,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with cmd_opt case _ => None } - cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, new RuntimeException("splice attempt failed: disconnected"))) + cmd_opt.foreach(cmd => cmd.replyTo ! RES_FAILURE(cmd, f)) } override def mdc(currentMessage: Any): MDC = { From 464569bc0c06e935d68f9a77eec73eaad04282a7 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Tue, 18 Jul 2023 16:02:28 +0200 Subject: [PATCH 10/25] Update handling of forbidden messages during a normal splice - now returns a warning (and disconnects) instead of force closing --- .../states/e/NormalSplicesStateSpec.scala | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala index d6e8c0fac3..6fb66711d7 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala @@ -649,21 +649,20 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik alice2bob.expectMsgType[SpliceInit] // we're holding the splice_init to create a race - val (preimage, cmdAdd: CMD_ADD_HTLC) = makeCmdAdd(5_000_000 msat, bob.underlyingActor.remoteNodeId, bob.underlyingActor.nodeParams.currentBlockHeight) + val (_, cmdAdd: CMD_ADD_HTLC) = makeCmdAdd(5_000_000 msat, bob.underlyingActor.remoteNodeId, bob.underlyingActor.nodeParams.currentBlockHeight) bob ! cmdAdd - val add = bob2alice.expectMsgType[UpdateAddHtlc] + bob2alice.expectMsgType[UpdateAddHtlc] bob2alice.forward(alice) // now we forward the splice_init alice2bob.forward(bob) - // this cancels the splice + // bob rejects the SpliceInit because they have a pending htlc bob2alice.expectMsgType[TxAbort] bob2alice.forward(alice) + // alice returns a warning and schedules a disconnect after receiving UpdateAddHtlc + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) + // alice confirms the splice abort alice2bob.expectMsgType[TxAbort] - alice2bob.forward(bob) - // but the htlc goes through normally - crossSign(bob, alice, bob2alice, alice2bob) - fulfillHtlc(add.id, preimage, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) + // the htlc is not added } test("recv UpdateAddHtlc while a splice is in progress") { f => @@ -680,12 +679,9 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik // have to build a htlc manually because eclair would refuse to accept this command as it's forbidden val fakeHtlc = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) bob2alice.forward(alice, fakeHtlc) - alice2bob.expectMsgType[Error] - assertPublished(alice2blockchain, "commit-tx") - assertPublished(alice2blockchain, "local-main-delayed") - alice2blockchain.expectMsgType[WatchTxConfirmed] - alice2blockchain.expectMsgType[WatchTxConfirmed] - alice2blockchain.expectNoMessage(100 millis) + // alice returns a warning and schedules a disconnect after receiving UpdateAddHtlc + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) + // the htlc is not added } test("recv UpdateAddHtlc before splice confirms (zero-conf)", Tag(ZeroConf), Tag(AnchorOutputsZeroFeeHtlcTxs)) { f => From ec26dab8e380a2d6ca907a0f24ba3d88825bbfd5 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 20 Jul 2023 10:30:57 +0200 Subject: [PATCH 11/25] Change so non-initiator is quiescent once they send stfu - initiator still remains in the negotiating state until they receive stfu from the non-initiator - also, simplify tests for forbidden messages --- .../fr/acinq/eclair/channel/ChannelData.scala | 2 +- .../fr/acinq/eclair/channel/fsm/Channel.scala | 3 + .../states/e/NormalQuiescentStateSpec.scala | 164 ++++++------------ 3 files changed, 57 insertions(+), 112 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala index 7ae4ba4454..12acedbe24 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala @@ -468,7 +468,7 @@ object SpliceStatus { /** Our peer is asked us to stop sending new updates and wait for our updates to be added to the local and remote commitments. */ case class ReceivedStfu(stfu: Stfu) extends QuiescenceNegotiation.NonInitiator /** Our updates to be added to the local and remote commitments, we wait for our peer to use the now quiescent channel. */ - case object NonInitiatorQuiescent extends QuiescenceNegotiation.NonInitiator + case object NonInitiatorQuiescent extends QuiescentSpliceStatus /** We told our peer we want to splice funds in the channel. */ case class SpliceRequested(cmd: CMD_SPLICE, init: SpliceInit) extends QuiescentSpliceStatus /** We both agreed to splice and are building the splice transaction. */ 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 ffc6602117..5a3bb07c7b 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 @@ -996,6 +996,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with log.info("our peer rejected our splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) cmd.replyTo ! RES_FAILURE(cmd, new RuntimeException(s"splice attempt rejected by our peer: ${msg.toAscii}")) stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling endQuiescence(d) + case SpliceStatus.NonInitiatorQuiescent => + log.info("our peer aborted their own splice attempt: ascii='{}' bin={}", msg.toAscii, msg.data) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending TxAbort(d.channelId, SpliceAttemptAborted(d.channelId).getMessage) calling endQuiescence(d) case SpliceStatus.SpliceAborted => log.debug("our peer acked our previous tx_abort") stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) calling endQuiescence(d) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 4c411b88bc..0190ea0069 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -208,7 +208,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv forbidden commands while quiescent") { f => import f._ - // alice should reject commands that change the commitment while quiescent + // both should reject commands that change the commitment while quiescent val sender1, sender2, sender3 = TestProbe() val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), @@ -218,6 +218,10 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] + safeSend(bob, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] } test("recv fulfill htlc command while initiator awaiting stfu from remote") { f => @@ -244,34 +248,34 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = false, resetConnection = true) } - test("recv fulfill htlc command after initiator receives stfu from remote") { f => + test("recv fulfill htlc command while quiescent") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true) } - test("recv fail htlc command after initiator receives stfu from remote") { f => + test("recv fail htlc command while quiescent") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true) } - test("recv fail malformed htlc command after initiator receives stfu from remote") { f => + test("recv fail malformed htlc command while quiescent") { f => receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true) } - test("recv fulfill htlc command when initiator receives stfu from remote and channel disconnects") { f => + test("recv fulfill htlc command while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true, resetConnection = true) } - test("recv fail htlc command after initiator receives stfu from remote and channel disconnects") { f => + test("recv fail htlc command while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true, resetConnection = true) } - test("recv fail malformed htlc command after initiator receives stfu from remote and channel disconnects") { f => + test("recv fail malformed htlc command while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true, resetConnection = true) } test("recv settlement commands while initiator awaiting stfu from remote") { f => import f._ - // alice should reject commands that change the commitment until the splice is complete + // initiator should reject commands that change the commitment until the splice is complete val sender1, sender2, sender3 = TestProbe() val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), @@ -283,152 +287,90 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] } - test("recv settlement commands after initiator receives stfu from remote") { f => - import f._ - - // alice should reject commands that change the commitment until the splice is complete - val sender1, sender2, sender3 = TestProbe() - val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), - CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), - CMD_CLOSE(sender3.ref, None, None)) - initiateQuiescence(f) - safeSend(alice, cmds) - sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] - sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] - sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] - } - test("recv second stfu while non-initiator waiting for local commitment to be signed") { f => import f._ initiateQuiescence(f, sendInitialStfu = false) val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) alice2bob.forward(bob) - // send a second stfu to bob + // second stfu to bob is ignored bob ! Stfu(channelId(bob), initiator = true) - bob2alice.expectMsgType[Warning] - // we should disconnect after giving alice time to receive the warning - bobPeer.fishForMessage(3 seconds) { - case Peer.Disconnect(nodeId, _) if nodeId == bob.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true - case _ => false - } + bob2alice.expectNoMessage(100 millis) } test("recv Shutdown message before initiator receives stfu from remote") { f => import f._ initiateQuiescence(f, sendInitialStfu = false) val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] - alice ! Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + val forbiddenMsg = Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + bob2alice.forward(alice, forbiddenMsg) + // handle Shutdown normally alice2bob.expectMsgType[Shutdown] } - test("recv (forbidden) Shutdown message after initiator receives stfu from remote") { f => + test("recv (forbidden) Shutdown message while quiescent") { f => import f._ initiateQuiescence(f) val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] - alice ! Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) - sendErrorAndClose(alice, alice2bob, alice2blockchain) + val forbiddenMsg = Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) + // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) + bob2alice.forward(alice, forbiddenMsg) + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "Shutdown").getMessage)) + alice2bob.forward(bob, forbiddenMsg) + bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "Shutdown").getMessage)) } - test("recv (forbidden) UpdateFulfillHtlc messages after initiator receives stfu from remote") { f => + test("recv (forbidden) UpdateFulfillHtlc messages while quiescent") { f => import f._ val (preimage, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) initiateQuiescence(f) - alice ! UpdateFulfillHtlc(channelId(bob), add.id, preimage) + val forbiddenMsg = UpdateFulfillHtlc(channelId(bob), add.id, preimage) + // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) + bob2alice.forward(alice, forbiddenMsg) + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFulfillHtlc").getMessage)) + alice2bob.forward(bob, forbiddenMsg) + bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFulfillHtlc").getMessage)) + // alice will forward the valid UpdateFulfilHtlc msg to their relayer alice2relayer.expectMsg(RelayForward(add)) - sendErrorAndClose(alice, alice2bob, alice2blockchain) - } - - test("recv (forbidden) UpdateFailHtlc messages after initiator receives stfu from remote") { f => - import f._ - val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) - crossSign(bob, alice, bob2alice, alice2bob) - initiateQuiescence(f) - alice ! UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) - sendErrorAndClose(alice, alice2bob, alice2blockchain) } - test("recv (forbidden) UpdateFailMalformedHtlc messages after initiator receives stfu from remote") { f => + test("recv (forbidden) UpdateFailHtlc messages while quiescent") { f => import f._ val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) initiateQuiescence(f) - alice ! UpdateFailMalformedHtlc(channelId(bob), add.id, randomBytes32(), FailureMessageCodecs.BADONION) - sendErrorAndClose(alice, alice2bob, alice2blockchain) + val forbiddenMsg = UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) + // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) + bob2alice.forward(alice, forbiddenMsg) + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFailHtlc").getMessage)) + alice2bob.forward(bob, forbiddenMsg) + bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFailHtlc").getMessage)) } - test("recv (forbidden) UpdateFee messages after initiator receives stfu from remote") { f => + test("recv (forbidden) UpdateFee messages while quiescent") { f => import f._ val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) initiateQuiescence(f) - alice ! UpdateFee(channelId(bob), FeeratePerKw(1 sat)) - sendErrorAndClose(alice, alice2bob, alice2blockchain) + val forbiddenMsg = UpdateFee(channelId(bob), FeeratePerKw(1 sat)) + // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) + bob2alice.forward(alice, forbiddenMsg) + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFee").getMessage)) + alice2bob.forward(bob, forbiddenMsg) + bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFee").getMessage)) } - test("recv UpdateAddHtlc message after initiator receives stfu from remote") { f => + test("recv (forbidden) UpdateAddHtlc message while quiescent") { f => import f._ initiateQuiescence(f) // have to build a htlc manually because eclair would refuse to accept this command as it's forbidden - val fakeHtlc = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) - bob2alice.forward(alice, fakeHtlc) - sendErrorAndClose(alice, alice2bob, alice2blockchain) - } - - test("recv (forbidden) Shutdown message after non-initiator receives stfu from initiator") { f => - import f._ - initiateQuiescence(f) - val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] - bob ! Shutdown(ByteVector32.Zeroes, bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) - sendErrorAndClose(bob, bob2alice, bob2blockchain) - } - - test("recv (forbidden) UpdateFulfillHtlc messages after non-initiator receives stfu from initiator") { f => - import f._ - val (preimage, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) - bob ! UpdateFulfillHtlc(channelId(bob), add.id, preimage) - bob2relayer.expectMsg(RelayForward(add)) - sendErrorAndClose(bob, bob2alice, bob2blockchain) - } - - test("recv (forbidden) UpdateFailHtlc messages after non-initiator receives stfu from initiator") { f => - import f._ - val (_, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) - bob ! UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) - sendErrorAndClose(bob, bob2alice, bob2blockchain) - } - - test("recv (forbidden) UpdateFailMalformedHtlc messages after non-initiator receives stfu from initiator") { f => - import f._ - val (_, add) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) - bob ! UpdateFailMalformedHtlc(channelId(bob), add.id, randomBytes32(), FailureMessageCodecs.BADONION) - sendErrorAndClose(bob, bob2alice, bob2blockchain) - } - - test("recv (forbidden) UpdateFee messages after non-initiator receives stfu from initiator") { f => - import f._ - val (_, _) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) - bob ! UpdateFee(channelId(bob), FeeratePerKw(1 sat)) - sendErrorAndClose(bob, bob2alice, bob2blockchain) - } - - test("recv UpdateAddHtlc message after non-initiator receives stfu from initiator") { f => - import f._ - val (_, _) = addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) - val fakeHtlc = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) - alice2bob.forward(bob, fakeHtlc) - sendErrorAndClose(bob, bob2alice, bob2blockchain) + val forbiddenMsg = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) + // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) + bob2alice.forward(alice, forbiddenMsg) + alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) + alice2bob.forward(bob, forbiddenMsg) + bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) } } From a42f6cb70b93522704dbc6e33fadd122f90ae90b Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 20 Jul 2023 10:41:12 +0200 Subject: [PATCH 12/25] Remove redundant fail malformed htlc tests - fail htlc tests are sufficient --- .../states/e/NormalQuiescentStateSpec.scala | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 0190ea0069..594ad62c9b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -232,10 +232,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false) } - test("recv fail malformed htlc command while initiator awaiting stfu from remote") { f => - receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = false) - } - test("recv fulfill htlc command while initiator awaiting stfu from remote and channel disconnects") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = false, resetConnection = true) } @@ -244,10 +240,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false, resetConnection = true) } - test("recv fail malformed htlc command while initiator awaiting stfu from remote and channel disconnects") { f => - receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = false, resetConnection = true) - } - test("recv fulfill htlc command while quiescent") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true) } @@ -256,10 +248,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true) } - test("recv fail malformed htlc command while quiescent") { f => - receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true) - } - test("recv fulfill htlc command while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true, resetConnection = true) } @@ -268,10 +256,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true, resetConnection = true) } - test("recv fail malformed htlc command while quiescent and channel disconnects") { f => - receiveSettlementCommand(f, FailMalformedHtlc, sendInitialStfu = true, resetConnection = true) - } - test("recv settlement commands while initiator awaiting stfu from remote") { f => import f._ From fce95c686c6b02f7812c12478f29da72f384a5da Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 20 Jul 2023 14:22:27 +0200 Subject: [PATCH 13/25] Refactored `isQuiescent` --- .../main/scala/fr/acinq/eclair/channel/Commitments.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 7262774c0d..d39129e2a4 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -351,7 +351,7 @@ case class Commitment(fundingTxIndex: Long, } } - private def hasNoPendingHtlcs: Boolean = localCommit.spec.htlcs.isEmpty && remoteCommit.spec.htlcs.isEmpty && nextRemoteCommit_opt.isEmpty + def hasNoPendingHtlcs: Boolean = localCommit.spec.htlcs.isEmpty && remoteCommit.spec.htlcs.isEmpty && nextRemoteCommit_opt.isEmpty def hasNoPendingHtlcsOrFeeUpdate(changes: CommitmentChanges): Boolean = hasNoPendingHtlcs && (changes.localChanges.signed ++ changes.localChanges.acked ++ changes.remoteChanges.signed ++ changes.remoteChanges.acked).collectFirst { case _: UpdateFee => true }.isEmpty @@ -360,8 +360,6 @@ case class Commitment(fundingTxIndex: Long, changes.localChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) || changes.remoteChanges.all.exists(_.isInstanceOf[UpdateAddHtlc]) - def isQuiescent(changes: CommitmentChanges, pendingHtlcsOk: Boolean): Boolean = (pendingHtlcsOk || hasNoPendingHtlcs) && changes.localChanges.all.isEmpty && changes.remoteChanges.all.isEmpty - def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = { def expired(add: UpdateAddHtlc): Boolean = currentHeight >= add.cltvExpiry.blockHeight @@ -797,8 +795,9 @@ case class Commitments(params: ChannelParams, // @formatter:off def localIsQuiescent: Boolean = changes.localChanges.all.isEmpty + def remoteIsQuiescent: Boolean = changes.remoteChanges.all.isEmpty // HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them. - def isQuiescent: Boolean = active.head.isQuiescent(changes, pendingHtlcsOk = params.useQuiescence) + def isQuiescent: Boolean = (params.useQuiescence || active.head.hasNoPendingHtlcs) && localIsQuiescent && remoteIsQuiescent def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes) def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes) def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight) From 1fc1c9cf13620acf96205f5378a1c9d2b6fd1379 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 20 Jul 2023 14:25:02 +0200 Subject: [PATCH 14/25] Add tests for remote with pending changes sending stfu --- .../fr/acinq/eclair/channel/fsm/Channel.scala | 57 +++++++++++-------- .../states/e/NormalQuiescentStateSpec.scala | 46 +++++++++++++++ 2 files changed, 78 insertions(+), 25 deletions(-) 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 5a3bb07c7b..aaa8c679ea 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 @@ -855,33 +855,40 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with case Event(msg: Stfu, d: DATA_NORMAL) => if (d.commitments.params.useQuiescence) { - d.spliceStatus match { - case SpliceStatus.NoSplice => - startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) - if (d.commitments.localIsQuiescent) { - stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false) - } else { + if (d.commitments.remoteIsQuiescent) { + d.spliceStatus match { + case SpliceStatus.NoSplice => + startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout) + if (d.commitments.localIsQuiescent) { + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false) + } else { + stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) + } + case SpliceStatus.QuiescenceRequested(_) => stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) - } - case SpliceStatus.QuiescenceRequested(_) => - stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) - case SpliceStatus.InitiatorQuiescent(cmd) => - // if both sides send stfu at the same time, the quiescence initiator is the channel initiator - if (!msg.initiator || d.commitments.params.localParams.isInitiator) { - initiateSplice(cmd, d) match { - case Left(f) => - cmd.replyTo ! RES_FAILURE(cmd, f) - context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) - stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, f.getMessage) - case Right(spliceInit) => - stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + case SpliceStatus.InitiatorQuiescent(cmd) => + // if both sides send stfu at the same time, the quiescence initiator is the channel initiator + if (!msg.initiator || d.commitments.params.localParams.isInitiator) { + initiateSplice(cmd, d) match { + case Left(f) => + cmd.replyTo ! RES_FAILURE(cmd, f) + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, f.getMessage) + case Right(spliceInit) => + stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit + } + } else { + stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) } - } else { - stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) - } - case _ => - log.warning("ignoring duplicate stfu") - stay() + case _ => + log.warning("ignoring duplicate stfu") + stay() + } + } else { + log.warning("our peer sent stfu but is not quiescent") + // NB: we use a small delay to ensure we've sent our warning before disconnecting. + context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId)) + stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage) } } else { log.warning("ignoring stfu because both peers do not advertise quiescence") diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 594ad62c9b..5a69c78fa2 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -357,4 +357,50 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) } + test("recv stfu from splice initiator that is not quiescent") { f => + import f._ + addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + + // alice has a pending add htlc + val sender = TestProbe() + alice ! CMD_SIGN(Some(sender.ref)) + sender.expectMsgType[RES_SUCCESS[CMD_SIGN]] + alice2bob.expectMsgType[CommitSig] + + val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + alice ! cmd + alice2bob.forward(bob, Stfu(channelId(bob), initiator = true)) + bob2alice.expectMsg(Warning(channelId(bob), InvalidSpliceNotQuiescent(channelId(bob)).getMessage)) + // we should disconnect after giving bob time to receive the warning + bobPeer.fishForMessage(3 seconds) { + case Peer.Disconnect(nodeId, _) if nodeId == bob.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case _ => false + } + } + + test("recv stfu from splice non-initiator that is not quiescent") { f => + import f._ + addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + + // bob has a pending add htlc + val sender = TestProbe() + bob ! CMD_SIGN(Some(sender.ref)) + sender.expectMsgType[RES_SUCCESS[CMD_SIGN]] + bob2alice.expectMsgType[CommitSig] + + val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + alice ! cmd + alice2bob.expectMsgType[Stfu] + alice2bob.forward(bob) + bob2alice.forward(alice, Stfu(channelId(alice), initiator = false)) + alice2bob.expectMsg(Warning(channelId(alice), InvalidSpliceNotQuiescent(channelId(alice)).getMessage)) + // we should disconnect after giving bob time to receive the warning + alicePeer.fishForMessage(3 seconds) { + case Peer.Disconnect(nodeId, _) if nodeId == alice.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case _ => false + } + } + } From 7b40fcea8e52e389674597b7c5b3b9c4d1ce929a Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 20 Jul 2023 14:58:41 +0200 Subject: [PATCH 15/25] Add test for exchanging stfu concurrently --- .../states/e/NormalQuiescentStateSpec.scala | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 5a69c78fa2..9ed90a8826 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -403,4 +403,21 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL } } + test("initiate quiescence concurrently") { f => + import f._ + + val (sender1, sender2) = (TestProbe(), TestProbe()) + val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) + val cmd1 = CMD_SPLICE(sender1.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + val cmd2 = CMD_SPLICE(sender2.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + alice ! cmd1 + alice2bob.expectMsgType[Stfu] + bob ! cmd2 + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + alice2bob.forward(bob) + alice2bob.expectMsgType[SpliceInit] + eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent)) + } + } From d250f39e88fe2b3f62d3706f0969560b58525c7c Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Fri, 21 Jul 2023 15:18:09 +0200 Subject: [PATCH 16/25] Add tests suggested by t-bast --- .../states/e/NormalQuiescentStateSpec.scala | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 9ed90a8826..8b5ef62c51 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -18,15 +18,19 @@ package fr.acinq.eclair.channel.states.e import akka.actor.typed.scaladsl.adapter.actorRefAdapter import akka.testkit.{TestFSMRef, TestProbe} -import fr.acinq.bitcoin.scalacompat.{ByteVector32, SatoshiLong, Script, Transaction} +import fr.acinq.bitcoin.scalacompat.{SatoshiLong, Script, Transaction} +import fr.acinq.eclair.TestConstants.Bob +import fr.acinq.eclair.blockchain.CurrentBlockHeight import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel._ import fr.acinq.eclair.channel.fsm.Channel +import fr.acinq.eclair.channel.fund.InteractiveTxBuilder import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishReplaceableTx, PublishTx} import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.io.Peer import fr.acinq.eclair.payment.relay.Relayer.RelayForward +import fr.acinq.eclair.transactions.Transactions.HtlcSuccessTx import fr.acinq.eclair.wire.protocol._ import fr.acinq.eclair.{channel, _} import org.scalatest.Outcome @@ -420,4 +424,75 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent)) } + test("htlc timeout during quiescence negotiation with pending preimage") { f => + import f._ + val (preimage, add) = addHtlc(50_000_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f) + + // bob receives the fulfill for htlc, which is ignored because the channel is quiescent + val fulfillHtlc = CMD_FULFILL_HTLC(add.id, preimage) + safeSend(bob, Seq(fulfillHtlc)) + + // the HTLC timeout from alice/upstream is near, bob needs to close the channel to avoid an on-chain race condition + val listener = TestProbe() + bob.underlying.system.eventStream.subscribe(listener.ref, classOf[ChannelErrorOccurred]) + bob ! CurrentBlockHeight(add.cltvExpiry.blockHeight - Bob.nodeParams.channelConf.fulfillSafetyBeforeTimeout.toInt) + val ChannelErrorOccurred(_, _, _, LocalError(err), isFatal) = listener.expectMsgType[ChannelErrorOccurred] + assert(isFatal) + assert(err.isInstanceOf[HtlcsWillTimeoutUpstream]) + + val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + val initialCommitTx = initialState.commitments.latest.localCommit.commitTxAndRemoteSig.commitTx.tx + val HtlcSuccessTx(_, htlcSuccessTx, _, _, _) = initialState.commitments.latest.localCommit.htlcTxsAndRemoteSigs.head.htlcTx + + // TODO: why is HtlcSuccessTx not published initially? + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == initialCommitTx.txid) + bob2blockchain.expectMsgType[PublishTx] // main delayed + //assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txOut == htlcSuccessTx.txOut) + assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == initialCommitTx.txid) + bob2blockchain.expectMsgType[WatchTxConfirmed] + bob2blockchain.expectMsgType[WatchOutputSpent] + + // TODO: why is HtlcSuccessTx now published during closing? + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == initialCommitTx.txid) + bob2blockchain.expectMsgType[PublishTx] // main delayed + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txOut == htlcSuccessTx.txOut) + assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == initialCommitTx.txid) + + channelUpdateListener.expectMsgType[LocalChannelDown] + alice2blockchain.expectNoMessage(500 millis) + } + + test("receive quiescence timeout while splice in progress") { f => + import f._ + initiateQuiescence(f) + + // alice sends splice-init to bob, bob responds with splice-ack + alice2bob.forward(bob) + bob2alice.expectMsgType[SpliceAck] + eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress])) + + // bob sends a warning and disconnects if the splice takes too long to complete + bob ! Channel.QuiescenceTimeout(bobPeer.ref) + bob2alice.expectMsg(Warning(channelId(bob), SpliceAttemptTimedOut(channelId(bob)).getMessage)) + } + + test("receive quiescence timeout while splice is waiting for tx_abort from peer") { f => + import f._ + initiateQuiescence(f) + + // bob sends tx-abort to alice but does not receive a tx-abort back from alice + alice2bob.forward(bob) + bob2alice.expectMsgType[SpliceAck] + eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress])) + alice2bob.forward(bob, InteractiveTxBuilder.LocalFailure(new ChannelException(channelId(bob), "abort"))) + bob2alice.expectMsgType[TxAbort] + eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.SpliceAborted)) + + // bob sends a warning and disconnects if the splice takes too long to complete + bob ! Channel.QuiescenceTimeout(bobPeer.ref) + bob2alice.expectMsg(Warning(channelId(bob), SpliceAttemptTimedOut(channelId(bob)).getMessage)) + } + } From a31a96f3d756fdc8919fae6c363cf90757047b14 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 13:44:47 +0200 Subject: [PATCH 17/25] Return error and log when concurrent splice causes failure --- .../scala/fr/acinq/eclair/channel/ChannelExceptions.scala | 1 + .../main/scala/fr/acinq/eclair/channel/fsm/Channel.scala | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala index 90805842ac..da8d8bb645 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala @@ -139,4 +139,5 @@ case class PleasePublishYourCommitment (override val channelId: Byte case class CommandUnavailableInThisState (override val channelId: ByteVector32, command: String, state: ChannelState) extends ChannelException(channelId, s"cannot execute command=$command in state=$state") case class ForbiddenDuringSplice (override val channelId: ByteVector32, command: String) extends ChannelException(channelId, s"cannot process $command while splicing") case class ForbiddenDuringQuiescence (override val channelId: ByteVector32, command: String) extends ChannelException(channelId, s"cannot process $command while quiescent") +case class ConcurrentRemoteSplice (override val channelId: ByteVector32) extends ChannelException(channelId, "splice attempt canceled, remote initiated splice before us") // @formatter:on \ No newline at end of file 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 aaa8c679ea..ac4bae80db 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 @@ -864,7 +864,11 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with } else { stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) } - case SpliceStatus.QuiescenceRequested(_) => + case SpliceStatus.QuiescenceRequested(cmd) => + // We could keep track of our splice attempt and merge it with the remote splice instead of cancelling it. + // But this is an edge case that should rarely occur, so it's probably not worth the additional complexity. + log.warning("our peer initiated quiescence before us, cancelling our splice attempt") + cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId)) stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg)) case SpliceStatus.InitiatorQuiescent(cmd) => // if both sides send stfu at the same time, the quiescence initiator is the channel initiator @@ -878,6 +882,8 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit } } else { + log.warning("concurrent stfu received and our peer is the channel initiator, cancelling our splice attempt") + cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId)) stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) } case _ => From 316f770424af684dd9ae04ec17cebc6fb3efb645 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 14:01:13 +0200 Subject: [PATCH 18/25] Clean up comments --- .../main/scala/fr/acinq/eclair/channel/ChannelData.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala index 12acedbe24..1b997b9b0a 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala @@ -463,11 +463,11 @@ object SpliceStatus { case object NoSplice extends SpliceStatus /** We stop sending new updates and wait for our updates to be added to the local and remote commitments. */ case class QuiescenceRequested(splice: CMD_SPLICE) extends QuiescenceNegotiation.Initiator - /** Our updates to be added to the local and remote commitments, we wait for our peer to do the same. */ + /** Our updates have been added to the local and remote commitments, we wait for our peer to do the same. */ case class InitiatorQuiescent(splice: CMD_SPLICE) extends QuiescenceNegotiation.Initiator - /** Our peer is asked us to stop sending new updates and wait for our updates to be added to the local and remote commitments. */ + /** Our peer has asked us to stop sending new updates and wait for our updates to be added to the local and remote commitments. */ case class ReceivedStfu(stfu: Stfu) extends QuiescenceNegotiation.NonInitiator - /** Our updates to be added to the local and remote commitments, we wait for our peer to use the now quiescent channel. */ + /** Our updates have been added to the local and remote commitments, we wait for our peer to use the now quiescent channel. */ case object NonInitiatorQuiescent extends QuiescentSpliceStatus /** We told our peer we want to splice funds in the channel. */ case class SpliceRequested(cmd: CMD_SPLICE, init: SpliceInit) extends QuiescentSpliceStatus From be10434dc66b19a5c8c4f28a3c7f99694bbcca51 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 15:51:20 +0200 Subject: [PATCH 19/25] Group tests and remove unused functions This commit contains almost no functional changes, we just group together and reorder tests, remove unused code and remove the default value for `sendInitialStfu` (which makes tests matrixes easier to read). --- .../states/e/NormalQuiescentStateSpec.scala | 177 ++++++++---------- 1 file changed, 79 insertions(+), 98 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 8b5ef62c51..bf4a096949 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -18,7 +18,7 @@ package fr.acinq.eclair.channel.states.e import akka.actor.typed.scaladsl.adapter.actorRefAdapter import akka.testkit.{TestFSMRef, TestProbe} -import fr.acinq.bitcoin.scalacompat.{SatoshiLong, Script, Transaction} +import fr.acinq.bitcoin.scalacompat.{SatoshiLong, Script} import fr.acinq.eclair.TestConstants.Bob import fr.acinq.eclair.blockchain.CurrentBlockHeight import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ @@ -26,7 +26,7 @@ import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel._ import fr.acinq.eclair.channel.fsm.Channel import fr.acinq.eclair.channel.fund.InteractiveTxBuilder -import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishReplaceableTx, PublishTx} +import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishTx} import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.io.Peer import fr.acinq.eclair.payment.relay.Relayer.RelayForward @@ -49,6 +49,8 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL val setup = init(tags = tags) import setup._ reachNormal(setup, tags) + alice2bob.ignoreMsg { case _: ChannelUpdate => true } + bob2alice.ignoreMsg { case _: ChannelUpdate => true } awaitCond(alice.stateName == NORMAL) awaitCond(bob.stateName == NORMAL) withFixture(test.toNoArgTest(setup)) @@ -90,7 +92,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL } } - private def initiateQuiescence(f: FixtureParam, sendInitialStfu: Boolean = true): TestProbe = { + private def initiateQuiescence(f: FixtureParam, sendInitialStfu: Boolean): TestProbe = { import f._ val sender = TestProbe() @@ -111,29 +113,66 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL sender } - private def assertPublished(probe: TestProbe, desc: String): Transaction = { - val p = probe.expectMsgType[PublishTx] - assert(desc == p.desc) - p match { - case p: PublishFinalTx => p.tx - case p: PublishReplaceableTx => p.txInfo.tx - } + test("recv stfu when there are pending local changes") { f => + import f._ + initiateQuiescence(f, sendInitialStfu = false) + // we're holding the stfu from alice so that bob can add a pending local change + addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + // bob will not reply to alice's stfu until bob has no pending local commitment changes + alice2bob.forward(bob) + bob2alice.expectNoMessage(100 millis) + crossSign(bob, alice, bob2alice, alice2bob) + eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent)) + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + // when both nodes are quiescent, alice will start the splice + alice2bob.expectMsgType[SpliceInit] + alice2bob.forward(bob) + bob2alice.expectMsgType[SpliceAck] + bob2alice.forward(alice) + } + + test("recv forbidden non-settlement commands while initiator awaiting stfu from remote") { f => + import f._ + // initiator should reject commands that change the commitment once it became quiescent + val sender1, sender2, sender3 = TestProbe() + val cmds = Seq( + CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), + CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), + CMD_CLOSE(sender3.ref, None, None) + ) + initiateQuiescence(f, sendInitialStfu = false) + safeSend(alice, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] } - private def sendErrorAndClose(s: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, s2blockchain: TestProbe): Unit = { - s2r.expectMsgType[Error] - assertPublished(s2blockchain, "commit-tx") - assertPublished(s2blockchain, "local-main-delayed") - s2blockchain.expectMsgType[WatchTxConfirmed] - s2blockchain.expectMsgType[WatchTxConfirmed] - s2blockchain.expectNoMessage(100 millis) - awaitCond(s.stateName == CLOSING) + test("recv forbidden non-settlement commands while quiescent") { f => + import f._ + // both should reject commands that change the commitment while quiescent + val sender1, sender2, sender3 = TestProbe() + val cmds = Seq( + CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), + CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), + CMD_CLOSE(sender3.ref, None, None) + ) + initiateQuiescence(f, sendInitialStfu = true) + safeSend(alice, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] + safeSend(bob, cmds) + sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] + sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] + sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] } + // @formatter:off sealed trait SettlementCommandEnum case object FulfillHtlc extends SettlementCommandEnum case object FailHtlc extends SettlementCommandEnum - case object FailMalformedHtlc extends SettlementCommandEnum + // @formatter:on private def receiveSettlementCommand(f: FixtureParam, c: SettlementCommandEnum, sendInitialStfu: Boolean, resetConnection: Boolean = false): Unit = { import f._ @@ -142,7 +181,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL val cmd = c match { case FulfillHtlc => CMD_FULFILL_HTLC(add.id, preimage) case FailHtlc => CMD_FAIL_HTLC(add.id, Left(randomBytes32())) - case FailMalformedHtlc => CMD_FAIL_MALFORMED_HTLC(add.id, randomBytes32(), FailureMessageCodecs.BADONION) } crossSign(bob, alice, bob2alice, alice2bob) val sender = initiateQuiescence(f, sendInitialStfu) @@ -162,13 +200,13 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.forward(bob) bob2alice.expectMsgType[TxAbort] } else { - // alice sends a warning and disconnects if an error occurs while waiting for stfu from bob + // alice sends a warning and disconnects if bob doesn't send stfu before the timeout alice ! Channel.QuiescenceTimeout(alicePeer.ref) alice2bob.expectMsgType[Warning] alice2bob.forward(bob) // we should disconnect after giving bob time to receive the warning alicePeer.fishForMessage(3 seconds) { - case Peer.Disconnect(nodeId, _) if nodeId == alice.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case Peer.Disconnect(nodeId, _) if nodeId == alice.underlyingActor.remoteNodeId => true case _ => false } disconnect(f) @@ -179,102 +217,45 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL assert(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice) reconnect(f) } - // alice sends pending settlement after quiescence ends - eventually { - c match { - case FulfillHtlc => alice2bob.expectMsgType[UpdateFulfillHtlc] - case FailHtlc => alice2bob.expectMsgType[UpdateFailHtlc] - case FailMalformedHtlc => alice2bob.expectMsgType[UpdateFailMalformedHtlc] - } + c match { + case FulfillHtlc => alice2bob.expectMsgType[UpdateFulfillHtlc] + case FailHtlc => alice2bob.expectMsgType[UpdateFailHtlc] } - alice2bob.forward(bob) } - test("recv stfu when there are pending local changes") { f => - import f._ - initiateQuiescence(f, sendInitialStfu = false) - // we're holding the stfu from alice so that bob can add a pending local change - addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) - // bob will not reply to alice's stfu until bob has no pending local commitment changes - alice2bob.forward(bob) - bob2alice.expectNoMessage(100 millis) - crossSign(bob, alice, bob2alice, alice2bob) - eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent)) - bob2alice.expectMsgType[Stfu] - bob2alice.forward(alice) - // when both nodes are quiescent, alice will start the splice - alice2bob.expectMsgType[SpliceInit] - alice2bob.forward(bob) - bob2alice.expectMsgType[SpliceAck] - bob2alice.forward(alice) - } - - test("recv forbidden commands while quiescent") { f => - import f._ - // both should reject commands that change the commitment while quiescent - val sender1, sender2, sender3 = TestProbe() - val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), - CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), - CMD_CLOSE(sender3.ref, None, None)) - initiateQuiescence(f) - safeSend(alice, cmds) - sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] - sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] - sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] - safeSend(bob, cmds) - sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] - sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] - sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] - } - - test("recv fulfill htlc command while initiator awaiting stfu from remote") { f => + test("recv CMD_FULFILL_HTLC while initiator awaiting stfu from remote") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = false) } - test("recv fail htlc command while initiator awaiting stfu from remote") { f => + test("recv CMD_FAIL_HTLC while initiator awaiting stfu from remote") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false) } - test("recv fulfill htlc command while initiator awaiting stfu from remote and channel disconnects") { f => + test("recv CMD_FULFILL_HTLC while initiator awaiting stfu from remote and channel disconnects") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = false, resetConnection = true) } - test("recv fail htlc command while initiator awaiting stfu from remote and channel disconnects") { f => + test("recv CMD_FAIL_HTLC while initiator awaiting stfu from remote and channel disconnects") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = false, resetConnection = true) } - test("recv fulfill htlc command while quiescent") { f => + test("recv CMD_FULFILL_HTLC while quiescent") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true) } - test("recv fail htlc command while quiescent") { f => + test("recv CMD_FAIL_HTLC while quiescent") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true) } - test("recv fulfill htlc command while quiescent and channel disconnects") { f => + test("recv CMD_FULFILL_HTLC while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FulfillHtlc, sendInitialStfu = true, resetConnection = true) } - test("recv fail htlc command while quiescent and channel disconnects") { f => + test("recv CMD_FAIL_HTLC while quiescent and channel disconnects") { f => receiveSettlementCommand(f, FailHtlc, sendInitialStfu = true, resetConnection = true) } - test("recv settlement commands while initiator awaiting stfu from remote") { f => - import f._ - - // initiator should reject commands that change the commitment until the splice is complete - val sender1, sender2, sender3 = TestProbe() - val cmds = Seq(CMD_ADD_HTLC(sender1.ref, 1_000_000 msat, randomBytes32(), CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, None, localOrigin(sender1.ref)), - CMD_UPDATE_FEE(FeeratePerKw(100 sat), replyTo_opt = Some(sender2.ref)), - CMD_CLOSE(sender3.ref, None, None)) - initiateQuiescence(f, sendInitialStfu = false) - safeSend(alice, cmds) - sender1.expectMsgType[RES_ADD_FAILED[ForbiddenDuringSplice]] - sender2.expectMsgType[RES_FAILURE[CMD_UPDATE_FEE, ForbiddenDuringSplice]] - sender3.expectMsgType[RES_FAILURE[CMD_CLOSE, ForbiddenDuringSplice]] - } - test("recv second stfu while non-initiator waiting for local commitment to be signed") { f => import f._ initiateQuiescence(f, sendInitialStfu = false) @@ -297,7 +278,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv (forbidden) Shutdown message while quiescent") { f => import f._ - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) val bobData = bob.stateData.asInstanceOf[DATA_NORMAL] val forbiddenMsg = Shutdown(channelId(bob), bob.underlyingActor.getOrGenerateFinalScriptPubKey(bobData)) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) @@ -311,7 +292,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL import f._ val (preimage, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) val forbiddenMsg = UpdateFulfillHtlc(channelId(bob), add.id, preimage) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) bob2alice.forward(alice, forbiddenMsg) @@ -326,7 +307,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL import f._ val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) val forbiddenMsg = UpdateFailHtlc(channelId(bob), add.id, randomBytes32()) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) bob2alice.forward(alice, forbiddenMsg) @@ -339,7 +320,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL import f._ val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) val forbiddenMsg = UpdateFee(channelId(bob), FeeratePerKw(1 sat)) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) bob2alice.forward(alice, forbiddenMsg) @@ -350,7 +331,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv (forbidden) UpdateAddHtlc message while quiescent") { f => import f._ - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) // have to build a htlc manually because eclair would refuse to accept this command as it's forbidden val forbiddenMsg = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) @@ -428,7 +409,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL import f._ val (preimage, add) = addHtlc(50_000_000 msat, alice, bob, alice2bob, bob2alice) crossSign(alice, bob, alice2bob, bob2alice) - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) // bob receives the fulfill for htlc, which is ignored because the channel is quiescent val fulfillHtlc = CMD_FULFILL_HTLC(add.id, preimage) @@ -466,7 +447,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("receive quiescence timeout while splice in progress") { f => import f._ - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) // alice sends splice-init to bob, bob responds with splice-ack alice2bob.forward(bob) @@ -480,7 +461,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("receive quiescence timeout while splice is waiting for tx_abort from peer") { f => import f._ - initiateQuiescence(f) + initiateQuiescence(f, sendInitialStfu = true) // bob sends tx-abort to alice but does not receive a tx-abort back from alice alice2bob.forward(bob) From 5704162feb57c4b8744ba8621a09d05667bb820c Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 16:46:35 +0200 Subject: [PATCH 20/25] Test quiescence requested state We were missing a test for the case where we request quiescence when we still have pending changes to apply. --- .../states/e/NormalQuiescentStateSpec.scala | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index bf4a096949..e944325095 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -97,7 +97,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL val sender = TestProbe() val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) - val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) alice ! cmd alice2bob.expectMsgType[Stfu] if (!sendInitialStfu) { @@ -113,6 +113,17 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL sender } + test("send stfu after pending local changes have been added") { f => + import f._ + // we have an unsigned htlc in our local changes + addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + alice ! CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(50_000 sat)), spliceOut_opt = None) + alice2bob.expectNoMessage(100 millis) + crossSign(alice, bob, alice2bob, bob2alice) + alice2bob.expectMsgType[Stfu] + assert(alice.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent) + } + test("recv stfu when there are pending local changes") { f => import f._ initiateQuiescence(f, sendInitialStfu = false) @@ -122,8 +133,8 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.forward(bob) bob2alice.expectNoMessage(100 millis) crossSign(bob, alice, bob2alice, alice2bob) - eventually(assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent)) bob2alice.expectMsgType[Stfu] + assert(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.isQuiescent) bob2alice.forward(alice) // when both nodes are quiescent, alice will start the splice alice2bob.expectMsgType[SpliceInit] From 38115be86a3eedefee2dd324acb080e5b7c6b040 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 16:57:01 +0200 Subject: [PATCH 21/25] Fix preimage forwarding test --- .../channel/states/e/NormalQuiescentStateSpec.scala | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index e944325095..788247fa6b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -299,10 +299,11 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "Shutdown").getMessage)) } - test("recv (forbidden) UpdateFulfillHtlc messages while quiescent") { f => + test("recv (forbidden) UpdateFulfillHtlc message while quiescent") { f => import f._ val (preimage, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) + alice2relayer.expectMsg(RelayForward(add)) initiateQuiescence(f, sendInitialStfu = true) val forbiddenMsg = UpdateFulfillHtlc(channelId(bob), add.id, preimage) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) @@ -310,11 +311,10 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFulfillHtlc").getMessage)) alice2bob.forward(bob, forbiddenMsg) bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFulfillHtlc").getMessage)) - // alice will forward the valid UpdateFulfilHtlc msg to their relayer - alice2relayer.expectMsg(RelayForward(add)) + assert(bob2relayer.expectMsgType[RES_ADD_SETTLED[_, HtlcResult.RemoteFulfill]].result.paymentPreimage == preimage) } - test("recv (forbidden) UpdateFailHtlc messages while quiescent") { f => + test("recv (forbidden) UpdateFailHtlc message while quiescent") { f => import f._ val (_, add) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) @@ -327,12 +327,12 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL bob2alice.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFailHtlc").getMessage)) } - test("recv (forbidden) UpdateFee messages while quiescent") { f => + test("recv (forbidden) UpdateFee message while quiescent") { f => import f._ val (_, _) = addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) crossSign(bob, alice, bob2alice, alice2bob) initiateQuiescence(f, sendInitialStfu = true) - val forbiddenMsg = UpdateFee(channelId(bob), FeeratePerKw(1 sat)) + val forbiddenMsg = UpdateFee(channelId(bob), FeeratePerKw(500 sat)) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) bob2alice.forward(alice, forbiddenMsg) alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateFee").getMessage)) @@ -343,7 +343,6 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv (forbidden) UpdateAddHtlc message while quiescent") { f => import f._ initiateQuiescence(f, sendInitialStfu = true) - // have to build a htlc manually because eclair would refuse to accept this command as it's forbidden val forbiddenMsg = UpdateAddHtlc(channelId = randomBytes32(), id = 5656, amountMsat = 50000000 msat, cltvExpiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), paymentHash = randomBytes32(), onionRoutingPacket = TestConstants.emptyOnionPacket, blinding_opt = None) // both parties will respond to a forbidden msg while quiescent with a warning (and disconnect) From a5747be38c8809804751c29d63e8da1183248b24 Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 17:17:14 +0200 Subject: [PATCH 22/25] More concurrent quiescent tests --- .../states/e/NormalQuiescentStateSpec.scala | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 788247fa6b..9a11dade02 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -355,21 +355,11 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv stfu from splice initiator that is not quiescent") { f => import f._ addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - - // alice has a pending add htlc - val sender = TestProbe() - alice ! CMD_SIGN(Some(sender.ref)) - sender.expectMsgType[RES_SUCCESS[CMD_SIGN]] - alice2bob.expectMsgType[CommitSig] - - val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) - val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) - alice ! cmd - alice2bob.forward(bob, Stfu(channelId(bob), initiator = true)) + alice2bob.forward(bob, Stfu(channelId(alice), initiator = true)) bob2alice.expectMsg(Warning(channelId(bob), InvalidSpliceNotQuiescent(channelId(bob)).getMessage)) - // we should disconnect after giving bob time to receive the warning + // we should disconnect after giving alice time to receive the warning bobPeer.fishForMessage(3 seconds) { - case Peer.Disconnect(nodeId, _) if nodeId == bob.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case Peer.Disconnect(nodeId, _) if nodeId == bob.underlyingActor.remoteNodeId => true case _ => false } } @@ -377,37 +367,24 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("recv stfu from splice non-initiator that is not quiescent") { f => import f._ addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) - - // bob has a pending add htlc - val sender = TestProbe() - bob ! CMD_SIGN(Some(sender.ref)) - sender.expectMsgType[RES_SUCCESS[CMD_SIGN]] - bob2alice.expectMsgType[CommitSig] - - val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) - val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) - alice ! cmd - alice2bob.expectMsgType[Stfu] + initiateQuiescence(f, sendInitialStfu = false) alice2bob.forward(bob) - bob2alice.forward(alice, Stfu(channelId(alice), initiator = false)) + bob2alice.forward(alice, Stfu(channelId(bob), initiator = false)) alice2bob.expectMsg(Warning(channelId(alice), InvalidSpliceNotQuiescent(channelId(alice)).getMessage)) // we should disconnect after giving bob time to receive the warning alicePeer.fishForMessage(3 seconds) { - case Peer.Disconnect(nodeId, _) if nodeId == alice.stateData.asInstanceOf[DATA_NORMAL].commitments.params.remoteParams.nodeId => true + case Peer.Disconnect(nodeId, _) if nodeId == alice.underlyingActor.remoteNodeId => true case _ => false } } - test("initiate quiescence concurrently") { f => + test("initiate quiescence concurrently (no pending changes)") { f => import f._ - val (sender1, sender2) = (TestProbe(), TestProbe()) - val scriptPubKey = Script.write(Script.pay2wpkh(randomKey().publicKey)) - val cmd1 = CMD_SPLICE(sender1.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) - val cmd2 = CMD_SPLICE(sender2.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = Some(SpliceOut(100_000 sat, scriptPubKey))) - alice ! cmd1 + val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + alice ! cmd alice2bob.expectMsgType[Stfu] - bob ! cmd2 + bob ! cmd bob2alice.expectMsgType[Stfu] bob2alice.forward(alice) alice2bob.forward(bob) @@ -415,6 +392,40 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent)) } + test("initiate quiescence concurrently (pending changes on initiator side)") { f => + import f._ + + addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) + val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + alice ! cmd + alice2bob.expectNoMessage(100 millis) // alice isn't quiescent yet + bob ! cmd + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + crossSign(alice, bob, alice2bob, bob2alice) + alice2bob.expectMsgType[Stfu] + alice2bob.forward(bob) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent) + bob2alice.expectMsgType[SpliceInit] + } + + test("initiate quiescence concurrently (pending changes on non-initiator side)") { f => + import f._ + + addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) + val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + alice ! cmd + alice2bob.expectMsgType[Stfu] + bob ! cmd + bob2alice.expectNoMessage(100 millis) // bob isn't quiescent yet + alice2bob.forward(bob) + crossSign(bob, alice, bob2alice, alice2bob) + bob2alice.expectMsgType[Stfu] + bob2alice.forward(alice) + assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent) + alice2bob.expectMsgType[SpliceInit] + } + test("htlc timeout during quiescence negotiation with pending preimage") { f => import f._ val (preimage, add) = addHtlc(50_000_000 msat, alice, bob, alice2bob, bob2alice) From 1f6af5d42a7ac177e3c35386e9b325fbcb262dda Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 26 Jul 2023 17:40:03 +0200 Subject: [PATCH 23/25] Improve force-close tests There are two scenarios to test: - one of our outgoing HTLC times out - one of our incoming HTLC is close to timing out and we have the preimage --- .../states/e/NormalQuiescentStateSpec.scala | 73 ++++++++++++------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 9a11dade02..07ebf62d6e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -30,7 +30,6 @@ import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishTx} import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.io.Peer import fr.acinq.eclair.payment.relay.Relayer.RelayForward -import fr.acinq.eclair.transactions.Transactions.HtlcSuccessTx import fr.acinq.eclair.wire.protocol._ import fr.acinq.eclair.{channel, _} import org.scalatest.Outcome @@ -426,44 +425,64 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.expectMsgType[SpliceInit] } - test("htlc timeout during quiescence negotiation with pending preimage") { f => + test("htlc timeout during quiescence negotiation") { f => + import f._ + val (_, add) = addHtlc(50_000_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + initiateQuiescence(f, sendInitialStfu = true) + + val aliceCommit = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localCommit + val commitTx = aliceCommit.commitTxAndRemoteSig.commitTx.tx + assert(aliceCommit.htlcTxsAndRemoteSigs.size == 1) + val htlcTimeoutTx = aliceCommit.htlcTxsAndRemoteSigs.head.htlcTx.tx + + // the HTLC times out, alice needs to close the channel + alice ! CurrentBlockHeight(add.cltvExpiry.blockHeight) + assert(alice2blockchain.expectMsgType[PublishFinalTx].tx.txid == commitTx.txid) + alice2blockchain.expectMsgType[PublishTx] // main delayed + assert(alice2blockchain.expectMsgType[PublishFinalTx].tx.txid == htlcTimeoutTx.txid) + assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == commitTx.txid) + alice2blockchain.expectMsgType[WatchTxConfirmed] // main delayed + alice2blockchain.expectMsgType[WatchOutputSpent] // htlc output + alice2blockchain.expectNoMessage(100 millis) + + channelUpdateListener.expectMsgType[LocalChannelDown] + } + + test("htlc timeout during quiescence negotiation (with pending preimage)") { f => import f._ val (preimage, add) = addHtlc(50_000_000 msat, alice, bob, alice2bob, bob2alice) crossSign(alice, bob, alice2bob, bob2alice) initiateQuiescence(f, sendInitialStfu = true) + val bobCommit = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localCommit + val commitTx = bobCommit.commitTxAndRemoteSig.commitTx.tx + assert(bobCommit.htlcTxsAndRemoteSigs.size == 1) + val htlcSuccessTx = bobCommit.htlcTxsAndRemoteSigs.head.htlcTx.tx + // bob receives the fulfill for htlc, which is ignored because the channel is quiescent val fulfillHtlc = CMD_FULFILL_HTLC(add.id, preimage) safeSend(bob, Seq(fulfillHtlc)) - // the HTLC timeout from alice/upstream is near, bob needs to close the channel to avoid an on-chain race condition - val listener = TestProbe() - bob.underlying.system.eventStream.subscribe(listener.ref, classOf[ChannelErrorOccurred]) + // the HTLC timeout from alice is near, bob needs to close the channel to avoid an on-chain race condition bob ! CurrentBlockHeight(add.cltvExpiry.blockHeight - Bob.nodeParams.channelConf.fulfillSafetyBeforeTimeout.toInt) - val ChannelErrorOccurred(_, _, _, LocalError(err), isFatal) = listener.expectMsgType[ChannelErrorOccurred] - assert(isFatal) - assert(err.isInstanceOf[HtlcsWillTimeoutUpstream]) - - val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] - val initialCommitTx = initialState.commitments.latest.localCommit.commitTxAndRemoteSig.commitTx.tx - val HtlcSuccessTx(_, htlcSuccessTx, _, _, _) = initialState.commitments.latest.localCommit.htlcTxsAndRemoteSigs.head.htlcTx - - // TODO: why is HtlcSuccessTx not published initially? - assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == initialCommitTx.txid) + // bob publishes a first set of force-close transactions + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == commitTx.txid) bob2blockchain.expectMsgType[PublishTx] // main delayed - //assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txOut == htlcSuccessTx.txOut) - assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == initialCommitTx.txid) + assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == commitTx.txid) bob2blockchain.expectMsgType[WatchTxConfirmed] - bob2blockchain.expectMsgType[WatchOutputSpent] + bob2blockchain.expectMsgType[WatchOutputSpent] // htlc output - // TODO: why is HtlcSuccessTx now published during closing? - assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == initialCommitTx.txid) + // when transitioning to the closing state, bob checks the pending commands DB and replays the HTLC fulfill + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == commitTx.txid) bob2blockchain.expectMsgType[PublishTx] // main delayed - assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txOut == htlcSuccessTx.txOut) - assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == initialCommitTx.txid) + assert(bob2blockchain.expectMsgType[PublishFinalTx].tx.txid == htlcSuccessTx.txid) + assert(bob2blockchain.expectMsgType[WatchTxConfirmed].txId == commitTx.txid) + bob2blockchain.expectMsgType[WatchTxConfirmed] // main delayed + bob2blockchain.expectMsgType[WatchOutputSpent] // htlc output + bob2blockchain.expectNoMessage(100 millis) channelUpdateListener.expectMsgType[LocalChannelDown] - alice2blockchain.expectNoMessage(500 millis) } test("receive quiescence timeout while splice in progress") { f => @@ -473,7 +492,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL // alice sends splice-init to bob, bob responds with splice-ack alice2bob.forward(bob) bob2alice.expectMsgType[SpliceAck] - eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress])) + assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress]) // bob sends a warning and disconnects if the splice takes too long to complete bob ! Channel.QuiescenceTimeout(bobPeer.ref) @@ -487,10 +506,10 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL // bob sends tx-abort to alice but does not receive a tx-abort back from alice alice2bob.forward(bob) bob2alice.expectMsgType[SpliceAck] - eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress])) - alice2bob.forward(bob, InteractiveTxBuilder.LocalFailure(new ChannelException(channelId(bob), "abort"))) + assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceInProgress]) + bob ! InteractiveTxBuilder.LocalFailure(new ChannelException(channelId(bob), "abort")) bob2alice.expectMsgType[TxAbort] - eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.SpliceAborted)) + assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.SpliceAborted) // bob sends a warning and disconnects if the splice takes too long to complete bob ! Channel.QuiescenceTimeout(bobPeer.ref) From f51710e427b850cc23344ac5a620b2871e1d1081 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 27 Jul 2023 09:53:00 +0200 Subject: [PATCH 24/25] Add check for cmd failure during concurrent splice tests --- .../channel/states/e/NormalQuiescentStateSpec.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala index 07ebf62d6e..476c7c46c5 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala @@ -380,7 +380,8 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL test("initiate quiescence concurrently (no pending changes)") { f => import f._ - val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + val sender = TestProbe() + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) alice ! cmd alice2bob.expectMsgType[Stfu] bob ! cmd @@ -389,13 +390,15 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.forward(bob) alice2bob.expectMsgType[SpliceInit] eventually(assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent)) + sender.expectMsgType[RES_FAILURE[CMD_SPLICE, ConcurrentRemoteSplice]] } test("initiate quiescence concurrently (pending changes on initiator side)") { f => import f._ addHtlc(10_000 msat, alice, bob, alice2bob, bob2alice) - val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + val sender = TestProbe() + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) alice ! cmd alice2bob.expectNoMessage(100 millis) // alice isn't quiescent yet bob ! cmd @@ -405,6 +408,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL alice2bob.expectMsgType[Stfu] alice2bob.forward(bob) assert(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent) + sender.expectMsgType[RES_FAILURE[CMD_SPLICE, ConcurrentRemoteSplice]] bob2alice.expectMsgType[SpliceInit] } @@ -412,7 +416,8 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL import f._ addHtlc(10_000 msat, bob, alice, bob2alice, alice2bob) - val cmd = CMD_SPLICE(TestProbe().ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) + val sender = TestProbe() + val cmd = CMD_SPLICE(sender.ref, spliceIn_opt = Some(SpliceIn(500_000 sat, pushAmount = 0 msat)), spliceOut_opt = None) alice ! cmd alice2bob.expectMsgType[Stfu] bob ! cmd @@ -422,6 +427,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL bob2alice.expectMsgType[Stfu] bob2alice.forward(alice) assert(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NonInitiatorQuiescent) + sender.expectMsgType[RES_FAILURE[CMD_SPLICE, ConcurrentRemoteSplice]] alice2bob.expectMsgType[SpliceInit] } From 7082c4ae7c71d02c10a5451881243bacfd36064a Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Thu, 27 Jul 2023 10:14:49 +0200 Subject: [PATCH 25/25] Add check that htlc not added during in-progress splice --- .../acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala index 6fb66711d7..98c27c5d00 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala @@ -663,6 +663,7 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik // alice confirms the splice abort alice2bob.expectMsgType[TxAbort] // the htlc is not added + assert(!alice.stateData.asInstanceOf[DATA_NORMAL].commitments.hasPendingOrProposedHtlcs) } test("recv UpdateAddHtlc while a splice is in progress") { f => @@ -682,6 +683,7 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik // alice returns a warning and schedules a disconnect after receiving UpdateAddHtlc alice2bob.expectMsg(Warning(channelId(alice), ForbiddenDuringSplice(channelId(alice), "UpdateAddHtlc").getMessage)) // the htlc is not added + assert(!alice.stateData.asInstanceOf[DATA_NORMAL].commitments.hasPendingOrProposedHtlcs) } test("recv UpdateAddHtlc before splice confirms (zero-conf)", Tag(ZeroConf), Tag(AnchorOutputsZeroFeeHtlcTxs)) { f =>