Commitments: take HTLC fee into account (#1152)

Our balance computation was slightly incorrect. If you want to know how much you can send (or receive), you need to take into account the fact that you'll add a new HTLC which adds weight to the commit tx (and thus adds fees).

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

@@ -88,14 +88,42 @@ case class Commitments(channelVersion: ChannelVersion,
     // we need to base the next current commitment on the last sig we sent, even if we didn't yet receive their revocation
     val remoteCommit1 = remoteNextCommitInfo.left.toOption.map(_.nextRemoteCommit).getOrElse(remoteCommit)
     val reduced = CommitmentSpec.reduce(remoteCommit1.spec, remoteChanges.acked, localChanges.proposed)
-    val feesMsat = if (localParams.isFunder) commitTxFee(remoteParams.dustLimit, reduced).toMilliSatoshi else 0.msat
-    (reduced.toRemote - remoteParams.channelReserve.toMilliSatoshi - feesMsat).max(0 msat)
+    val balanceNoFees = (reduced.toRemote - remoteParams.channelReserve).max(0 msat)
+    if (localParams.isFunder) {
+      // The funder always pays the on-chain fees, so we must subtract that from the amount we can send.
+      val commitFees = commitTxFee(remoteParams.dustLimit, reduced).toMilliSatoshi
+      val htlcFees = htlcOutputFee(reduced.feeratePerKw)
+      if (balanceNoFees - commitFees < offeredHtlcTrimThreshold(remoteParams.dustLimit, reduced)) {
+        // htlc will be trimmed
+        (balanceNoFees - commitFees).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)
+      }
+    } else {
+      // The fundee doesn't pay on-chain fees.
+      balanceNoFees
+    }
   }
 
   lazy val availableBalanceForReceive: MilliSatoshi = {
     val reduced = CommitmentSpec.reduce(localCommit.spec, localChanges.acked, remoteChanges.proposed)
-    val feesMsat = if (localParams.isFunder) 0.msat else commitTxFee(localParams.dustLimit, reduced).toMilliSatoshi
-    (reduced.toRemote - localParams.channelReserve.toMilliSatoshi - feesMsat).max(0 msat)
+    val balanceNoFees = (reduced.toRemote - localParams.channelReserve).max(0 msat)
+    if (localParams.isFunder) {
+      // The fundee doesn't pay on-chain fees so we don't take those into account when receiving.
+      balanceNoFees
+    } else {
+      // The funder always pays the on-chain fees, so we must subtract that from the amount we can receive.
+      val commitFees = commitTxFee(localParams.dustLimit, reduced).toMilliSatoshi
+      val htlcFees = htlcOutputFee(reduced.feeratePerKw)
+      if (balanceNoFees - commitFees < receivedHtlcTrimThreshold(localParams.dustLimit, reduced)) {
+        // htlc will be trimmed
+        (balanceNoFees - commitFees).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)
+      }
+    }
   }
 }
 

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

