From 5d6a1db9fb39ed004a157fe79aa6aed748df8ed9 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Thu, 22 Feb 2024 09:21:13 +0100 Subject: [PATCH] Skip anchor tx when remote commit has been evicted (#2830) When we detect that the remote commit has been published, we spend our anchor output from that commit if the fees are too low and we have funds at risk. But if the remote commit is then evicted from our mempool, we cannot publish our anchor tx and must instead skip it, since we don't provide the fully signed remote commitment to the publisher. We otherwise error when calling `fundrawtransaction`, where `bitcoind` fails because it cannot find the external non-wallet utxo. This change gets rid of those errors that can be quite confusing for node operators. --- .../publish/ReplaceableTxPrePublisher.scala | 13 +++++++++++-- .../publish/ReplaceableTxPublisherSpec.scala | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala index bb12bf30f..7847682c6 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala @@ -48,6 +48,7 @@ object ReplaceableTxPrePublisher { private case object ParentTxOk extends Command private case object FundingTxNotFound extends RuntimeException with Command private case object CommitTxAlreadyConfirmed extends RuntimeException with Command + private case object RemoteCommitTxNotInMempool extends RuntimeException with Command private case object LocalCommitTxConfirmed extends Command private case object RemoteCommitTxConfirmed extends Command private case object RemoteCommitTxPublished extends Command @@ -139,8 +140,11 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, val remoteCommits = Set(Some(cmd.commitment.remoteCommit.txid), cmd.commitment.nextRemoteCommit_opt.map(_.commit.txid)).flatten if (remoteCommits.contains(localAnchorTx.input.outPoint.txid)) { // We're trying to bump the remote commit tx: we must make sure it is in our mempool first. - // If it isn't, we will publish our local commit tx instead. - bitcoinClient.getMempoolTx(localAnchorTx.input.outPoint.txid).map(_.txid) + bitcoinClient.getMempoolTx(localAnchorTx.input.outPoint.txid).map(_.txid).transformWith { + // We could improve this: we've seen the remote commit in our mempool at least once, so we could try to republish it ourselves. + case Failure(_) => Future.failed(RemoteCommitTxNotInMempool) + case Success(remoteCommitTxId) => Future.successful(remoteCommitTxId) + } } else { // We must ensure our local commit tx is in the mempool before publishing the anchor transaction. // If it's already published, this call will be a no-op. @@ -155,6 +159,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, case Success(_) => ParentTxOk case Failure(FundingTxNotFound) => FundingTxNotFound case Failure(CommitTxAlreadyConfirmed) => CommitTxAlreadyConfirmed + case Failure(RemoteCommitTxNotInMempool) => RemoteCommitTxNotInMempool case Failure(reason) if reason.getMessage.contains("rejecting replacement") => RemoteCommitTxPublished case Failure(reason) => UnknownFailure(reason) } @@ -170,6 +175,10 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams, log.debug("commit tx is already confirmed, no need to claim our anchor") replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false)) Behaviors.stopped + case RemoteCommitTxNotInMempool => + log.debug("remote commit tx cannot be found in our mempool: we can't spend our anchor") + replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = true)) + Behaviors.stopped case RemoteCommitTxPublished => log.warn("cannot publish commit tx: there is a conflicting tx in the mempool") // We retry until that conflicting commit tx is confirmed or we're able to publish our local commit tx. 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 568723d01..5c5b6ac84 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 @@ -571,6 +571,25 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w } } + test("remote commit tx not published, not spending remote anchor output") { + withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f => + import f._ + + val commitTx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.fullySignedLocalCommitTx(bob.underlyingActor.nodeParams.channelKeyManager).tx + // Note that we don't publish the remote commit, to simulate the case where the watch triggers but the remote commit is then evicted from our mempool. + probe.send(alice, WatchFundingSpentTriggered(commitTx)) + val publishAnchor = alice2blockchain.expectMsgType[PublishReplaceableTx] + assert(publishAnchor.txInfo.input.outPoint.txid == commitTx.txid) + assert(publishAnchor.txInfo.isInstanceOf[ClaimLocalAnchorOutputTx]) + val anchorTx = publishAnchor.copy(txInfo = publishAnchor.txInfo.asInstanceOf[ClaimLocalAnchorOutputTx].copy(confirmationTarget = ConfirmationTarget.Absolute(aliceBlockHeight() + 6))) + publisher ! Publish(probe.ref, anchorTx) + + val result = probe.expectMsgType[TxRejected] + assert(result.cmd == anchorTx) + assert(result.reason == TxSkipped(retryNextBlock = true)) + } + } + test("commit tx feerate too low, spending anchor outputs with multiple wallet inputs") { val utxos = Seq( // channel funding