Skip to content

Commit 1b749e1

Browse files
t-bastt-bast
authored
Remove support for splicing without quiescence (#2922)
We initially supported splicing with a poor man's quiescence, where we allowed splice messages if the commitments were already quiescent. We've shipped support for quiescence since then, which means that new even nodes relying on experimental splicing should support quiescence. We can thus remove support for the non-quiescent version.
1 parent 2a3d7d7 commit 1b749e1

File tree

6 files changed

+123
-276
lines changed

6 files changed

+123
-276
lines changed

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,6 @@ case class ChannelParams(channelId: ByteVector32,
133133
else Right(remoteScriptPubKey)
134134
}
135135

136-
/** If both peers support quiescence, we have to exchange stfu when splicing. */
137-
def useQuiescence: Boolean = Features.canUseFeature(localParams.initFeatures, remoteParams.initFeatures, Features.Quiescence)
138-
139136
}
140137

141138
object ChannelParams {
@@ -822,7 +819,7 @@ case class Commitments(params: ChannelParams,
822819
def localIsQuiescent: Boolean = changes.localChanges.all.isEmpty
823820
def remoteIsQuiescent: Boolean = changes.remoteChanges.all.isEmpty
824821
// HTLCs and pending changes are the same for all active commitments, so we don't need to loop through all of them.
825-
def isQuiescent: Boolean = (params.useQuiescence || active.head.hasNoPendingHtlcs) && localIsQuiescent && remoteIsQuiescent
822+
def isQuiescent: Boolean = localIsQuiescent && remoteIsQuiescent
826823
def hasNoPendingHtlcsOrFeeUpdate: Boolean = active.head.hasNoPendingHtlcsOrFeeUpdate(changes)
827824
def hasPendingOrProposedHtlcs: Boolean = active.head.hasPendingOrProposedHtlcs(changes)
828825
def timedOutOutgoingHtlcs(currentHeight: BlockHeight): Set[UpdateAddHtlc] = active.head.timedOutOutgoingHtlcs(currentHeight)

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala

Lines changed: 41 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -854,21 +854,13 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
854854
case Event(cmd: CMD_SPLICE, d: DATA_NORMAL) =>
855855
if (d.commitments.params.remoteParams.initFeatures.hasFeature(Features.SplicePrototype)) {
856856
d.spliceStatus match {
857-
case SpliceStatus.NoSplice if d.commitments.params.useQuiescence =>
857+
case SpliceStatus.NoSplice =>
858858
startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout)
859859
if (d.commitments.localIsQuiescent) {
860860
stay() using d.copy(spliceStatus = SpliceStatus.InitiatorQuiescent(cmd)) sending Stfu(d.channelId, initiator = true)
861861
} else {
862862
stay() using d.copy(spliceStatus = SpliceStatus.QuiescenceRequested(cmd))
863863
}
864-
case SpliceStatus.NoSplice if !d.commitments.params.useQuiescence =>
865-
initiateSplice(cmd, d) match {
866-
case Left(f) =>
867-
cmd.replyTo ! RES_FAILURE(cmd, f)
868-
stay()
869-
case Right(spliceInit) =>
870-
stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit
871-
}
872864
case _ =>
873865
log.warning("cannot initiate splice, another one is already in progress")
874866
cmd.replyTo ! RES_FAILURE(cmd, InvalidSpliceAlreadyInProgress(d.channelId))
@@ -886,62 +878,53 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
886878
stay()
887879

888880
case Event(msg: Stfu, d: DATA_NORMAL) =>
889-
if (d.commitments.params.useQuiescence) {
890-
if (d.commitments.remoteIsQuiescent) {
891-
d.spliceStatus match {
892-
case SpliceStatus.NoSplice =>
893-
startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout)
894-
if (d.commitments.localIsQuiescent) {
895-
stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false)
896-
} else {
897-
stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg))
898-
}
899-
case SpliceStatus.QuiescenceRequested(cmd) =>
900-
// We could keep track of our splice attempt and merge it with the remote splice instead of cancelling it.
901-
// But this is an edge case that should rarely occur, so it's probably not worth the additional complexity.
902-
log.warning("our peer initiated quiescence before us, cancelling our splice attempt")
903-
cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId))
881+
if (d.commitments.remoteIsQuiescent) {
882+
d.spliceStatus match {
883+
case SpliceStatus.NoSplice =>
884+
startSingleTimer(QuiescenceTimeout.toString, QuiescenceTimeout(peer), nodeParams.channelConf.quiescenceTimeout)
885+
if (d.commitments.localIsQuiescent) {
886+
stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent) sending Stfu(d.channelId, initiator = false)
887+
} else {
904888
stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg))
905-
case SpliceStatus.InitiatorQuiescent(cmd) =>
906-
// if both sides send stfu at the same time, the quiescence initiator is the channel opener
907-
if (!msg.initiator || d.commitments.params.localParams.isChannelOpener) {
908-
initiateSplice(cmd, d) match {
909-
case Left(f) =>
910-
cmd.replyTo ! RES_FAILURE(cmd, f)
911-
context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId))
912-
stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, f.getMessage)
913-
case Right(spliceInit) =>
914-
stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit
915-
}
916-
} else {
917-
log.warning("concurrent stfu received and our peer is the channel initiator, cancelling our splice attempt")
918-
cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId))
919-
stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent)
889+
}
890+
case SpliceStatus.QuiescenceRequested(cmd) =>
891+
// We could keep track of our splice attempt and merge it with the remote splice instead of cancelling it.
892+
// But this is an edge case that should rarely occur, so it's probably not worth the additional complexity.
893+
log.warning("our peer initiated quiescence before us, cancelling our splice attempt")
894+
cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId))
895+
stay() using d.copy(spliceStatus = SpliceStatus.ReceivedStfu(msg))
896+
case SpliceStatus.InitiatorQuiescent(cmd) =>
897+
// if both sides send stfu at the same time, the quiescence initiator is the channel opener
898+
if (!msg.initiator || d.commitments.params.localParams.isChannelOpener) {
899+
initiateSplice(cmd, d) match {
900+
case Left(f) =>
901+
cmd.replyTo ! RES_FAILURE(cmd, f)
902+
context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId))
903+
stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, f.getMessage)
904+
case Right(spliceInit) =>
905+
stay() using d.copy(spliceStatus = SpliceStatus.SpliceRequested(cmd, spliceInit)) sending spliceInit
920906
}
921-
case _ =>
922-
log.warning("ignoring duplicate stfu")
923-
stay()
924-
}
925-
} else {
926-
log.warning("our peer sent stfu but is not quiescent")
927-
// NB: we use a small delay to ensure we've sent our warning before disconnecting.
928-
context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId))
929-
stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage)
907+
} else {
908+
log.warning("concurrent stfu received and our peer is the channel initiator, cancelling our splice attempt")
909+
cmd.replyTo ! RES_FAILURE(cmd, ConcurrentRemoteSplice(d.channelId))
910+
stay() using d.copy(spliceStatus = SpliceStatus.NonInitiatorQuiescent)
911+
}
912+
case _ =>
913+
log.warning("ignoring duplicate stfu")
914+
stay()
930915
}
931916
} else {
932-
log.warning("ignoring stfu because both peers do not advertise quiescence")
933-
stay()
917+
log.warning("our peer sent stfu but is not quiescent")
918+
// NB: we use a small delay to ensure we've sent our warning before disconnecting.
919+
context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId))
920+
stay() using d.copy(spliceStatus = SpliceStatus.NoSplice) sending Warning(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage)
934921
}
935922

