Funder reserve for future fee increase (#1319)

See lightning/bolts#728

Add an additional reserve on the funder to prevent emptying and then
being stuck with an unusable channel.

As fundee we don't verify funders comply with that change.
We may enforce it in the future when we're confident the network as a
whole enforces that.

Author: t-bast

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

@@ -94,13 +94,16 @@ case class Commitments(channelVersion: ChannelVersion,
     if (localParams.isFunder) {
       // The funder always pays the on-chain fees, so we must subtract that from the amount we can send.
       val commitFees = commitTxFeeMsat(remoteParams.dustLimit, reduced)
+      // the funder needs to keep an extra reserve to be able to handle fee increase without getting the channel stuck
+      // (see https://github.com/lightningnetwork/lightning-rfc/issues/728)
+      val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw)
       val htlcFees = htlcOutputFee(reduced.feeratePerKw)
       if (balanceNoFees - commitFees < offeredHtlcTrimThreshold(remoteParams.dustLimit, reduced)) {
         // htlc will be trimmed
-        (balanceNoFees - commitFees).max(0 msat)
+        (balanceNoFees - commitFees - funderFeeReserve).max(0 msat)
       } else {
         // htlc will have an output in the commitment tx, so there will be additional fees.
-        (balanceNoFees - commitFees - htlcFees).max(0 msat)
+        (balanceNoFees - commitFees - funderFeeReserve - htlcFees).max(0 msat)
       }
     } else {
       // The fundee doesn't pay on-chain fees.
@@ -117,13 +120,16 @@ case class Commitments(channelVersion: ChannelVersion,
     } else {
       // The funder always pays the on-chain fees, so we must subtract that from the amount we can receive.
       val commitFees = commitTxFeeMsat(localParams.dustLimit, reduced)
+      // we expect the funder to keep an extra reserve to be able to handle fee increase without getting the channel stuck
+      // (see https://github.com/lightningnetwork/lightning-rfc/issues/728)
+      val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw)
       val htlcFees = htlcOutputFee(reduced.feeratePerKw)
       if (balanceNoFees - commitFees < receivedHtlcTrimThreshold(localParams.dustLimit, reduced)) {
         // htlc will be trimmed
-        (balanceNoFees - commitFees).max(0 msat)
+        (balanceNoFees - commitFees - funderFeeReserve).max(0 msat)
       } else {
         // htlc will have an output in the commitment tx, so there will be additional fees.
-        (balanceNoFees - commitFees - htlcFees).max(0 msat)
+        (balanceNoFees - commitFees - funderFeeReserve - htlcFees).max(0 msat)
       }
     }
   }
