diff --git a/core/src/main/scala/org/bitcoins/core/protocol/transaction/TxUtil.scala b/core/src/main/scala/org/bitcoins/core/protocol/transaction/TxUtil.scala index b760bff1d2..f6046b9d1d 100644 --- a/core/src/main/scala/org/bitcoins/core/protocol/transaction/TxUtil.scala +++ b/core/src/main/scala/org/bitcoins/core/protocol/transaction/TxUtil.scala @@ -24,6 +24,11 @@ import scala.util.{Failure, Success, Try} object TxUtil extends BitcoinSLogger { + def isRBFEnabled(transaction: Transaction): Boolean = { + transaction.inputs.exists( + _.sequence < TransactionConstants.disableRBFSequence) + } + private def computeNextLockTime( currentLockTimeOpt: Option[UInt32], locktime: Long): Try[UInt32] = { 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 10256d031b..0a942ed356 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala @@ -198,6 +198,40 @@ class WalletIntegrationTest extends BitcoinSWalletTest { } } + it should "fail to RBF a confirmed transaction" in { walletWithBitcoind => + val WalletWithBitcoindRpc(wallet, bitcoind) = walletWithBitcoind + + for { + // Fund wallet + addr <- wallet.getNewAddress() + txId <- bitcoind.sendToAddress(addr, valueFromBitcoind) + _ <- bitcoind.getNewAddress.flatMap(bitcoind.generateToAddress(6, _)) + rawTx <- bitcoind.getRawTransaction(txId) + _ <- wallet.processTransaction(rawTx.hex, rawTx.blockhash) + + // Verify we funded the wallet + balance <- wallet.getBalance() + _ = assert(balance == valueFromBitcoind) + + // Create rbf tx + rbf <- bitcoind.getNewAddress.flatMap { + wallet.sendToAddress(_, valueToBitcoind, SatoshisPerVirtualByte.one) + } + _ <- bitcoind.sendRawTransaction(rbf) + + // Confirm transaction + _ <- bitcoind.getNewAddress.flatMap(bitcoind.generateToAddress(1, _)) + rawTx1 <- bitcoind.getRawTransaction(rbf.txIdBE) + _ = require(rawTx1.blockhash.isDefined) + _ <- wallet.processTransaction(rbf, rawTx1.blockhash) + + // fail to RBF confirmed tx + res <- recoverToSucceededIf[IllegalArgumentException] { + wallet.bumpFeeRBF(rbf.txIdBE, SatoshisPerVirtualByte.fromLong(20)) + } + } yield res + } + it should "correctly bump fees with CPFP" in { walletWithBitcoind => val WalletWithBitcoindRpc(wallet, bitcoind) = walletWithBitcoind diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala index 7a8f2f6130..92ade51e87 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala @@ -2,16 +2,22 @@ package org.bitcoins.wallet import org.bitcoins.core.api.wallet.{CoinSelectionAlgo, CoinSelector} import org.bitcoins.core.currency._ +import org.bitcoins.core.number.{Int32, UInt32} import org.bitcoins.core.protocol.BitcoinAddress +import org.bitcoins.core.protocol.script.EmptyScriptSignature import org.bitcoins.core.protocol.transaction.{ + BaseTransaction, EmptyTransaction, + TransactionConstants, + TransactionInput, TransactionOutput } +import org.bitcoins.core.psbt.PSBT import org.bitcoins.core.script.constant.{BytesToPushOntoStack, ScriptConstant} import org.bitcoins.core.script.control.OP_RETURN import org.bitcoins.core.wallet.fee._ import org.bitcoins.core.wallet.utxo.TxoState -import org.bitcoins.crypto.CryptoUtil +import org.bitcoins.crypto.{CryptoUtil, DoubleSha256DigestBE} import org.bitcoins.testkit.Implicits.GeneratorOps import org.bitcoins.testkit.core.gen.FeeUnitGen import org.bitcoins.testkit.wallet.BitcoinSWalletTest @@ -288,6 +294,50 @@ class WalletSendingTest extends BitcoinSWalletTest { } } + it should "fail to RBF a confirmed transaction" in { fundedWallet => + val wallet = fundedWallet.wallet + + val feeRate = FeeUnitGen.satsPerByte.sampleSome + val newFeeRate = SatoshisPerByte(feeRate.currencyUnit + Satoshis.one) + + for { + tx <- wallet.sendToAddress(testAddress, amountToSend, feeRate) + _ <- wallet.processTransaction(tx, Some(DoubleSha256DigestBE.empty)) + + res <- recoverToSucceededIf[IllegalArgumentException] { + wallet.bumpFeeRBF(tx.txIdBE, newFeeRate) + } + } yield res + } + + it should "fail to RBF a non-signaling transaction" in { fundedWallet => + val wallet = fundedWallet.wallet + + for { + addr <- wallet.getNewAddress() + utxo <- wallet.listUtxos().map(_.head) + + // Create tx not signaling RBF + input = TransactionInput(utxo.outPoint, + EmptyScriptSignature, + TransactionConstants.disableRBFSequence) + output = + TransactionOutput(utxo.output.value - Satoshis(500), addr.scriptPubKey) + tx = + BaseTransaction(Int32.two, Vector(input), Vector(output), UInt32.zero) + psbt = PSBT.fromUnsignedTx(tx) + + // Have wallet sign and process transaction + signedPSBT <- wallet.signPSBT(psbt) + signedTx = signedPSBT.finalizePSBT.get.extractTransactionAndValidate.get + _ <- wallet.processTransaction(signedTx, None) + + res <- recoverToSucceededIf[IllegalArgumentException] { + wallet.bumpFeeRBF(signedTx.txIdBE, SatoshisPerVirtualByte.fromLong(100)) + } + } yield res + } + it should "correctly CPFP a transaction" in { fundedWallet => val wallet = fundedWallet.wallet for { @@ -324,6 +374,21 @@ class WalletSendingTest extends BitcoinSWalletTest { } } + it should "fail to CPFP a confirmed transaction" in { fundedWallet => + val wallet = fundedWallet.wallet + + val feeRate = FeeUnitGen.satsPerByte.sampleSome + + for { + tx <- wallet.sendToAddress(testAddress, amountToSend, feeRate) + _ <- wallet.processTransaction(tx, Some(DoubleSha256DigestBE.empty)) + + res <- recoverToSucceededIf[IllegalArgumentException] { + wallet.bumpFeeCPFP(tx.txIdBE, feeRate) + } + } yield res + } + it should "fail to CPFP a transaction we don't own" in { fundedWallet => val wallet = fundedWallet.wallet diff --git a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala index ca945c8f1a..1118cc7aa1 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala @@ -506,6 +506,8 @@ abstract class Wallet new RuntimeException(s"Unable to find transaction ${txId.hex}")) } + _ = require(TxUtil.isRBFEnabled(tx), "Transaction is not signaling RBF") + outPoints = tx.inputs.map(_.previousOutput).toVector spks = tx.outputs.map(_.scriptPubKey).toVector @@ -513,6 +515,13 @@ abstract class Wallet _ = require(utxos.nonEmpty, "Can only bump fee for our own transaction") _ = require(utxos.size == tx.inputs.size, "Can only bump fee for a transaction we own all the inputs") + + oldOutputs <- spendingInfoDAO.findDbsForTx(txId) + blockHashes = oldOutputs.flatMap(_.blockHash).distinct + _ = require( + blockHashes.isEmpty, + s"Cannot replace a confirmed transaction, ${blockHashes.map(_.hex)}") + spendingInfos <- FutureUtil.sequentially(utxos) { utxo => transactionDAO .findByOutPoint(utxo.outPoint) @@ -558,9 +567,8 @@ abstract class Wallet } // Mark old outputs as replaced - oldUtxos <- spendingInfoDAO.findDbsForTx(txId) _ <- spendingInfoDAO.updateAll( - oldUtxos.map(_.copyWithState(TxoState.DoesNotExist))) + oldOutputs.map(_.copyWithState(TxoState.DoesNotExist))) sequence = tx.inputs.head.sequence + UInt32.one outputs = tx.outputs.filterNot(_.scriptPubKey == changeSpk) @@ -721,6 +729,12 @@ abstract class Wallet _ = require(spendingInfos.nonEmpty, s"Transaction ${txId.hex} must have an output we own") + oldOutputs <- spendingInfoDAO.findDbsForTx(txId) + blockHashes = oldOutputs.flatMap(_.blockHash).distinct + _ = require( + blockHashes.isEmpty, + s"No need to fee bump a confirmed transaction, ${blockHashes.map(_.hex)}") + changeSpendingInfos = spendingInfos.flatMap { db => if (db.privKeyPath.chain.chainType == HDChainType.Change) { Some(db)