936923
case Event(_: QuiescenceTimeout, d: DATA_NORMAL) => handleQuiescenceTimeout(d)
937924

938-
case Event(_: SpliceInit, d: DATA_NORMAL) if d.spliceStatus == SpliceStatus.NoSplice && d.commitments.params.useQuiescence =>
939-
log.info("rejecting splice attempt: quiescence not negotiated")
940-
stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage)
941-
942925
case Event(msg: SpliceInit, d: DATA_NORMAL) =>
943926
d.spliceStatus match {
944-
case SpliceStatus.NoSplice | SpliceStatus.NonInitiatorQuiescent =>
927+
case SpliceStatus.NonInitiatorQuiescent =>
945928
if (!d.commitments.isQuiescent) {
946929
log.info("rejecting splice request: channel not quiescent")
947930
stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage)
@@ -993,6 +976,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
993976
stay() using d.copy(spliceStatus = SpliceStatus.SpliceInProgress(cmd_opt = None, sessionId, txBuilder, remoteCommitSig = None)) sending spliceAck
994977
}
995978
}
979+
case SpliceStatus.NoSplice =>
980+
log.info("rejecting splice attempt: quiescence not negotiated")
981+
stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, InvalidSpliceNotQuiescent(d.channelId).getMessage)
996982
case SpliceStatus.SpliceAborted =>
997983
log.info("rejecting splice attempt: our previous tx_abort was not acked")
998984
stay() sending Warning(d.channelId, InvalidSpliceTxAbortNotAcked(d.channelId).getMessage)

