From 874bd4bc263611021f0dfbda66cd0279a828fe85 Mon Sep 17 00:00:00 2001 From: Fabrice Drouin Date: Tue, 5 May 2020 11:03:26 +0200 Subject: [PATCH] Unlock transaction inputs if tx cannot be published (#1404) * Unlock transaction inputs if tx cannot be published In some cases, funding a tx will work but publishing may fail (because mempool fees are not met for example). In that case we need to make sure that the tx inputs are unlocked. --- .../bitcoind/BitcoinCoreWallet.scala | 40 +++++-- .../bitcoind/BitcoinCoreWalletSpec.scala | 103 +++++++++++++++++- 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala index cdbdbfe4c..6057aaf5c 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala @@ -26,6 +26,7 @@ import org.json4s.JsonAST._ import scodec.bits.ByteVector import scala.concurrent.{ExecutionContext, Future} +import scala.util.{Failure, Success} /** * Created by PM on 06/07/2017. @@ -63,7 +64,30 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC def publishTransaction(tx: Transaction)(implicit ec: ExecutionContext): Future[String] = bitcoinClient.publishTransaction(tx) - def unlockOutpoints(outPoints: Seq[OutPoint])(implicit ec: ExecutionContext): Future[Boolean] = rpcClient.invoke("lockunspent", true, outPoints.toList.map(outPoint => Utxo(outPoint.txid, outPoint.index))) collect { case JBool(result) => result } + /** + * + * @param outPoints outpoints to unlock + * @return true if all outpoints were successfully unlocked, false otherwise + */ + def unlockOutpoints(outPoints: Seq[OutPoint])(implicit ec: ExecutionContext): Future[Boolean] = { + // we unlock utxos one by one and not as a list as it would fail at the first utxo that is not actually lock and the rest would not be processed + val futures = outPoints + .map(outPoint => Utxo(outPoint.txid, outPoint.index)) + .map(utxo => rpcClient + .invoke("lockunspent", true, List(utxo)) + .mapTo[JBool] + .transformWith { + case Success(JBool(result)) => Future.successful(result) + case Failure(JsonRPCError(error)) if error.message.contains("expected locked output") => + Future.successful(true) // we consider that the outpoint was successfully unlocked (since it was not locked to begin with) + case Failure(t) => + logger.warn(s"Cannot unlock utxo=$utxo", t) + Future.successful(false) + }) + val future = Future.sequence(futures) + // return true if all outpoints were unlocked false otherwise + future.map(_.forall(b => b)) + } override def getBalance: Future[Satoshi] = rpcClient.invoke("getbalance") collect { case JDecimal(balance) => Satoshi(balance.bigDecimal.scaleByPowerOfTen(8).longValue) } @@ -103,13 +127,15 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC } yield MakeFundingTxResponse(fundingTx, outputIndex, fee) } - override def commit(tx: Transaction): Future[Boolean] = publishTransaction(tx) - .map(_ => true) // if bitcoind says OK, then we consider the tx successfully published - .recoverWith { case JsonRPCError(e) => + override def commit(tx: Transaction): Future[Boolean] = publishTransaction(tx).transformWith { + case Success(_) => Future.successful(true) + case Failure(e) => logger.warn(s"txid=${tx.txid} error=$e") - bitcoinClient.getTransaction(tx.txid).map(_ => true).recover { case _ => false } // if we get a parseable error from bitcoind AND the tx is NOT in the mempool/blockchain, then we consider that the tx was not published - } - .recover { case _ => true } // in all other cases we consider that the tx has been published + bitcoinClient.getTransaction(tx.txid).transformWith { + case Success(_) => Future.successful(true) // tx is in the mempool, we consider that it was published + case Failure(_) => rollback(tx).transform { case _ => Success(false) } // we use transform here because we want to return false in all cases even if rollback fails + } + } override def rollback(tx: Transaction): Future[Boolean] = unlockOutpoints(tx.txIn.map(_.outPoint)) // we unlock all utxos used by the tx diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala index 97944a070..82a93c361 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala @@ -20,9 +20,9 @@ import akka.actor.Status.Failure import akka.pattern.pipe import akka.testkit.TestProbe import com.typesafe.config.ConfigFactory -import fr.acinq.bitcoin.{Block, ByteVector32, MilliBtc, OutPoint, Satoshi, Script, Transaction, TxIn, TxOut} +import fr.acinq.bitcoin.{Block, Btc, ByteVector32, MilliBtc, OutPoint, Satoshi, Script, Transaction, TxIn, TxOut} import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.bitcoind.BitcoinCoreWallet.FundTransactionResponse +import fr.acinq.eclair.blockchain.bitcoind.BitcoinCoreWallet.{FundTransactionResponse, SignTransactionResponse} import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.BitcoinReq import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, JsonRPCError} import fr.acinq.eclair.transactions.Scripts @@ -68,6 +68,105 @@ class BitcoinCoreWalletSpec extends TestKitBaseClass with BitcoindService with A waitForBitcoindReady() } + def getLocks(sender: TestProbe = TestProbe()) : Set[OutPoint] = { + implicit val formats = DefaultFormats + sender.send(bitcoincli, BitcoinReq("listlockunspent")) + val JArray(locks) = sender.expectMsgType[JValue] + val txids = locks.map { item => + val JString(txid) = item \ "txid" + val JInt(vout) = item \ "vout" + OutPoint(ByteVector32.fromValidHex(txid).reverse, vout.toInt) + } + txids.toSet + } + + test("unlock transaction inputs if publishing fails") { + val bitcoinClient = new BasicBitcoinJsonRPCClient( + user = config.getString("bitcoind.rpcuser"), + password = config.getString("bitcoind.rpcpassword"), + host = config.getString("bitcoind.host"), + port = config.getInt("bitcoind.rpcport")) + val wallet = new BitcoinCoreWallet(bitcoinClient) + + val sender = TestProbe() + val pubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(randomKey.publicKey, randomKey.publicKey))) + + // create a huge tx so we make sure it has > 1 inputs + wallet.makeFundingTx(pubkeyScript, Btc(250), 1000).pipeTo(sender.ref) + val MakeFundingTxResponse(fundingTx, outputIndex, _) = sender.expectMsgType[MakeFundingTxResponse] + + // spend the first 2 inputs + val tx1 = fundingTx.copy( + txIn = fundingTx.txIn.take(2), + txOut = fundingTx.txOut.updated(outputIndex, fundingTx.txOut(outputIndex).copy(amount = Btc(50))) + ) + wallet.signTransaction(tx1).pipeTo(sender.ref) + val SignTransactionResponse(tx2, true) = sender.expectMsgType[SignTransactionResponse] + + wallet.commit(tx2).pipeTo(sender.ref) + assert(sender.expectMsgType[Boolean]) + + // fundingTx inputs are still locked except for the first 2 that were just spent + val expectedLocks = fundingTx.txIn.drop(2).map(_.outPoint).toSet + awaitCond({ + val locks = getLocks(sender) + expectedLocks -- locks isEmpty + }, max = 10 seconds, interval = 1 second) + + // publishing fundingTx will fail as its first 2 inputs are already spent by tx above in the mempool + wallet.commit(fundingTx).pipeTo(sender.ref) + val result = sender.expectMsgType[Boolean] + assert(!result) + + // and all locked inputs should now be unlocked + awaitCond({ + val locks = getLocks(sender) + locks isEmpty + }, max = 10 seconds, interval = 1 second) + } + + test("unlock outpoints correcly") { + val bitcoinClient = new BasicBitcoinJsonRPCClient( + user = config.getString("bitcoind.rpcuser"), + password = config.getString("bitcoind.rpcpassword"), + host = config.getString("bitcoind.host"), + port = config.getInt("bitcoind.rpcport")) + + val wallet = new BitcoinCoreWallet(bitcoinClient) + + val sender = TestProbe() + val pubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(randomKey.publicKey, randomKey.publicKey))) + + { + // test #1: unlock outpoints that are actually locked + // create a huge tx so we make sure it has > 1 inputs + wallet.makeFundingTx(pubkeyScript, Btc(250), 1000).pipeTo(sender.ref) + val MakeFundingTxResponse(fundingTx, outputIndex, _) = sender.expectMsgType[MakeFundingTxResponse] + assert(fundingTx.txIn.size > 2) + assert(getLocks(sender) == fundingTx.txIn.map(_.outPoint).toSet) + wallet.rollback(fundingTx).pipeTo(sender.ref) + assert(sender.expectMsgType[Boolean]) + } + { + // test #2: some outpoints are locked, some are unlocked + wallet.makeFundingTx(pubkeyScript, Btc(250), 1000).pipeTo(sender.ref) + val MakeFundingTxResponse(fundingTx, outputIndex, _) = sender.expectMsgType[MakeFundingTxResponse] + assert(fundingTx.txIn.size > 2) + assert(getLocks(sender) == fundingTx.txIn.map(_.outPoint).toSet) + + // unlock the first 2 outpoints + val tx1 = fundingTx.copy(txIn = fundingTx.txIn.take(2)) + wallet.rollback(tx1).pipeTo(sender.ref) + assert(sender.expectMsgType[Boolean]) + assert(getLocks(sender) == fundingTx.txIn.drop(2).map(_.outPoint).toSet) + + // and try to unlock all outpoints: it should work too + wallet.rollback(fundingTx).pipeTo(sender.ref) + assert(sender.expectMsgType[Boolean]) + assert(getLocks(sender) isEmpty) + } + } + test("absence of rounding") { val hexIn = "020000000001404b4c0000000000220020822eb4234126c5fc84910e51a161a9b7af94eb67a2344f7031db247e0ecc2f9200000000" val hexOut = "02000000013361e994f6bd5cbe9dc9e8cb3acdc12bc1510a3596469d9fc03cfddd71b223720000000000feffffff02c821354a00000000160014b6aa25d6f2a692517f2cf1ad55f243a5ba672cac404b4c0000000000220020822eb4234126c5fc84910e51a161a9b7af94eb67a2344f7031db247e0ecc2f9200000000"