@@ -95,6 +95,7 @@ object Transactions {
    * these values are defined in the RFC
    */
   val commitWeight = 724
+  val htlcOutputWeight = 172
   val htlcTimeoutWeight = 663
   val htlcSuccessWeight = 703
 
@@ -118,26 +119,35 @@ object Transactions {
    */
   def fee2rate(fee: Satoshi, weight: Int) = (fee.toLong * 1000L) / weight
 
+  /** Offered HTLCs below this amount will be trimmed. */
+  def offeredHtlcTrimThreshold(dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = dustLimit + weight2fee(spec.feeratePerKw, htlcTimeoutWeight)
+
   def trimOfferedHtlcs(dustLimit: Satoshi, spec: CommitmentSpec): Seq[DirectedHtlc] = {
-    val htlcTimeoutFee = weight2fee(spec.feeratePerKw, htlcTimeoutWeight)
+    val threshold = offeredHtlcTrimThreshold(dustLimit, spec)
     spec.htlcs
       .filter(_.direction == OUT)
-      .filter(htlc => htlc.add.amountMsat >= (dustLimit + htlcTimeoutFee))
+      .filter(htlc => htlc.add.amountMsat >= threshold)
       .toSeq
   }
 
+  /** Received HTLCs below this amount will be trimmed. */
+  def receivedHtlcTrimThreshold(dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = dustLimit + weight2fee(spec.feeratePerKw, htlcSuccessWeight)
+
   def trimReceivedHtlcs(dustLimit: Satoshi, spec: CommitmentSpec): Seq[DirectedHtlc] = {
-    val htlcSuccessFee = weight2fee(spec.feeratePerKw, htlcSuccessWeight)
+    val threshold = receivedHtlcTrimThreshold(dustLimit, spec)
     spec.htlcs
       .filter(_.direction == IN)
-      .filter(htlc => htlc.add.amountMsat >= (dustLimit + htlcSuccessFee))
+      .filter(htlc => htlc.add.amountMsat >= threshold)
       .toSeq
   }
 
+  /** Fee for an un-trimmed HTLC. */
+  def htlcOutputFee(feeratePerKw: Long): Satoshi = weight2fee(feeratePerKw, htlcOutputWeight)
+
   def commitTxFee(dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = {
     val trimmedOfferedHtlcs = trimOfferedHtlcs(dustLimit, spec)
     val trimmedReceivedHtlcs = trimReceivedHtlcs(dustLimit, spec)
-    val weight = commitWeight + 172 * (trimmedOfferedHtlcs.size + trimmedReceivedHtlcs.size)
+    val weight = commitWeight + htlcOutputWeight * (trimmedOfferedHtlcs.size + trimmedReceivedHtlcs.size)
     weight2fee(spec.feeratePerKw, weight)
   }
 

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

@@ -44,12 +44,32 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
     }
   }
 
+  test("take additional HTLC fee into account") { f =>
+    import f._
+    val htlcOutputFee = 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
+    // we need to take the additional HTLC fee into account because balances are above the trim threshold.
+    assert(ac0.availableBalanceForSend == a - htlcOutputFee)
+    assert(bc0.availableBalanceForReceive == a - htlcOutputFee)
+
+    val (_, cmdAdd) = makeCmdAdd(a - htlcOutputFee - 1000.msat, bob.underlyingActor.nodeParams.nodeId, currentBlockHeight)
+    val Right((ac1, add)) = sendAdd(ac0, cmdAdd, Local(UUID.randomUUID, None), currentBlockHeight)
+    val bc1 = receiveAdd(bc0, add)
+    val (_, commit1) = sendCommit(ac1, alice.underlyingActor.nodeParams.keyManager)
+    val (bc2, _) = receiveCommit(bc1, commit1, bob.underlyingActor.nodeParams.keyManager)
+    // we don't take into account the additional HTLC fee since Alice's balance is below the trim threshold.
+    assert(ac1.availableBalanceForSend == 1000.msat)
+    assert(bc2.availableBalanceForReceive == 1000.msat)
+  }
+
   test("correct values for availableForSend/availableForReceive (success case)") { f =>
     import f._
 
-    val a = 772760000 msat // initial balance alice
-    val b = 190000000 msat // initial balance bob
     val fee = 1720000 msat // fee due to the additional htlc output
+    val a = (772760000 msat) - fee // initial balance alice
+    val b = 190000000 msat // initial balance bob
     val p = 42000000 msat // a->b payment
 
     val ac0 = alice.stateData.asInstanceOf[DATA_NORMAL].commitments
@@ -131,9 +151,9 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
   test("correct values for availableForSend/availableForReceive (failure case)") { f =>
     import f._
 
-    val a = 772760000 msat // initial balance alice
-    val b = 190000000 msat // initial balance bob
     val fee = 1720000 msat // fee due to the additional htlc output
+    val a = (772760000 msat) - fee // initial balance alice
+    val b = 190000000 msat // initial balance bob
     val p = 42000000 msat // a->b payment
 
     val ac0 = alice.stateData.asInstanceOf[DATA_NORMAL].commitments
@@ -215,9 +235,9 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods {
   test("correct values for availableForSend/availableForReceive (multiple htlcs)") { f =>
     import f._
 
-    val a = 772760000 msat // initial balance alice
-    val b = 190000000 msat // initial balance bob
     val fee = 1720000 msat // fee due to the additional htlc output
+    val a = (772760000 msat) - fee // initial balance alice
+    val b = 190000000 msat // initial balance bob
     val p1 = 10000000 msat // a->b payment
     val p2 = 20000000 msat // a->b payment
     val p3 = 40000000 msat // b->a payment

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

@@ -184,15 +184,15 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
   test("recv CMD_ADD_HTLC (HTLC dips remote funder below reserve)") { f =>
     import f._
     val sender = TestProbe()
-    addHtlc(MilliSatoshi(771000000), alice, bob, alice2bob, bob2alice)
+    addHtlc(771000000 msat, alice, bob, alice2bob, bob2alice)
     crossSign(alice, bob, alice2bob, bob2alice)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend === 40000.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(MilliSatoshi(120000000), randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, upstream = Left(UUID.randomUUID()))
+    val add = CMD_ADD_HTLC(120000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, upstream = Left(UUID.randomUUID()))
     sender.send(bob, add)
-    val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), add.amount, missing = 1680.sat, 10000.sat, 10680.sat)
+    val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), add.amount, missing = 1680 sat, 10000 sat, 10680 sat)
     sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), add.paymentHash, error, Local(add.upstream.left.get, Some(sender.ref)), Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate), Some(add))))
   }