From b6cc97a663373100a346ea42f4e578a2e12e6088 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Thu, 21 Nov 2024 08:59:15 -0600 Subject: [PATCH] 2024 11 20 prevoutmap ordering (#5776) * Add test case and add invariant to RawTxSigner.sign() * Add InputInfo.sortPreviousOutputMap() * Fix bug where we were sorting prevoutputmap when it didn't need to be sorted --- .../core/wallet/builder/RawTxSigner.scala | 27 +++++++++++++---- .../bitcoins/core/wallet/utxo/InputInfo.scala | 29 +++++++++++++++++++ .../wallet/WalletIntegrationTest.scala | 26 +++++++++++++++++ .../internal/SendFundsHandlingHandling.scala | 11 ++++--- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/core/src/main/scala/org/bitcoins/core/wallet/builder/RawTxSigner.scala b/core/src/main/scala/org/bitcoins/core/wallet/builder/RawTxSigner.scala index b230ecd1d5..6bc488e918 100644 --- a/core/src/main/scala/org/bitcoins/core/wallet/builder/RawTxSigner.scala +++ b/core/src/main/scala/org/bitcoins/core/wallet/builder/RawTxSigner.scala @@ -1,8 +1,14 @@ package org.bitcoins.core.wallet.builder import org.bitcoins.core.crypto.TxSigComponent -import org.bitcoins.core.protocol.script.ScriptWitness -import org.bitcoins.core.protocol.transaction._ +import org.bitcoins.core.protocol.script.{ + NonWitnessScriptPubKey, + ScriptWitness, + TaprootScriptPubKey, + UnassignedWitnessScriptPubKey, + WitnessScriptPubKeyV0 +} +import org.bitcoins.core.protocol.transaction.* import org.bitcoins.core.wallet.fee.FeeUnit import org.bitcoins.core.wallet.signer.BitcoinSigner import org.bitcoins.core.wallet.utxo.{ @@ -105,9 +111,18 @@ object RawTxSigner { s"Must provide exactly one UTXOSatisfyingInfo per input, ${utxoInfos.length} != ${utx.inputs.length}") require(utxoInfos.distinct.length == utxoInfos.length, "All UTXOSatisfyingInfos must be unique. ") - require(utxoInfos.forall(utxo => - utx.inputs.exists(_.previousOutput == utxo.outPoint)), - "All UTXOSatisfyingInfos must correspond to an input.") + val utxOutPoints = utx.inputs.map(_.previousOutput) + val sortedUtxoInfos = utxoInfos.map { u => + u.output.scriptPubKey match { + case _: NonWitnessScriptPubKey | _: WitnessScriptPubKeyV0 | + _: UnassignedWitnessScriptPubKey => + // no sorting needed for these spk types as the sighash algorithm + // doesn't include all outputs + u + case _: TaprootScriptPubKey => + u.copy(inputInfo = u.inputInfo.sortPreviousOutputMap(utxOutPoints)) + } + } val signedTx = if ( @@ -121,7 +136,7 @@ object RawTxSigner { .setLockTime(utx.lockTime) .++=(utx.outputs) - val inputsAndWitnesses = utxoInfos.map { utxo => + val inputsAndWitnesses = sortedUtxoInfos.map { utxo => val txSigComp = BitcoinSigner.sign(utxo, utx) val scriptWitnessOpt = TxSigComponent.getScriptWitness(txSigComp) diff --git a/core/src/main/scala/org/bitcoins/core/wallet/utxo/InputInfo.scala b/core/src/main/scala/org/bitcoins/core/wallet/utxo/InputInfo.scala index 821e71051c..8b79a46a74 100644 --- a/core/src/main/scala/org/bitcoins/core/wallet/utxo/InputInfo.scala +++ b/core/src/main/scala/org/bitcoins/core/wallet/utxo/InputInfo.scala @@ -8,6 +8,11 @@ import org.bitcoins.core.protocol.transaction.* import org.bitcoins.core.script.constant.{OP_TRUE, ScriptConstant} import org.bitcoins.core.script.util.PreviousOutputMap import org.bitcoins.core.util.{BitcoinScriptUtil, BytesUtil} +import org.bitcoins.core.wallet.utxo.InputInfo.{ + getHashPreImages, + getRedeemScript, + getScriptWitness +} import org.bitcoins.crypto.{ ECDigitalSignature, ECPublicKey, @@ -83,6 +88,30 @@ sealed trait InputInfo { } def previousOutputMap: PreviousOutputMap + + /** Sorts our [[previousOutputMap]] to be in the same ordering as the given + * outPoints This is necessary as the outpoints must be signed in the exact + * order they appear in the inputs of our [[Transaction]] according to BIP341 + * @return + * InputInfo with the [[previousOutputMap]] sorted correctly + */ + def sortPreviousOutputMap( + outPoints: Vector[TransactionOutPoint]): InputInfo = { + require( + outPoints.forall(o => previousOutputMap.get(o).isDefined), + s"Could not find all outPoints in map, outPoints=$outPoints previousOutputMap=${previousOutputMap.outputMap.keys}" + ) + val sorted = outPoints.map(o => o -> previousOutputMap(o)).toMap + InputInfo( + outPoint = outPoint, + output = output, + redeemScriptOpt = getRedeemScript(this), + scriptWitnessOpt = getScriptWitness(this), + conditionalPath = conditionalPath, + previousOutputMap = PreviousOutputMap(sorted), + hashPreImages = getHashPreImages(this) + ) + } } object InputInfo { diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala index 484f1d7b64..a93af1ad0e 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala @@ -411,4 +411,30 @@ class WalletIntegrationTest extends BitcoinSWalletTestCachedBitcoindNewest { newBalance <- bitcoind.getBalance } yield assert(newBalance == oldBalance + amountToSend + Bitcoins(50)) } + + it must "sweep the wallet with multiple utxos" in { walletWithBitcoind => + val wallet = walletWithBitcoind.wallet + val bitcoind = walletWithBitcoind.bitcoind + val addr1F = wallet.getNewAddress() + val addr2F = wallet.getNewAddress() + val bitcoindAddr1F = bitcoind.getNewAddress + for { + addr1 <- addr1F + addr2 <- addr2F + txId1 <- bitcoind.sendToAddress(addr1, valueFromBitcoind) + txId2 <- bitcoind.sendToAddress(addr2, valueFromBitcoind) + tx1 <- bitcoind.getRawTransactionRaw(txId1) + tx2 <- bitcoind.getRawTransactionRaw(txId2) + _ <- wallet.transactionProcessing.processTransaction(tx1, None) + _ <- wallet.transactionProcessing.processTransaction(tx2, None) + balance1 <- wallet.getBalance() + _ = assert(balance1 == valueFromBitcoind * 2) + bitcoindAddr1 <- bitcoindAddr1F + sweepTx <- wallet.sendFundsHandling.sweepWallet(bitcoindAddr1, None) + _ <- bitcoind.sendRawTransaction(sweepTx) + balance2 <- wallet.getBalance() + } yield { + assert(balance2 == Satoshis.zero) + } + } } diff --git a/wallet/src/main/scala/org/bitcoins/wallet/internal/SendFundsHandlingHandling.scala b/wallet/src/main/scala/org/bitcoins/wallet/internal/SendFundsHandlingHandling.scala index d6eb626431..ee00c2b5e7 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/internal/SendFundsHandlingHandling.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/internal/SendFundsHandlingHandling.scala @@ -488,9 +488,9 @@ case class SendFundsHandlingHandling( txBuilder = RawTxBuilder() ++= inputs += dummyOutput finalizer = SubtractFeeFromOutputsFinalizer( - inputInfos, - feeRate, - Vector(address.scriptPubKey) + inputInfos = inputInfos, + feeRate = feeRate, + spks = Vector(address.scriptPubKey) ) .andThen(ShuffleFinalizer) .andThen(AddWitnessDataFinalizer(inputInfos)) @@ -503,7 +503,10 @@ case class SendFundsHandlingHandling( tmp.outputs.size == 1, s"Created tx is not as expected, does not have 1 output, got $tmp" ) - rawTxHelper = FundRawTxHelper(withFinalizer, utxos, feeRate, Future.unit) + rawTxHelper = FundRawTxHelper(txBuilderWithFinalizer = withFinalizer, + scriptSigParams = utxos, + feeRate = feeRate, + reservedUTXOsCallbackF = Future.unit) tx <- finishSend( rawTxHelper, tmp.outputs.head.value,