mirrors/eclair
1
0
Fork 0
mirror of https://github.com/ACINQ/eclair.git synced 2025-02-22 22:25:26 +01:00
Code Issues Projects Releases Wiki Activity

Check HTLC output status before funding HTLC tx (#2944)

Browse source 
When a channel force-closes, we publish our commit tx and HTLC txs.
HTLC transactions conflict with our peer's transactions that also spend
the HTLC outputs. If our peer is able to get their transaction confirmed
before ours, we should stop retrying to publish our HTLC transaction as
that will never succeed.

Since we didn't check the output status, we kept retrying until the
channel was closed (which requires waiting for the `to_self_delay`).
The retries always fail at funding time: `bitcoind` returns an error
saying that the UTXO cannot be found (which is expected because it has
already been spent by our peer). This creates a lot of unnecessary
retries and a lot of noise in the logs.

This scenario usually happens when our peer didn't send the preimage
before force-closing the channel, but reveals it on-chain before the
HTLC timeout: when that happens we kept retrying to publish our HTLC
timeout transaction, which cannot succeed.

We now check the output status in our publishing preconditions, and
immediately abort if the output has already been spent by a confirmed
transaction.
This commit is contained in:
Bastien Teinturier 2024-12-03 15:49:37 +01:00 • committed by GitHub
parent 304290d841
commit f47acd0fb4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 79 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala
View file

@ -125,7 +125,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag
* Return true if this output has already been spent by a confirmed transaction.
* Note that a reorg may invalidate the result of this function and make a spent output spendable again.
*/
private def isTransactionOutputSpent(txid: TxId, outputIndex: Int)(implicit ec: ExecutionContext): Future[Boolean] = {
def isTransactionOutputSpent(txid: TxId, outputIndex: Int)(implicit ec: ExecutionContext): Future[Boolean] = {
getTxConfirmations(txid).flatMap {
case Some(confirmations) if confirmations > 0 =>
// There is an important limitation when using isTransactionOutputSpendable: if it returns false, it can mean a

3
eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
View file

@ -997,7 +997,8 @@ object Helpers {
val remoteHtlcPubkey = Generators.derivePubKey(commitment.remoteParams.htlcBasepoint, remoteCommit.remotePerCommitmentPoint)
val remoteRevocationPubkey = Generators.revocationPubKey(keyManager.revocationPoint(channelKeyPath).publicKey, remoteCommit.remotePerCommitmentPoint)
val remoteDelayedPaymentPubkey = Generators.derivePubKey(commitment.remoteParams.delayedPaymentBasepoint, remoteCommit.remotePerCommitmentPoint)
val localPaymentPubkey = Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remoteCommit.remotePerCommitmentPoint)
val localPaymentBasepoint = commitment.localParams.walletStaticPaymentBasepoint.getOrElse(keyManager.paymentPoint(channelKeyPath).publicKey)
val localPaymentPubkey = if (commitment.params.channelFeatures.hasFeature(Features.StaticRemoteKey)) localPaymentBasepoint else Generators.derivePubKey(localPaymentBasepoint, remoteCommit.remotePerCommitmentPoint)
val outputs = makeCommitTxOutputs(!commitment.localParams.paysCommitTxFees, commitment.remoteParams.dustLimit, remoteRevocationPubkey, commitment.localParams.toSelfDelay, remoteDelayedPaymentPubkey, localPaymentPubkey, remoteHtlcPubkey, localHtlcPubkey, commitment.remoteFundingPubKey, localFundingPubkey, remoteCommit.spec, commitment.params.commitmentFormat)
// we need to use a rather high fee for htlc-claim because we compete with the counterparty
val feeratePerKwHtlc = feerates.fast

57
eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala
View file

@ -52,6 +52,7 @@ object ReplaceableTxPrePublisher {
private case object LocalCommitTxConfirmed extends Command
private case object RemoteCommitTxConfirmed extends Command
private case object RemoteCommitTxPublished extends Command
private case object HtlcOutputAlreadySpent extends Command
private case class UnknownFailure(reason: Throwable) extends Command
// @formatter:on
@ -203,17 +204,29 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
}
}
/**
* We verify that:
* - their commit is not confirmed: if it is, there is no need to publish our htlc transactions
* - the HTLC output isn't already spent by a confirmed transaction (race between HTLC-timeout and HTLC-success)
*/
private def checkHtlcOutput(commitment: FullCommitment, htlcTx: HtlcTx): Future[Command] = {
getRemoteCommitConfirmations(commitment).flatMap {
case Some(depth) if depth >= nodeParams.channelConf.minDepthBlocks => Future.successful(RemoteCommitTxConfirmed)
case _ => bitcoinClient.isTransactionOutputSpent(htlcTx.input.outPoint.txid, htlcTx.input.outPoint.index.toInt).map {
case true => HtlcOutputAlreadySpent
case false => ParentTxOk
}
}
}
private def checkHtlcPreconditions(htlcTx: HtlcTx): Behavior[Command] = {
// We verify that:
// - their commit is not confirmed: if it is, there is no need to publish our htlc transactions
// - if this is an htlc-success transaction, we have the preimage
context.pipeToSelf(getRemoteCommitConfirmations(cmd.commitment)) {
case Success(Some(depth)) if depth >= nodeParams.channelConf.minDepthBlocks => RemoteCommitTxConfirmed
case Success(_) => ParentTxOk
context.pipeToSelf(checkHtlcOutput(cmd.commitment, htlcTx)) {
case Success(result) => result
case Failure(reason) => UnknownFailure(reason)
}
Behaviors.receiveMessagePartial {
case ParentTxOk =>
// We make sure that if this is an htlc-success transaction, we have the preimage.
extractHtlcWitnessData(htlcTx, cmd.commitment) match {
case Some(txWithWitnessData) => replyTo ! PreconditionsOk(txWithWitnessData)
case None => replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false))
@ -223,6 +236,10 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
log.warn("cannot publish {}: remote commit has been confirmed", cmd.desc)
replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.ConflictingTxConfirmed)
Behaviors.stopped
case HtlcOutputAlreadySpent =>
log.warn("cannot publish {}: htlc output has already been spent", cmd.desc)
replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.ConflictingTxConfirmed)
Behaviors.stopped
case UnknownFailure(reason) =>
log.error(s"could not check ${cmd.desc} preconditions, proceeding anyway: ", reason)
// If our checks fail, we don't want it to prevent us from trying to publish our htlc transactions.
@ -265,17 +282,29 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
}
}
/**
* We verify that:
* - our commit is not confirmed: if it is, there is no need to publish our claim-htlc transactions
* - the HTLC output isn't already spent by a confirmed transaction (race between HTLC-timeout and HTLC-success)
*/
private def checkClaimHtlcOutput(commitment: FullCommitment, claimHtlcTx: ClaimHtlcTx): Future[Command] = {
bitcoinClient.getTxConfirmations(commitment.localCommit.commitTxAndRemoteSig.commitTx.tx.txid).flatMap {
case Some(depth) if depth >= nodeParams.channelConf.minDepthBlocks => Future.successful(LocalCommitTxConfirmed)
case _ => bitcoinClient.isTransactionOutputSpent(claimHtlcTx.input.outPoint.txid, claimHtlcTx.input.outPoint.index.toInt).map {
case true => HtlcOutputAlreadySpent
case false => ParentTxOk
}
}
}
private def checkClaimHtlcPreconditions(claimHtlcTx: ClaimHtlcTx): Behavior[Command] = {
// We verify that:
// - our commit is not confirmed: if it is, there is no need to publish our claim-htlc transactions
// - if this is a claim-htlc-success transaction, we have the preimage
context.pipeToSelf(bitcoinClient.getTxConfirmations(cmd.commitment.localCommit.commitTxAndRemoteSig.commitTx.tx.txid)) {
case Success(Some(depth)) if depth >= nodeParams.channelConf.minDepthBlocks => LocalCommitTxConfirmed
case Success(_) => ParentTxOk
context.pipeToSelf(checkClaimHtlcOutput(cmd.commitment, claimHtlcTx)) {
case Success(result) => result
case Failure(reason) => UnknownFailure(reason)
}
Behaviors.receiveMessagePartial {
case ParentTxOk =>
// We verify that if this is a claim-htlc-success transaction, we have the preimage.
extractClaimHtlcWitnessData(claimHtlcTx, cmd.commitment) match {
case Some(txWithWitnessData) => replyTo ! PreconditionsOk(txWithWitnessData)
case None => replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false))
@ -285,6 +314,10 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
log.warn("cannot publish {}: local commit has been confirmed", cmd.desc)
replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.ConflictingTxConfirmed)
Behaviors.stopped
case HtlcOutputAlreadySpent =>
log.warn("cannot publish {}: htlc output has already been spent", cmd.desc)
replyTo ! PreconditionsFailed(TxPublisher.TxRejectedReason.ConflictingTxConfirmed)
Behaviors.stopped
case UnknownFailure(reason) =>
log.error(s"could not check ${cmd.desc} preconditions, proceeding anyway: ", reason)
// If our checks fail, we don't want it to prevent us from trying to publish our htlc transactions.

31
eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala
View file

@ -1278,6 +1278,37 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
}
}
test("htlc success tx double-spent by claim-htlc-timeout") {
withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f =>
import f._
val feerate = FeeratePerKw(15_000 sat)
setFeerate(feerate, fastest = feerate)
val (commitTx, htlcSuccess, _) = closeChannelWithHtlcs(f, aliceBlockHeight() + 144)
// We reach the HTLC timeout before publishing our HTLC-success transaction.
generateBlocks(144)
system.eventStream.publish(CurrentBlockHeight(currentBlockHeight(probe)))
// Bob detects Alice's commit tx and publishes his claim-htlc-timeout, which gets confirmed.
probe.send(bob, WatchFundingSpentTriggered(commitTx))
bob2blockchain.expectMsgType[PublishReplaceableTx] // claim anchor
bob2blockchain.expectMsgType[PublishFinalTx] // claim main output
val claimHtlcTimeout = bob2blockchain.expectMsgType[PublishReplaceableTx] // claim-htlc-timeout
assert(claimHtlcTimeout.txInfo.isInstanceOf[ClaimHtlcTimeoutTx])
wallet.publishTransaction(claimHtlcTimeout.txInfo.tx).pipeTo(probe.ref)
probe.expectMsg(claimHtlcTimeout.txInfo.tx.txid)
generateBlocks(1)
// When Alice tries to publish her HTLC-success, it is immediately aborted.
val htlcSuccessPublisher = createPublisher()
htlcSuccessPublisher ! Publish(probe.ref, htlcSuccess)
val result = probe.expectMsgType[TxRejected]
assert(result.cmd == htlcSuccess)
assert(result.reason == ConflictingTxConfirmed)
htlcSuccessPublisher ! Stop
}
}
test("htlc success tx confirmation target reached, increasing fees") {
withFixture(Seq(50 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f =>
import f._