diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisher.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisher.scala index f18ca3574b..4fcc308584 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisher.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisher.scala @@ -51,6 +51,7 @@ object ReplaceableTxPublisher { private case object TimeLocksOk extends Command private case object CommitTxAlreadyConfirmed extends RuntimeException with Command private case object RemoteCommitTxPublished extends RuntimeException with Command + private case object RemoteCommitTxConfirmed extends Command private case object PreconditionsOk extends Command private case class FundingFailed(reason: Throwable) extends Command private case class SignFundedTx(tx: ReplaceableTransactionWithInputInfo, fee: Satoshi) extends Command @@ -178,23 +179,12 @@ private class ReplaceableTxPublisher(nodeParams: NodeParams, def start(): Behavior[Command] = { Behaviors.receiveMessagePartial { - case Publish(replyTo, cmd, targetFeerate) => checkTimeLocks(replyTo, cmd, targetFeerate) - case Stop => Behaviors.stopped - } - } - - def checkTimeLocks(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, targetFeerate: FeeratePerKw): Behavior[Command] = { - val timeLocksChecker = context.spawn(TxTimeLocksMonitor(nodeParams, watcher, loggingInfo), "time-locks-monitor") - timeLocksChecker ! CheckTx(context.messageAdapter[TxTimeLocksMonitor.TimeLocksOk](_ => TimeLocksOk), cmd.txInfo.tx, cmd.desc) - Behaviors.receiveMessagePartial { - case TimeLocksOk => + case Publish(replyTo, cmd, targetFeerate) => cmd.txInfo match { case _: ClaimLocalAnchorOutputTx => checkAnchorPreconditions(replyTo, cmd, targetFeerate) case htlcTx: HtlcTx => checkHtlcPreconditions(replyTo, cmd, htlcTx, targetFeerate) } - case Stop => - timeLocksChecker ! TxTimeLocksMonitor.Stop - Behaviors.stopped + case Stop => Behaviors.stopped } } @@ -243,23 +233,48 @@ private class ReplaceableTxPublisher(nodeParams: NodeParams, } def checkHtlcPreconditions(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, htlcTx: HtlcTx, targetFeerate: FeeratePerKw): Behavior[Command] = { - val htlcFeerate = cmd.commitments.localCommit.spec.htlcTxFeerate(cmd.commitments.commitmentFormat) - // HTLC transactions have a 1-block relative delay when using anchor outputs, which ensures our commit tx has already - // been confirmed (we don't need to check again here). - HtlcTxAndWitnessData(htlcTx, cmd.commitments) match { - case Some(txWithWitnessData) if targetFeerate <= htlcFeerate => - val channelKeyPath = keyManager.keyPath(cmd.commitments.localParams, cmd.commitments.channelConfig) - val localPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, cmd.commitments.localCommit.index) - val localHtlcBasepoint = keyManager.htlcPoint(channelKeyPath) - val localSig = keyManager.sign(htlcTx, localHtlcBasepoint, localPerCommitmentPoint, TxOwner.Local, cmd.commitments.commitmentFormat) - val signedHtlcTx = txWithWitnessData.addSigs(localSig, cmd.commitments.commitmentFormat) - log.info("publishing {} without adding inputs: txid={}", htlcTx.desc, signedHtlcTx.tx.txid) - publish(replyTo, cmd, signedHtlcTx.tx, htlcTx.fee) - case Some(_) => - fund(replyTo, cmd, targetFeerate) - case None => - log.error("witness data not found for htlcId={}, skipping...", htlcTx.htlcId) - sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false))) + // We verify that: + // - their commit is not confirmed: if it is, there is no need to publish our HTLC transactions + context.pipeToSelf(bitcoinClient.getTxConfirmations(cmd.commitments.remoteCommit.txid)) { + case Success(Some(depth)) if depth >= nodeParams.minDepthBlocks => RemoteCommitTxConfirmed + case Success(_) => PreconditionsOk + case Failure(_) => PreconditionsOk // if our checks fail, we don't want it to prevent us from publishing HTLC transactions + } + Behaviors.receiveMessagePartial { + case PreconditionsOk => + HtlcTxAndWitnessData(htlcTx, cmd.commitments) match { + case Some(txWithWitnessData) => checkTimeLocks(replyTo, cmd, txWithWitnessData, targetFeerate) + case None => + log.error("witness data not found for htlcId={}, skipping...", htlcTx.htlcId) + sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false))) + } + case RemoteCommitTxConfirmed => + log.warn("cannot publish {}: remote commit has been confirmed", cmd.desc) + sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.ConflictingTxConfirmed)) + case Stop => Behaviors.stopped + } + } + + def checkTimeLocks(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, htlcTxWithWitnessData: HtlcTxAndWitnessData, targetFeerate: FeeratePerKw): Behavior[Command] = { + val timeLocksChecker = context.spawn(TxTimeLocksMonitor(nodeParams, watcher, loggingInfo), "time-locks-monitor") + timeLocksChecker ! CheckTx(context.messageAdapter[TxTimeLocksMonitor.TimeLocksOk](_ => TimeLocksOk), cmd.txInfo.tx, cmd.desc) + Behaviors.receiveMessagePartial { + case TimeLocksOk => + val htlcFeerate = cmd.commitments.localCommit.spec.htlcTxFeerate(cmd.commitments.commitmentFormat) + if (targetFeerate <= htlcFeerate) { + val channelKeyPath = keyManager.keyPath(cmd.commitments.localParams, cmd.commitments.channelConfig) + val localPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, cmd.commitments.localCommit.index) + val localHtlcBasepoint = keyManager.htlcPoint(channelKeyPath) + val localSig = keyManager.sign(htlcTxWithWitnessData.txInfo, localHtlcBasepoint, localPerCommitmentPoint, TxOwner.Local, cmd.commitments.commitmentFormat) + val signedHtlcTx = htlcTxWithWitnessData.addSigs(localSig, cmd.commitments.commitmentFormat) + log.info("publishing {} without adding inputs: txid={}", cmd.desc, signedHtlcTx.tx.txid) + publish(replyTo, cmd, signedHtlcTx.tx, signedHtlcTx.fee) + } else { + fund(replyTo, cmd, targetFeerate) + } + case Stop => + timeLocksChecker ! TxTimeLocksMonitor.Stop + Behaviors.stopped } } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala index 202ccc78a3..7cf235afb2 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala @@ -29,7 +29,7 @@ import fr.acinq.eclair.blockchain.bitcoind.rpc.{BitcoinCoreClient, BitcoinJsonRP import fr.acinq.eclair.blockchain.fee.FeeratePerKw import fr.acinq.eclair.channel._ import fr.acinq.eclair.channel.publish.ReplaceableTxPublisher.{Publish, Stop} -import fr.acinq.eclair.channel.publish.TxPublisher.TxRejectedReason.{ConflictingTxUnconfirmed, CouldNotFund, TxSkipped, WalletInputGone} +import fr.acinq.eclair.channel.publish.TxPublisher.TxRejectedReason._ import fr.acinq.eclair.channel.publish.TxPublisher._ import fr.acinq.eclair.channel.states.{ChannelStateTestsHelperMethods, ChannelStateTestsTags} import fr.acinq.eclair.transactions.Transactions.{ClaimLocalAnchorOutputTx, HtlcSuccessTx, HtlcTimeoutTx} @@ -446,6 +446,53 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w }) } + test("remote commit tx confirmed, not publishing htlc tx") { + withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx, f => { + import f._ + + // Add htlcs in both directions and ensure that preimages are available. + addHtlc(5_000_000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + val (r, htlc) = addHtlc(4_000_000 msat, bob, alice, bob2alice, alice2bob) + crossSign(bob, alice, bob2alice, alice2bob) + probe.send(alice, CMD_FULFILL_HTLC(htlc.id, r, replyTo_opt = Some(probe.ref))) + probe.expectMsgType[CommandSuccess[CMD_FULFILL_HTLC]] + + // Force-close channel. + probe.send(alice, CMD_FORCECLOSE(probe.ref)) + probe.expectMsgType[CommandSuccess[CMD_FORCECLOSE]] + alice2blockchain.expectMsgType[PublishRawTx] + assert(alice2blockchain.expectMsgType[PublishReplaceableTx].txInfo.isInstanceOf[ClaimLocalAnchorOutputTx]) + alice2blockchain.expectMsgType[PublishRawTx] // claim main output + val htlcSuccess = alice2blockchain.expectMsgType[PublishReplaceableTx] + assert(htlcSuccess.txInfo.isInstanceOf[HtlcSuccessTx]) + val htlcTimeout = alice2blockchain.expectMsgType[PublishReplaceableTx] + assert(htlcTimeout.txInfo.isInstanceOf[HtlcTimeoutTx]) + + // Ensure remote commit tx confirms. + val remoteCommitTx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.fullySignedLocalCommitTx(bob.underlyingActor.nodeParams.channelKeyManager) + wallet.publishTransaction(remoteCommitTx.tx).pipeTo(probe.ref) + probe.expectMsg(remoteCommitTx.tx.txid) + generateBlocks(5) + + // Verify that HTLC transactions immediately fail to publish. + val targetFeerate = FeeratePerKw(15_000 sat) + val htlcSuccessPublisher = createPublisher() + htlcSuccessPublisher ! Publish(probe.ref, htlcSuccess, targetFeerate) + val result1 = probe.expectMsgType[TxRejected] + assert(result1.cmd === htlcSuccess) + assert(result1.reason === ConflictingTxConfirmed) + htlcSuccessPublisher ! Stop + + val htlcTimeoutPublisher = createPublisher() + htlcTimeoutPublisher ! Publish(probe.ref, htlcTimeout, targetFeerate) + val result2 = probe.expectMsgType[TxRejected] + assert(result2.cmd === htlcTimeout) + assert(result2.reason === ConflictingTxConfirmed) + htlcTimeoutPublisher ! Stop + }) + } + def closeChannelWithHtlcs(f: Fixture): (Transaction, PublishReplaceableTx, PublishReplaceableTx) = { import f._