Remove the htlcsAmount parameter from Output.Shared

 - instead of an extra parameter, add the incoming/outgoing htlcs balance to the local/remote balances, respectively

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -166,6 +166,9 @@ object InteractiveTxBuilder {
     def commitTxFeerate: FeeratePerKw
     def fundingTxIndex: Long
     def localHtlcs: Set[DirectedHtlc]
+    def incomingHtlcsBalance: MilliSatoshi = localHtlcs.collect(DirectedHtlc.incoming).toSeq.map(_.amountMsat).sum
+    def outgoingHtlcsBalance: MilliSatoshi = localHtlcs.collect(DirectedHtlc.outgoing).toSeq.map(_.amountMsat).sum
+    def previousHtlcsBalance: MilliSatoshi = incomingHtlcsBalance + outgoingHtlcsBalance
   }
   case class FundingTx(commitTxFeerate: FeeratePerKw, remotePerCommitmentPoint: PublicKey) extends Purpose {
     override val previousLocalBalance: MilliSatoshi = 0 msat
@@ -251,9 +254,9 @@ object InteractiveTxBuilder {
     case class Remote(serialId: UInt64, amount: Satoshi, pubkeyScript: ByteVector) extends Output with Incoming
 
     /** The shared output can be added by us or by our peer, depending on who initiated the protocol. */
-    case class Shared(serialId: UInt64, pubkeyScript: ByteVector, localAmount: MilliSatoshi, remoteAmount: MilliSatoshi, htlcsAmount: MilliSatoshi) extends Output with Incoming with Outgoing {
+    case class Shared(serialId: UInt64, pubkeyScript: ByteVector, localAmount: MilliSatoshi, remoteAmount: MilliSatoshi) extends Output with Incoming with Outgoing {
       // Note that the truncation is a no-op: the sum of balances in a channel must be a satoshi amount.
-      override val amount: Satoshi = (localAmount + remoteAmount + htlcsAmount).truncateToSatoshi
+      override val amount: Satoshi = (localAmount + remoteAmount).truncateToSatoshi
     }
   }
 
@@ -347,10 +350,10 @@ object InteractiveTxBuilder {
         Behaviors.withMdc(Logs.mdc(remoteNodeId_opt = Some(channelParams.remoteParams.nodeId), channelId_opt = Some(fundingParams.channelId))) {
           Behaviors.receiveMessagePartial {
             case Start(replyTo) =>
-              val nextLocalFundingAmount = purpose.previousLocalBalance + fundingParams.localContribution
-              val nextRemoteFundingAmount = purpose.previousRemoteBalance + fundingParams.remoteContribution
-              if (fundingParams.fundingAmount < fundingParams.dustLimit) {
-                replyTo ! LocalFailure(FundingAmountTooLow(channelParams.channelId, fundingParams.fundingAmount, fundingParams.dustLimit))
+              val nextLocalFundingAmount = purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution
+              val nextRemoteFundingAmount = purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution
+              if (fundingParams.fundingAmount < fundingParams.dustLimit || fundingParams.fundingAmount < purpose.previousHtlcsBalance.truncateToSatoshi) {
+                replyTo ! LocalFailure(FundingAmountTooLow(channelParams.channelId, fundingParams.fundingAmount, fundingParams.dustLimit + purpose.previousHtlcsBalance.truncateToSatoshi))
                 Behaviors.stopped
               } else if (nextLocalFundingAmount < 0.msat || nextRemoteFundingAmount < 0.msat) {
                 replyTo ! LocalFailure(InvalidFundingBalances(channelParams.channelId, fundingParams.fundingAmount, nextLocalFundingAmount, nextRemoteFundingAmount))
@@ -482,7 +485,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
       case None =>
         (addInput.sharedInput_opt, fundingParams.sharedInput_opt) match {
           case (Some(outPoint), Some(sharedInput)) if outPoint == sharedInput.info.outPoint =>
-            Input.Shared(addInput.serialId, outPoint, addInput.sequence, purpose.previousLocalBalance, purpose.previousRemoteBalance)
+            Input.Shared(addInput.serialId, outPoint, addInput.sequence, purpose.previousLocalBalance + purpose.incomingHtlcsBalance, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance)
           case _ =>
             return Left(PreviousTxMissing(fundingParams.channelId, addInput.serialId))
         }
@@ -509,7 +512,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
       Left(InvalidSpliceOutputScript(fundingParams.channelId, addOutput.serialId, addOutput.pubkeyScript))
     } else if (addOutput.pubkeyScript == fundingPubkeyScript) {
       val htlcsAmount = fundingParams.sharedInput_opt.map(_.info.txOut.amount - purpose.previousLocalBalance - purpose.previousRemoteBalance).getOrElse(0 msat)
-      Right(Output.Shared(addOutput.serialId, addOutput.pubkeyScript, purpose.previousLocalBalance + fundingParams.localContribution, purpose.previousRemoteBalance + fundingParams.remoteContribution, htlcsAmount))
+      Right(Output.Shared(addOutput.serialId, addOutput.pubkeyScript, purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution))
     } else {
       Right(Output.Remote(addOutput.serialId, addOutput.amount, addOutput.pubkeyScript))
     }
@@ -729,8 +732,8 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
     val fundingOutputIndex = fundingTx.txOut.indexWhere(_.publicKeyScript == fundingPubkeyScript)
     Funding.makeCommitTxs(keyManager, channelParams,
       fundingAmount = fundingParams.fundingAmount,
-      toLocal = completeTx.sharedOutput.localAmount - localPushAmount + remotePushAmount,
-      toRemote = completeTx.sharedOutput.remoteAmount - remotePushAmount + localPushAmount,
+      toLocal = completeTx.sharedOutput.localAmount - localPushAmount + remotePushAmount - purpose.incomingHtlcsBalance,
+      toRemote = completeTx.sharedOutput.remoteAmount - remotePushAmount + localPushAmount - purpose.outgoingHtlcsBalance,
       purpose.commitTxFeerate,
       fundingTxIndex = purpose.fundingTxIndex,
       fundingTx.hash, fundingOutputIndex,

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxFunder.scala

@@ -25,7 +25,7 @@ import fr.acinq.eclair.blockchain.fee.FeeratePerKw
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
 import fr.acinq.eclair.transactions.Transactions
 import fr.acinq.eclair.wire.protocol.TxAddInput
-import fr.acinq.eclair.{Logs, MilliSatoshiLong, UInt64}
+import fr.acinq.eclair.{Logs, UInt64}
 import scodec.bits.ByteVector
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -156,9 +156,8 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
       // force us to add wallet inputs. The caller may manually decrease the output amounts if it wants to actually
       // contribute to the RBF attempt.
       if (fundingParams.isInitiator) {
-        val sharedInput = fundingParams.sharedInput_opt.toSeq.map(sharedInput => Input.Shared(UInt64(0), sharedInput.info.outPoint, 0xfffffffdL, purpose.previousLocalBalance, purpose.previousRemoteBalance))
-        val htlcsAmount = fundingParams.sharedInput_opt.map(_.info.txOut.amount - purpose.previousLocalBalance - purpose.previousRemoteBalance).getOrElse(0 msat)
-        val sharedOutput = Output.Shared(UInt64(0), fundingPubkeyScript, purpose.previousLocalBalance + fundingParams.localContribution, purpose.previousRemoteBalance + fundingParams.remoteContribution, htlcsAmount)
+        val sharedInput = fundingParams.sharedInput_opt.toSeq.map(sharedInput => Input.Shared(UInt64(0), sharedInput.info.outPoint, 0xfffffffdL, purpose.previousLocalBalance + purpose.incomingHtlcsBalance, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance))
+        val sharedOutput = Output.Shared(UInt64(0), fundingPubkeyScript, purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution)
         val nonChangeOutputs = fundingParams.localOutputs.map(txOut => Output.Local.NonChange(UInt64(0), txOut.amount, txOut.publicKeyScript))
         val fundingContributions = sortFundingContributions(fundingParams, sharedInput ++ previousWalletInputs, sharedOutput +: nonChangeOutputs)
         replyTo ! fundingContributions
@@ -233,8 +232,7 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
           val fundingContributions = if (fundingParams.isInitiator) {
             // The initiator is responsible for adding the shared output and the shared input.
             val inputs = inputDetails.usableInputs
-            val htlcsAmount = fundingParams.sharedInput_opt.map(_.info.txOut.amount - purpose.previousLocalBalance - purpose.previousRemoteBalance).getOrElse(0 msat)
-            val fundingOutput = Output.Shared(UInt64(0), fundingPubkeyScript, purpose.previousLocalBalance + fundingParams.localContribution, purpose.previousRemoteBalance + fundingParams.remoteContribution, htlcsAmount)
+            val fundingOutput = Output.Shared(UInt64(0), fundingPubkeyScript, purpose.previousLocalBalance + purpose.incomingHtlcsBalance + fundingParams.localContribution, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance + fundingParams.remoteContribution)
             val outputs = Seq(fundingOutput) ++ nonChangeOutputs ++ changeOutput_opt.toSeq
             sortFundingContributions(fundingParams, inputs, outputs)
           } else {
@@ -293,7 +291,7 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
       case None => fundingParams.sharedInput_opt match {
         case Some(sharedInput) if sharedInput.info.outPoint == txIn.outPoint =>
           // We don't need to validate the shared input, it comes from a valid lightning channel.
-          Future.successful(Right(Input.Shared(UInt64(0), sharedInput.info.outPoint, txIn.sequence, purpose.previousLocalBalance, purpose.previousRemoteBalance)))
+          Future.successful(Right(Input.Shared(UInt64(0), sharedInput.info.outPoint, txIn.sequence, purpose.previousLocalBalance + purpose.incomingHtlcsBalance, purpose.previousRemoteBalance + purpose.outgoingHtlcsBalance)))
         case _ =>
           for {
             previousTx <- wallet.getTransaction(txIn.outPoint.txid)

eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version4/ChannelCodecs4.scala

@@ -261,8 +261,7 @@ private[channel] object ChannelCodecs4 {
         ("serialId" | uint64) ::
           ("scriptPubKey" | lengthDelimited(bytes)) ::
           ("localAmount" | millisatoshi) ::
-          ("remoteAmount" | millisatoshi) ::
-          ("htlcsAmount" | millisatoshi)).as[InteractiveTxBuilder.Output.Shared])
+          ("remoteAmount" | millisatoshi)).as[InteractiveTxBuilder.Output.Shared])
 
     private val localOnlyInteractiveTxInputCodec: Codec[InteractiveTxBuilder.Input.Local] = (
       ("serialId" | uint64) ::

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

@@ -2511,19 +2511,13 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
   test("reference test vector") {
     val channelId = ByteVector32.Zeroes
     val parentTx = Transaction.read("02000000000101f86fd1d0db3ac5a72df968622f31e6b5e6566a09e29206d7c7a55df90e181de800000000171600141fb9623ffd0d422eacc450fd1e967efc477b83ccffffffff0580b2e60e00000000220020fd89acf65485df89797d9ba7ba7a33624ac4452f00db08107f34257d33e5b94680b2e60e0000000017a9146a235d064786b49e7043e4a042d4cc429f7eb6948780b2e60e00000000160014fbb4db9d85fba5e301f4399e3038928e44e37d3280b2e60e0000000017a9147ecd1b519326bc13b0ec716e469b58ed02b112a087f0006bee0000000017a914f856a70093da3a5b5c4302ade033d4c2171705d387024730440220696f6cee2929f1feb3fd6adf024ca0f9aa2f4920ed6d35fb9ec5b78c8408475302201641afae11242160101c6f9932aeb4fcd1f13a9c6df5d1386def000ea259a35001210381d7d5b1bc0d7600565d827242576d9cb793bfe0754334af82289ee8b65d137600000000")
-    val localAmountIn = 1_749_990_000_000L msat
-    val remoteAmountIn = 2_000_000_000_000L msat
-    val localAmountOut = localAmountIn + (200_000_000_000L msat)
-    val remoteAmountOut = remoteAmountIn + (200_000_000_000L msat)
-    val htlcsAmount = parentTx.txOut(4).amount - (localAmountIn + remoteAmountIn)
-    val sharedOutput = Output.Shared(UInt64(44), hex"0020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec5", localAmountOut, remoteAmountOut, htlcsAmount)
-    val sharedInput = Input.Shared(UInt64(22), OutPoint(parentTx, 4), 4294967293L, localAmountIn, remoteAmountIn)
+    val sharedOutput = Output.Shared(UInt64(44), hex"0020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec5", 200_000_000_000L msat, 200_000_000_000L msat)
     val initiatorTx = {
-      val initiatorInput = Input.Local(UInt64(20), parentTx, 0, 4294967293L) // 2_500_000_000 sat
+      val initiatorInput = Input.Local(UInt64(20), parentTx, 0, 4294967293L)
       val initiatorOutput = Output.Local.Change(UInt64(30), 49_999_845 sat, hex"00141ca1cca8855bad6bc1ea5436edd8cff10b7e448b")
-      val nonInitiatorInput = Input.Remote(UInt64(11), OutPoint(parentTx, 2), parentTx.txOut(2), 4294967293L) // 2_500_000_000 sat
+      val nonInitiatorInput = Input.Remote(UInt64(11), OutPoint(parentTx, 2), parentTx.txOut(2), 4294967293L)
       val nonInitiatorOutput = Output.Remote(UInt64(33), 49_999_900 sat, hex"001444cb0c39f93ecc372b5851725bd29d865d333b10")
-      SharedTransaction(Some(sharedInput), sharedOutput, List(initiatorInput), List(nonInitiatorInput), List(initiatorOutput), List(nonInitiatorOutput), lockTime = 120)
+      SharedTransaction(None, sharedOutput, List(initiatorInput), List(nonInitiatorInput), List(initiatorOutput), List(nonInitiatorOutput), lockTime = 120)
     }
     assert(initiatorTx.localFees == 155_000.msat)
     assert(initiatorTx.remoteFees == 100_000.msat)
@@ -2534,23 +2528,23 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
       val initiatorOutput = Output.Remote(UInt64(30), 49_999_845 sat, hex"00141ca1cca8855bad6bc1ea5436edd8cff10b7e448b")
       val nonInitiatorInput = Input.Local(UInt64(11), parentTx, 2, 4294967293L)
       val nonInitiatorOutput = Output.Local.Change(UInt64(33), 49_999_900 sat, hex"001444cb0c39f93ecc372b5851725bd29d865d333b10")
-      SharedTransaction(Some(sharedInput), sharedOutput, List(nonInitiatorInput), List(initiatorInput), List(nonInitiatorOutput), List(initiatorOutput), lockTime = 120)
+      SharedTransaction(None, sharedOutput, List(nonInitiatorInput), List(initiatorInput), List(nonInitiatorOutput), List(initiatorOutput), lockTime = 120)
     }
     assert(nonInitiatorTx.localFees == 100_000.msat)
     assert(nonInitiatorTx.remoteFees == 155_000.msat)
     assert(nonInitiatorTx.fees == 255.sat)
 
-    val unsignedTx = Transaction.read("0200000003b932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430200000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430000000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430400000000fdffffff03e5effa02000000001600141ca1cca8855bad6bc1ea5436edd8cff10b7e448b1cf0fa020000000016001444cb0c39f93ecc372b5851725bd29d865d333b10f084420601000000220020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec578000000")
+    val unsignedTx = Transaction.read("0200000002b932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430200000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430000000000fdffffff03e5effa02000000001600141ca1cca8855bad6bc1ea5436edd8cff10b7e448b1cf0fa020000000016001444cb0c39f93ecc372b5851725bd29d865d333b100084d71700000000220020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec578000000")
     assert(initiatorTx.buildUnsignedTx().txid == unsignedTx.txid)
     assert(nonInitiatorTx.buildUnsignedTx().txid == unsignedTx.txid)
 
     val initiatorSigs = TxSignatures(channelId, unsignedTx, Seq(ScriptWitness(Seq(hex"68656c6c6f2074686572652c2074686973206973206120626974636f6e212121", hex"82012088a820add57dfe5277079d069ca4ad4893c96de91f88ffb981fdc6a2a34d5336c66aff87"))), None)
     val nonInitiatorSigs = TxSignatures(channelId, unsignedTx, Seq(ScriptWitness(Seq(hex"304402207de9ba56bb9f641372e805782575ee840a899e61021c8b1572b3ec1d5b5950e9022069e9ba998915dae193d3c25cb89b5e64370e6a3a7755e7f31cf6d7cbc2a49f6d01", hex"034695f5b7864c580bf11f9f8cb1a94eb336f2ce9ef872d2ae1a90ee276c772484"))), None)
     val initiatorSignedTx = FullySignedSharedTransaction(initiatorTx, initiatorSigs, nonInitiatorSigs, None)
-    assert(initiatorSignedTx.feerate == FeeratePerKw(224 sat))
+    assert(initiatorSignedTx.feerate == FeeratePerKw(262 sat))
     val nonInitiatorSignedTx = FullySignedSharedTransaction(nonInitiatorTx, nonInitiatorSigs, initiatorSigs, None)
-    assert(nonInitiatorSignedTx.feerate == FeeratePerKw(224 sat))
-    val signedTx = Transaction.read("02000000000103b932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430200000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430000000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430400000000fdffffff03e5effa02000000001600141ca1cca8855bad6bc1ea5436edd8cff10b7e448b1cf0fa020000000016001444cb0c39f93ecc372b5851725bd29d865d333b10f084420601000000220020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec50247304402207de9ba56bb9f641372e805782575ee840a899e61021c8b1572b3ec1d5b5950e9022069e9ba998915dae193d3c25cb89b5e64370e6a3a7755e7f31cf6d7cbc2a49f6d0121034695f5b7864c580bf11f9f8cb1a94eb336f2ce9ef872d2ae1a90ee276c772484022068656c6c6f2074686572652c2074686973206973206120626974636f6e2121212782012088a820add57dfe5277079d069ca4ad4893c96de91f88ffb981fdc6a2a34d5336c66aff870078000000")
+    assert(nonInitiatorSignedTx.feerate == FeeratePerKw(262 sat))
+    val signedTx = Transaction.read("02000000000102b932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430200000000fdffffffb932b0669cd0394d0d5bcc27e01ab8c511f1662a6799925b346c0cf18fca03430000000000fdffffff03e5effa02000000001600141ca1cca8855bad6bc1ea5436edd8cff10b7e448b1cf0fa020000000016001444cb0c39f93ecc372b5851725bd29d865d333b100084d71700000000220020297b92c238163e820b82486084634b4846b86a3c658d87b9384192e6bea98ec50247304402207de9ba56bb9f641372e805782575ee840a899e61021c8b1572b3ec1d5b5950e9022069e9ba998915dae193d3c25cb89b5e64370e6a3a7755e7f31cf6d7cbc2a49f6d0121034695f5b7864c580bf11f9f8cb1a94eb336f2ce9ef872d2ae1a90ee276c772484022068656c6c6f2074686572652c2074686973206973206120626974636f6e2121212782012088a820add57dfe5277079d069ca4ad4893c96de91f88ffb981fdc6a2a34d5336c66aff8778000000")
     assert(initiatorSignedTx.signedTx == signedTx)
     assert(initiatorSignedTx.signedTx == nonInitiatorSignedTx.signedTx)
   }

eclair-core/src/test/scala/fr/acinq/eclair/wire/internal/channel/version4/ChannelCodecs4Spec.scala

@@ -123,7 +123,7 @@ class ChannelCodecs4Spec extends AnyFunSuite {
     val fundingInput = InputInfo(OutPoint(randomBytes32(), 3), TxOut(175_000 sat, Script.pay2wpkh(randomKey().publicKey)), Nil)
     val fundingTx = SharedTransaction(
       sharedInput_opt = None,
-      sharedOutput = InteractiveTxBuilder.Output.Shared(UInt64(8), ByteVector.empty, 100_000_600 msat, 74_000_400 msat, 150_000_000 msat),
+      sharedOutput = InteractiveTxBuilder.Output.Shared(UInt64(8), ByteVector.empty, 100_000_600 msat, 74_000_400 msat),
       localInputs = Nil, remoteInputs = Nil,
       localOutputs = Nil, remoteOutputs = Nil,
       lockTime = 0