Address review comments

Rename `wallet` field to `walletName`
Improve comments related to signature workflow and verification of wallet inputs/outputs
Remove `descriptorChecksum` method: it's provided by bitcoin-kmp and does not need to be exposed

eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala

@@ -150,10 +150,10 @@ class Setup(val datadir: File,
           case JArray(values) => values.map(value => value.extract[String])
         }
       eclairBackedWalletOk <- onChainKeyManager_opt match {
-        case Some(keyManager) if !wallets.contains(keyManager.wallet) => keyManager.createWallet(bitcoinClient)
+        case Some(keyManager) if !wallets.contains(keyManager.walletName) => keyManager.createWallet(bitcoinClient)
         case _ => Future.successful(true)
       }
-      _ = assert(eclairBackedWalletOk || onChainKeyManager_opt.map(_.wallet) != wallet, s"cannot create eclair-backed wallet=${onChainKeyManager_opt.map(_.wallet)}, check logs for details")
+      _ = assert(eclairBackedWalletOk || onChainKeyManager_opt.map(_.walletName) != wallet, s"cannot create eclair-backed wallet=${onChainKeyManager_opt.map(_.walletName)}, check logs for details")
       progress = (json \ "verificationprogress").extract[Double]
       ibd = (json \ "initialblockdownload").extract[Boolean]
       blocks = (json \ "blocks").extract[Long]
