Skip to content

Commit

Permalink
Reject unspendable inputs in interactive-tx (#2870)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
t-bast authored Jun 26, 2024
1 parent 71bad3a commit c53b32c
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit c53b32c

Please sign in to comment.