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-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)
-
 }

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)

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

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)

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)
 }

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)))