Skip to content

Commit

Permalink
Avoid unusable channels after a large splice (#2761)
Browse files Browse the repository at this point in the history 
Splicing (and dual funding as well) introduce a new scenario that could
not happen before, where the channel initiator (who pays the fees for the
commit tx) can end up below the channel reserve, or right above it.

In that case it means that most of the channels funds are on the non
initiator side, so we should allow HTLCs from the non-initiator to the
initiator to move funds towards the initiator. We allow slightly dipping
into the channel reserve in that case, for at most 5 pending HTLCs.
  • Loading branch information
@t-bast
t-bast authored Nov 3, 2023
1 parent 12adf87 commit 830335f
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 57 deletions.
13 changes: 13 additions & 0 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,19 @@ case class Commitment(fundingTxIndex: Long,
} else if (missingForReceiver < 0.msat) {
if (params.localParams.isInitiator) {
// receiver is not the channel initiator; it is ok if it can't maintain its channel_reserve for now, as long as its balance is increasing, which is the case if it is receiving a payment
} else if (reduced.toLocal > fees && reduced.htlcs.size < 5) {
// Receiver is the channel initiator; we usually don't want to let them dip into their channel reserve, because
// that may give them a commitment transaction where they have nothing at stake, which would create an incentive
// for them to force-close using that commitment after it has been revoked.
// But we let them dip slightly into their channel reserve to pay the fees, to ensure that the channel is not
// stuck and unusable, because we can end up in that state in the following scenario:
// - they were above their channel reserve
// - we spliced a lot of funds into the channel, which increased the reserve requirements
// - they are now below the new reserve, but if we don't allow htlcs to them, they have no way of increasing their balance
// Since we only allow a limited number of htlcs, that doesn't let them dip into their reserve much.
// We could also keep track of the previous channel reserve, but this is additional state that is awkward to
// store and not trivial to correctly keep up-to-date. This simpler solution has a similar result with less
// complexity.
} else {
return Left(RemoteCannotAffordFeesForNewHtlc(params.channelId, amount = amount, missing = -missingForReceiver.truncateToSatoshi, reserve = remoteChannelReserve(params), fees = fees))
}
Expand Down
150 changes: 93 additions & 57 deletions eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,98 +68,103 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik

private val defaultSpliceOutScriptPubKey = hex"0020aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

private def useQuiescence(f: FixtureParam): Boolean = f.alice.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.useQuiescence
private def useQuiescence(s: TestFSMRef[ChannelState, ChannelData, Channel]): Boolean = s.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.useQuiescence

private def initiateSpliceWithoutSigs(f: FixtureParam, spliceIn_opt: Option[SpliceIn] = None, spliceOut_opt: Option[SpliceOut] = None): TestProbe = {
import f._
private def useQuiescence(f: FixtureParam): Boolean = useQuiescence(f.alice)

private def initiateSpliceWithoutSigs(s: TestFSMRef[ChannelState, ChannelData, Channel], r: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, r2s: TestProbe, spliceIn_opt: Option[SpliceIn], spliceOut_opt: Option[SpliceOut]): TestProbe = {
val sender = TestProbe()
val cmd = CMD_SPLICE(sender.ref, spliceIn_opt, spliceOut_opt)
alice ! cmd
if (useQuiescence(f)) {
exchangeStfu(f)
s ! cmd
if (useQuiescence(s)) {
exchangeStfu(s, r, s2r, r2s)
}
alice2bob.expectMsgType[SpliceInit]
alice2bob.forward(bob)
bob2alice.expectMsgType[SpliceAck]
bob2alice.forward(alice)

alice2bob.expectMsgType[TxAddInput]
alice2bob.forward(bob)
bob2alice.expectMsgType[TxComplete]
bob2alice.forward(alice)
s2r.expectMsgType[SpliceInit]
s2r.forward(r)
r2s.expectMsgType[SpliceAck]
r2s.forward(s)

s2r.expectMsgType[TxAddInput]
s2r.forward(r)
r2s.expectMsgType[TxComplete]
r2s.forward(s)
if (spliceIn_opt.isDefined) {
alice2bob.expectMsgType[TxAddInput]
alice2bob.forward(bob)
bob2alice.expectMsgType[TxComplete]
bob2alice.forward(alice)
alice2bob.expectMsgType[TxAddOutput]
alice2bob.forward(bob)
bob2alice.expectMsgType[TxComplete]
bob2alice.forward(alice)
s2r.expectMsgType[TxAddInput]
s2r.forward(r)
r2s.expectMsgType[TxComplete]
r2s.forward(s)
s2r.expectMsgType[TxAddOutput]
s2r.forward(r)
r2s.expectMsgType[TxComplete]
r2s.forward(s)
}
if (spliceOut_opt.isDefined) {
alice2bob.expectMsgType[TxAddOutput]
alice2bob.forward(bob)
bob2alice.expectMsgType[TxComplete]
bob2alice.forward(alice)
s2r.expectMsgType[TxAddOutput]
s2r.forward(r)
r2s.expectMsgType[TxComplete]
r2s.forward(s)
}
alice2bob.expectMsgType[TxAddOutput]
alice2bob.forward(bob)
bob2alice.expectMsgType[TxComplete]
bob2alice.forward(alice)
alice2bob.expectMsgType[TxComplete]
alice2bob.forward(bob)
s2r.expectMsgType[TxAddOutput]
s2r.forward(r)
r2s.expectMsgType[TxComplete]
r2s.forward(s)
s2r.expectMsgType[TxComplete]
s2r.forward(r)
sender
}

private def exchangeSpliceSigs(f: FixtureParam, sender: TestProbe): Transaction = {
import f._
private def initiateSpliceWithoutSigs(f: FixtureParam, spliceIn_opt: Option[SpliceIn] = None, spliceOut_opt: Option[SpliceOut] = None): TestProbe = initiateSpliceWithoutSigs(f.alice, f.bob, f.alice2bob, f.bob2alice, spliceIn_opt, spliceOut_opt)

val commitSigBob = bob2alice.fishForMessage() {
private def exchangeSpliceSigs(s: TestFSMRef[ChannelState, ChannelData, Channel], r: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, r2s: TestProbe, sender: TestProbe): Transaction = {
val commitSigR = r2s.fishForMessage() {
case _: CommitSig => true
case _: ChannelReady => false
}
bob2alice.forward(alice, commitSigBob)
val commitSigAlice = alice2bob.fishForMessage() {
r2s.forward(s, commitSigR)
val commitSigS = s2r.fishForMessage() {
case _: CommitSig => true
case _: ChannelReady => false
}
alice2bob.forward(bob, commitSigAlice)
s2r.forward(r, commitSigS)

val txSigsBob = bob2alice.fishForMessage() {
val txSigsR = r2s.fishForMessage() {
case _: TxSignatures => true
case _: ChannelUpdate => false
}
bob2alice.forward(alice, txSigsBob)
val txSigsAlice = alice2bob.fishForMessage() {
r2s.forward(s, txSigsR)
val txSigsS = s2r.fishForMessage() {
case _: TxSignatures => true
case _: ChannelUpdate => false
}
alice2bob.forward(bob, txSigsAlice)
s2r.forward(r, txSigsS)

sender.expectMsgType[RES_SPLICE]

awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.asInstanceOf[DualFundedUnconfirmedFundingTx].sharedTx.isInstanceOf[FullySignedSharedTransaction])
awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.asInstanceOf[DualFundedUnconfirmedFundingTx].sharedTx.isInstanceOf[FullySignedSharedTransaction])
alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get
awaitCond(s.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
awaitCond(r.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
awaitCond(s.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.asInstanceOf[DualFundedUnconfirmedFundingTx].sharedTx.isInstanceOf[FullySignedSharedTransaction])
awaitCond(r.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.asInstanceOf[DualFundedUnconfirmedFundingTx].sharedTx.isInstanceOf[FullySignedSharedTransaction])
s.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get
}

private def initiateSplice(f: FixtureParam, spliceIn_opt: Option[SpliceIn] = None, spliceOut_opt: Option[SpliceOut] = None): Transaction = {
val sender = initiateSpliceWithoutSigs(f, spliceIn_opt, spliceOut_opt)
exchangeSpliceSigs(f, sender)
private def exchangeSpliceSigs(f: FixtureParam, sender: TestProbe): Transaction = exchangeSpliceSigs(f.alice, f.bob, f.alice2bob, f.bob2alice, sender)

private def initiateSplice(s: TestFSMRef[ChannelState, ChannelData, Channel], r: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, r2s: TestProbe, spliceIn_opt: Option[SpliceIn], spliceOut_opt: Option[SpliceOut]): Transaction = {
val sender = initiateSpliceWithoutSigs(s, r, s2r, r2s, spliceIn_opt, spliceOut_opt)
exchangeSpliceSigs(s, r, s2r, r2s, sender)
}

private def exchangeStfu(f: FixtureParam): Unit = {
import f._
alice2bob.expectMsgType[Stfu]
alice2bob.forward(bob)
bob2alice.expectMsgType[Stfu]
bob2alice.forward(alice)
private def initiateSplice(f: FixtureParam, spliceIn_opt: Option[SpliceIn] = None, spliceOut_opt: Option[SpliceOut] = None): Transaction = initiateSplice(f.alice, f.bob, f.alice2bob, f.bob2alice, spliceIn_opt, spliceOut_opt)

private def exchangeStfu(s: TestFSMRef[ChannelState, ChannelData, Channel], r: TestFSMRef[ChannelState, ChannelData, Channel], s2r: TestProbe, r2s: TestProbe): Unit = {
s2r.expectMsgType[Stfu]
s2r.forward(r)
r2s.expectMsgType[Stfu]
r2s.forward(s)
}

private def exchangeStfu(f: FixtureParam): Unit = exchangeStfu(f.alice, f.bob, f.alice2bob, f.bob2alice)

case class TestHtlcs(aliceToBob: Seq[(ByteVector32, UpdateAddHtlc)], bobToAlice: Seq[(ByteVector32, UpdateAddHtlc)])

private def setupHtlcs(f: FixtureParam): TestHtlcs = {
Expand Down Expand Up @@ -416,6 +421,37 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik
awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
}

test("recv CMD_SPLICE (remote splices in which takes us below reserve)", Tag(ChannelStateTestsTags.NoMaxHtlcValueInFlight)) { f =>
import f._

assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localCommit.spec.toLocal == 800_000_000.msat)
val (r1, htlc1) = addHtlc(750_000_000 msat, alice, bob, alice2bob, bob2alice)
crossSign(alice, bob, alice2bob, bob2alice)
fulfillHtlc(htlc1.id, r1, bob, alice, bob2alice, alice2bob)
crossSign(bob, alice, bob2alice, alice2bob)

// Bob makes a large splice: Alice doesn't meet the new reserve requirements, but she met the previous one, so we allow this.
initiateSplice(bob, alice, bob2alice, alice2bob, spliceIn_opt = Some(SpliceIn(4_000_000 sat)), spliceOut_opt = None)
val postSpliceState = alice.stateData.asInstanceOf[DATA_NORMAL]
assert(postSpliceState.commitments.latest.localCommit.spec.toLocal < postSpliceState.commitments.latest.localChannelReserve)

// Since Alice is below the reserve and most of the funds are on Bob's side, Alice cannot send HTLCs.
val probe = TestProbe()
val (_, cmd) = makeCmdAdd(5_000_000 msat, bob.nodeParams.nodeId, bob.nodeParams.currentBlockHeight)
alice ! cmd.copy(replyTo = probe.ref)
probe.expectMsgType[RES_ADD_FAILED[InsufficientFunds]]

// But Bob can send HTLCs to take Alice above the reserve.
val (r2, htlc2) = addHtlc(50_000_000 msat, bob, alice, bob2alice, alice2bob)
crossSign(bob, alice, bob2alice, alice2bob)
fulfillHtlc(htlc2.id, r2, alice, bob, alice2bob, bob2alice)
crossSign(alice, bob, alice2bob, bob2alice)

// Alice can now send HTLCs as well.
addHtlc(10_000_000 msat, alice, bob, alice2bob, bob2alice)
crossSign(alice, bob, alice2bob, bob2alice)
}

def testSpliceInAndOutCmd(f: FixtureParam): Unit = {
val htlcs = setupHtlcs(f)

Expand Down

0 comments on commit 830335f

Please sign in to comment.