From c53b32c7813b0706d6b4537e9515682420cf2423 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Wed, 26 Jun 2024 16:17:17 +0200 Subject: [PATCH] Reject unspendable inputs in `interactive-tx` (#2870) When we require inputs to be confirmed, we can reliably check whether they are unspent. We can't reliably check this for unconfirmed inputs, because they could be valid but simply not in our mempool, in which case bitcoind would incorrectly consider them unspendable. We want to reject unspendable inputs early to immediately fail the funding attempt, instead of waiting to detect the double-spend later. --- .../acinq/eclair/blockchain/OnChainWallet.scala | 8 ++++++++ .../channel/fund/InteractiveTxBuilder.scala | 15 ++++++++++----- .../eclair/blockchain/DummyOnChainWallet.scala | 6 ++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala index d6e87993dc..3544636678 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala @@ -75,6 +75,14 @@ trait OnChainChannelFunder { /** Get the number of confirmations of a given transaction. */ def getTxConfirmations(txId: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] + /** + * Return true if this output can potentially be spent. + * + * Note that if this function returns false, that doesn't mean the output cannot be spent. The output could be unknown + * (not in the blockchain nor in the mempool) but could reappear later and be spendable at that point. + */ + def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] + /** Rollback a transaction that we failed to commit: this probably translates to "release locks on utxos". */ def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] 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 b1ff8e5878..51a6b1645c 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 @@ -630,11 +630,16 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon private def checkInputsConfirmed(inputs: Seq[Input.Remote]): Future[Boolean] = { // We check inputs sequentially and stop at the first unconfirmed one. - inputs.map(_.outPoint.txid).toSet.foldLeft(Future.successful(true)) { - case (current, txId) => current.transformWith { - case Success(true) => wallet.getTxConfirmations(txId).map { - case None => false - case Some(confirmations) => confirmations > 0 + inputs.map(_.outPoint).toSet.foldLeft(Future.successful(true)) { + case (current, outpoint) => current.transformWith { + case Success(true) => wallet.getTxConfirmations(outpoint.txid).flatMap { + case Some(confirmations) if confirmations > 0 => + // The input is confirmed, so we can reliably check whether it is unspent, We don't check this for + // unconfirmed inputs, because if they are valid but not in our mempool we would incorrectly consider + // them unspendable (unknown). We want to reject unspendable inputs to immediately fail the funding + // attempt, instead of waiting to detect the double-spend later. + wallet.isTransactionOutputSpendable(outpoint.txid, outpoint.index.toInt, includeMempool = true) + case _ => Future.successful(false) } case Success(false) => Future.successful(false) case Failure(t) => Future.failed(t) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala index 6b3e059e27..a3c6c04863 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala @@ -74,6 +74,8 @@ class DummyOnChainWallet extends OnChainWallet with OnchainPubkeyCache { override def getTxConfirmations(txid: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] = Future.failed(new RuntimeException("transaction not found")) + override def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true) + override def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] = { rolledback = rolledback + tx Future.successful(true) @@ -117,6 +119,8 @@ class NoOpOnChainWallet extends OnChainWallet with OnchainPubkeyCache { override def getTxConfirmations(txid: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] = Promise().future // will never be completed + override def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true) + override def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] = { rolledback = rolledback :+ tx Future.successful(true) @@ -228,6 +232,8 @@ class SingleKeyOnChainWallet extends OnChainWallet with OnchainPubkeyCache { override def getTxConfirmations(txid: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] = Future.successful(None) + override def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true) + override def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] = { rolledback = rolledback :+ tx Future.successful(true)