‎eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ object TestConstants {
106106
Features.PaymentMetadata -> FeatureSupport.Optional,
107107
Features.RouteBlinding -> FeatureSupport.Optional,
108108
Features.StaticRemoteKey -> FeatureSupport.Mandatory,
109+
Features.Quiescence -> FeatureSupport.Optional,
110+
Features.SplicePrototype -> FeatureSupport.Optional,
109111
),
110112
unknown = Set(UnknownFeature(TestFeature.optional))
111113
),
@@ -282,6 +284,8 @@ object TestConstants {
282284
Features.RouteBlinding -> FeatureSupport.Optional,
283285
Features.StaticRemoteKey -> FeatureSupport.Mandatory,
284286
Features.AnchorOutputsZeroFeeHtlcTx -> FeatureSupport.Optional,
287+
Features.Quiescence -> FeatureSupport.Optional,
288+
Features.SplicePrototype -> FeatureSupport.Optional,
285289
),
286290
pluginParams = Nil,
287291
overrideInitFeatures = Map.empty,

‎eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ object ChannelStateTestsTags {
5353
val DualFunding = "dual_funding"
5454
/** If set, a liquidity ads will be used when opening a channel. */
5555
val LiquidityAds = "liquidity_ads"
56-
/** If set, peers will support splicing. */
57-
val Splicing = "splicing"
5856
/** If set, channels will use option_static_remotekey. */
5957
val StaticRemoteKey = "static_remotekey"
6058
/** If set, channels will use option_anchor_outputs. */
@@ -93,8 +91,6 @@ object ChannelStateTestsTags {
9391
val RejectRbfAttempts = "reject_rbf_attempts"
9492
/** If set, the non-initiator will require a 1-block delay between RBF attempts. */
9593
val DelayRbfAttempts = "delay_rbf_attempts"
96-
/** If set, peers will support the quiesce protocol. */
97-
val Quiescence = "quiescence"
9894
/** If set, channels will adapt their max HTLC amount to the available balance */
9995
val AdaptMaxHtlcAmount = "adapt-max-htlc-amount"
10096
}
@@ -165,7 +161,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
165161
.modify(_.channelConf.balanceThresholds).setToIf(tags.contains(ChannelStateTestsTags.AdaptMaxHtlcAmount))(Seq(Channel.BalanceThreshold(1_000 sat, 0 sat), Channel.BalanceThreshold(5_000 sat, 1_000 sat), Channel.BalanceThreshold(10_000 sat, 5_000 sat)))
166162
val wallet = wallet_opt match {
167163
case Some(wallet) => wallet
168-
case None => if (tags.contains(ChannelStateTestsTags.DualFunding) || tags.contains(ChannelStateTestsTags.Splicing)) new SingleKeyOnChainWallet() else new DummyOnChainWallet()
164+
case None => if (tags.contains(ChannelStateTestsTags.DualFunding)) new SingleKeyOnChainWallet() else new DummyOnChainWallet()
169165
}
170166
val alice: TestFSMRef[ChannelState, ChannelData, Channel] = {
171167
implicit val system: ActorSystem = systemA
@@ -192,8 +188,6 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
192188
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ZeroConf))(_.updated(Features.ZeroConf, FeatureSupport.Optional))
193189
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional))
194190
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional))
195-
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional))
196-
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.Quiescence, FeatureSupport.Optional))
197191
.initFeatures()
198192
val bobInitFeatures = Bob.nodeParams.features
199193
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DisableWumbo))(_.removed(Features.Wumbo))
@@ -206,8 +200,6 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
206200
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ZeroConf))(_.updated(Features.ZeroConf, FeatureSupport.Optional))
207201
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ScidAlias))(_.updated(Features.ScidAlias, FeatureSupport.Optional))
208202
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DualFunding))(_.updated(Features.DualFunding, FeatureSupport.Optional))
209-
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Splicing))(_.updated(Features.SplicePrototype, FeatureSupport.Optional))
210-
.modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.Quiescence))(_.updated(Features.Quiescence, FeatureSupport.Optional))
211203
.initFeatures()
212204

213205
val channelType = ChannelTypes.defaultFromFeatures(aliceInitFeatures, bobInitFeatures, announceChannel = channelFlags.announceChannel)

‎eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalQuiescentStateSpec.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class NormalQuiescentStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL
4545
implicit val log: akka.event.LoggingAdapter = akka.event.NoLogging
4646

4747
override def withFixture(test: OneArgTest): Outcome = {
48-
val tags = test.tags + ChannelStateTestsTags.DualFunding + ChannelStateTestsTags.Splicing + ChannelStateTestsTags.Quiescence
48+
val tags = test.tags + ChannelStateTestsTags.DualFunding
4949
val setup = init(tags = tags)
5050
import setup._
5151
reachNormal(setup, tags)

0 commit comments

Comments
 (0)