From ffb86c947b2782c603c0d7a0ba51384151686fee Mon Sep 17 00:00:00 2001 From: t-bast Date: Mon, 10 Jun 2024 10:25:58 +0200 Subject: [PATCH] Allow underpaying feerate when using future HTLCs When an interactive-tx session is created for a liquidity purchase that uses future HTLCs to pay fees, the initiator may not have enough funds to honor the target feerate. We allow the transaction anyway, because we want to get paid for the liquidity we're providing. If the feerate is too low and the transaction doesn't confirm, we can double-spend it if we need that liquidity elsewhere. --- .../channel/fund/InteractiveTxBuilder.scala | 19 +++++- .../channel/InteractiveTxBuilderSpec.scala | 65 +++++++++++++++++-- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala index 7ab6f0584f..44862124ad 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala @@ -738,7 +738,18 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon return Left(InvalidCompleteInteractiveTx(fundingParams.channelId)) } case None => - val minimumFee = Transactions.weight2fee(fundingParams.targetFeerate, tx.weight()) + val feeWithoutWitness = Transactions.weight2fee(fundingParams.targetFeerate, tx.weight()) + val minimumFee = liquidityLease_opt.map(_.paymentDetails) match { + case Some(paymentDetails) => paymentDetails match { + case PaymentDetails.FromChannelBalance => feeWithoutWitness + // We allow the feerate to be lower than requested when using on-the-fly funding, because our peer may not + // be able to contribute as much as expected to the funding transaction itself since they don't have funds. + // It's acceptable because they will be paying liquidity fees from future HTLCs. + case PaymentDetails.FromFutureHtlc(_) => feeWithoutWitness * 0.5 + case PaymentDetails.FromFutureHtlcWithPreimage(_) => feeWithoutWitness * 0.5 + } + case None => feeWithoutWitness + } if (sharedTx.fees < minimumFee) { log.warn("invalid interactive tx: below the target feerate (target={}, actual={})", fundingParams.targetFeerate, Transactions.fee2rate(sharedTx.fees, tx.weight())) return Left(InvalidCompleteInteractiveTx(fundingParams.channelId)) @@ -948,7 +959,11 @@ object InteractiveTxSigningSession { return Left(InvalidFundingSignature(fundingParams.channelId, Some(partiallySignedTx.txId))) } // We allow a 5% error margin since witness size prediction could be inaccurate. - if (fundingParams.localContribution != 0.sat && txWithSigs.feerate < fundingParams.targetFeerate * 0.95) { + // If they didn't contribute to the transaction, they're not responsible, so we don't check the feerate. + // If we didn't contribute to the transaction, we don't care if they use a lower feerate than expected. + val localContributed = txWithSigs.tx.localInputs.nonEmpty || txWithSigs.tx.localOutputs.nonEmpty + val remoteContributed = txWithSigs.tx.remoteInputs.nonEmpty || txWithSigs.tx.remoteOutputs.nonEmpty + if (localContributed && remoteContributed && txWithSigs.feerate < fundingParams.targetFeerate * 0.95) { return Left(InvalidFundingFeerate(fundingParams.channelId, fundingParams.targetFeerate, txWithSigs.feerate)) } val previousOutputs = { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala index caffd0cbcd..4c4fcd04e0 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala @@ -211,11 +211,11 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit } } - private def createFixtureParams(fundingAmountA: Satoshi, fundingAmountB: Satoshi, targetFeerate: FeeratePerKw, dustLimit: Satoshi, lockTime: Long, requireConfirmedInputs: RequireConfirmedInputs = RequireConfirmedInputs(forLocal = false, forRemote = false)): FixtureParams = { + private def createFixtureParams(fundingAmountA: Satoshi, fundingAmountB: Satoshi, targetFeerate: FeeratePerKw, dustLimit: Satoshi, lockTime: Long, requireConfirmedInputs: RequireConfirmedInputs = RequireConfirmedInputs(forLocal = false, forRemote = false), nonInitiatorPaysCommitTxFees: Boolean = false): FixtureParams = { val channelFeatures = ChannelFeatures(ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), Features[InitFeature](Features.DualFunding -> FeatureSupport.Optional), Features[InitFeature](Features.DualFunding -> FeatureSupport.Optional), announceChannel = true) val Seq(nodeParamsA, nodeParamsB) = Seq(TestConstants.Alice.nodeParams, TestConstants.Bob.nodeParams).map(_.copy(features = Features(channelFeatures.features.map(f => f -> FeatureSupport.Optional).toMap[Feature, FeatureSupport]))) - val localParamsA = makeChannelParams(nodeParamsA, nodeParamsA.features.initFeatures(), None, None, isChannelOpener = true, dualFunded = true, fundingAmountA, unlimitedMaxHtlcValueInFlight = false) - val localParamsB = makeChannelParams(nodeParamsB, nodeParamsB.features.initFeatures(), None, None, isChannelOpener = false, dualFunded = true, fundingAmountB, unlimitedMaxHtlcValueInFlight = false) + val localParamsA = makeChannelParams(nodeParamsA, nodeParamsA.features.initFeatures(), None, None, isChannelOpener = true, dualFunded = true, fundingAmountA, unlimitedMaxHtlcValueInFlight = false).copy(payCommitTxFees = !nonInitiatorPaysCommitTxFees) + val localParamsB = makeChannelParams(nodeParamsB, nodeParamsB.features.initFeatures(), None, None, isChannelOpener = false, dualFunded = true, fundingAmountB, unlimitedMaxHtlcValueInFlight = false).copy(payCommitTxFees = nonInitiatorPaysCommitTxFees) val Seq(remoteParamsA, remoteParamsB) = Seq((nodeParamsA, localParamsA), (nodeParamsB, localParamsB)).map { case (nodeParams, localParams) => @@ -287,7 +287,7 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit utxosB.foreach(amount => addUtxo(walletB, amount, probe)) generateBlocks(1) - val fixtureParams = createFixtureParams(fundingAmountA, fundingAmountB, targetFeerate, dustLimit, lockTime, requireConfirmedInputs) + val fixtureParams = createFixtureParams(fundingAmountA, fundingAmountB, targetFeerate, dustLimit, lockTime, requireConfirmedInputs, nonInitiatorPaysCommitTxFees = lease_opt.nonEmpty) val alice = fixtureParams.spawnTxBuilderAlice(walletA, lease_opt = lease_opt) val bob = fixtureParams.spawnTxBuilderBob(walletB, lease_opt = lease_opt) testFun(Fixture(alice, bob, fixtureParams, walletA, rpcClientA, walletB, rpcClientB, TestProbe(), TestProbe())) @@ -564,6 +564,59 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit } } + test("initiator does not contribute -- on-the-fly funding") { + val targetFeerate = FeeratePerKw(5000 sat) + val fundingB = 150_000.sat + val utxosB = Seq(200_000 sat) + // When on-the-fly funding is used, the initiator may not contribute to the funding transaction. + // It will receive HTLCs later that use the purchased inbound liquidity, and liquidity fees will be deduced from those HTLCs. + val lease = LiquidityAds.Lease(fundingB, LiquidityAds.LeaseFees(2500 sat, 7500 sat), LiquidityAds.PaymentDetails.FromFutureHtlc(Nil), ByteVector64.Zeroes, LiquidityAds.FundingLeaseWitness.Basic(hex"deadbeef")) + withFixture(0 sat, Nil, fundingB, utxosB, targetFeerate, 330 sat, 0, RequireConfirmedInputs(forLocal = false, forRemote = false), Some(lease)) { f => + import f._ + + alice ! Start(alice2bob.ref) + bob ! Start(bob2alice.ref) + + // Alice --- tx_add_output --> Bob + fwd.forwardAlice2Bob[TxAddOutput] + // Alice <-- tx_add_input --- Bob + fwd.forwardBob2Alice[TxAddInput] + // Alice --- tx_complete --> Bob + fwd.forwardAlice2Bob[TxComplete] + // Alice <-- tx_add_output --- Bob + fwd.forwardBob2Alice[TxAddOutput] + // Alice --- tx_complete --> Bob + fwd.forwardAlice2Bob[TxComplete] + // Alice <-- tx_complete --- Bob + fwd.forwardBob2Alice[TxComplete] + + // Alice is responsible for adding the shared output, but Bob is paying for everything. + assert(aliceParams.fundingAmount == fundingB) + + // Alice sends signatures first as she did not contribute at all. + val successA = alice2bob.expectMsgType[Succeeded] + val successB = bob2alice.expectMsgType[Succeeded] + val (txA, _, txB, commitmentB) = fixtureParams.exchangeSigsAliceFirst(aliceParams, successA, successB) + // Alice doesn't pay any fees to Bob during the interactive-tx, fees will be paid from future HTLCs. + assert(commitmentB.localCommit.spec.toLocal == fundingB.toMilliSatoshi) + + // The resulting transaction is valid but has a lower feerate than expected. + assert(txA.txId == txB.txId) + assert(txA.tx.localAmountIn == 0.msat) + assert(txA.tx.localFees == 0.msat) + assert(txB.tx.remoteAmountIn == 0.msat) + assert(txB.tx.remoteFees == 0.msat) + assert(txB.tx.localFees > 0.msat) + val probe = TestProbe() + walletA.publishTransaction(txA.signedTx).pipeTo(probe.ref) + probe.expectMsg(txA.txId) + walletA.getMempoolTx(txA.txId).pipeTo(probe.ref) + val mempoolTx = probe.expectMsgType[MempoolTx] + assert(mempoolTx.fees == txA.tx.fees) + assert(targetFeerate * 0.5 <= txA.feerate && txA.feerate < targetFeerate, s"unexpected feerate (target=$targetFeerate actual=${txA.feerate})") + } + } + test("initiator and non-initiator splice-in") { val targetFeerate = FeeratePerKw(1000 sat) // We chose those amounts to ensure that Bob always signs first: @@ -2193,6 +2246,10 @@ class InteractiveTxBuilderSpec extends TestKitBaseClass with AnyFunSuiteLike wit val bobSplice = params.spawnTxBuilderSpliceBob(spliceParams, previousCommitment, wallet, Some(lease)) bobSplice ! Start(probe.ref) assert(probe.expectMsgType[LocalFailure].cause == InvalidFundingBalances(params.channelId, 620_000 sat, 625_000_000 msat, -5_000_000 msat)) + // If we use a payment type where fees are paid outside of the interactive-tx session, the funding attempt is valid. + val bobFutureHtlc = params.spawnTxBuilderBob(wallet, params.fundingParamsB, Some(lease.copy(paymentDetails = LiquidityAds.PaymentDetails.FromFutureHtlc(Nil)))) + bobFutureHtlc ! Start(probe.ref) + probe.expectNoMessage(100 millis) } test("invalid input") {