Better feerate estimation without change outputs

When asking bitcoind to fund the transaction, we always include the
shared input and output in the transaction even when we're not
the initiator, and later deduce the corresponding fees from our change
output. But if bitcoind doesn't add a change output, we end up paying
a lot more fees than the targeted feerate.

In the special cases where we're responding to a liquidity ads, we
don't need to include the shared input in the tx construction,
which reduces the potential overpayment.

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

@@ -141,6 +141,8 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
     case _ => Nil
   }
 
+  private val spliceInOnly = fundingParams.sharedInput_opt.nonEmpty && fundingParams.localContribution > 0.sat && fundingParams.localOutputs.isEmpty
+
   def start(): Behavior[Command] = {
     // We always double-spend all our previous inputs. It's technically overkill because we only really need to double
     // spend one input of each previous tx, but it's simpler and less error-prone this way. It also ensures that in
@@ -169,10 +171,21 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
         replyTo ! fundingContributions
         Behaviors.stopped
       }
+    } else if (!fundingParams.isInitiator && spliceInOnly) {
+      // We are splicing funds in without being the initiator (most likely responding to a liquidity ads).
+      // We don't need to include the shared input, the other node will pay for its weight.
+      // We create a dummy shared output with the amount we want to splice in, and bitcoind will make sure we match that
+      // amount.
+      val sharedTxOut = TxOut(fundingParams.localContribution, fundingPubkeyScript)
+      val previousWalletTxIn = previousWalletInputs.map(i => TxIn(i.outPoint, ByteVector.empty, i.sequence))
+      val dummyTx = Transaction(2, previousWalletTxIn, Seq(sharedTxOut), fundingParams.lockTime)
+      fund(dummyTx, previousWalletInputs, Set.empty)
     } else {
       // The shared input contains funds that belong to us *and* funds that belong to our peer, so we add the previous
       // funding amount to our shared output to make sure bitcoind adds what is required for our local contribution.
       // We always include the shared input in our transaction and will let bitcoind make sure the target feerate is reached.
+      // We will later subtract the fees for that input to ensure we don't overshoot the feerate: however, if bitcoind
+      // doesn't add a change output, we won't be able to do so and will overpay miner fees.
       // Note that if the shared output amount is smaller than the dust limit, bitcoind will reject the funding attempt.
       val sharedTxOut = TxOut(purpose.previousFundingAmount + fundingParams.localContribution, fundingPubkeyScript)
       val sharedTxIn = fundingParams.sharedInput_opt.toSeq.map(sharedInput => TxIn(sharedInput.info.outPoint, ByteVector.empty, 0xfffffffdL))
@@ -249,13 +262,16 @@ private class InteractiveTxFunder(replyTo: ActorRef[InteractiveTxFunder.Response
             // By using bitcoind's fundrawtransaction we are currently paying fees for those fields, but we can fix that
             // by increasing our change output accordingly.
             // If we don't have a change output, we will slightly overpay the fees: fixing this is not worth the extra
-            // complexity of adding a change output, which would require a call to bitcoind to get a change address.
+            // complexity of adding a change output, which would require a call to bitcoind to get a change address and
+            // create a tiny change output that would most likely be unusable and costly to spend.
             val outputs = changeOutput_opt match {
               case Some(changeOutput) =>
                 val txWeightWithoutInput = Transaction(2, Nil, Seq(TxOut(fundingParams.fundingAmount, fundingPubkeyScript)), 0).weight()
                 val commonWeight = fundingParams.sharedInput_opt match {
-                  case Some(sharedInput) => sharedInput.weight + txWeightWithoutInput
-                  case None => txWeightWithoutInput
+                  // If we are only splicing in, we didn't include the shared input in the funding transaction, but
+                  // otherwise we did and must thus claim the corresponding fee back.
+                  case Some(sharedInput) if !spliceInOnly => sharedInput.weight + txWeightWithoutInput
+                  case _ => txWeightWithoutInput
                 }
                 val overpaidFees = Transactions.weight2fee(fundingParams.targetFeerate, commonWeight)
                 nonChangeOutputs :+ changeOutput.copy(amount = changeOutput.amount + overpaidFees)

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

@@ -23,7 +23,7 @@ import akka.testkit.TestProbe
 import com.softwaremill.quicklens.{ModifyPimp, QuicklensAt}
 import fr.acinq.bitcoin.psbt.Psbt
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
-import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, ByteVector64, OP_1, OutPoint, Satoshi, SatoshiLong, Script, ScriptWitness, Transaction, TxHash, TxId, TxOut, addressToPublicKeyScript}
+import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, ByteVector64, OP_1, OutPoint, Satoshi, SatoshiLong, Script, ScriptWitness, Transaction, TxHash, TxId, TxIn, TxOut, addressToPublicKeyScript}
 import fr.acinq.eclair.TestUtils.randomTxId
 import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, ProcessPsbtResponse}
 import fr.acinq.eclair.blockchain.bitcoind.BitcoindService
@@ -34,8 +34,8 @@ import fr.acinq.eclair.blockchain.{OnChainWallet, SingleKeyOnChainWallet}
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
 import fr.acinq.eclair.channel.fund.{InteractiveTxBuilder, InteractiveTxSigningSession}
 import fr.acinq.eclair.io.OpenChannelInterceptor.makeChannelParams
-import fr.acinq.eclair.transactions.Scripts
 import fr.acinq.eclair.transactions.Transactions.InputInfo
+import fr.acinq.eclair.transactions.{Scripts, Transactions}
 import fr.acinq.eclair.wire.protocol._
 import fr.acinq.eclair.{Feature, FeatureSupport, Features, InitFeature, MilliSatoshiLong, NodeParams, TestConstants, TestKitBaseClass, ToMilliSatoshiConversion, UInt64, randomBytes32, randomKey}
 import org.scalatest.BeforeAndAfterAll
@@ -1612,6 +1612,85 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit
     }
   }
 