@@ -185,7 +191,10 @@ object Commitments {
 
     // note that the funder pays the fee, so if sender != funder, both sides will have to afford this payment
     val fees = commitTxFee(commitments1.remoteParams.dustLimit, reduced)
-    val missingForSender = reduced.toRemote - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees else 0.sat)
+    // the funder needs to keep an extra reserve to be able to handle fee increase without getting the channel stuck
+    // (see https://github.com/lightningnetwork/lightning-rfc/issues/728)
+    val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw)
+    val missingForSender = reduced.toRemote - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees + funderFeeReserve else 0.msat)
     val missingForReceiver = reduced.toLocal - commitments1.localParams.channelReserve - (if (commitments1.localParams.isFunder) 0.sat else fees)
     if (missingForSender < 0.msat) {
       return Failure(InsufficientFunds(commitments.channelId, amount = cmd.amount, missing = -missingForSender.truncateToSatoshi, reserve = commitments1.remoteParams.channelReserve, fees = if (commitments1.localParams.isFunder) fees else 0.sat))
@@ -227,6 +236,8 @@ object Commitments {
 
     // note that the funder pays the fee, so if sender != funder, both sides will have to afford this payment
     val fees = commitTxFee(commitments1.remoteParams.dustLimit, reduced)
+    // NB: we don't enforce the funderFeeReserve (see sendAdd) because it would confuse a remote funder that doesn't have this mitigation in place
+    // We could enforce it once we're confident a large portion of the network implements it.
     val missingForSender = reduced.toRemote - commitments1.localParams.channelReserve - (if (commitments1.localParams.isFunder) 0.sat else fees)
     val missingForReceiver = reduced.toLocal - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees else 0.sat)
     if (missingForSender < 0.sat) {

eclair-core/src/test/scala/fr/acinq/eclair/channel/CommitmentsSpec.scala

@@ -53,7 +53,9 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
 
   test("take additional HTLC fee into account") { f =>
     import f._
-    val htlcOutputFee = 1720000 msat
+    // The fee for a single HTLC is 1720000 msat but the funder keeps an extra reserve to make sure we're able to handle
+    // an additional HTLC at twice the feerate (hence the multiplier).
+    val htlcOutputFee = 3 * 1720000 msat
     val a = 772760000 msat // initial balance alice
     val ac0 = alice.stateData.asInstanceOf[DATA_NORMAL].commitments
     val bc0 = bob.stateData.asInstanceOf[DATA_NORMAL].commitments
@@ -75,7 +77,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
     import f._
 
     val fee = 1720000 msat // fee due to the additional htlc output
-    val a = (772760000 msat) - fee // initial balance alice
+    val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase
+    val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice
     val b = 190000000 msat // initial balance bob
     val p = 42000000 msat // a->b payment
 
@@ -159,7 +162,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
     import f._
 
     val fee = 1720000 msat // fee due to the additional htlc output
-    val a = (772760000 msat) - fee // initial balance alice
+    val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase
+    val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice
     val b = 190000000 msat // initial balance bob
     val p = 42000000 msat // a->b payment
 
@@ -243,7 +247,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
     import f._
 
     val fee = 1720000 msat // fee due to the additional htlc output
-    val a = (772760000 msat) - fee // initial balance alice
+    val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase
+    val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice
     val b = 190000000 msat // initial balance bob
     val p1 = 10000000 msat // a->b payment
     val p2 = 20000000 msat // a->b payment
@@ -386,6 +391,23 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
     assert(ac16.availableBalanceForReceive == b + p1 - p3)
   }
 
+  // See https://github.com/lightningnetwork/lightning-rfc/issues/728
+  test("funder keeps additional reserve to avoid channel being stuck") { f =>
+    val isFunder = true
+    val c = CommitmentsSpec.makeCommitments(100000000 msat, 50000000 msat, 2500, 546 sat, isFunder)
+    val (_, cmdAdd) = makeCmdAdd(c.availableBalanceForSend, randomKey.publicKey, f.currentBlockHeight)
+    val Success((c1, _)) = sendAdd(c, cmdAdd, Local(UUID.randomUUID, None), f.currentBlockHeight)
+    assert(c1.availableBalanceForSend === 0.msat)
+
+    // We should be able to handle a fee increase.
+    val Success((c2, _)) = sendFee(c1, CMD_UPDATE_FEE(3000))
+
+    // Now we shouldn't be able to send until we receive enough to handle the updated commit tx fee (even trimmed HTLCs shouldn't be sent).
+    val (_, cmdAdd1) = makeCmdAdd(100 msat, randomKey.publicKey, f.currentBlockHeight)
+    val Failure(e) = sendAdd(c2, cmdAdd1, Local(UUID.randomUUID, None), f.currentBlockHeight)
+    assert(e.isInstanceOf[InsufficientFunds])
+  }
+
   test("can send availableForSend") { f =>
     for (isFunder <- Seq(true, false)) {
       val c = CommitmentsSpec.makeCommitments(702000000 msat, 52000000 msat, 2679, 546 sat, isFunder)

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

@@ -190,7 +190,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     val initialState = alice.stateData.asInstanceOf[DATA_NORMAL]
     val add = CMD_ADD_HTLC(MilliSatoshi(Int.MaxValue), randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
     sender.send(alice, add)
-    val error = InsufficientFunds(channelId(alice), amount = MilliSatoshi(Int.MaxValue), missing = 1376443 sat, reserve = 20000 sat, fees = 8960 sat)
+    val error = InsufficientFunds(channelId(alice), amount = MilliSatoshi(Int.MaxValue), missing = 1379883 sat, reserve = 20000 sat, fees = 8960 sat)
     sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add))))
     alice2bob.expectNoMsg(200 millis)
   }
