From a733b230280e936ac99dfa314133d73bd674a6bc Mon Sep 17 00:00:00 2001 From: sstone Date: Mon, 20 Mar 2023 20:29:04 +0100 Subject: [PATCH] Move BitcoinCoreClient's signTransaction() method to test package It's unsafe (it does not validate the inputs it is signing) and only used in tests. --- .../eclair/blockchain/OnChainWallet.scala | 3 --- .../bitcoind/rpc/BitcoinCoreClient.scala | 13 ++----------- .../eclair/blockchain/DummyOnChainWallet.scala | 3 ++- .../bitcoind/BitcoinCoreClientSpec.scala | 18 +++++++++--------- .../blockchain/bitcoind/BitcoindService.scala | 11 +++++++++++ .../blockchain/bitcoind/ZmqWatcherSpec.scala | 5 +++-- 6 files changed, 27 insertions(+), 26 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 9382509551..47c5bb0c0d 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 @@ -125,7 +125,4 @@ object OnChainWallet { final case class FundTransactionResponse(tx: Transaction, fee: Satoshi, changePosition: Option[Int]) { val amountIn: Satoshi = fee + tx.txOut.map(_.amount).sum } - - final case class SignTransactionResponse(tx: Transaction, complete: Boolean) - } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala index 91ae1ba48f..57116294c1 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala @@ -22,7 +22,7 @@ import fr.acinq.bitcoin.scalacompat._ import fr.acinq.bitcoin.{Bech32, Block, SigHash} import fr.acinq.eclair.ShortChannelId.coordinates import fr.acinq.eclair.blockchain.OnChainWallet -import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance, SignTransactionResponse} +import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance} import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher.{GetTxWithMetaResponse, UtxoStatus, ValidateResult} import fr.acinq.eclair.blockchain.fee.{FeeratePerKB, FeeratePerKw} import fr.acinq.eclair.crypto.keymanager.OnchainKeyManager @@ -349,15 +349,6 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onchainKeyManag } yield signed } - def signTransaction(tx: Transaction)(implicit ec: ExecutionContext): Future[SignTransactionResponse] = signTransaction(tx, Nil) - - def signTransaction(tx: Transaction, allowIncomplete: Boolean)(implicit ec: ExecutionContext): Future[SignTransactionResponse] = signTransaction(tx, Nil, allowIncomplete) - - def signTransaction(tx: Transaction, previousTxs: Seq[PreviousTx], allowIncomplete: Boolean = false)(implicit ec: ExecutionContext): Future[SignTransactionResponse] = { - import KotlinUtils._ - signPsbt(new Psbt(tx), tx.txIn.indices, Nil).map(p => SignTransactionResponse(p.extractFinalTx.getOrElse(p.extractPartiallySignedTx), p.extractFinalTx.isRight)) - } - //------------------------- PUBLISHING -------------------------// /** @@ -615,7 +606,7 @@ object BitcoinCoreClient { import KotlinUtils._ // Extract a fully signed transaction from `psbt` - // If the transaction is just partially signed, this method wil fail and you must call extractPartiallySignedTx instead + // If the transaction is just partially signed, this method will fail and you must call extractPartiallySignedTx instead def extractFinalTx: Either[UpdateFailure, Transaction] = { val extracted = psbt.extract() if (extracted.isLeft) Left(extracted.getLeft) else Right(extracted.getRight) 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 37cc378b2b..4692138e60 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 @@ -21,7 +21,8 @@ import fr.acinq.bitcoin.psbt.Psbt import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey import fr.acinq.bitcoin.scalacompat.{ByteVector32, Crypto, OutPoint, Satoshi, SatoshiLong, Script, Transaction, TxIn, TxOut} import fr.acinq.bitcoin.{Bech32, SigHash, SigVersion} -import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance, SignTransactionResponse} +import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance} +import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.SignTransactionResponse import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient.ProcessPsbtResponse import fr.acinq.eclair.blockchain.fee.FeeratePerKw diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala index 4f7ec34c67..b4b11045c6 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala @@ -25,9 +25,9 @@ import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey import fr.acinq.bitcoin.scalacompat.{Block, Btc, BtcDouble, ByteVector32, DeterministicWallet, MilliBtcDouble, MnemonicCode, OP_DROP, OP_PUSHDATA, OutPoint, Satoshi, SatoshiLong, Script, ScriptWitness, Transaction, TxIn, TxOut, computeBIP84Address, computeP2PkhAddress, computeP2WpkhAddress} import fr.acinq.bitcoin.utils.EitherKt import fr.acinq.bitcoin.{Bech32, SigHash, SigVersion} -import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance, SignTransactionResponse} +import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance} import fr.acinq.eclair.blockchain.WatcherSpec.{createSpendManyP2WPKH, createSpendP2WPKH} -import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.BitcoinReq +import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.{BitcoinReq, SignTransactionResponse} import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient._ import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinJsonRPCAuthMethod.UserPassword import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, BitcoinCoreClient, JsonRPCError} @@ -857,14 +857,14 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val opts = FundTransactionOptions(TestConstants.feeratePerKw, changePosition = Some(1)) bitcoinClient.fundTransaction(Transaction(2, Nil, Seq(TxOut(250000 sat, Script.pay2wpkh(randomKey().publicKey))), 0), opts).pipeTo(sender.ref) val fundedTx1 = sender.expectMsgType[FundTransactionResponse].tx - bitcoinClient.signTransaction(fundedTx1, Nil).pipeTo(sender.ref) + signTransaction(bitcoinClient, fundedTx1, Nil).pipeTo(sender.ref) val signedTx1 = sender.expectMsgType[SignTransactionResponse].tx bitcoinClient.publishTransaction(signedTx1).pipeTo(sender.ref) sender.expectMsg(signedTx1.txid) // Double-spend that transaction. val fundedTx2 = fundedTx1.copy(txOut = TxOut(200000 sat, Script.pay2wpkh(randomKey().publicKey)) +: fundedTx1.txOut.tail) - bitcoinClient.signTransaction(fundedTx2, Nil).pipeTo(sender.ref) + signTransaction(bitcoinClient, fundedTx2, Nil).pipeTo(sender.ref) val signedTx2 = sender.expectMsgType[SignTransactionResponse].tx assert(signedTx2.txid != signedTx1.txid) bitcoinClient.publishTransaction(signedTx2).pipeTo(sender.ref) @@ -895,12 +895,12 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val noInputTx1 = Transaction(2, Nil, Seq(TxOut(500_000 sat, Script.pay2wpkh(randomKey().publicKey))), 0) bitcoinClient.fundTransaction(noInputTx1, FundTransactionOptions(FeeratePerKw(2500 sat))).pipeTo(sender.ref) val unsignedTx1 = sender.expectMsgType[FundTransactionResponse].tx - bitcoinClient.signTransaction(unsignedTx1).pipeTo(sender.ref) + signTransaction(bitcoinClient, unsignedTx1).pipeTo(sender.ref) val tx1 = sender.expectMsgType[SignTransactionResponse].tx // let's then generate another tx that double spends the first one val unsignedTx2 = tx1.copy(txOut = Seq(TxOut(tx1.txOut.map(_.amount).sum, Script.pay2wpkh(randomKey().publicKey)))) - bitcoinClient.signTransaction(unsignedTx2).pipeTo(sender.ref) + signTransaction(bitcoinClient, unsignedTx2).pipeTo(sender.ref) val tx2 = sender.expectMsgType[SignTransactionResponse].tx // tx1/tx2 haven't been published, so tx1 isn't double-spent @@ -936,7 +936,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val noInputTx = Transaction(2, Nil, Seq(TxOut(amount, Script.pay2wpkh(priv.publicKey))), 0) bitcoinClient.fundTransaction(noInputTx, FundTransactionOptions(FeeratePerKw(2500 sat))).pipeTo(sender.ref) val unsignedTx = sender.expectMsgType[FundTransactionResponse].tx - bitcoinClient.signTransaction(unsignedTx).pipeTo(sender.ref) + signTransaction(bitcoinClient, unsignedTx).pipeTo(sender.ref) sender.expectMsgType[SignTransactionResponse].tx }) bitcoinClient.publishTransaction(txs.head).pipeTo(sender.ref) @@ -962,7 +962,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A { val previousAmountOut = unconfirmedParentTx.txOut.map(_.amount).sum val unsignedTx = unconfirmedParentTx.copy(txOut = Seq(TxOut(previousAmountOut - 50_000.sat, Script.pay2wpkh(randomKey().publicKey)))) - bitcoinClient.signTransaction(unsignedTx).pipeTo(sender.ref) + signTransaction(bitcoinClient, unsignedTx).pipeTo(sender.ref) val signedTx = sender.expectMsgType[SignTransactionResponse].tx bitcoinClient.publishTransaction(signedTx).pipeTo(sender.ref) sender.expectMsg(signedTx.txid) @@ -1092,7 +1092,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val txNotFunded = Transaction(2, Nil, mainOutput +: outputsWithLargeScript, 0) miner.fundTransaction(txNotFunded, FundTransactionOptions(FeeratePerKw(500 sat), changePosition = Some(outputs.length))).pipeTo(sender.ref) val fundedTx = sender.expectMsgType[FundTransactionResponse].tx - miner.signTransaction(fundedTx, allowIncomplete = false).pipeTo(sender.ref) + signTransaction(miner, fundedTx, allowIncomplete = false).pipeTo(sender.ref) val signedTx = sender.expectMsgType[SignTransactionResponse].tx miner.publishTransaction(signedTx).pipeTo(sender.ref) sender.expectMsg(signedTx.txid) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala index e52ccd61d2..5755682482 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala @@ -22,6 +22,7 @@ import akka.testkit.{TestKitBase, TestProbe} import fr.acinq.bitcoin.psbt.Psbt import fr.acinq.bitcoin.scalacompat.Crypto.PrivateKey import fr.acinq.bitcoin.scalacompat.{Block, Btc, BtcAmount, MilliBtc, Satoshi, Transaction, TxOut, computeP2WpkhAddress} +import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient.PreviousTx import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinJsonRPCAuthMethod.{SafeCookie, UserPassword} import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, BitcoinCoreClient, BitcoinJsonRPCAuthMethod, BitcoinJsonRPCClient} import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKB, FeeratePerKw} @@ -37,6 +38,7 @@ import java.io.File import java.nio.charset.StandardCharsets import java.nio.file.Files import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future import scala.concurrent.duration._ import scala.io.Source @@ -268,10 +270,19 @@ trait BitcoindService extends Logging { Transaction.read(rawTx) } + def signTransaction(client: BitcoinCoreClient, tx: Transaction): Future[SignTransactionResponse] = signTransaction(client, tx, Nil) + + def signTransaction(client: BitcoinCoreClient, tx: Transaction, allowIncomplete: Boolean): Future[SignTransactionResponse] = signTransaction(client, tx, Nil, allowIncomplete) + + def signTransaction(client: BitcoinCoreClient, tx: Transaction, previousTxs: Seq[PreviousTx], allowIncomplete: Boolean = false): Future[SignTransactionResponse] = { + import fr.acinq.bitcoin.scalacompat.KotlinUtils._ + client.signPsbt(new Psbt(tx), tx.txIn.indices, Nil).map(p => SignTransactionResponse(p.extractFinalTx.getOrElse(p.extractPartiallySignedTx), p.extractFinalTx.isRight)) + } } object BitcoindService { case class BitcoinReq(method: String, params: Any*) + final case class SignTransactionResponse(tx: Transaction, complete: Boolean) } \ No newline at end of file diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcherSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcherSpec.scala index 0f3fb250fd..1584316b1f 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcherSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcherSpec.scala @@ -22,8 +22,9 @@ import akka.actor.{ActorRef, Props, typed} import akka.pattern.pipe import akka.testkit.TestProbe import fr.acinq.bitcoin.scalacompat.{Block, Btc, MilliBtcDouble, OutPoint, SatoshiLong, Script, Transaction, TxOut} -import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, SignTransactionResponse} +import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse} import fr.acinq.eclair.blockchain.WatcherSpec._ +import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.SignTransactionResponse import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient.FundTransactionOptions @@ -305,7 +306,7 @@ class ZmqWatcherSpec extends TestKitBaseClass with AnyFunSuiteLike with Bitcoind val tx1 = { bitcoinClient.fundTransaction(Transaction(2, Nil, TxOut(150000 sat, Script.pay2wpkh(priv.publicKey)) :: Nil, 0), FundTransactionOptions(FeeratePerKw(250 sat))).pipeTo(probe.ref) val funded = probe.expectMsgType[FundTransactionResponse].tx - bitcoinClient.signTransaction(funded).pipeTo(probe.ref) + signTransaction(bitcoinClient, funded).pipeTo(probe.ref) probe.expectMsgType[SignTransactionResponse].tx } val outputIndex = tx1.txOut.indexWhere(_.publicKeyScript == Script.write(Script.pay2wpkh(priv.publicKey)))