+  test("fund splice transaction from non-initiator without change output") {
+    val targetFeerate = FeeratePerKw(10_000 sat)
+    val fundingA = 100_000 sat
+    val utxosA = Seq(150_000 sat)
+    val fundingB = 92_000 sat
+    val utxosB = Seq(50_000 sat, 50_000 sat, 50_000 sat, 50_000 sat)
+    withFixture(fundingA, utxosA, fundingB, utxosB, targetFeerate, 660 sat, 0, RequireConfirmedInputs(forLocal = false, forRemote = false)) { f =>
+      import f._
+
+      val probe = TestProbe()
+      alice ! Start(alice2bob.ref)
+      bob ! Start(bob2alice.ref)
+
+      // Alice --- tx_add_input --> Bob
+      fwd.forwardAlice2Bob[TxAddInput]
+      // Alice <-- tx_add_input --- Bob
+      fwd.forwardBob2Alice[TxAddInput]
+      // Alice --- tx_add_output --> Bob
+      fwd.forwardAlice2Bob[TxAddOutput]
+      // Alice <-- tx_add_input --- Bob
+      fwd.forwardBob2Alice[TxAddInput]
+      // Alice --- tx_add_output --> Bob
+      fwd.forwardAlice2Bob[TxAddOutput]
+      // Alice <-- tx_complete --- Bob
+      fwd.forwardBob2Alice[TxComplete]
+      // Alice --- tx_complete --> Bob
+      fwd.forwardAlice2Bob[TxComplete]
+
+      val successA1 = alice2bob.expectMsgType[Succeeded]
+      val successB1 = bob2alice.expectMsgType[Succeeded]
+      val (txA1, commitmentA1, _, commitmentB1) = fixtureParams.exchangeSigsBobFirst(bobParams, successA1, successB1)
+      assert(targetFeerate * 0.9 <= txA1.feerate && txA1.feerate <= targetFeerate * 1.25)
+      walletA.publishTransaction(txA1.signedTx).pipeTo(probe.ref)
+      probe.expectMsg(txA1.txId)
+
+      // Alice initiates a splice that is only funded by Bob (e.g. liquidity ads).
+      // Alice pays fees for the common fields of the transaction, by decreasing her balance in the shared output.
+      val spliceFeeA = {
+        val dummySpliceTx = Transaction(
+          version = 2,
+          txIn = Seq(TxIn(commitmentA1.commitInput.outPoint, ByteVector.empty, 0, Scripts.witness2of2(Transactions.PlaceHolderSig, Transactions.PlaceHolderSig, Transactions.PlaceHolderPubKey, Transactions.PlaceHolderPubKey))),
+          txOut = Seq(commitmentA1.commitInput.txOut),
+          lockTime = 0
+        )
+        Transactions.weight2fee(targetFeerate, dummySpliceTx.weight())
+      }
+      val (sharedInputA, sharedInputB) = sharedInputs(commitmentA1, commitmentB1)
+      val spliceFixtureParams = fixtureParams.createSpliceFixtureParams(fundingTxIndex = 1, fundingAmountA = -spliceFeeA, fundingAmountB = fundingB, targetFeerate, aliceParams.dustLimit, aliceParams.lockTime, sharedInputA = sharedInputA, sharedInputB = sharedInputB, spliceOutputsA = Nil, spliceOutputsB = Nil, requireConfirmedInputs = aliceParams.requireConfirmedInputs)
+      val fundingParamsA1 = spliceFixtureParams.fundingParamsA
+      val fundingParamsB1 = spliceFixtureParams.fundingParamsB
+      val aliceSplice = fixtureParams.spawnTxBuilderSpliceAlice(fundingParamsA1, commitmentA1, walletA)
+      val bobSplice = fixtureParams.spawnTxBuilderSpliceBob(fundingParamsB1, commitmentB1, walletB)
+      val fwdSplice = TypeCheckedForwarder(aliceSplice, bobSplice, alice2bob, bob2alice)
+
+      aliceSplice ! Start(alice2bob.ref)
+      bobSplice ! Start(bob2alice.ref)
+
+      // Alice --- tx_add_input --> Bob
+      fwdSplice.forwardAlice2Bob[TxAddInput]
+      // Alice <-- tx_add_input --- Bob
+      fwdSplice.forwardBob2Alice[TxAddInput]
+      // Alice --- tx_add_output --> Bob
+      fwdSplice.forwardAlice2Bob[TxAddOutput]
+      // Alice <-- tx_add_input --- Bob
+      fwdSplice.forwardBob2Alice[TxAddInput]
+      // Alice --- tx_complete --> Bob
+      fwdSplice.forwardAlice2Bob[TxComplete]
+      // Alice <-- tx_complete --- Bob
+      fwdSplice.forwardBob2Alice[TxComplete]
+
+      val successA2 = alice2bob.expectMsgType[Succeeded]
+      val successB2 = bob2alice.expectMsgType[Succeeded]
+      val (spliceTxA1, _, _, _) = fixtureParams.exchangeSigsBobFirst(fundingParamsB1, successA2, successB2)
+      assert(targetFeerate * 0.9 <= spliceTxA1.feerate && spliceTxA1.feerate <= targetFeerate * 1.25)
+      walletA.publishTransaction(spliceTxA1.signedTx).pipeTo(probe.ref)
+      probe.expectMsg(spliceTxA1.txId)
+    }
+  }
+
   test("funding splice transaction with previous inputs (different balance)") {
     val targetFeerate = FeeratePerKw(2_500 sat)
     val fundingA1 = 100_000 sat