@@ -207,19 +207,27 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     alice2bob.expectNoMsg(200 millis)
   }
 
-  test("recv CMD_ADD_HTLC (HTLC dips remote funder below reserve)") { f =>
+  test("recv CMD_ADD_HTLC (HTLC dips into remote funder fee reserve)") { f =>
     import f._
     val sender = TestProbe()
-    addHtlc(771000000 msat, alice, bob, alice2bob, bob2alice)
+    addHtlc(767600000 msat, alice, bob, alice2bob, bob2alice)
     crossSign(alice, bob, alice2bob, bob2alice)
-    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend === 40000.msat)
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend === 0.msat)
 
     // actual test begins
-    // at this point alice has the minimal amount to sustain a channel (29000 sat ~= alice reserve + commit fee)
-    val add = CMD_ADD_HTLC(120000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
-    sender.send(bob, add)
-    val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), add.amount, missing = 1680 sat, 10000 sat, 10680 sat)
-    sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate), Some(add))))
+    // at this point alice has the minimal amount to sustain a channel
+    // alice maintains an extra reserve to accommodate for a few more HTLCs, so the first two HTLCs should be allowed
+    sender.send(bob, CMD_ADD_HTLC(12000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())))
+    sender.expectMsg(ChannelCommandResponse.Ok)
+
+    sender.send(bob, CMD_ADD_HTLC(12500000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())))
+    sender.expectMsg(ChannelCommandResponse.Ok)
+
+    // but this one will dip alice below her reserve: we must wait for the two previous HTLCs to settle before sending any more
+    val failedAdd = CMD_ADD_HTLC(11000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
+    sender.send(bob, failedAdd)
+    val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), failedAdd.amount, missing = 1720 sat, 10000 sat, 14120 sat)
+    sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), failedAdd.paymentHash, error, Origin.Local(failedAdd.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate), Some(failedAdd))))
   }
 
   test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs and 0 balance)") { f =>
@@ -232,7 +240,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     sender.send(alice, CMD_ADD_HTLC(200000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())))
     sender.expectMsg(ChannelCommandResponse.Ok)
     alice2bob.expectMsgType[UpdateAddHtlc]
-    sender.send(alice, CMD_ADD_HTLC(67600000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())))
+    sender.send(alice, CMD_ADD_HTLC(64160000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())))
     sender.expectMsg(ChannelCommandResponse.Ok)
     alice2bob.expectMsgType[UpdateAddHtlc]
     val add = CMD_ADD_HTLC(1000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
@@ -254,7 +262,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     alice2bob.expectMsgType[UpdateAddHtlc]
     val add = CMD_ADD_HTLC(500000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
     sender.send(alice, add)
-    val error = InsufficientFunds(channelId(alice), amount = 500000000 msat, missing = 332400 sat, reserve = 20000 sat, fees = 12400 sat)
+    val error = InsufficientFunds(channelId(alice), amount = 500000000 msat, missing = 335840 sat, reserve = 20000 sat, fees = 12400 sat)
     sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add))))
     alice2bob.expectNoMsg(200 millis)
   }
@@ -316,7 +324,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     // this is over channel-capacity
     val add2 = CMD_ADD_HTLC(TestConstants.fundingSatoshis.toMilliSatoshi * 2 / 3, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))
     sender.send(alice, add2)
-    val error = InsufficientFunds(channelId(alice), add2.amount, 564013 sat, 20000 sat, 10680 sat)
+    val error = InsufficientFunds(channelId(alice), add2.amount, 567453 sat, 20000 sat, 10680 sat)
     sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add2.paymentHash, error, Origin.Local(add2.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add2))))
     alice2bob.expectNoMsg(200 millis)
   }