From 9c4aad0688ba25bc108aeeb5c8b212ff7628caea Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Fri, 8 Dec 2023 10:38:54 +0100 Subject: [PATCH] Dip into remote initiator reserve only for splices (#2797) #2761 introduced the ability for the HTLC sender to let a remote initiator dip into its reserve to unblock channels after a large splice. However, we relaxed that condition for all channels, even those that don't use splice. This creates compatibility issues with other implementations that are stricter than what the specification requires, and will force-close in those situations. --- .../fr/acinq/eclair/channel/Commitments.scala | 2 +- .../channel/states/e/NormalStateSpec.scala | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) 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 6df8ed5fda..5dedab5146 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 @@ -458,7 +458,7 @@ 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) { + } else if (reduced.toLocal > fees && reduced.htlcs.size < 5 && fundingTxIndex > 0) { // 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. diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index d49bda0d51..0f46aa934a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -273,6 +273,35 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.changes.remoteChanges.proposed.size == proposedChanges + 1) } + test("recv CMD_ADD_HTLC (HTLC dips into remote funder channel reserve)", Tag(ChannelStateTestsTags.NoMaxHtlcValueInFlight)) { f => + import f._ + val sender = TestProbe() + addHtlc(758_640_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend == 0.msat) + // We increase the feerate to get Alice's balance closer to her channel reserve. + bob.underlyingActor.nodeParams.setFeerates(FeeratesPerKw.single(FeeratePerKw(17_500 sat))) + updateFee(FeeratePerKw(17_500 sat), alice, bob, alice2bob, bob2alice) + + // At this point alice has the minimal amount to sustain a channel. + // Alice maintains an extra reserve to accommodate for a one more HTLCs, so the first few HTLCs should be allowed. + bob ! CMD_ADD_HTLC(sender.ref, 25_000_000 msat, randomBytes32(), CltvExpiry(400144), TestConstants.emptyOnionPacket, None, localOrigin(sender.ref)) + sender.expectMsgType[RES_SUCCESS[CMD_ADD_HTLC]] + val add = bob2alice.expectMsgType[UpdateAddHtlc] + bob2alice.forward(alice, add) + + // But this one will dip alice below her reserve: we must wait for the previous HTLCs to settle before sending any more. + val failedAdd = CMD_ADD_HTLC(sender.ref, 25_000_000 msat, randomBytes32(), CltvExpiry(400144), TestConstants.emptyOnionPacket, None, localOrigin(sender.ref)) + bob ! failedAdd + val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), failedAdd.amount, missing = 340 sat, 20_000 sat, 21_700 sat) + sender.expectMsg(RES_ADD_FAILED(failedAdd, error, Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate))) + + // If Bob had sent this HTLC, Alice would have accepted dipping into her reserve. + val proposedChanges = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.changes.remoteChanges.proposed.size + alice ! add.copy(id = add.id + 1) + awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.changes.remoteChanges.proposed.size == proposedChanges + 1) + } + test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs and 0 balance)", Tag(ChannelStateTestsTags.NoMaxHtlcValueInFlight)) { f => import f._ val sender = TestProbe()