@@ -252,7 +252,7 @@ class Setup(val datadir: File,
 
       finalPubkey = new AtomicReference[PublicKey](null)
       pubkeyRefreshDelay = FiniteDuration(config.getDuration("bitcoind.final-pubkey-refresh-delay").getSeconds, TimeUnit.SECONDS)
-      bitcoinClient = new BitcoinCoreClient(bitcoin, if (bitcoin.wallet == onChainKeyManager_opt.map(_.wallet)) onChainKeyManager_opt else None) with OnchainPubkeyCache {
+      bitcoinClient = new BitcoinCoreClient(bitcoin, if (bitcoin.wallet == onChainKeyManager_opt.map(_.walletName)) onChainKeyManager_opt else None) with OnchainPubkeyCache {
         val refresher: typed.ActorRef[OnchainPubkeyRefresher.Command] = system.spawn(Behaviors.supervise(OnchainPubkeyRefresher(this, finalPubkey, pubkeyRefreshDelay)).onFailure(typed.SupervisorStrategy.restart), name = "onchain-address-manager")
 
         override def getP2wpkhPubkey(renew: Boolean): PublicKey = {
@@ -314,8 +314,7 @@ class Setup(val datadir: File,
       routerTimeout = after(FiniteDuration(config.getDuration("router.init-timeout").getSeconds, TimeUnit.SECONDS), using = system.scheduler)(Future.failed(new RuntimeException("Router initialization timed out")))
       _ <- Future.firstCompletedOf(routerInitialized.future :: routerTimeout :: Nil)
 
-      _ = bitcoinClient.getReceiveAddress().map(address => logger.info(s"initial address=$address for bitcoin wallet=${bitcoinClient.rpcClient.wallet.getOrElse("")}"))
-
+      _ = bitcoinClient.getReceiveAddress().map(address => logger.info(s"initial address=$address for bitcoin wallet=${bitcoinClient.rpcClient.wallet.getOrElse("(default)")}"))
       channelsListener = system.spawn(ChannelsListener(channelsListenerReady), name = "channels-listener")
       _ <- channelsListenerReady.future
 

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala

@@ -39,7 +39,13 @@ trait OnChainChannelFunder {
    */
   def fundTransaction(tx: Transaction, feeRate: FeeratePerKw, replaceable: Boolean, externalInputsWeight: Map[OutPoint, Long] = Map.empty)(implicit ec: ExecutionContext): Future[FundTransactionResponse]
 
-  /** Sign a PSBT. Result may be partially signed: only inputs known to our bitcoin wallet will be signed. */
+  /**
+   * Sign a PSBT. Result may be partially signed: only inputs known to our bitcoin wallet will be signed. *
+   *
+   * @param psbt       PSBT to sign
+   * @param ourInputs  our wallet inputs. If Eclair is managing Bitcoin Core wallet keys, only these inputs will be signed.
+   * @param ourOutputs our wallet outputs. If Eclair is managing Bitcoin Core wallet keys, it will check that it can actually spend them (i.e re-compute private keys for them)
+   */
   def signPsbt(psbt: Psbt, ourInputs: Seq[Int], ourOutputs: Seq[Int])(implicit ec: ExecutionContext): Future[ProcessPsbtResponse]
 
   /**

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala

@@ -57,7 +57,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onChainKeyManag
   implicit val formats: Formats = org.json4s.DefaultFormats
 
   onChainKeyManager_opt.foreach { keyManager =>
-    require(rpcClient.wallet.contains(keyManager.wallet), s"eclair-backed bitcoin wallet mismatch: eclair-signer.conf uses wallet=${keyManager.wallet}, but eclair.conf uses wallet=${rpcClient.wallet.getOrElse("")}")
+    require(rpcClient.wallet.contains(keyManager.walletName), s"eclair-backed bitcoin wallet mismatch: eclair-signer.conf uses wallet=${keyManager.walletName}, but eclair.conf uses wallet=${rpcClient.wallet.getOrElse("")}")
   }
 
   val useEclairSigner = onChainKeyManager_opt.nonEmpty
@@ -753,7 +753,21 @@ object BitcoinCoreClient {
 
   def toSatoshi(btcAmount: BigDecimal): Satoshi = Satoshi(btcAmount.bigDecimal.scaleByPowerOfTen(8).longValue)
 
+  /**
+   * Bitcoin Core descriptor, as used in Bitcoin Core's RPC API.
+   *
+   * @param desc      Descriptor string representation
+   * @param internal  True if this descriptor is used to generate change addresses. False if this descriptor is used to generate receiving addresses; defined only for active descriptors
+   * @param timestamp The creation time of the descriptor
+   * @param active    Whether this descriptor is currently used to generate new addresses
+   */
   case class Descriptor(desc: String, internal: Boolean = false, timestamp: Long, active: Boolean = true)
 
+  /**
+   * Descriptors for a specific Bitcoin wallet.
+   *
+   * @param wallet_name wallet name
+   * @param descriptors list of wallet descriptors
+   */
   case class Descriptors(wallet_name: String, descriptors: Seq[Descriptor])
 }

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -773,7 +773,6 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
     if (unsignedTx.localInputs.isEmpty) {
       context.self ! SignTransactionResult(PartiallySignedSharedTransaction(unsignedTx, TxSignatures(fundingParams.channelId, tx, Nil, sharedSig_opt)))
     } else {
-      // We only sign our wallet inputs, and check that we can spend our wallet outputs.
       val ourWalletInputs = unsignedTx.localInputs.map(i => tx.txIn.indexWhere(_.outPoint == i.outPoint))
       val ourWalletOutputs = unsignedTx.localOutputs.flatMap {
         case Output.Local.Change(_, amount, pubkeyScript) => Some(tx.txOut.indexWhere(output => output.amount == amount && output.publicKeyScript == pubkeyScript))
@@ -782,6 +781,8 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
         // We trust that non-change outputs are valid: this only works if the entry point for creating such outputs is trusted (for example, a secure API call).
         case _: Output.Local.NonChange => None
       }
+      // We track our wallet inputs and outputs, so we can verify them when we sign the transaction: if Eclair is managing bitcoin core wallet keys, it will
+      // only sign our wallet inputs, and check that it can re-compute private keys for our wallet outputs
       context.pipeToSelf(wallet.signPsbt(new Psbt(tx), ourWalletInputs, ourWalletOutputs).map {
         response =>
           val localOutpoints = unsignedTx.localInputs.map(_.outPoint).toSet

eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalOnChainKeyManager.scala

@@ -34,7 +34,6 @@ import scala.jdk.CollectionConverters.{CollectionHasAsScala, MapHasAsScala}
 import scala.util.{Failure, Success, Try}
 
 object LocalOnChainKeyManager extends Logging {
-  def descriptorChecksum(span: String): String = fr.acinq.bitcoin.Descriptor.checksum(span)
 
   /**
    * Load a configuration file and create an on-chain key manager
@@ -62,7 +61,13 @@ object LocalOnChainKeyManager extends Logging {
   }
 }
 
-class LocalOnChainKeyManager(override val wallet: String, seed: ByteVector, override val walletTimestamp: TimestampSecond, chainHash: ByteVector32) extends OnChainKeyManager with Logging {
+/**
+ * A manager for on-chain keys used by Eclair, to be used in combination with a watch-only descriptor-based wallet managed by Bitcoin Core.
+ * In this setup, Bitcoin Core handles all non-sensitive wallet tasks (including watching the blockchain and building transactions), while
+ * Eclair is in charge of signing transactions.
+ * This is an advanced feature particularly suited when Eclair runs in a secure runtime.
+ */
+class LocalOnChainKeyManager(override val walletName: String, seed: ByteVector, override val walletTimestamp: TimestampSecond, chainHash: ByteVector32) extends OnChainKeyManager with Logging {
 
   import LocalOnChainKeyManager._
 
@@ -100,11 +105,11 @@ class LocalOnChainKeyManager(override val wallet: String, seed: ByteVector, over
 
   override def createWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionContext): Future[Boolean] = {
     if (walletTimestamp < (TimestampSecond.now() - 2.hours)) {
-      logger.warn(s"eclair-backed wallet descriptors for wallet=$wallet are too old to be automatically imported into bitcoin core, you will need to manually import them and select how far back to rescan")
+      logger.warn(s"eclair-backed wallet descriptors for wallet=$walletName are too old to be automatically imported into bitcoin core, you will need to manually import them and select how far back to rescan")
       Future.successful(false)
     } else {
-      logger.info(s"creating a new on-chain eclair-backed wallet in bitcoind: $wallet")
-      rpcClient.invoke("createwallet", wallet, /* disable_private_keys */ true, /* blank */ true, /* passphrase */ "", /* avoid_reuse */ false, /* descriptors */ true, /* load_on_startup */ true).flatMap(_ => {
+      logger.info(s"creating a new on-chain eclair-backed wallet in bitcoind: $walletName")
+      rpcClient.invoke("createwallet", walletName, /* disable_private_keys */ true, /* blank */ true, /* passphrase */ "", /* avoid_reuse */ false, /* descriptors */ true, /* load_on_startup */ true).flatMap(_ => {
         logger.info(s"importing new descriptors ${descriptors(0).descriptors}")
         rpcClient.invoke("importdescriptors", descriptors(0).descriptors).collect {
           case JArray(results) => results.forall(item => {
@@ -132,9 +137,9 @@ class LocalOnChainKeyManager(override val wallet: String, seed: ByteVector, over
     // 84'/{0'/1'}/0'/1/* for change addresses
     val receiveDesc = s"wpkh([$fingerPrintHex/$keyPath]${encode(accountPub, prefix)}/0/*)"
     val changeDesc = s"wpkh([$fingerPrintHex/$keyPath]${encode(accountPub, prefix)}/1/*)"
-    Descriptors(wallet_name = wallet, descriptors = List(
-      Descriptor(desc = s"$receiveDesc#${descriptorChecksum(receiveDesc)}", internal = false, active = true, timestamp = walletTimestamp.toLong),
-      Descriptor(desc = s"$changeDesc#${descriptorChecksum(changeDesc)}", internal = true, active = true, timestamp = walletTimestamp.toLong),
+    Descriptors(wallet_name = walletName, descriptors = List(
+      Descriptor(desc = s"$receiveDesc#${fr.acinq.bitcoin.Descriptor.checksum(receiveDesc)}", internal = false, active = true, timestamp = walletTimestamp.toLong),
+      Descriptor(desc = s"$changeDesc#${fr.acinq.bitcoin.Descriptor.checksum(changeDesc)}", internal = true, active = true, timestamp = walletTimestamp.toLong),
     ))
   }
 

eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/OnChainKeyManager.scala

@@ -27,7 +27,7 @@ import scala.concurrent.{ExecutionContext, Future}
 import scala.util.Try
 
 trait OnChainKeyManager {
-  def wallet: String
+  def walletName: String
 
   /**
    * @return the creation time of the wallet managed by this key manager

eclair-core/src/test/scala/fr/acinq/eclair/crypto/keymanager/LocalOnChainKeyManagerSpec.scala

@@ -114,21 +114,4 @@ class LocalOnChainKeyManagerSpec extends AnyFunSuite {
       assert(error.getMessage.contains("input sighash must be SIGHASH_ALL"))
     }
   }
-
-  test("compute descriptor checksums") {
-    val data = Seq(
-      "pkh([6ded4eb8/44h/0h/0h]xpub6C6N5WVF5zmurBR52MZZj8Jxm6eDiKyM4wFCm7xTYBEsAvJPqBKp2u2K7RTsZaYDN8duBWq4acrD4vrwjaKHTYuntGjL334nVHtLNuaj5Mu/0/*)#5mzpq0w6",
-      "wpkh([6ded4eb8/84h/0h/0h]xpub6CDeom4xT3Wg7BuyXU2Sd9XerTKttyfxRwJE36mi5HxFYpYdtdwM76Zx8swPnc6zxuArMYJgjNy91fJ13YtGPHgf49YqA8KdXg6D69tzNFh/0/*)#refya6f0",
-      "sh(wpkh([6ded4eb8/49h/0h/0h]xpub6Cb8jR9kYsfC6kj9CsE18SyudWjW2V3FnBFkT2oqq6n7NWWvJrjhFin3sAYg8X7ApX8iPophBa98mo4nMvSxnqrXvpnwaRopecQz859Ai1s/0/*))#xrhyhtvl",
-      "tr([6ded4eb8/86h/0h/0h]xpub6CDp1iw76taes3pkqfiJ6PYhwURkaYksJ62CrrdTVr6ow9wR9mKAtUGoZQqb8pRDiq2F8k31tYrrJjVGTRSLYGQ7nYpmewH94ThsAgDxJ4h/0/*)#2nm7drky",
-      "pkh([6ded4eb8/44h/0h/0h]xpub6C6N5WVF5zmurBR52MZZj8Jxm6eDiKyM4wFCm7xTYBEsAvJPqBKp2u2K7RTsZaYDN8duBWq4acrD4vrwjaKHTYuntGjL334nVHtLNuaj5Mu/1/*)#908qa67z",
-      "wpkh([6ded4eb8/84h/0h/0h]xpub6CDeom4xT3Wg7BuyXU2Sd9XerTKttyfxRwJE36mi5HxFYpYdtdwM76Zx8swPnc6zxuArMYJgjNy91fJ13YtGPHgf49YqA8KdXg6D69tzNFh/1/*)#jdv9q0eh",
-      "sh(wpkh([6ded4eb8/49h/0h/0h]xpub6Cb8jR9kYsfC6kj9CsE18SyudWjW2V3FnBFkT2oqq6n7NWWvJrjhFin3sAYg8X7ApX8iPophBa98mo4nMvSxnqrXvpnwaRopecQz859Ai1s/1/*))#nzej05eq",
-      "tr([6ded4eb8/86h/0h/0h]xpub6CDp1iw76taes3pkqfiJ6PYhwURkaYksJ62CrrdTVr6ow9wR9mKAtUGoZQqb8pRDiq2F8k31tYrrJjVGTRSLYGQ7nYpmewH94ThsAgDxJ4h/1/*)#m87lskxu"
-    )
-    data.foreach(dnc => {
-      val Array(desc, checksum) = dnc.split('#')
-      assert(checksum == LocalOnChainKeyManager.descriptorChecksum(desc))
-    })
-  }
 }