Refactored fee calculation (added trim functions) (#25)

* refactored fee calculation (added trim functions)

* now running BOLT 2 fee tests automatically

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

@@ -73,48 +73,27 @@ object Transactions {
 
   def weight2fee(feeRatePerKw: Long, weight: Int) = Satoshi((feeRatePerKw * weight) / 1000)
 
-  def commitTxFee(feeRatePerKw: Long, dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = {
-    val trimmedOfferedHtlcs = spec.htlcs
+  def trimOfferedHtlcs(dustLimit: Satoshi, spec: CommitmentSpec): Seq[Htlc] = {
+    val htlcTimeoutFee = weight2fee(spec.feeRatePerKw, htlcTimeoutWeight)
+    spec.htlcs
       .filter(_.direction == OUT)
-      .map(htlc => MilliSatoshi(htlc.add.amountMsat))
-      .filter(htlcAmount => htlcAmount.compare(dustLimit + weight2fee(feeRatePerKw, htlcTimeoutWeight)) >= 0)
-
-    val trimmedReceivedHtlcs = spec.htlcs
-      .filter(_.direction == IN)
-      .map(htlc => MilliSatoshi(htlc.add.amountMsat))
-      .filter(htlcAmount => htlcAmount.compare(dustLimit + weight2fee(feeRatePerKw, htlcSuccessWeight)) >= 0)
-
-    val weight = commitWeight + 172 * (trimmedOfferedHtlcs.size + trimmedReceivedHtlcs.size)
-    weight2fee(feeRatePerKw, weight)
+      .filter(htlc => MilliSatoshi(htlc.add.amountMsat).compare(dustLimit + htlcTimeoutFee) >= 0)
+      .toSeq
   }
 
-  def commitTxFeeOld(feeRatePerKw: Long, dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = {
-
-    case class Fee(weight: Int, amount: Satoshi)
-
-    val fee1 = Fee(commitWeight, Satoshi(0))
-
-    val fee2 = spec.htlcs
-      .filter(_.direction == OUT)
-      .map(htlc => MilliSatoshi(htlc.add.amountMsat))
-      .foldLeft(fee1) {
-        case (fee, htlcAmount) if (htlcAmount).compare(dustLimit + weight2fee(feeRatePerKw, htlcTimeoutWeight)) >= 0 =>
-          fee.copy(weight = fee.weight + 172)
-        case (fee, htlcAmount) =>
-          fee.copy(amount = fee.amount + htlcAmount)
-      }
-
-    val fee3 = spec.htlcs
+  def trimReceivedHtlcs(dustLimit: Satoshi, spec: CommitmentSpec): Seq[Htlc] = {
+    val htlcSuccessFee = weight2fee(spec.feeRatePerKw, htlcSuccessWeight)
+    spec.htlcs
       .filter(_.direction == IN)
-      .map(htlc => MilliSatoshi(htlc.add.amountMsat))
-      .foldLeft(fee2) {
-        case (fee, htlcAmount) if (htlcAmount).compare(dustLimit + weight2fee(feeRatePerKw, htlcSuccessWeight)) >= 0 =>
-          fee.copy(weight = fee.weight + 172)
-        case (fee, htlcAmount) =>
-          fee.copy(amount = fee.amount + htlcAmount)
-      }
+      .filter(htlc => MilliSatoshi(htlc.add.amountMsat).compare(dustLimit + htlcSuccessFee) >= 0)
+      .toSeq
+  }
 
-    weight2fee(feeRatePerKw, fee3.weight) + fee3.amount
+  def commitTxFee(dustLimit: Satoshi, spec: CommitmentSpec): Satoshi = {
+    val trimmedOfferedHtlcs = trimOfferedHtlcs(dustLimit, spec).map(htlc => MilliSatoshi(htlc.add.amountMsat))
+    val trimmedReceivedHtlcs = trimReceivedHtlcs(dustLimit, spec).map(htlc => MilliSatoshi(htlc.add.amountMsat))
+    val weight = commitWeight + 172 * (trimmedOfferedHtlcs.size + trimmedReceivedHtlcs.size)
+    weight2fee(spec.feeRatePerKw, weight)
   }
 
   /**
@@ -158,7 +137,7 @@ object Transactions {
 
   def makeCommitTx(commitTxInput: InputInfo, commitTxNumber: Long, localPaymentBasePoint: Point, remotePaymentBasePoint: Point, localIsFunder: Boolean, localDustLimit: Satoshi, localPubKey: PublicKey, localRevocationPubkey: PublicKey, toLocalDelay: Int, localDelayedPubkey: PublicKey, remotePubkey: PublicKey, spec: CommitmentSpec): CommitTx = {
 
-    val commitFee = commitTxFee(spec.feeRatePerKw, localDustLimit, spec)
+    val commitFee = commitTxFee(localDustLimit, spec)
 
     val (toLocalAmount: Satoshi, toRemoteAmount: Satoshi) = (MilliSatoshi(spec.toLocalMsat), MilliSatoshi(spec.toRemoteMsat)) match {
       case (local, remote) if localIsFunder && local.compare(commitFee) < 0 => (Satoshi(0), millisatoshi2satoshi(remote)) //TODO: check: can't pay fees! => pay whatever you can
@@ -169,15 +148,9 @@ object Transactions {
     val toLocalDelayedOutput_opt = if (toLocalAmount.compare(localDustLimit) >= 0) Some(TxOut(toLocalAmount, pay2wsh(toLocalDelayed(localRevocationPubkey, toLocalDelay, localDelayedPubkey)))) else None
     val toRemoteOutput_opt = if (toRemoteAmount.compare(localDustLimit) >= 0) Some(TxOut(toRemoteAmount, pay2wpkh(remotePubkey))) else None
 
-    val htlcTimeoutFee = weight2fee(spec.feeRatePerKw, htlcTimeoutWeight)
-    val htlcSuccessFee = weight2fee(spec.feeRatePerKw, htlcSuccessWeight)
-    val htlcOfferedOutputs = spec.htlcs.toSeq
-      .filter(_.direction == OUT)
-      .filter(htlc => (MilliSatoshi(htlc.add.amountMsat) - htlcTimeoutFee).compare(localDustLimit) >= 0)
+    val htlcOfferedOutputs = trimOfferedHtlcs(localDustLimit, spec)
       .map(htlc => TxOut(MilliSatoshi(htlc.add.amountMsat), pay2wsh(htlcOffered(localPubKey, remotePubkey, ripemd160(htlc.add.paymentHash)))))
-    val htlcReceivedOutputs = spec.htlcs.toSeq
-      .filter(_.direction == IN)
-      .filter(htlc => (MilliSatoshi(htlc.add.amountMsat) - htlcSuccessFee).compare(localDustLimit) >= 0)
+    val htlcReceivedOutputs = trimReceivedHtlcs(localDustLimit, spec)
       .map(htlc => TxOut(MilliSatoshi(htlc.add.amountMsat), pay2wsh(htlcReceived(localPubKey, remotePubkey, ripemd160(htlc.add.paymentHash), htlc.add.expiry))))
 
     val txnumber = obscuredCommitTxNumber(commitTxNumber, localPaymentBasePoint, remotePaymentBasePoint)
@@ -220,18 +193,10 @@ object Transactions {
   }
 
   def makeHtlcTxs(commitTx: Transaction, localDustLimit: Satoshi, localRevocationPubkey: PublicKey, toLocalDelay: Int, localPubkey: PublicKey, localDelayedPubkey: PublicKey, remotePubkey: PublicKey, spec: CommitmentSpec): (Seq[HtlcTimeoutTx], Seq[HtlcSuccessTx]) = {
-    val htlcTimeoutFee = weight2fee(spec.feeRatePerKw, htlcTimeoutWeight)
-    val htlcSuccessFee = weight2fee(spec.feeRatePerKw, htlcSuccessWeight)
-    val htlcTimeoutTxs = spec.htlcs
-      .filter(_.direction == OUT)
-      .filter(htlc => (MilliSatoshi(htlc.add.amountMsat) - htlcTimeoutFee).compare(localDustLimit) > 0)
+    val htlcTimeoutTxs = trimOfferedHtlcs(localDustLimit, spec)
       .map(htlc => makeHtlcTimeoutTx(commitTx, localRevocationPubkey, toLocalDelay, localPubkey, localDelayedPubkey, remotePubkey, spec.feeRatePerKw, htlc.add))
-      .toSeq
-    val htlcSuccessTxs = spec.htlcs
-      .filter(_.direction == IN)
-      .filter(htlc => (MilliSatoshi(htlc.add.amountMsat) - htlcSuccessFee).compare(localDustLimit) > 0)
+    val htlcSuccessTxs = trimReceivedHtlcs(localDustLimit, spec)
       .map(htlc => makeHtlcSuccessTx(commitTx, localRevocationPubkey, toLocalDelay, localPubkey, localDelayedPubkey, remotePubkey, spec.feeRatePerKw, htlc.add))
-      .toSeq
     (htlcTimeoutTxs, htlcSuccessTxs)
   }
 

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

@@ -1065,7 +1065,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
       Transaction.correctlySpends(penaltyTx, Seq(revokedTx), ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
 
       // two main outputs are 760 000 and 200 000
-      assert(mainTx.txOut(0).amount == Satoshi(746670))
+      assert(mainTx.txOut(0).amount == Satoshi(741510))
       assert(penaltyTx.txOut(0).amount == Satoshi(195170))
 
       awaitCond(alice.stateName == CLOSING)

eclair-node/src/test/scala/fr/acinq/eclair/transactions/TestVectorsSpec.scala

@@ -168,7 +168,7 @@ class TestVectorsSpec extends FunSuite {
       Transactions.addSigs(tx, Local.funding_pubkey, Remote.funding_pubkey, local_sig, remote_sig)
     }
 
-    val baseFee = Transactions.commitTxFee(spec.feeRatePerKw, Local.dustLimit, spec)
+    val baseFee = Transactions.commitTxFee(Local.dustLimit, spec)
     println(s"# base commitment transaction fee = $baseFee")
     val actualFee = fundingAmount - commitTx.tx.txOut.map(_.amount).sum
     println(s"# actual commitment transaction fee = $actualFee")

eclair-node/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala

@@ -13,6 +13,9 @@ import org.junit.runner.RunWith
 import org.scalatest.FunSuite
 import org.scalatest.junit.JUnitRunner
 
+import scala.io.Source
+import scala.util.matching.Regex
+import scala.util.matching.Regex.MatchIterator
 import scala.util.{Failure, Random, Success, Try}
 
 /**
@@ -52,7 +55,7 @@ class TransactionsSpec extends FunSuite {
       Htlc(IN, UpdateAddHtlc(0, 0, MilliSatoshi(800000).amount, 551, Hash.Zeroes, BinaryData("")), None)
     )
     val spec = CommitmentSpec(htlcs, feeRatePerKw = 5000, toLocalMsat = 0, toRemoteMsat = 0)
-    val fee = Transactions.commitTxFee(5000, Satoshi(546), spec)
+    val fee = Transactions.commitTxFee(Satoshi(546), spec)
     assert(fee == Satoshi(5340))
   }
 
@@ -244,4 +247,51 @@ class TransactionsSpec extends FunSuite {
     case Success(_) => ()
     case Failure(t) => fail(t)
   }
+
+  def htlc(direction: Direction, amount: Satoshi): Htlc =
+    Htlc(direction, UpdateAddHtlc(0, 0, amount.amount * 1000, 144, "00" * 32, ""), None)
+
+  test("BOLT 2 fee tests") {
+
+    val bolt3 = Source
+      .fromURL("https://raw.githubusercontent.com/lightningnetwork/lightning-rfc/master/03-transactions.md")
+      .mkString
+      .replace("    name:", "$   name:")
+    // character '$' separates tests
+
+    // this regex extract params from a given test
+    val testRegex = ("""name: (.*)\n""" +
+      """.*to_local_msat: ([0-9]+)\n""" +
+      """.*to_remote_msat: ([0-9]+)\n""" +
+      """.*feerate_per_kw: ([0-9]+)\n""" +
+      """.*base commitment transaction fee = ([0-9]+)\n""" +
+      """[^$]+""").r
+    // this regex extracts htlc direction and amounts
+    val htlcRegex =
+    """.*HTLC ([a-z]+) amount ([0-9]+).*""".r
+
+    val dustLimit = Satoshi(546)
+    case class TestSetup(name: String, dustLimit: Satoshi, spec: CommitmentSpec, expectedFee: Satoshi)
+
+    val tests = testRegex.findAllIn(bolt3).map(s => {
+      val testRegex(name, to_local_msat, to_remote_msat, feerate_per_kw, fee) = s
+      val htlcs = htlcRegex.findAllIn(s).map(l => {
+        val htlcRegex(direction, amount) = l
+        direction match {
+          case "offered" => htlc(OUT, Satoshi(amount.toLong))
+          case "received" => htlc(IN, Satoshi(amount.toLong))
+        }
+      }).toSet
+      TestSetup(name, dustLimit, CommitmentSpec(htlcs = htlcs, feeRatePerKw = feerate_per_kw.toLong, toLocalMsat = to_local_msat.toLong, toRemoteMsat = to_remote_msat.toLong), Satoshi(fee.toLong))
+    })
+
+    // simple non-reg test making sure we are not missing tests
+    assert(tests.size === 15, "there were 15 tests at ec99f893f320e8c88f564c1c8566f3454f0f1f5f")
+
+    tests.foreach(test => {
+      println(s"running BOLT 2 test: '${test.name}'")
+      val fee = commitTxFee(test.dustLimit, test.spec)
+      assert(fee === test.expectedFee)
+    })
+  }
 }