From 0cf396e65778ccf5631c7bac728b31ad83e3f22e Mon Sep 17 00:00:00 2001 From: t-bast Date: Fri, 30 Apr 2021 12:54:38 +0200 Subject: [PATCH 1/7] Bitcoin client allows CPFP Our bitcoin client can create a CPFP tx that spends unconfirmed outpoints that belong to our wallet. Note that this only works for wallet outpoints, not outpoints using lightning pubkey scripts. --- .../bitcoind/rpc/BitcoinCoreClient.scala | 93 +++++++- .../bitcoind/BitcoinCoreClientSpec.scala | 202 +++++++++++++++++- 2 files changed, 282 insertions(+), 13 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 5298af0dce..319a1eeb5b 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 @@ -255,6 +255,75 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall } } + /** + * Create a child-pays-for-parent transaction to increase the effective feerate of a set of unconfirmed transactions. + * These unconfirmed transactions must: + * - be in our mempool (evicted transactions cannot be used) + * - have an output that can be spent by our bitcoin wallet (provided in the outpoints set) + * - the total amount of the set of outpoints must be high enough to pay the target feerate + * + * @param outpoints outpoints that should be spent by the CPFP transaction. + * @param targetFeerate feerate to apply to the package of unconfirmed transactions. + */ + def cpfp(outpoints: Set[OutPoint], targetFeerate: FeeratePerKw)(implicit ec: ExecutionContext): Future[Transaction] = { + getMempoolPackage(outpoints.map(_.txid), Map.empty).transformWith { + case Failure(ex) => Future.failed(new IllegalArgumentException("unable to analyze mempool package: some transactions could not be found in your mempool", ex)) + case Success(mempoolPackage) => + getTxOutputs(outpoints).transformWith { + case Failure(ex) => Future.failed(new IllegalArgumentException("some transactions could not be found", ex)) + case Success(txOutputs) => + getChangeAddress().transformWith { + case Failure(ex) => Future.failed(new IllegalArgumentException("change address generation failed", ex)) + case Success(changeAddress) => + val amountIn = txOutputs.values.map(_.amount).sum + // We build a transaction spending all the inputs provided to a single change output. Our inputs are + // using either p2wpkh or p2tr: p2tr inputs are slightly smaller, but we don't bother doing an exact + // calculation and always use the weight of p2wpkh inputs for simplicity. + val p2wpkhInputWeight = 272 + val txWeight = p2wpkhInputWeight * outpoints.size + Transaction(2, Nil, Seq(TxOut(amountIn, Script.pay2wpkh(changeAddress))), 0).weight() + val totalWeight = mempoolPackage.values.map(_.weight).sum + txWeight + val targetFees = Transactions.weight2fee(targetFeerate, totalWeight.toInt) + val currentFees = mempoolPackage.values.map(_.fees).sum + val missingFees = targetFees - currentFees + if (missingFees <= 0.sat) { + Future.failed(new IllegalArgumentException("package feerate is already higher than the target feerate")) + } else if (amountIn <= missingFees + 660.sat) { + Future.failed(new IllegalArgumentException("input amount is not sufficient to cover the target feerate")) + } else { + val unsignedTx = Transaction(2, outpoints.toSeq.map(o => TxIn(o, Seq.empty, 0)), Seq(TxOut(amountIn - missingFees, Script.pay2wpkh(changeAddress))), 0) + signTransaction(unsignedTx, Nil).transformWith { + case Failure(ex) => Future.failed(new IllegalArgumentException("tx signing failed: some inputs don't belong to our wallet", ex)) + case Success(signedTx) => publishTransaction(signedTx.tx).map(_ => signedTx.tx) + } + } + } + } + } + } + + /** Recursively fetch unconfirmed parents and return the complete unconfirmed ancestors tree. */ + private def getMempoolPackage(leaves: Set[ByteVector32], current: Map[ByteVector32, MempoolTx])(implicit ec: ExecutionContext): Future[Map[ByteVector32, MempoolTx]] = { + Future.sequence(leaves.map(txid => getMempoolTx(txid))).flatMap(txs => { + val current2 = current.concat(txs.map(tx => tx.txid -> tx)) + val remainingParents = txs.flatMap(_.unconfirmedParents) -- current2.keySet + if (remainingParents.isEmpty) { + Future.successful(current2) + } else { + getMempoolPackage(remainingParents, current2) + } + }) + } + + /** Fetch transaction output details for the given outpoints. */ + private def getTxOutputs(outpoints: Set[OutPoint])(implicit ec: ExecutionContext): Future[Map[OutPoint, TxOut]] = { + Future.sequence(outpoints.map(_.txid).map(txid => getTransaction(txid))).map(txs => { + outpoints.flatMap(o => txs.find(tx => tx.txid == o.txid && o.index < tx.txOut.length) match { + case Some(tx) => Some(o -> tx.txOut(o.index.toInt)) + case None => None + }).toMap + }) + } + //------------------------- SIGNING -------------------------// def signTransaction(tx: Transaction)(implicit ec: ExecutionContext): Future[SignTransactionResponse] = signTransaction(tx, Nil) @@ -422,8 +491,9 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall val JDecimal(ancestorFees) = json \ "fees" \ "ancestor" val JDecimal(descendantFees) = json \ "fees" \ "descendant" val JBool(replaceable) = json \ "bip125-replaceable" + val unconfirmedParents = (json \ "depends").extract[List[String]].map(ByteVector32.fromValidHex).toSet // NB: bitcoind counts the transaction itself as its own ancestor and descendant, which is confusing: we fix that by decrementing these counters. - MempoolTx(txid, vsize.toLong, weight.toLong, replaceable, toSatoshi(fees), ancestorCount.toInt - 1, toSatoshi(ancestorFees), descendantCount.toInt - 1, toSatoshi(descendantFees)) + MempoolTx(txid, vsize.toLong, weight.toLong, replaceable, toSatoshi(fees), ancestorCount.toInt - 1, toSatoshi(ancestorFees), descendantCount.toInt - 1, toSatoshi(descendantFees), unconfirmedParents) }) } @@ -528,17 +598,18 @@ object BitcoinCoreClient { /** * Information about a transaction currently in the mempool. * - * @param txid transaction id. - * @param vsize virtual transaction size as defined in BIP 141. - * @param weight transaction weight as defined in BIP 141. - * @param replaceable Whether this transaction could be replaced with RBF (BIP125). - * @param fees transaction fees. - * @param ancestorCount number of unconfirmed parent transactions. - * @param ancestorFees transactions fees for the package consisting of this transaction and its unconfirmed parents. - * @param descendantCount number of unconfirmed child transactions. - * @param descendantFees transactions fees for the package consisting of this transaction and its unconfirmed children (without its unconfirmed parents). + * @param txid transaction id. + * @param vsize virtual transaction size as defined in BIP 141. + * @param weight transaction weight as defined in BIP 141. + * @param replaceable Whether this transaction could be replaced with RBF (BIP125). + * @param fees transaction fees. + * @param ancestorCount number of unconfirmed parent transactions. + * @param ancestorFees transactions fees for the package consisting of this transaction and its unconfirmed parents. + * @param descendantCount number of unconfirmed child transactions. + * @param descendantFees transactions fees for the package consisting of this transaction and its unconfirmed children (without its unconfirmed parents). + * @param unconfirmedParents unconfirmed transactions used as inputs for this transaction. */ - case class MempoolTx(txid: ByteVector32, vsize: Long, weight: Long, replaceable: Boolean, fees: Satoshi, ancestorCount: Int, ancestorFees: Satoshi, descendantCount: Int, descendantFees: Satoshi) + case class MempoolTx(txid: ByteVector32, vsize: Long, weight: Long, replaceable: Boolean, fees: Satoshi, ancestorCount: Int, ancestorFees: Satoshi, descendantCount: Int, descendantFees: Satoshi, unconfirmedParents: Set[ByteVector32]) case class WalletTx(address: String, amount: Satoshi, fees: Satoshi, blockHash: ByteVector32, confirmations: Long, txid: ByteVector32, timestamp: Long) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala index c7b3aff0df..7d365d6519 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala @@ -21,7 +21,7 @@ import akka.pattern.pipe import akka.testkit.TestProbe import fr.acinq.bitcoin import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey -import fr.acinq.bitcoin.scalacompat.{Block, Btc, BtcDouble, ByteVector32, MilliBtcDouble, OP_DROP, OP_PUSHDATA, OutPoint, Satoshi, SatoshiLong, Script, ScriptWitness, Transaction, TxIn, TxOut, computeP2PkhAddress, computeP2WpkhAddress} +import fr.acinq.bitcoin.scalacompat.{Block, Btc, BtcDouble, ByteVector32, Crypto, MilliBtcDouble, OP_DROP, OP_PUSHDATA, OutPoint, Satoshi, SatoshiLong, Script, ScriptWitness, Transaction, TxIn, TxOut, computeP2PkhAddress, computeP2WpkhAddress} import fr.acinq.bitcoin.{Bech32, SigHash, SigVersion} import fr.acinq.eclair.blockchain.OnChainWallet.{FundTransactionResponse, MakeFundingTxResponse, OnChainBalance, SignTransactionResponse} import fr.acinq.eclair.blockchain.WatcherSpec.{createSpendManyP2WPKH, createSpendP2WPKH} @@ -31,7 +31,7 @@ import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinJsonRPCAuthMethod.UserPass import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, BitcoinCoreClient, JsonRPCError} import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw} import fr.acinq.eclair.transactions.{Scripts, Transactions} -import fr.acinq.eclair.{BlockHeight, TestConstants, TestKitBaseClass, addressToPublicKeyScript, randomKey} +import fr.acinq.eclair.{BlockHeight, TestConstants, TestKitBaseClass, addressToPublicKeyScript, randomBytes32, randomKey} import grizzled.slf4j.Logging import org.json4s.JsonAST._ import org.json4s.{DefaultFormats, Formats} @@ -842,6 +842,204 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A sender.expectMsg(false) } + test("bump transaction fees with child-pays-for-parent (single tx)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val tx = sendToAddress(getNewAddress(sender), 150_000 sat, sender) + assert(tx.txOut.length === 2) // there must be a change output + val changeOutput = if (tx.txOut.head.amount === 150_000.sat) 1 else 0 + + bitcoinClient.getMempoolTx(tx.txid).pipeTo(sender.ref) + val mempoolTx = sender.expectMsgType[MempoolTx] + val currentFeerate = FeeratePerKw(mempoolTx.fees * 1000 / tx.weight()) + + val targetFeerate = currentFeerate * 1.5 + bitcoinClient.cpfp(Set(OutPoint(tx, changeOutput)), targetFeerate).pipeTo(sender.ref) + val cpfpTx = sender.expectMsgType[Transaction] + bitcoinClient.getMempoolTx(cpfpTx.txid).pipeTo(sender.ref) + val mempoolCpfpTx = sender.expectMsgType[MempoolTx] + assert(mempoolCpfpTx.ancestorFees === mempoolCpfpTx.fees + mempoolTx.fees) + val expectedFees = Transactions.weight2fee(targetFeerate, tx.weight() + cpfpTx.weight()) + assert(expectedFees * 0.95 <= mempoolCpfpTx.ancestorFees && mempoolCpfpTx.ancestorFees <= expectedFees * 1.05) + assert(mempoolCpfpTx.replaceable) + + generateBlocks(1) + } + + test("bump transaction fees with child-pays-for-parent (small package)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + + val fundingFeerate = FeeratePerKw(1000 sat) + val remoteFundingPrivKey = randomKey() + val walletFundingPrivKey = randomKey() + val fundingScript = Scripts.multiSig2of2(remoteFundingPrivKey.publicKey, walletFundingPrivKey.publicKey) + val fundingTx = { + val txNotFunded = Transaction(2, Nil, TxOut(250_000 sat, Script.pay2wsh(fundingScript)) :: Nil, 0) + bitcoinClient.fundTransaction(txNotFunded, FundTransactionOptions(fundingFeerate, changePosition = Some(1))).pipeTo(sender.ref) + val fundTxResponse = sender.expectMsgType[FundTransactionResponse] + assert(fundTxResponse.changePosition === Some(1)) + bitcoinClient.signTransaction(fundTxResponse.tx, Nil).pipeTo(sender.ref) + val signTxResponse = sender.expectMsgType[SignTransactionResponse] + assert(signTxResponse.complete) + bitcoinClient.publishTransaction(signTxResponse.tx).pipeTo(sender.ref) + sender.expectMsg(signTxResponse.tx.txid) + signTxResponse.tx + } + + val mutualCloseTx = { + bitcoinClient.getP2wpkhPubkey().pipeTo(sender.ref) + val walletClosePubKey = sender.expectMsgType[PublicKey] + val remoteClosePubKey = randomKey().publicKey + // NB: the output amounts are chosen so that the feerate is ~750 sat/kw + val unsignedTx = Transaction(2, TxIn(OutPoint(fundingTx, 0), Seq.empty, 0) :: Nil, TxOut(130_000 sat, Script.pay2wpkh(walletClosePubKey)) :: TxOut(119_500 sat, Script.pay2wpkh(remoteClosePubKey)) :: Nil, 0) + val walletSig = Transaction.signInput(unsignedTx, 0, fundingScript, SigHash.SIGHASH_ALL, fundingTx.txOut.head.amount, SigVersion.SIGVERSION_WITNESS_V0, walletFundingPrivKey) + val remoteSig = Transaction.signInput(unsignedTx, 0, fundingScript, SigHash.SIGHASH_ALL, fundingTx.txOut.head.amount, SigVersion.SIGVERSION_WITNESS_V0, remoteFundingPrivKey) + val witness = Scripts.witness2of2(Crypto.der2compact(walletSig), Crypto.der2compact(remoteSig), walletFundingPrivKey.publicKey, remoteFundingPrivKey.publicKey) + val signedTx = unsignedTx.updateWitness(0, witness) + bitcoinClient.publishTransaction(signedTx).pipeTo(sender.ref) + sender.expectMsg(signedTx.txid) + signedTx + } + + val targetFeerate = FeeratePerKw(5000 sat) + bitcoinClient.cpfp(Set(OutPoint(fundingTx, 1), OutPoint(mutualCloseTx, 0)), targetFeerate).pipeTo(sender.ref) + val cpfpTx = sender.expectMsgType[Transaction] + bitcoinClient.getMempoolTx(cpfpTx.txid).pipeTo(sender.ref) + val mempoolCpfpTx = sender.expectMsgType[MempoolTx] + val packageWeight = fundingTx.weight() + mutualCloseTx.weight() + cpfpTx.weight() + val expectedFees = Transactions.weight2fee(targetFeerate, packageWeight) + assert(expectedFees * 0.95 <= mempoolCpfpTx.ancestorFees && mempoolCpfpTx.ancestorFees <= expectedFees * 1.05) + assert(mempoolCpfpTx.replaceable) + + generateBlocks(1) + } + + test("bump transaction fees with child-pays-for-parent (complex package)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val currentFeerate = FeeratePerKw(500 sat) + + // We create two separate trees of transactions that will be bumped together: + // TxA1 TxB1 + // / \ \ + // / \ \ + // TxA2* TxA5 TxB2 + // / \ \ + // / \ \ + // TxA3 TxA6* TxB3* + // / \ \ + // / \ \ + // TxA4* TxA7 TxB4 + + def createTx(dest: Seq[(PublicKey, Satoshi)], walletInput_opt: Option[OutPoint]): Transaction = { + val txIn = walletInput_opt match { + case Some(walletInput) => TxIn(walletInput, Seq.empty, 0) :: Nil + case None => Nil + } + val txOut = dest.map { case (pubKey, amount) => TxOut(amount, Script.pay2wpkh(pubKey)) } + val txNotFunded = Transaction(2, txIn, txOut, 0) + bitcoinClient.fundTransaction(txNotFunded, FundTransactionOptions(currentFeerate, changePosition = Some(txOut.length))).pipeTo(sender.ref) + val fundTxResponse = sender.expectMsgType[FundTransactionResponse] + bitcoinClient.signTransaction(fundTxResponse.tx, Nil).pipeTo(sender.ref) + val signTxResponse = sender.expectMsgType[SignTransactionResponse] + assert(signTxResponse.complete) + bitcoinClient.publishTransaction(signTxResponse.tx).pipeTo(sender.ref) + sender.expectMsg(signTxResponse.tx.txid) + signTxResponse.tx + } + + def getWalletPubKey(sender: TestProbe): PublicKey = { + bitcoinClient.getP2wpkhPubkey().pipeTo(sender.ref) + sender.expectMsgType[PublicKey] + } + + val pubKeyA2 = getWalletPubKey(sender) + val pubKeyA3 = getWalletPubKey(sender) + val pubKeyA4 = getWalletPubKey(sender) + val pubKeyA5 = getWalletPubKey(sender) + val pubKeyA6 = getWalletPubKey(sender) + val pubKeyA7 = getWalletPubKey(sender) + val pubKeyB2 = getWalletPubKey(sender) + val pubKeyB3 = getWalletPubKey(sender) + val pubKeyB4 = getWalletPubKey(sender) + + // NB: we use the same amount to ensure that an additional input will be added and there will be a change output. + val txB1 = createTx(Seq((pubKeyB2, 100_000 sat)), None) + val txB2 = createTx(Seq((pubKeyB3, 100_000 sat)), Some(OutPoint(txB1, 0))) + val txB3 = createTx(Seq((pubKeyB4, 100_000 sat)), Some(OutPoint(txB2, 0))) + val txB4 = createTx(Seq((randomKey().publicKey, 100_000 sat)), Some(OutPoint(txB3, 0))) + val txA1 = createTx(Seq((pubKeyA2, 75_000 sat), (pubKeyA5, 50_000 sat)), None) + val txA2 = createTx(Seq((pubKeyA3, 75_000 sat)), Some(OutPoint(txA1, 0))) + val txA3 = createTx(Seq((pubKeyA4, 75_000 sat)), Some(OutPoint(txA2, 0))) + val txA4 = createTx(Seq((randomKey().publicKey, 75_000 sat)), Some(OutPoint(txA3, 0))) + val txA5 = createTx(Seq((pubKeyA6, 50_000 sat)), Some(OutPoint(txA1, 1))) + val txA6 = createTx(Seq((pubKeyA7, 50_000 sat)), Some(OutPoint(txA5, 0))) + val txA7 = createTx(Seq((randomKey().publicKey, 50_000 sat)), Some(OutPoint(txA6, 0))) + + // We bump a subset of the mempool: TxA1 -> TxA6 and TxB1 -> TxB3 + val targetFeerate = FeeratePerKw(5000 sat) + val bumpedOutpoints = Set(OutPoint(txA2, 1), OutPoint(txA4, 1), OutPoint(txA6, 1), OutPoint(txB3, 1)) + bitcoinClient.cpfp(bumpedOutpoints, targetFeerate).pipeTo(sender.ref) + val cpfpTx = sender.expectMsgType[Transaction] + assert(cpfpTx.txIn.find(_.outPoint.txid == txB4.txid) === None) + assert(cpfpTx.txIn.find(_.outPoint.txid == txA7.txid) === None) + + bitcoinClient.getMempoolTx(cpfpTx.txid).pipeTo(sender.ref) + val mempoolCpfpTx = sender.expectMsgType[MempoolTx] + val packageWeight = txA1.weight() + txA2.weight() + txA3.weight() + txA4.weight() + txA5.weight() + txA6.weight() + txB1.weight() + txB2.weight() + txB3.weight() + cpfpTx.weight() + val expectedFees = Transactions.weight2fee(targetFeerate, packageWeight) + assert(expectedFees * 0.95 <= mempoolCpfpTx.ancestorFees && mempoolCpfpTx.ancestorFees <= expectedFees * 1.05) + assert(mempoolCpfpTx.replaceable) + + generateBlocks(1) + } + + test("cannot bump transaction fees (unknown transaction)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + bitcoinClient.cpfp(Set(OutPoint(randomBytes32(), 0), OutPoint(randomBytes32(), 3)), FeeratePerKw(1500 sat)).pipeTo(sender.ref) + val failure = sender.expectMsgType[Failure] + assert(failure.cause.getMessage.contains("some transactions could not be found")) + } + + test("cannot bump transaction fees (transaction already confirmed)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val tx = sendToAddress(getNewAddress(sender), 45_000 sat, sender) + generateBlocks(1) + + bitcoinClient.cpfp(Set(OutPoint(tx, 0)), FeeratePerKw(2500 sat)).pipeTo(sender.ref) + val failure = sender.expectMsgType[Failure] + assert(failure.cause.getMessage.contains("some transactions could not be found")) + } + + test("cannot bump transaction fees (non-wallet input)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val txNotFunded = Transaction(2, Nil, TxOut(50_000 sat, Script.pay2wpkh(randomKey().publicKey)) :: Nil, 0) + bitcoinClient.fundTransaction(txNotFunded, FundTransactionOptions(FeeratePerKw(1000 sat), changePosition = Some(1))).pipeTo(sender.ref) + val fundTxResponse = sender.expectMsgType[FundTransactionResponse] + bitcoinClient.signTransaction(fundTxResponse.tx, Nil).pipeTo(sender.ref) + val signTxResponse = sender.expectMsgType[SignTransactionResponse] + bitcoinClient.publishTransaction(signTxResponse.tx).pipeTo(sender.ref) + sender.expectMsg(signTxResponse.tx.txid) + + bitcoinClient.cpfp(Set(OutPoint(signTxResponse.tx, 0)), FeeratePerKw(1500 sat)).pipeTo(sender.ref) + val failure = sender.expectMsgType[Failure] + assert(failure.cause.getMessage.contains("some inputs don't belong to our wallet")) + } + + test("cannot bump transaction fees (amount too low)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val tx = sendToAddress(getNewAddress(sender), 2500 sat, sender) + val outputIndex = if (tx.txOut.head.amount == 2500.sat) 0 else 1 + bitcoinClient.cpfp(Set(OutPoint(tx, outputIndex)), FeeratePerKw(50_000 sat)).pipeTo(sender.ref) + val failure = sender.expectMsgType[Failure] + assert(failure.cause.getMessage.contains("input amount is not sufficient to cover the target feerate")) + } + test("detect if tx has been double-spent") { val sender = TestProbe() val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) From 1a4a9a9810c95849f2a99bf95ad906dfac003f9a Mon Sep 17 00:00:00 2001 From: t-bast Date: Fri, 30 Apr 2021 14:35:50 +0200 Subject: [PATCH 2/7] Add API for CPFP fee bumping Add API to let node operators initiate CPFP. It's up to the node operator to select which outpoints to spend and ensure that they are spendable by the bitcoind wallet. --- eclair-core/eclair-cli | 1 + .../main/scala/fr/acinq/eclair/Eclair.scala | 11 ++++- .../api/directives/ExtraDirectives.scala | 3 +- .../acinq/eclair/api/handlers/OnChain.scala | 10 ++++- .../api/serde/FormParamExtractors.scala | 7 ++- .../fr/acinq/eclair/api/ApiServiceSpec.scala | 44 ++++++++++++++++++- 6 files changed, 70 insertions(+), 6 deletions(-) diff --git a/eclair-core/eclair-cli b/eclair-core/eclair-cli index c5a9a962e6..3bfe4c6d3a 100755 --- a/eclair-core/eclair-cli +++ b/eclair-core/eclair-cli @@ -85,6 +85,7 @@ and COMMAND is one of the available commands: === OnChain === - getnewaddress - sendonchain + - cpfpbumpfees - onchainbalance - onchaintransactions - globalbalance diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala index 3cac3e9511..c0f6897ad7 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala @@ -24,7 +24,7 @@ import akka.pattern._ import akka.util.Timeout import com.softwaremill.quicklens.ModifyPimp import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey -import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, Satoshi, Script, addressToPublicKeyScript} +import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, OutPoint, Satoshi, Script, addressToPublicKeyScript} import fr.acinq.eclair.ApiTypes.ChannelNotFound import fr.acinq.eclair.balance.CheckBalance.GlobalBalance import fr.acinq.eclair.balance.{BalanceActor, ChannelsListener} @@ -129,6 +129,8 @@ trait Eclair { def sendOnChain(address: String, amount: Satoshi, confirmationTarget: Long): Future[ByteVector32] + def cpfpBumpFees(targetFeeratePerByte: FeeratePerByte, outpoints: Set[OutPoint]): Future[ByteVector32] + def findRoute(targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], extraEdges: Seq[Invoice.ExtraEdge] = Seq.empty, includeLocalChannelCost: Boolean = false, ignoreNodeIds: Seq[PublicKey] = Seq.empty, ignoreShortChannelIds: Seq[ShortChannelId] = Seq.empty, maxFee_opt: Option[MilliSatoshi] = None)(implicit timeout: Timeout): Future[RouteResponse] def findRouteBetween(sourceNodeId: PublicKey, targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], extraEdges: Seq[Invoice.ExtraEdge] = Seq.empty, includeLocalChannelCost: Boolean = false, ignoreNodeIds: Seq[PublicKey] = Seq.empty, ignoreShortChannelIds: Seq[ShortChannelId] = Seq.empty, maxFee_opt: Option[MilliSatoshi] = None)(implicit timeout: Timeout): Future[RouteResponse] @@ -335,6 +337,13 @@ class EclairImpl(appKit: Kit) extends Eclair with Logging { } } + override def cpfpBumpFees(targetFeeratePerByte: FeeratePerByte, outpoints: Set[OutPoint]): Future[ByteVector32] = { + appKit.wallet match { + case w: BitcoinCoreClient => w.cpfp(outpoints, FeeratePerKw(targetFeeratePerByte)).map(_.txid) + case _ => Future.failed(new IllegalArgumentException("this call is only available with a bitcoin core backend")) + } + } + override def findRoute(targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], extraEdges: Seq[Invoice.ExtraEdge] = Seq.empty, includeLocalChannelCost: Boolean = false, ignoreNodeIds: Seq[PublicKey] = Seq.empty, ignoreShortChannelIds: Seq[ShortChannelId] = Seq.empty, maxFee_opt: Option[MilliSatoshi] = None)(implicit timeout: Timeout): Future[RouteResponse] = findRouteBetween(appKit.nodeParams.nodeId, targetNodeId, amount, pathFindingExperimentName_opt, extraEdges, includeLocalChannelCost, ignoreNodeIds, ignoreShortChannelIds, maxFee_opt) diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/directives/ExtraDirectives.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/directives/ExtraDirectives.scala index 153ec87c44..035ecbe23d 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/api/directives/ExtraDirectives.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/api/directives/ExtraDirectives.scala @@ -21,7 +21,7 @@ import akka.http.scaladsl.marshalling.ToResponseMarshaller import akka.http.scaladsl.model.StatusCodes.NotFound import akka.http.scaladsl.model.{ContentTypes, HttpResponse} import akka.http.scaladsl.server.{Directive1, Directives, MalformedFormFieldRejection, Route} -import fr.acinq.bitcoin.scalacompat.ByteVector32 +import fr.acinq.bitcoin.scalacompat.{ByteVector32, OutPoint} import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey import fr.acinq.eclair.ApiTypes.ChannelIdentifier import fr.acinq.eclair.api.serde.FormParamExtractors._ @@ -43,6 +43,7 @@ trait ExtraDirectives extends Directives { val channelIdsFormParam: NameUnmarshallerReceptacle[List[ByteVector32]] = "channelIds".as[List[ByteVector32]](bytes32ListUnmarshaller) val nodeIdFormParam: NameReceptacle[PublicKey] = "nodeId".as[PublicKey] val nodeIdsFormParam: NameUnmarshallerReceptacle[List[PublicKey]] = "nodeIds".as[List[PublicKey]](pubkeyListUnmarshaller) + val outPointsFormParam: NameUnmarshallerReceptacle[List[OutPoint]] = "outpoints".as[List[OutPoint]](outPointListUnmarshaller) val paymentHashFormParam: NameUnmarshallerReceptacle[ByteVector32] = "paymentHash".as[ByteVector32](bytes32Unmarshaller) val amountMsatFormParam: NameReceptacle[MilliSatoshi] = "amountMsat".as[MilliSatoshi] val invoiceFormParam: NameReceptacle[Bolt11Invoice] = "invoice".as[Bolt11Invoice] diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/OnChain.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/OnChain.scala index 286d0fd583..876b2305f7 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/OnChain.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/OnChain.scala @@ -21,6 +21,7 @@ import fr.acinq.bitcoin.scalacompat.Satoshi import fr.acinq.eclair.api.Service import fr.acinq.eclair.api.directives.EclairDirectives import fr.acinq.eclair.api.serde.FormParamExtractors._ +import fr.acinq.eclair.blockchain.fee.FeeratePerByte trait OnChain { this: Service with EclairDirectives => @@ -38,6 +39,13 @@ trait OnChain { } } + val cpfpBumpFees: Route = postRequest("cpfpbumpfees") { implicit t => + formFields("targetFeerateSatByte".as[FeeratePerByte], outPointsFormParam) { + (targetFeerate, outPoints) => + complete(eclairApi.cpfpBumpFees(targetFeerate, outPoints.toSet)) + } + } + val onChainBalance: Route = postRequest("onchainbalance") { implicit t => complete(eclairApi.onChainBalance()) } @@ -56,6 +64,6 @@ trait OnChain { complete(eclairApi.globalBalance()) } - val onChainRoutes: Route = getNewAddress ~ sendOnChain ~ onChainBalance ~ onChainTransactions ~ globalBalance + val onChainRoutes: Route = getNewAddress ~ sendOnChain ~ cpfpBumpFees ~ onChainBalance ~ onChainTransactions ~ globalBalance } diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala b/eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala index 466d6750d3..3e3b703cca 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala @@ -19,7 +19,7 @@ package fr.acinq.eclair.api.serde import akka.http.scaladsl.unmarshalling.Unmarshaller import akka.util.Timeout import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey -import fr.acinq.bitcoin.scalacompat.{ByteVector32, Satoshi} +import fr.acinq.bitcoin.scalacompat.{ByteVector32, OutPoint, Satoshi} import fr.acinq.eclair.api.directives.RouteFormat import fr.acinq.eclair.api.serde.JsonSupport._ import fr.acinq.eclair.blockchain.fee.FeeratePerByte @@ -65,6 +65,11 @@ object FormParamExtractors { implicit val feeratePerByteUnmarshaller: Unmarshaller[String, FeeratePerByte] = Unmarshaller.strict { str => FeeratePerByte(Satoshi(str.toLong)) } + implicit val outPointListUnmarshaller: Unmarshaller[String, List[OutPoint]] = listUnmarshaller(outPoint => { + val parts = outPoint.split(":") + OutPoint(ByteVector32.fromValidHex(parts.head).reverse, parts.last.toLong) + }) + implicit val base64DataUnmarshaller: Unmarshaller[String, ByteVector] = Unmarshaller.strict { str => ByteVector.fromValidBase64(str) } implicit val routeFormatUnmarshaller: Unmarshaller[String, RouteFormat] = Unmarshaller.strict { str => RouteFormat.fromString(str) } diff --git a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala index c4ebc7f628..9b14f60ed7 100644 --- a/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala +++ b/eclair-node/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala @@ -25,14 +25,14 @@ import akka.http.scaladsl.testkit.{RouteTestTimeout, ScalatestRouteTest, WSProbe import akka.util.Timeout import de.heikoseeberger.akkahttpjson4s.Json4sSupport import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} -import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, ByteVector64, SatoshiLong} +import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, ByteVector64, OutPoint, SatoshiLong} import fr.acinq.eclair.ApiTypes.ChannelIdentifier import fr.acinq.eclair.FeatureSupport.{Mandatory, Optional} import fr.acinq.eclair.Features.{ChannelRangeQueriesExtended, DataLossProtect} import fr.acinq.eclair._ import fr.acinq.eclair.api.directives.{EclairDirectives, ErrorResponse} import fr.acinq.eclair.api.serde.JsonSupport -import fr.acinq.eclair.blockchain.fee.FeeratePerKw +import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw} import fr.acinq.eclair.channel.Helpers.Closing import fr.acinq.eclair.channel._ import fr.acinq.eclair.crypto.Sphinx @@ -573,6 +573,46 @@ class ApiServiceSpec extends AnyFunSuite with ScalatestRouteTest with IdiomaticM } } + test("'cpfpbumpfees'") { + val eclair = mock[Eclair] + eclair.cpfpBumpFees(any, any) returns Future.successful(randomBytes32()) + val mockService = new MockService(eclair) + val (txId1, txId2) = (randomBytes32(), randomBytes32()) + + Post("/cpfpbumpfees", FormData("targetFeerateSatByte" -> "10", "outpoints" -> s"$txId1:2,$txId2:0,$txId1:13").toEntity) ~> + addCredentials(BasicHttpCredentials("", mockApi().password)) ~> + Route.seal(mockService.route) ~> + check { + assert(handled) + assert(status == OK) + eclair.cpfpBumpFees(FeeratePerByte(10 sat), Set(OutPoint(txId1.reverse, 2), OutPoint(txId1.reverse, 13), OutPoint(txId2.reverse, 0))).wasCalled(once) + } + + Post("/cpfpbumpfees", FormData("targetFeerateSatByte" -> "10").toEntity) ~> + addCredentials(BasicHttpCredentials("", mockApi().password)) ~> + Route.seal(mockService.route) ~> + check { + assert(handled) + assert(status == BadRequest) + } + + Post("/cpfpbumpfees", FormData("outpoints" -> s"$txId1:2").toEntity) ~> + addCredentials(BasicHttpCredentials("", mockApi().password)) ~> + Route.seal(mockService.route) ~> + check { + assert(handled) + assert(status == BadRequest) + } + + Post("/cpfpbumpfees", FormData("targetFeerateSatByte" -> "10", "outpoints" -> s"$txId1,2").toEntity) ~> + addCredentials(BasicHttpCredentials("", mockApi().password)) ~> + Route.seal(mockService.route) ~> + check { + assert(handled) + assert(status == BadRequest) + } + } + test("'connect' method should accept a nodeId") { val remoteNodeId = PublicKey(hex"030bb6a5e0c6b203c7e2180fb78c7ba4bdce46126761d8201b91ddac089cdecc87") From 486d186d66c3e0fb32170862f87e6af9ab1d1563 Mon Sep 17 00:00:00 2001 From: t-bast Date: Fri, 30 Apr 2021 17:30:22 +0200 Subject: [PATCH 3/7] Add documentation section Explain how to configure bitcoind to ensure you'll be able to use CPFP without hitting unconfirmed transaction chains mempool limits. --- README.md | 21 +++++++++++++++++++++ docs/release-notes/eclair-vnext.md | 1 + 2 files changed, 22 insertions(+) diff --git a/README.md b/README.md index 2cc3cada13..3533c72701 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,22 @@ If you want to use a different wallet from the default one, you must set `eclair Eclair will return BTC from closed channels to the wallet configured. Any BTC found in the wallet can be used to fund the channels you choose to open. +We also recommend tweaking the following parameters in `bitcoin.conf`: + +```conf +# This parameter ensures that your wallet will not create chains of unconfirmed +# transactions that would be rejected by other nodes. +walletrejectlongchains=1 +# The following parameters set the maximum length of chains of unconfirmed +# transactions to 20 instead of the default value of 25. +limitancestorcount=20 +limitdescendantcount=20 +``` + +Setting these parameters lets you unblock long chains of unconfirmed channel funding transactions by using child-pays-for-parent (CPFP) to make them confirm. + +With the default `bitcoind` parameters, if your node created a chain of 25 unconfirmed funding transactions with a low-feerate, you wouldn't be able to use CPFP to raise their fees because your CPFP transaction would likely be rejected by the rest of the network. + ### Java Environment Variables Some advanced parameters can be changed with java environment variables. Most users won't need this and can skip this section. @@ -278,9 +294,14 @@ so you can easily run your Bitcoin node on both mainnet and testnet. For example ```conf server=1 txindex=1 + addresstype=bech32 changetype=bech32 +walletrejectlongchains=1 +limitancestorcount=20 +limitdescendantcount=20 + [main] rpcuser= rpcpassword= diff --git a/docs/release-notes/eclair-vnext.md b/docs/release-notes/eclair-vnext.md index 319d5eacd5..db4b1ca6cc 100644 --- a/docs/release-notes/eclair-vnext.md +++ b/docs/release-notes/eclair-vnext.md @@ -40,6 +40,7 @@ All this data is signed and encrypted so that it can not be read or forged by th - `channel-opened` websocket event was updated to contain the final `channel_id` and be published when a channel is ready to process payments (#2567) - `getsentinfo` can now be used with `--offer` to list payments sent to a specific offer. - `listreceivedpayments` lists payments received by your node (#2607) +- `cpfpbumpfees` can be used to unblock chains of unconfirmed transactions by creating a child transaction that pays a high fee (#1783) ### Miscellaneous improvements and bug fixes From e7b13fc4ac98a3f555c93be43726cbf15c164500 Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 4 May 2023 13:49:06 +0200 Subject: [PATCH 4/7] Rename `getChangeAddress` --- .../blockchain/bitcoind/rpc/BitcoinCoreClient.scala | 10 +++++----- .../eclair/channel/publish/ReplaceableTxFunder.scala | 2 +- 2 files changed, 6 insertions(+), 6 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 319a1eeb5b..4f0ddf7c8c 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 @@ -272,15 +272,15 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall getTxOutputs(outpoints).transformWith { case Failure(ex) => Future.failed(new IllegalArgumentException("some transactions could not be found", ex)) case Success(txOutputs) => - getChangeAddress().transformWith { + getP2wpkhPubkeyHashForChange().transformWith { case Failure(ex) => Future.failed(new IllegalArgumentException("change address generation failed", ex)) - case Success(changeAddress) => + case Success(changePubkeyHash) => val amountIn = txOutputs.values.map(_.amount).sum // We build a transaction spending all the inputs provided to a single change output. Our inputs are // using either p2wpkh or p2tr: p2tr inputs are slightly smaller, but we don't bother doing an exact // calculation and always use the weight of p2wpkh inputs for simplicity. val p2wpkhInputWeight = 272 - val txWeight = p2wpkhInputWeight * outpoints.size + Transaction(2, Nil, Seq(TxOut(amountIn, Script.pay2wpkh(changeAddress))), 0).weight() + val txWeight = p2wpkhInputWeight * outpoints.size + Transaction(2, Nil, Seq(TxOut(amountIn, Script.pay2wpkh(changePubkeyHash))), 0).weight() val totalWeight = mempoolPackage.values.map(_.weight).sum + txWeight val targetFees = Transactions.weight2fee(targetFeerate, totalWeight.toInt) val currentFees = mempoolPackage.values.map(_.fees).sum @@ -290,7 +290,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall } else if (amountIn <= missingFees + 660.sat) { Future.failed(new IllegalArgumentException("input amount is not sufficient to cover the target feerate")) } else { - val unsignedTx = Transaction(2, outpoints.toSeq.map(o => TxIn(o, Seq.empty, 0)), Seq(TxOut(amountIn - missingFees, Script.pay2wpkh(changeAddress))), 0) + val unsignedTx = Transaction(2, outpoints.toSeq.map(o => TxIn(o, Seq.empty, 0)), Seq(TxOut(amountIn - missingFees, Script.pay2wpkh(changePubkeyHash))), 0) signTransaction(unsignedTx, Nil).transformWith { case Failure(ex) => Future.failed(new IllegalArgumentException("tx signing failed: some inputs don't belong to our wallet", ex)) case Success(signedTx) => publishTransaction(signedTx.tx).map(_ => signedTx.tx) @@ -450,7 +450,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall /** * @return the public key hash of a bech32 raw change address. */ - def getChangeAddress()(implicit ec: ExecutionContext): Future[ByteVector] = { + def getP2wpkhPubkeyHashForChange()(implicit ec: ExecutionContext): Future[ByteVector] = { rpcClient.invoke("getrawchangeaddress", "bech32").collect { case JString(changeAddress) => val pubkeyHash = ByteVector.view(Bech32.decodeWitnessAddress(changeAddress).getThird) 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 78923e13a6..3a331df8b6 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 @@ -381,7 +381,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams, (anchorTx.updateTx(fundedTx), fundTxResponse.amountIn) }) case None => - bitcoinClient.getChangeAddress().map(pubkeyHash => { + bitcoinClient.getP2wpkhPubkeyHashForChange().map(pubkeyHash => { val fundedTx = fundTxResponse.tx.copy(txOut = Seq(TxOut(dustLimit, Script.pay2wpkh(pubkeyHash)))) (anchorTx.updateTx(fundedTx), fundTxResponse.amountIn) }) From ef5e3c541fe0334a095694ea644b356ed48e00eb Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 4 May 2023 14:35:20 +0200 Subject: [PATCH 5/7] Fail on invalid outpoints --- .../bitcoind/rpc/BitcoinCoreClient.scala | 10 ++++--- .../bitcoind/BitcoinCoreClientSpec.scala | 27 ++++++++++++++----- 2 files changed, 28 insertions(+), 9 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 4f0ddf7c8c..58034dfeed 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 @@ -270,7 +270,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall case Failure(ex) => Future.failed(new IllegalArgumentException("unable to analyze mempool package: some transactions could not be found in your mempool", ex)) case Success(mempoolPackage) => getTxOutputs(outpoints).transformWith { - case Failure(ex) => Future.failed(new IllegalArgumentException("some transactions could not be found", ex)) + case Failure(ex) => Future.failed(new IllegalArgumentException("some outpoints are invalid or cannot be resolved", ex)) case Success(txOutputs) => getP2wpkhPubkeyHashForChange().transformWith { case Failure(ex) => Future.failed(new IllegalArgumentException("change address generation failed", ex)) @@ -316,11 +316,15 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall /** Fetch transaction output details for the given outpoints. */ private def getTxOutputs(outpoints: Set[OutPoint])(implicit ec: ExecutionContext): Future[Map[OutPoint, TxOut]] = { - Future.sequence(outpoints.map(_.txid).map(txid => getTransaction(txid))).map(txs => { - outpoints.flatMap(o => txs.find(tx => tx.txid == o.txid && o.index < tx.txOut.length) match { + Future.sequence(outpoints.map(_.txid).map(txid => getTransaction(txid))).flatMap(txs => { + val txOuts = outpoints.flatMap(o => txs.find(tx => tx.txid == o.txid && o.index < tx.txOut.length) match { case Some(tx) => Some(o -> tx.txOut(o.index.toInt)) case None => None }).toMap + outpoints.find(o => !txOuts.contains(o)) match { + case Some(o) => Future.failed(new IllegalArgumentException(s"invalid outpoint $o")) + case None => Future.successful(txOuts) + } }) } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala index 7d365d6519..931fc8b9b7 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala @@ -846,8 +846,8 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val sender = TestProbe() val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) val tx = sendToAddress(getNewAddress(sender), 150_000 sat, sender) - assert(tx.txOut.length === 2) // there must be a change output - val changeOutput = if (tx.txOut.head.amount === 150_000.sat) 1 else 0 + assert(tx.txOut.length == 2) // there must be a change output + val changeOutput = if (tx.txOut.head.amount == 150_000.sat) 1 else 0 bitcoinClient.getMempoolTx(tx.txid).pipeTo(sender.ref) val mempoolTx = sender.expectMsgType[MempoolTx] @@ -858,7 +858,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val cpfpTx = sender.expectMsgType[Transaction] bitcoinClient.getMempoolTx(cpfpTx.txid).pipeTo(sender.ref) val mempoolCpfpTx = sender.expectMsgType[MempoolTx] - assert(mempoolCpfpTx.ancestorFees === mempoolCpfpTx.fees + mempoolTx.fees) + assert(mempoolCpfpTx.ancestorFees == mempoolCpfpTx.fees + mempoolTx.fees) val expectedFees = Transactions.weight2fee(targetFeerate, tx.weight() + cpfpTx.weight()) assert(expectedFees * 0.95 <= mempoolCpfpTx.ancestorFees && mempoolCpfpTx.ancestorFees <= expectedFees * 1.05) assert(mempoolCpfpTx.replaceable) @@ -878,7 +878,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val txNotFunded = Transaction(2, Nil, TxOut(250_000 sat, Script.pay2wsh(fundingScript)) :: Nil, 0) bitcoinClient.fundTransaction(txNotFunded, FundTransactionOptions(fundingFeerate, changePosition = Some(1))).pipeTo(sender.ref) val fundTxResponse = sender.expectMsgType[FundTransactionResponse] - assert(fundTxResponse.changePosition === Some(1)) + assert(fundTxResponse.changePosition.contains(1)) bitcoinClient.signTransaction(fundTxResponse.tx, Nil).pipeTo(sender.ref) val signTxResponse = sender.expectMsgType[SignTransactionResponse] assert(signTxResponse.complete) @@ -982,8 +982,8 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val bumpedOutpoints = Set(OutPoint(txA2, 1), OutPoint(txA4, 1), OutPoint(txA6, 1), OutPoint(txB3, 1)) bitcoinClient.cpfp(bumpedOutpoints, targetFeerate).pipeTo(sender.ref) val cpfpTx = sender.expectMsgType[Transaction] - assert(cpfpTx.txIn.find(_.outPoint.txid == txB4.txid) === None) - assert(cpfpTx.txIn.find(_.outPoint.txid == txA7.txid) === None) + assert(!cpfpTx.txIn.exists(_.outPoint.txid == txB4.txid)) + assert(!cpfpTx.txIn.exists(_.outPoint.txid == txA7.txid)) bitcoinClient.getMempoolTx(cpfpTx.txid).pipeTo(sender.ref) val mempoolCpfpTx = sender.expectMsgType[MempoolTx] @@ -1003,6 +1003,21 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A assert(failure.cause.getMessage.contains("some transactions could not be found")) } + test("cannot bump transaction fees (invalid outpoint index)") { + val sender = TestProbe() + val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) + val tx = sendToAddress(getNewAddress(sender), 150_000 sat, sender) + assert(tx.txOut.length == 2) // there must be a change output + bitcoinClient.getMempoolTx(tx.txid).pipeTo(sender.ref) + val mempoolTx = sender.expectMsgType[MempoolTx] + val currentFeerate = FeeratePerKw(mempoolTx.fees * 1000 / tx.weight()) + + val targetFeerate = currentFeerate * 1.5 + bitcoinClient.cpfp(Set(OutPoint(tx, 3)), targetFeerate).pipeTo(sender.ref) + val failure = sender.expectMsgType[Failure] + assert(failure.cause.getMessage.contains("some outpoints are invalid or cannot be resolved")) + } + test("cannot bump transaction fees (transaction already confirmed)") { val sender = TestProbe() val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient) From 50e9b6b1fd8d4b350092153b34b97f9a1b0f5baf Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 4 May 2023 14:39:18 +0200 Subject: [PATCH 6/7] Add bash completion --- contrib/eclair-cli.bash-completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/eclair-cli.bash-completion b/contrib/eclair-cli.bash-completion index 47b76ecd76..5c7c53935b 100644 --- a/contrib/eclair-cli.bash-completion +++ b/contrib/eclair-cli.bash-completion @@ -21,7 +21,7 @@ _eclair-cli() *) # works fine, but is too slow at the moment. # allopts=$($eclaircli help 2>&1 | awk '$1 ~ /^"/ { sub(/,/, ""); print $1}' | sed 's/[":]//g') - allopts="getinfo connect open close forceclose updaterelayfee peers channels channel allnodes allchannels allupdates findroute findroutetonode findroutebetweennodes parseinvoice payinvoice sendtonode getsentinfo createinvoice getinvoice listinvoices listpendinginvoices listreceivedpayments getreceivedinfo audit networkfees channelstats" + allopts="getinfo connect open cpfpbumpfees close forceclose updaterelayfee peers channels channel allnodes allchannels allupdates findroute findroutetonode findroutebetweennodes parseinvoice payinvoice sendtonode getsentinfo createinvoice getinvoice listinvoices listpendinginvoices listreceivedpayments getreceivedinfo audit networkfees channelstats" if ! [[ " $allopts " =~ " $prev " ]]; then # prevent double arguments if [[ -z "$cur" || "$cur" =~ ^[a-z] ]]; then From 28bf87bb7a8e8dce5ddd5310a8a94a5caacccb7e Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 4 May 2023 17:47:58 +0200 Subject: [PATCH 7/7] Add unit tests for mempool package fetching --- .../bitcoind/rpc/BitcoinCoreClient.scala | 4 ++- .../bitcoind/BitcoinCoreClientSpec.scala | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) 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 58034dfeed..476fd8a777 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 @@ -266,7 +266,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall * @param targetFeerate feerate to apply to the package of unconfirmed transactions. */ def cpfp(outpoints: Set[OutPoint], targetFeerate: FeeratePerKw)(implicit ec: ExecutionContext): Future[Transaction] = { - getMempoolPackage(outpoints.map(_.txid), Map.empty).transformWith { + getMempoolPackage(outpoints.map(_.txid)).transformWith { case Failure(ex) => Future.failed(new IllegalArgumentException("unable to analyze mempool package: some transactions could not be found in your mempool", ex)) case Success(mempoolPackage) => getTxOutputs(outpoints).transformWith { @@ -302,6 +302,8 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall } /** Recursively fetch unconfirmed parents and return the complete unconfirmed ancestors tree. */ + def getMempoolPackage(leaves: Set[ByteVector32])(implicit ec: ExecutionContext): Future[Map[ByteVector32, MempoolTx]] = getMempoolPackage(leaves, Map.empty) + private def getMempoolPackage(leaves: Set[ByteVector32], current: Map[ByteVector32, MempoolTx])(implicit ec: ExecutionContext): Future[Map[ByteVector32, MempoolTx]] = { Future.sequence(leaves.map(txid => getMempoolTx(txid))).flatMap(txs => { val current2 = current.concat(txs.map(tx => tx.txid -> tx)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala index 931fc8b9b7..ed7e8b526d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala @@ -853,6 +853,9 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val mempoolTx = sender.expectMsgType[MempoolTx] val currentFeerate = FeeratePerKw(mempoolTx.fees * 1000 / tx.weight()) + bitcoinClient.getMempoolPackage(Set(tx.txid)).pipeTo(sender.ref) + sender.expectMsg(Map(tx.txid -> mempoolTx)) + val targetFeerate = currentFeerate * 1.5 bitcoinClient.cpfp(Set(OutPoint(tx, changeOutput)), targetFeerate).pipeTo(sender.ref) val cpfpTx = sender.expectMsgType[Transaction] @@ -902,6 +905,13 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A signedTx } + bitcoinClient.getMempoolTx(fundingTx.txid).pipeTo(sender.ref) + val mempoolFundingTx = sender.expectMsgType[MempoolTx] + bitcoinClient.getMempoolTx(mutualCloseTx.txid).pipeTo(sender.ref) + val mempoolMutualCloseTx = sender.expectMsgType[MempoolTx] + bitcoinClient.getMempoolPackage(Set(fundingTx.txid, mutualCloseTx.txid)).pipeTo(sender.ref) + sender.expectMsg(Map(fundingTx.txid -> mempoolFundingTx, mutualCloseTx.txid -> mempoolMutualCloseTx)) + val targetFeerate = FeeratePerKw(5000 sat) bitcoinClient.cpfp(Set(OutPoint(fundingTx, 1), OutPoint(mutualCloseTx, 0)), targetFeerate).pipeTo(sender.ref) val cpfpTx = sender.expectMsgType[Transaction] @@ -949,6 +959,12 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A signTxResponse.tx } + def getMempoolTx(txid: ByteVector32): MempoolTx = { + val probe = TestProbe() + bitcoinClient.getMempoolTx(txid).pipeTo(probe.ref) + probe.expectMsgType[MempoolTx] + } + def getWalletPubKey(sender: TestProbe): PublicKey = { bitcoinClient.getP2wpkhPubkey().pipeTo(sender.ref) sender.expectMsgType[PublicKey] @@ -977,6 +993,15 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val txA6 = createTx(Seq((pubKeyA7, 50_000 sat)), Some(OutPoint(txA5, 0))) val txA7 = createTx(Seq((randomKey().publicKey, 50_000 sat)), Some(OutPoint(txA6, 0))) + bitcoinClient.getMempoolPackage(Set(txA3.txid)).pipeTo(sender.ref) + sender.expectMsg(Set(txA1, txA2, txA3).map(tx => tx.txid -> getMempoolTx(tx.txid)).toMap) + + bitcoinClient.getMempoolPackage(Set(txA2.txid, txA5.txid)).pipeTo(sender.ref) + sender.expectMsg(Set(txA1, txA2, txA5).map(tx => tx.txid -> getMempoolTx(tx.txid)).toMap) + + bitcoinClient.getMempoolPackage(Set(txA2.txid, txA4.txid, txA6.txid, txB3.txid)).pipeTo(sender.ref) + sender.expectMsg(Set(txA1, txA2, txA3, txA4, txA5, txA6, txB1, txB2, txB3).map(tx => tx.txid -> getMempoolTx(tx.txid)).toMap) + // We bump a subset of the mempool: TxA1 -> TxA6 and TxB1 -> TxB3 val targetFeerate = FeeratePerKw(5000 sat) val bumpedOutpoints = Set(OutPoint(txA2, 1), OutPoint(txA4, 1), OutPoint(txA6, 1), OutPoint(txB3, 1))