From bc05070a46176b63c5fc731ab9b69030b23b5edd Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 1 Apr 2025 12:33:54 +0200 Subject: [PATCH] Stricter requirements on input details for signing We verify that details about all inputs are provided to the `sign` function. While this isn't mandatory for segwit v0, it ensures that all of our existing tests exercise this codepath and reduces the risk that we forget to provide some wallet inputs, which would result in an invalid signature which would be hard to investigate. With this change, some of the unit tests started failing, which showed that we weren't correctly setting wallet inputs in the fee-bumping case in `ReplaceableTxFunder`, which we've fixed. We also add a test in `TransactionsSpec.scala` to verify that signing fails when details about some inputs are missing. --- .../bitcoind/rpc/BitcoinCoreClient.scala | 2 +- .../channel/fund/InteractiveTxBuilder.scala | 13 +++-- .../channel/publish/ReplaceableTxFunder.scala | 41 ++++++-------- .../crypto/keymanager/ChannelKeyManager.scala | 6 +- .../keymanager/LocalChannelKeyManager.scala | 6 +- .../eclair/transactions/Transactions.scala | 29 +++++++--- .../publish/ReplaceableTxPublisherSpec.scala | 56 ++++--------------- .../transactions/TransactionsSpec.scala | 35 +++++++++--- 8 files changed, 88 insertions(+), 100 deletions(-) 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 91b6bcff23..5dfecb60ce 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 @@ -305,7 +305,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool }) } - def utxoUpdatePsbt(psbt: Psbt)(implicit ec: ExecutionContext): Future[Psbt] = { + private def utxoUpdatePsbt(psbt: Psbt)(implicit ec: ExecutionContext): Future[Psbt] = { val encoded = Base64.getEncoder.encodeToString(Psbt.write(psbt).toByteArray) rpcClient.invoke("utxoupdatepsbt", encoded).map(json => { val JString(base64) = json diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala index 225be50bda..94dec79916 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala @@ -105,18 +105,15 @@ object InteractiveTxBuilder { // @formatter:off def info: InputInfo def weight: Int - // we don't need to provide extra inputs here, as this method will only be called for multisig-2-of-2 inputs which only require the output that is spent for signing - // for taproot channels, we'll use a musig2 input that is not signed like this: instead, the signature will be the Musig2 aggregation if a local and remote partial signature - def sign(keyManager: ChannelKeyManager, params: ChannelParams, tx: Transaction): ByteVector64 // @formatter:on } case class Multisig2of2Input(info: InputInfo, fundingTxIndex: Long, remoteFundingPubkey: PublicKey) extends SharedFundingInput { override val weight: Int = 388 - override def sign(keyManager: ChannelKeyManager, params: ChannelParams, tx: Transaction): ByteVector64 = { + def sign(keyManager: ChannelKeyManager, params: ChannelParams, tx: Transaction, spentUtxos: Map[OutPoint, TxOut]): ByteVector64 = { val localFundingPubkey = keyManager.fundingPublicKey(params.localParams.fundingKeyPath, fundingTxIndex) - keyManager.sign(Transactions.SpliceTx(info, tx), localFundingPubkey, TxOwner.Local, params.commitmentFormat, Map.empty) + keyManager.sign(Transactions.SpliceTx(info, tx), localFundingPubkey, TxOwner.Local, params.commitmentFormat, spentUtxos) } } @@ -339,6 +336,8 @@ object InteractiveTxBuilder { val remoteFees: MilliSatoshi = remoteAmountIn - remoteAmountOut // Note that the truncation is a no-op: sub-satoshi balances are carried over from inputs to outputs and cancel out. val fees: Satoshi = (localFees + remoteFees).truncateToSatoshi + // When signing transactions that include taproot inputs, we must provide details about all of the transaction's inputs. + val inputDetails: Map[OutPoint, TxOut] = (sharedInput_opt.toSeq.map(i => i.outPoint -> i.txOut) ++ localInputs.map(i => i.outPoint -> i.txOut) ++ remoteInputs.map(i => i.outPoint -> i.txOut)).toMap def localOnlyNonChangeOutputs: List[Output.Local.NonChange] = localOutputs.collect { case o: Local.NonChange => o } @@ -914,7 +913,9 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon import fr.acinq.bitcoin.scalacompat.KotlinUtils._ val tx = unsignedTx.buildUnsignedTx() - val sharedSig_opt = fundingParams.sharedInput_opt.map(_.sign(keyManager, channelParams, tx)) + val sharedSig_opt = fundingParams.sharedInput_opt.collect { + case i: Multisig2of2Input => i.sign(keyManager, channelParams, tx, unsignedTx.inputDetails) + } if (unsignedTx.localInputs.isEmpty) { context.self ! SignTransactionResult(PartiallySignedSharedTransaction(unsignedTx, TxSignatures(fundingParams.channelId, tx, Nil, sharedSig_opt))) } else { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala index 33fcc2b478..10301aaa8a 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala @@ -274,7 +274,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, Behaviors.stopped case AdjustPreviousTxOutputResult.TxOutputAdjusted(updatedTx) => log.debug("bumping {} fees without adding new inputs: txid={}", cmd.desc, updatedTx.txInfo.tx.txid) - sign(updatedTx, targetFeerate, previousTx.totalAmountIn, Map.empty) + sign(updatedTx, targetFeerate, previousTx.totalAmountIn, previousTx.walletInputs) case AdjustPreviousTxOutputResult.AddWalletInputs(tx) => log.debug("bumping {} fees requires adding new inputs (feerate={})", cmd.desc, targetFeerate) // We restore the original transaction (remove previous attempt's wallet inputs). @@ -426,32 +426,23 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, } } - private def addInputs(tx: ReplaceableTxWithWalletInputs, targetFeerate: FeeratePerKw, commitment: FullCommitment): Future[(ReplaceableTxWithWalletInputs, Satoshi, Map[OutPoint, TxOut])] = { - - def getWalletUtxos(txInfo: TransactionWithInputInfo): Future[Map[OutPoint, TxOut]] = { - val inputs = txInfo.tx.txIn.filterNot(_.outPoint == txInfo.input.outPoint) - val txids = inputs.map(_.outPoint.txid).toSet - for { - txs <- Future.sequence(txids.toSeq.map(bitcoinClient.getTransaction)) - txMap = txs.map(tx => tx.txid -> tx).toMap - } yield { - inputs.map(_.outPoint).map(outPoint => { - require(txMap.contains(outPoint.txid) && txMap(outPoint.txid).txOut.size >= outPoint.index, s"missing wallet input $outPoint") - outPoint -> txMap(outPoint.txid).txOut(outPoint.index.toInt) - }).toMap + private def getWalletUtxos(txInfo: TransactionWithInputInfo): Future[Map[OutPoint, TxOut]] = { + Future.sequence(txInfo.tx.txIn.filter(_.outPoint != txInfo.input.outPoint).map(txIn => { + bitcoinClient.getTransaction(txIn.outPoint.txid).flatMap { + case inputTx if inputTx.txOut.size <= txIn.outPoint.index => Future.failed(new IllegalArgumentException(s"input ${inputTx.txid}:${txIn.outPoint.index} doesn't exist")) + case inputTx => Future.successful(txIn.outPoint -> inputTx.txOut(txIn.outPoint.index.toInt)) } - } + })).map(_.toMap) + } - tx match { - case anchorTx: ClaimLocalAnchorWithWitnessData => for { - (fundedTx, amountIn) <- addInputs(anchorTx, targetFeerate, commitment) - spentUtxos <- getWalletUtxos(fundedTx.txInfo) - } yield (fundedTx, amountIn, spentUtxos) - case htlcTx: HtlcWithWitnessData => for { - (fundedTx, amountIn) <- addInputs(htlcTx, targetFeerate, commitment) - spentUtxos <- getWalletUtxos(fundedTx.txInfo) - } yield (fundedTx, amountIn, spentUtxos) - } + private def addInputs(tx: ReplaceableTxWithWalletInputs, targetFeerate: FeeratePerKw, commitment: FullCommitment): Future[(ReplaceableTxWithWalletInputs, Satoshi, Map[OutPoint, TxOut])] = { + for { + (fundedTx, amountIn) <- tx match { + case anchorTx: ClaimLocalAnchorWithWitnessData => addInputs(anchorTx, targetFeerate, commitment) + case htlcTx: HtlcWithWitnessData => addInputs(htlcTx, targetFeerate, commitment) + } + spentUtxos <- getWalletUtxos(fundedTx.txInfo) + } yield (fundedTx, amountIn, spentUtxos) } private def addInputs(anchorTx: ClaimLocalAnchorWithWitnessData, targetFeerate: FeeratePerKw, commitment: FullCommitment): Future[(ClaimLocalAnchorWithWitnessData, Satoshi)] = { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/ChannelKeyManager.scala b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/ChannelKeyManager.scala index 50f71781d7..78a2b981bb 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/ChannelKeyManager.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/ChannelKeyManager.scala @@ -64,7 +64,7 @@ trait ChannelKeyManager { * @param publicKey extended public key * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with the private key that matches the input extended public key */ def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 @@ -78,7 +78,7 @@ trait ChannelKeyManager { * @param remotePoint remote point * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with a private key generated from the input key's matching private key and the remote point. */ def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, remotePoint: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 @@ -91,7 +91,7 @@ trait ChannelKeyManager { * @param remoteSecret remote secret * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with a private key generated from the input key's matching private key and the remote secret. */ def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, remoteSecret: PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalChannelKeyManager.scala b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalChannelKeyManager.scala index 96e0acded0..01be5a95c4 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalChannelKeyManager.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalChannelKeyManager.scala @@ -104,7 +104,7 @@ class LocalChannelKeyManager(seed: ByteVector, chainHash: BlockHash) extends Cha * @param publicKey extended public key * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with the private key that matches the input extended public key */ override def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { @@ -125,7 +125,7 @@ class LocalChannelKeyManager(seed: ByteVector, chainHash: BlockHash) extends Cha * @param remotePoint remote point * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with a private key generated from the input key's matching private key and the remote point. */ override def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, remotePoint: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { @@ -147,7 +147,7 @@ class LocalChannelKeyManager(seed: ByteVector, chainHash: BlockHash) extends Cha * @param remoteSecret remote secret * @param txOwner owner of the transaction (local/remote) * @param commitmentFormat format of the commitment tx - * @param extraUtxos extra outputs spent by this transaction (in addition to our [[fr.acinq.eclair.transactions.Transactions.InputInfo]] output, which is assumed to always be the first spent output) + * @param extraUtxos extra outputs spent by this transaction (in addition to [[fr.acinq.eclair.transactions.Transactions.InputInfo]]) * @return a signature generated with a private key generated from the input key's matching private key and the remote secret. */ override def sign(tx: TransactionWithInputInfo, publicKey: ExtendedPublicKey, remoteSecret: PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala index cc8dd3b50c..9cb1ea9194 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala @@ -131,18 +131,31 @@ object Transactions { /** Sighash flags to use when signing the transaction. */ def sighash(txOwner: TxOwner, commitmentFormat: CommitmentFormat): Int = SIGHASH_ALL + /** + * @param extraUtxos extra outputs spent by this transaction (in addition to the main [[input]]). + */ def sign(key: PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { sign(key, sighash(txOwner, commitmentFormat), extraUtxos) } - def sign(key: PrivateKey, sighashType: Int, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = input match { - case _: InputInfo.TaprootInput => ByteVector64.Zeroes - case InputInfo.SegwitInput(outPoint, txOut, redeemScript) => - // NB: the tx may have multiple inputs, we will only sign the one provided in txinfo.input. Bear in mind that the - // signature will be invalidated if other inputs are added *afterwards* and sighashType was SIGHASH_ALL. - val inputIndex = tx.txIn.indexWhere(_.outPoint == outPoint) - val sigDER = Transaction.signInput(tx, inputIndex, redeemScript, sighashType, txOut.amount, SIGVERSION_WITNESS_V0, key) - Crypto.der2compact(sigDER) + def sign(key: PrivateKey, sighashType: Int, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { + val inputsMap = extraUtxos + (input.outPoint -> input.txOut) + tx.txIn.foreach(txIn => { + // Note that using a require here is dangerous, because callers don't except this function to throw. + // But we want to ensure that we're correctly providing input details, otherwise our signature will silently be + // invalid when using taproot. We verify this in all cases, even when using segwit v0, to ensure that we have as + // many tests as possible that exercise this codepath. + require(inputsMap.contains(txIn.outPoint), s"cannot sign $desc with txId=${tx.txid}: missing input details for ${txIn.outPoint}") + }) + input match { + case InputInfo.SegwitInput(outPoint, txOut, redeemScript) => + // NB: the tx may have multiple inputs, we will only sign the one provided in txinfo.input. Bear in mind that the + // signature will be invalidated if other inputs are added *afterwards* and sighashType was SIGHASH_ALL. + val inputIndex = tx.txIn.indexWhere(_.outPoint == outPoint) + val sigDER = Transaction.signInput(tx, inputIndex, redeemScript, sighashType, txOut.amount, SIGVERSION_WITNESS_V0, key) + Crypto.der2compact(sigDER) + case _: InputInfo.TaprootInput => ??? + } } def checkSig(sig: ByteVector64, pubKey: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat): Boolean = input match { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala index a2fbf82ec3..64fe89219a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala @@ -22,7 +22,7 @@ import akka.pattern.pipe import akka.testkit.{TestFSMRef, TestProbe} import com.softwaremill.quicklens.ModifyPimp import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey -import fr.acinq.bitcoin.scalacompat.{Block, BtcAmount, ByteVector64, Crypto, DeterministicWallet, MilliBtcDouble, MnemonicCode, OutPoint, SatoshiLong, ScriptElt, Transaction, TxId, TxOut} +import fr.acinq.bitcoin.scalacompat.{Block, BtcAmount, MilliBtcDouble, MnemonicCode, OutPoint, SatoshiLong, ScriptElt, Transaction, TxId} import fr.acinq.eclair.NotificationsLogger.NotifyNodeOperator import fr.acinq.eclair.blockchain.bitcoind.BitcoindService import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ @@ -36,19 +36,18 @@ import fr.acinq.eclair.channel.publish.ReplaceableTxPublisher.{Publish, Stop, Up import fr.acinq.eclair.channel.publish.TxPublisher.TxRejectedReason._ import fr.acinq.eclair.channel.publish.TxPublisher._ import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} -import fr.acinq.eclair.crypto.keymanager.{ChannelKeyManager, LocalOnChainKeyManager} +import fr.acinq.eclair.crypto.keymanager.LocalOnChainKeyManager import fr.acinq.eclair.transactions.Transactions import fr.acinq.eclair.transactions.Transactions._ import fr.acinq.eclair.wire.protocol.{CommitSig, RevokeAndAck, UpdateFee} import fr.acinq.eclair.{BlockHeight, MilliSatoshi, MilliSatoshiLong, NodeParams, NotificationsLogger, TestConstants, TestKitBaseClass, TimestampSecond, randomKey} -import org.json4s.JString import org.scalatest.BeforeAndAfterAll import org.scalatest.Inside.inside import org.scalatest.funsuite.AnyFunSuiteLike import scodec.bits.ByteVector import java.util.UUID -import java.util.concurrent.atomic.{AtomicLong, AtomicReference} +import java.util.concurrent.atomic.AtomicLong import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.duration.DurationInt @@ -144,7 +143,7 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w } // NB: we can't use ScalaTest's fixtures, they would see uninitialized bitcoind fields because they sandbox each test. - private def withFixture(utxos: Seq[BtcAmount], channelType: SupportedChannelType, channelKeyManager_opt: Option[ChannelKeyManager] = None)(testFun: Fixture => Any): Unit = { + private def withFixture(utxos: Seq[BtcAmount], channelType: SupportedChannelType)(testFun: Fixture => Any): Unit = { // Create a unique wallet for this test and ensure it has some btc. val testId = UUID.randomUUID() val (walletRpcClient, walletClient) = createTestWallet(s"lightning-$testId") @@ -161,7 +160,7 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w // Setup a valid channel between alice and bob. val blockHeight = new AtomicLong() blockHeight.set(currentBlockHeight(probe).toLong) - val aliceNodeParams = channelKeyManager_opt.map(ckm => TestConstants.Alice.nodeParams.copy(blockHeight = blockHeight, channelKeyManager = ckm)).getOrElse(TestConstants.Alice.nodeParams.copy(blockHeight = blockHeight)) + val aliceNodeParams = TestConstants.Alice.nodeParams.copy(blockHeight = blockHeight) val setup = init(aliceNodeParams, TestConstants.Bob.nodeParams.copy(blockHeight = blockHeight), wallet_opt = Some(walletClient)) val testTags = channelType match { case _: ChannelTypes.AnchorOutputsZeroFeeHtlcTx => Set(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs) @@ -408,36 +407,7 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w } test("commit tx feerate too low, spending anchor output (local commit)") { - //we create a new channel key manager that delegates all call to Alice's key manager but track extra utxos passed to the sign() methods - val walletInputs = new AtomicReference[Map[OutPoint, TxOut]](Map.empty) - val channelKeyManagerA = TestConstants.Alice.nodeParams.channelKeyManager - // @formatter:off - val channelKeyManager = new ChannelKeyManager { - override def fundingPublicKey(fundingKeyPath: DeterministicWallet.KeyPath, fundingTxIndex: Long): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.fundingPublicKey(fundingKeyPath, fundingTxIndex) - override def revocationPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.revocationPoint(channelKeyPath) - override def paymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.paymentPoint(channelKeyPath) - override def delayedPaymentPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.delayedPaymentPoint(channelKeyPath) - override def htlcPoint(channelKeyPath: DeterministicWallet.KeyPath): DeterministicWallet.ExtendedPublicKey = channelKeyManagerA.htlcPoint(channelKeyPath) - override def commitmentSecret(channelKeyPath: DeterministicWallet.KeyPath, index: Long): Crypto.PrivateKey = channelKeyManagerA.commitmentSecret(channelKeyPath, index) - override def commitmentPoint(channelKeyPath: DeterministicWallet.KeyPath, index: Long): PublicKey = channelKeyManagerA.commitmentPoint(channelKeyPath, index) - override def newFundingKeyPath(isChannelOpener: Boolean): DeterministicWallet.KeyPath = channelKeyManagerA.newFundingKeyPath(isChannelOpener) - override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { - walletInputs.set(extraUtxos) - channelKeyManagerA.sign(tx, publicKey, txOwner, commitmentFormat, extraUtxos) - } - override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remotePoint: PublicKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { - walletInputs.set(extraUtxos) - channelKeyManagerA.sign(tx, publicKey, remotePoint, txOwner, commitmentFormat, extraUtxos) - } - override def sign(tx: TransactionWithInputInfo, publicKey: DeterministicWallet.ExtendedPublicKey, remoteSecret: Crypto.PrivateKey, txOwner: TxOwner, commitmentFormat: CommitmentFormat, extraUtxos: Map[OutPoint, TxOut]): ByteVector64 = { - walletInputs.set(extraUtxos) - channelKeyManagerA.sign(tx, publicKey, remoteSecret, txOwner, commitmentFormat, extraUtxos) - } - override def signChannelAnnouncement(witness: ByteVector, fundingKeyPath: DeterministicWallet.KeyPath): ByteVector64 = channelKeyManagerA.signChannelAnnouncement(witness, fundingKeyPath) - } - // @formatter:on - - withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx(), Some(channelKeyManager)) { f => + withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx()) { f => import f._ val (commitTx, anchorTx) = closeChannelWithoutHtlcs(f, aliceBlockHeight() + 30) @@ -452,19 +422,13 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w // wait for the commit tx and anchor tx to be published val mempoolTxs = getMempoolTxs(2) - // check that the wallet input added by the tx funder was passed to the key manager assert(mempoolTxs.map(_.txid).contains(commitTx.tx.txid)) - def getTransaction(txid: TxId): Transaction = { - bitcoinrpcclient.invoke("getrawtransaction", txid).pipeTo(probe.ref) - Transaction.read(probe.expectMsgType[JString].values) - } - + // we check that the anchor tx contains additional wallet inputs // there are 2 transactions in the mempool, the one that is not the commit tx has to be the anchor tx - val publishedAnchorTx = getTransaction(mempoolTxs.filterNot(_.txid == commitTx.tx.txid).head.txid) - val extraInputs = publishedAnchorTx.txIn.tail.map(input => input.outPoint -> getTransaction(input.outPoint.txid).txOut(input.outPoint.index.toInt)).toMap - // check that inputs added to the anchor tx match what was passed to our key manager's sign() method - assert(walletInputs.get() == extraInputs) + wallet.getTransaction(mempoolTxs.filterNot(_.txid == commitTx.tx.txid).head.txid).pipeTo(probe.ref) + val publishedAnchorTx = probe.expectMsgType[Transaction] + assert(publishedAnchorTx.txIn.size > 1) val targetFee = Transactions.weight2fee(targetFeerate, mempoolTxs.map(_.weight).sum.toInt) val actualFee = mempoolTxs.map(_.fees).sum diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala index 6a7bffb52e..b6ce30a205 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala @@ -23,7 +23,6 @@ import fr.acinq.bitcoin.scalacompat.{Btc, ByteVector32, Crypto, MilliBtc, MilliB import fr.acinq.bitcoin.{ScriptFlags, ScriptTree, SigHash, SigVersion} import fr.acinq.eclair.TestUtils.randomTxId import fr.acinq.eclair._ -import fr.acinq.eclair.blockchain.fee.ConfirmationPriority.Fast import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerKw} import fr.acinq.eclair.channel.Helpers.Funding import fr.acinq.eclair.transactions.CommitmentOutput.{InHtlc, OutHtlc} @@ -37,7 +36,7 @@ import scodec.bits._ import java.nio.ByteOrder import scala.io.Source -import scala.util.Random +import scala.util.{Random, Try} /** * Created by PM on 16/12/2016. @@ -489,7 +488,9 @@ class TransactionsSpec extends AnyFunSuite with Logging { } test("generate valid commitment and htlc transactions (anchor outputs)") { - val finalPubKeyScript = Script.write(Script.pay2wpkh(PrivateKey(randomBytes32()).publicKey)) + val walletPriv = randomKey() + val walletPub = walletPriv.publicKey + val finalPubKeyScript = Script.write(Script.pay2wpkh(walletPub)) val commitInput = Funding.makeFundingInputInfo(randomTxId(), 0, Btc(1), localFundingPriv.publicKey, remoteFundingPriv.publicKey) // htlc1, htlc2a and htlc2b are regular IN/OUT htlcs @@ -582,12 +583,30 @@ class TransactionsSpec extends AnyFunSuite with Logging { assert(checkSpendable(signedTx).isSuccess) } { - // local spends local anchor - val Right(claimAnchorOutputTx) = makeClaimLocalAnchorOutputTx(commitTx.tx, localFundingPriv.publicKey, ConfirmationTarget.Absolute(BlockHeight(0))) - assert(checkSpendable(claimAnchorOutputTx).isFailure) - val localSig = claimAnchorOutputTx.sign(localFundingPriv, TxOwner.Local, UnsafeLegacyAnchorOutputsCommitmentFormat, Map.empty) + // local spends local anchor with additional wallet inputs + val walletAmount = 50_000 sat + val walletInputs = Map( + OutPoint(randomTxId(), 3) -> TxOut(walletAmount, Script.pay2wpkh(walletPub)), + OutPoint(randomTxId(), 0) -> TxOut(walletAmount, Script.pay2wpkh(walletPub)), + ) + val Right(claimAnchorOutputTx) = makeClaimLocalAnchorOutputTx(commitTx.tx, localFundingPriv.publicKey, ConfirmationTarget.Absolute(BlockHeight(0))).map(anchorTx => { + val walletTxIn = walletInputs.map { case (outpoint, _) => TxIn(outpoint, ByteVector.empty, 0) } + val unsignedTx = anchorTx.tx.copy(txIn = anchorTx.tx.txIn ++ walletTxIn) + val sig1 = unsignedTx.signInput(1, Script.pay2pkh(walletPub), SIGHASH_ALL, walletAmount, SigVersion.SIGVERSION_WITNESS_V0, walletPriv) + val sig2 = unsignedTx.signInput(2, Script.pay2pkh(walletPub), SIGHASH_ALL, walletAmount, SigVersion.SIGVERSION_WITNESS_V0, walletPriv) + val walletSignedTx = unsignedTx + .updateWitness(1, Script.witnessPay2wpkh(walletPub, sig1)) + .updateWitness(2, Script.witnessPay2wpkh(walletPub, sig2)) + anchorTx.copy(tx = walletSignedTx) + }) + val allInputs = walletInputs + (claimAnchorOutputTx.input.outPoint -> claimAnchorOutputTx.input.txOut) + assert(Try(Transaction.correctlySpends(claimAnchorOutputTx.tx, allInputs, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)).isFailure) + // All wallet inputs must be provided when signing. + assert(Try(claimAnchorOutputTx.sign(localFundingPriv, TxOwner.Local, UnsafeLegacyAnchorOutputsCommitmentFormat, Map.empty)).isFailure) + assert(Try(claimAnchorOutputTx.sign(localFundingPriv, TxOwner.Local, UnsafeLegacyAnchorOutputsCommitmentFormat, walletInputs.take(1))).isFailure) + val localSig = claimAnchorOutputTx.sign(localFundingPriv, TxOwner.Local, UnsafeLegacyAnchorOutputsCommitmentFormat, walletInputs) val signedTx = addSigs(claimAnchorOutputTx, localSig) - assert(checkSpendable(signedTx).isSuccess) + Transaction.correctlySpends(signedTx.tx, allInputs, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS) } { // remote spends remote anchor