diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala index 2843d1b9fb..611306659a 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala @@ -44,7 +44,7 @@ import fr.acinq.eclair.remote.EclairInternalsSerializer.RemoteTypes import fr.acinq.eclair.router.Router import fr.acinq.eclair.wire.protocol import fr.acinq.eclair.wire.protocol.FailureMessageCodecs.createBadOnionFailure -import fr.acinq.eclair.wire.protocol.{AddFeeCredit, ChannelTlv, CurrentFeeCredit, Error, HasChannelId, HasTemporaryChannelId, LightningMessage, LiquidityAds, NodeAddress, OnTheFlyFundingFailureMessage, OnionMessage, OnionRoutingPacket, RecommendedFeerates, RoutingMessage, SpliceInit, TlvStream, TxAbort, UnknownMessage, Warning, WillAddHtlc, WillFailHtlc, WillFailMalformedHtlc} +import fr.acinq.eclair.wire.protocol.{AddFeeCredit, ChannelTlv, CurrentFeeCredit, Error, HasChannelId, HasTemporaryChannelId, LightningMessage, LiquidityAds, NodeAddress, OnTheFlyFundingFailureMessage, OnionMessage, OnionRoutingPacket, RecommendedFeerates, RoutingMessage, SpliceInit, TemporaryChannelFailure, TlvStream, TxAbort, UnknownMessage, Warning, WillAddHtlc, WillFailHtlc, WillFailMalformedHtlc} /** * This actor represents a logical peer. There is one [[Peer]] per unique remote node id at all time. @@ -276,8 +276,12 @@ class Peer(val nodeParams: NodeParams, proposal.createFulfillCommands(status.preimage).foreach { case (channelId, cmd) => PendingCommandsDb.safeSend(register, nodeParams.db.pendingCommands, channelId, cmd) } pending.copy(proposed = pending.proposed :+ proposal) case status: OnTheFlyFunding.Status.Funded => - log.info("received extra payment for on-the-fly funding that has already been funded with txId={} (payment_hash={}, amount={})", status.txId, cmd.paymentHash, cmd.amount) - pending.copy(proposed = pending.proposed :+ OnTheFlyFunding.Proposal(htlc, cmd.upstream)) + log.info("rejecting extra payment for on-the-fly funding that has already been funded with txId={} (payment_hash={}, amount={})", status.txId, cmd.paymentHash, cmd.amount) + // The payer is buggy and is paying the same payment_hash multiple times. We could simply claim that + // extra payment for ourselves, but we're nice and instead immediately fail it. + val proposal = OnTheFlyFunding.Proposal(htlc, cmd.upstream) + proposal.createFailureCommands(None).foreach { case (channelId, cmd) => PendingCommandsDb.safeSend(register, nodeParams.db.pendingCommands, channelId, cmd) } + pending } case None => self ! Peer.OutgoingMessage(htlc, d.peerConnection) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala index 7ec46939c6..b12316ea62 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala @@ -292,9 +292,7 @@ object OnTheFlyFunding { // This lets us detect that this HTLC is an on-the-fly funded HTLC. val htlcFees = LiquidityAds.FundingFee(remainingFees.min(p.maxFees(htlcMinimum)), cmd.status.txId) val origin = Origin.Hot(htlcSettledAdapter.toClassic, p.upstream) - // We only sign at the end of the whole batch. - val commit = p.htlc.id == cmd.proposed.last.htlc.id - val add = CMD_ADD_HTLC(cmdAdapter.toClassic, p.htlc.amount - htlcFees.amount, paymentHash, p.htlc.expiry, p.htlc.finalPacket, p.htlc.blinding_opt, 1.0, Some(htlcFees), origin, commit) + val add = CMD_ADD_HTLC(cmdAdapter.toClassic, p.htlc.amount - htlcFees.amount, paymentHash, p.htlc.expiry, p.htlc.finalPacket, p.htlc.blinding_opt, 1.0, Some(htlcFees), origin, commit = true) cmd.channel ! add remainingFees - htlcFees.amount } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/OnTheFlyFundingSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/OnTheFlyFundingSpec.scala index f538decb78..00ff39e944 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/OnTheFlyFundingSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/OnTheFlyFundingSpec.scala @@ -381,6 +381,7 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { proposeFunding(40_000_000 msat, CltvExpiry(515), paymentHash2, upstream2.head) signLiquidityPurchase(100_000 sat, LiquidityAds.PaymentDetails.FromFutureHtlc(paymentHash2 :: Nil)) proposeExtraFunding(50_000_000 msat, CltvExpiry(525), paymentHash2, upstream2.last) + register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]] // A third funding is signed coming from a trampoline payment. val paymentHash3 = randomBytes32() @@ -396,16 +397,16 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { val upstream4 = Upstream.Hot.Trampoline(List(upstreamChannel(60_000_000 msat, CltvExpiry(560), paymentHash4))) proposeFunding(50_000_000 msat, CltvExpiry(516), paymentHash4, upstream4) - // The first three proposals reach their CLTV expiry. + // The first three proposals reach their CLTV expiry (the extra htlc was already failed). peer ! CurrentBlockHeight(BlockHeight(515)) - val fwds = (0 until 6).map(_ => register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]) + val fwds = (0 until 5).map(_ => register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]) register.expectNoMessage(100 millis) fwds.foreach(fwd => { assert(fwd.message.reason == Right(UnknownNextPeer())) assert(fwd.message.commit) }) - assert(fwds.map(_.channelId).toSet == (upstream1 ++ upstream2 ++ upstream3.received).map(_.add.channelId).toSet) - assert(fwds.map(_.message.id).toSet == (upstream1 ++ upstream2 ++ upstream3.received).map(_.add.id).toSet) + assert(fwds.map(_.channelId).toSet == (upstream1 ++ upstream2.slice(0, 1) ++ upstream3.received).map(_.add.channelId).toSet) + assert(fwds.map(_.message.id).toSet == (upstream1 ++ upstream2.slice(0, 1) ++ upstream3.received).map(_.add.id).toSet) awaitCond(nodeParams.db.liquidity.listPendingOnTheFlyFunding(remoteNodeId).isEmpty, interval = 100 millis) } @@ -848,6 +849,7 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { val purchase = signLiquidityPurchase(200_000 sat, LiquidityAds.PaymentDetails.FromFutureHtlcWithPreimage(preimage :: Nil), channelId, fees, fundingTxIndex = 5, htlcMinimum) // We receive the last payment *after* signing the funding transaction. proposeExtraFunding(50_000_000 msat, expiryOut, paymentHash, upstream(2)) + register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]] // Once the splice with the right funding index is locked, we forward HTLCs matching the proposed will_add_htlc. val channelData = makeChannelData(htlcMinimum) @@ -858,17 +860,15 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { val adds1 = Seq( channel.expectMsgType[CMD_ADD_HTLC], channel.expectMsgType[CMD_ADD_HTLC], - channel.expectMsgType[CMD_ADD_HTLC], ) adds1.foreach(add => { assert(add.paymentHash == paymentHash) assert(add.fundingFee_opt.nonEmpty) assert(add.fundingFee_opt.get.fundingTxId == purchase.txId) }) - adds1.take(2).foreach(add => assert(!add.commit)) - assert(adds1.last.commit) + adds1.foreach(add => assert(add.commit)) assert(adds1.map(_.fundingFee_opt.get.amount).sum == fees.total.toMilliSatoshi) - assert(adds1.map(add => add.amount + add.fundingFee_opt.get.amount).sum == 160_000_000.msat) + assert(adds1.map(add => add.amount + add.fundingFee_opt.get.amount).sum == 110_000_000.msat) channel.expectNoMessage(100 millis) // The recipient fails the payments: we don't relay the failure upstream and will retry. @@ -887,7 +887,6 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { val adds2 = Seq( channel.expectMsgType[CMD_ADD_HTLC], channel.expectMsgType[CMD_ADD_HTLC], - channel.expectMsgType[CMD_ADD_HTLC], ) adds2.foreach(add => add.replyTo ! RES_SUCCESS(add, purchase.channelId)) channel.expectNoMessage(100 millis) @@ -900,9 +899,8 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike { val fwds = Seq( register.expectMsgType[Register.Forward[CMD_FULFILL_HTLC]], register.expectMsgType[Register.Forward[CMD_FULFILL_HTLC]], - register.expectMsgType[Register.Forward[CMD_FULFILL_HTLC]], ) - val (channelsIn, htlcsIn) = upstream.flatMap { + val (channelsIn, htlcsIn) = upstream.take(2).flatMap { case u: Hot.Channel => Seq(u) case u: Hot.Trampoline => u.received case _: Upstream.Local => Nil