From 9535f2762845d845dad8e5ace04c56a18dff505a Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Mon, 3 Oct 2022 23:45:11 +0300 Subject: [PATCH 01/15] CPFP method first version (untested) --- packages/fastbtc-node/src/btc/multisig.ts | 149 ++++++++++++++++++++-- 1 file changed, 135 insertions(+), 14 deletions(-) diff --git a/packages/fastbtc-node/src/btc/multisig.ts b/packages/fastbtc-node/src/btc/multisig.ts index 4cb2bf4..f1ec0df 100644 --- a/packages/fastbtc-node/src/btc/multisig.ts +++ b/packages/fastbtc-node/src/btc/multisig.ts @@ -2,7 +2,7 @@ * Bitcoin multisig signature logic, Bitcoin transaction sending and reading data from the Bitcoin network */ import {inject, injectable} from 'inversify'; -import {bip32, ECPair, Network, networks, Payment, payments, Psbt, script} from "bitcoinjs-lib"; +import {bip32, ECPair, Network, networks, Payment, payments, Psbt, script, address as bitcoinjsAddress} from "bitcoinjs-lib"; import {normalizeKey, xprvToPublic} from './utils'; import getByteCount from './bytecount'; import BitcoinNodeWrapper, {IBitcoinNodeWrapper} from './nodewrapper'; @@ -21,6 +21,9 @@ export interface PartiallySignedBitcoinTransaction { requiredSignatures: number; derivationPaths: string[]; noChange: boolean; + isCpfp?: boolean; + feeSatsPerVB?: number; + totalFee?: number; } export interface BtcTransfer { @@ -29,6 +32,11 @@ export interface BtcTransfer { nonce: number; } +export interface CPFPOpts { + feeMultiplier?: number; + signSelf?: boolean; +} + // https://developer.bitcoin.org/reference/rpc/gettransaction.html // this is only partially reflected here because we don't need everything export interface BitcoinRPCGetTransactionResponse { @@ -293,12 +301,19 @@ export class BitcoinMultisig { response = withDerivationPaths; - const amountSatoshi: BigNumber = transfers.map(t => t.amountSatoshi).reduce( + const totalPayoutValueSat: BigNumber = transfers.map(t => t.amountSatoshi).reduce( (a, b) => a.add(b), BigNumber.from(0), ); + const minChangeSat = ( + noChange + ? BigNumber.from(0) + : BigNumber.from(1e7) // 0.1 BTC to ensure enough change for cpfp + ); + const totalPayoutValueWithMinChangeSat = totalPayoutValueSat.add(minChangeSat); + const psbt = new Psbt({network}); - let totalSum = BigNumber.from(0); + let totalInputValueSat = BigNumber.from(0); let outputCounts = { 'P2WSH': 2, // OP_RETURN data + change address; this actually always exceeds the byte size of the OP_RETURN }; @@ -307,7 +322,7 @@ export class BitcoinMultisig { }; const derivationPaths: Set = new Set(); - let fee = BigNumber.from(0); + let feeSat = BigNumber.from(0); let totalInputCount = 0; for (const utxo of response) { const tx = await this.getRawTx(utxo.txid); @@ -326,9 +341,9 @@ export class BitcoinMultisig { psbt.addInput(input); inputCounts[inputType]++; totalInputCount++; - totalSum = totalSum.add(BigNumber.from(Math.round(utxo.amount * 1e8))); + totalInputValueSat = totalInputValueSat.add(BigNumber.from(Math.round(utxo.amount * 1e8))); - fee = BigNumber.from( + feeSat = BigNumber.from( Math.round( getByteCount( inputCounts, @@ -339,7 +354,7 @@ export class BitcoinMultisig { * feeRate ) ); - if (totalSum.gte(amountSatoshi.add(fee))) { + if (totalInputValueSat.gte(totalPayoutValueWithMinChangeSat.add(feeSat))) { break; } if (maxInputs && totalInputCount >= maxInputs) { @@ -348,18 +363,18 @@ export class BitcoinMultisig { } } - const transferSumIncludingFee = amountSatoshi.add(fee); - if (totalSum.lt(transferSumIncludingFee)) { - if (maxInputs && noChange && !totalSum.isZero()) { + const transferSumIncludingFee = totalPayoutValueWithMinChangeSat.add(feeSat); + if (totalInputValueSat.lt(transferSumIncludingFee)) { + if (maxInputs && noChange && !totalInputValueSat.isZero()) { this.logger.warning( `Number of inputs is capped at ${maxInputs} -- can only send ` + - `${totalSum.toString()} satoshi out of ${transferSumIncludingFee.toString()} satoshi wanted. ` + + `${totalInputValueSat.toString()} satoshi out of ${transferSumIncludingFee.toString()} satoshi wanted. ` + `(But that's ok for a replenish tx.)` ) } else { throw new Error( - `balance is too low (can only send up to ${totalSum.toString()} satoshi out of ` + + `balance is too low (can only send up to ${totalInputValueSat.toString()} satoshi out of ` + `${transferSumIncludingFee.toString()} required)` ); } @@ -387,7 +402,7 @@ export class BitcoinMultisig { // change money! psbt.addOutput({ address: this.changePayment.address!, - value: totalSum.sub(fee).sub(amountSatoshi).toNumber(), + value: totalInputValueSat.sub(feeSat).sub(totalPayoutValueSat).toNumber(), }); } else { @@ -398,7 +413,7 @@ export class BitcoinMultisig { psbt.addOutput({ address: transfers[0].btcAddress, - value: totalSum.sub(fee).toNumber() + value: totalInputValueSat.sub(feeSat).toNumber() }); } @@ -408,6 +423,9 @@ export class BitcoinMultisig { requiredSignatures: this.cosigners, derivationPaths: [...derivationPaths], noChange: Boolean(noChange), + isCpfp: false, + feeSatsPerVB: feeRate, + totalFee: feeSat.toNumber(), }; if (signSelf) { ret = this.signTransaction(ret); @@ -417,6 +435,106 @@ export class BitcoinMultisig { return ret; } + /** + * Create a child-pays-for-parent (CPFP) transaction that bumps `bumpedTx`. + * + * @param bumpedTx + * @param opts + */ + async createPartiallySignedCpfpTransaction( + bumpedTx: PartiallySignedBitcoinTransaction, + opts?: CPFPOpts, + ): Promise { + const { + feeMultiplier = 10, + signSelf = true, + } = (opts ?? {}); + if (!this.changePayment.address || !this.changePayment.redeem) { + throw new Error("no change address"); + } + if (!bumpedTx.totalFee) { + throw new Error("bumpedTx.totalFee is required"); + } + + const feeBtcPerKB = await this.feeEstimator.estimateFeeBtcPerKB(); + let feeSatsPerVB = feeMultiplier * feeBtcPerKB / 1000 * 1e8; + + const outputCounts = { + 'P2WSH': 1, + }; + const inputType = `MULTISIG-P2WSH:${this.cosigners}-${this.masterPublicKeys.length}`; + const inputCounts = { + [inputType]: 1, + }; + + // we add the original fee to this to ensure the network gets enough fee per vB + const byteCount = getByteCount( + inputCounts, + outputCounts, + [this.changePayment.address], + this.network + ); + const fee = Math.round(byteCount * feeSatsPerVB) + bumpedTx.totalFee; + feeSatsPerVB = fee / byteCount; // save this for later :D + + const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); + bumpedPsbt.validateSignaturesOfAllInputs(); + bumpedPsbt.finalizeAllInputs(); + + const rawBumpedTx = bumpedPsbt.extractTransaction(); + + let changeOutput: any = null; + let changeOutputVout: number = -1; + const derivationPath = this.keyDerivationPath; // probably correct + + for (let i = 0; i < rawBumpedTx.outs.length; i++) { + const output = rawBumpedTx.outs[i]; + try { + const address = bitcoinjsAddress.fromOutputScript(output.script, this.network); + if (address === this.changePayment.address) { + changeOutput = output; + changeOutputVout = i; + break; + } + } catch (e) { + // ignore + } + } + if (!changeOutput) { + throw new Error("bumpedTx has no change output"); + } + + if (fee >= changeOutput.value) { + throw new Error(`fee ${fee} is greater or equal than change output value ${changeOutput.value}`); + } + + const cpfpPsbt = new Psbt({network: this.network}); + cpfpPsbt.addInput({ + hash: rawBumpedTx.getId(), + index: changeOutputVout, + nonWitnessUtxo: rawBumpedTx.toBuffer(), + witnessScript: this.changePayment.redeem.output, + }); + cpfpPsbt.addOutput({ + address: this.changePayment.address, + value: changeOutput.value - fee, + }); + let ret: PartiallySignedBitcoinTransaction = { + serializedTransaction: cpfpPsbt.toBase64(), + signedPublicKeys: [], + requiredSignatures: this.cosigners, + derivationPaths: [derivationPath], + noChange: false, + isCpfp: true, + feeSatsPerVB, + totalFee: fee, + }; + if (signSelf) { + ret = this.signTransaction(ret); + } + return ret; + } + getTransactionTransfers(tx: PartiallySignedBitcoinTransaction): BtcTransfer[] { const psbtUnserialized = Psbt.fromBase64(tx.serializedTransaction, {network: this.network}); let end; @@ -531,6 +649,9 @@ export class BitcoinMultisig { requiredSignatures: tx.requiredSignatures, derivationPaths: [...tx.derivationPaths], noChange: Boolean(tx.noChange), + isCpfp: Boolean(tx.isCpfp), + feeSatsPerVB: tx.feeSatsPerVB, + totalFee: tx.totalFee, } } From afcba3e3e387e538f81f1176108da0b3dc59c648 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Tue, 4 Oct 2022 00:22:41 +0300 Subject: [PATCH 02/15] Adjust CPFP fee calculation --- packages/fastbtc-node/src/btc/multisig.ts | 69 +++++++++++++++-------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/packages/fastbtc-node/src/btc/multisig.ts b/packages/fastbtc-node/src/btc/multisig.ts index f1ec0df..4e07568 100644 --- a/packages/fastbtc-node/src/btc/multisig.ts +++ b/packages/fastbtc-node/src/btc/multisig.ts @@ -22,8 +22,10 @@ export interface PartiallySignedBitcoinTransaction { derivationPaths: string[]; noChange: boolean; isCpfp?: boolean; + // cumulativeByteCount includes the transaction's byte count, and the byte count + // of all CPFP'd transactions in the chain + cumulativeByteCount?: number; feeSatsPerVB?: number; - totalFee?: number; } export interface BtcTransfer { @@ -324,6 +326,12 @@ export class BitcoinMultisig { const derivationPaths: Set = new Set(); let feeSat = BigNumber.from(0); let totalInputCount = 0; + let cumulativeByteCount = getByteCount( + inputCounts, + outputCounts, + transfers.map(t => t.btcAddress), + this.network + ); for (const utxo of response) { const tx = await this.getRawTx(utxo.txid); @@ -343,15 +351,15 @@ export class BitcoinMultisig { totalInputCount++; totalInputValueSat = totalInputValueSat.add(BigNumber.from(Math.round(utxo.amount * 1e8))); + cumulativeByteCount = getByteCount( + inputCounts, + outputCounts, + transfers.map(t => t.btcAddress), + this.network + ); feeSat = BigNumber.from( Math.round( - getByteCount( - inputCounts, - outputCounts, - transfers.map(t => t.btcAddress), - this.network - ) - * feeRate + cumulativeByteCount * feeRate ) ); if (totalInputValueSat.gte(totalPayoutValueWithMinChangeSat.add(feeSat))) { @@ -424,8 +432,8 @@ export class BitcoinMultisig { derivationPaths: [...derivationPaths], noChange: Boolean(noChange), isCpfp: false, + cumulativeByteCount, feeSatsPerVB: feeRate, - totalFee: feeSat.toNumber(), }; if (signSelf) { ret = this.signTransaction(ret); @@ -446,18 +454,28 @@ export class BitcoinMultisig { opts?: CPFPOpts, ): Promise { const { - feeMultiplier = 10, + feeMultiplier = 2, // TODO: experiment with this signSelf = true, } = (opts ?? {}); if (!this.changePayment.address || !this.changePayment.redeem) { throw new Error("no change address"); } - if (!bumpedTx.totalFee) { - throw new Error("bumpedTx.totalFee is required"); + if (!bumpedTx.cumulativeByteCount || !bumpedTx.feeSatsPerVB) { + throw new Error("bumpedTx.cumulativeByteCount and bumpedTx.feeSatsPerVB are required"); } + const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); + bumpedPsbt.validateSignaturesOfAllInputs(); + bumpedPsbt.finalizeAllInputs(); + const rawBumpedTx = bumpedPsbt.extractTransaction(); + + this.logger.info( + `Attempting to CPFP ${rawBumpedTx.getId()} with fee multiplier ${feeMultiplier}` + ); + const feeBtcPerKB = await this.feeEstimator.estimateFeeBtcPerKB(); let feeSatsPerVB = feeMultiplier * feeBtcPerKB / 1000 * 1e8; + feeSatsPerVB = Math.max(feeSatsPerVB, bumpedTx.feeSatsPerVB); const outputCounts = { 'P2WSH': 1, @@ -467,21 +485,28 @@ export class BitcoinMultisig { [inputType]: 1, }; - // we add the original fee to this to ensure the network gets enough fee per vB - const byteCount = getByteCount( + // we add the original cumulativeByteCount to this to ensure the network gets enough fee per vB + const cumulativeByteCount = getByteCount( inputCounts, outputCounts, [this.changePayment.address], this.network + ) + bumpedTx.cumulativeByteCount; + const fee = Math.round( + cumulativeByteCount * feeSatsPerVB ); - const fee = Math.round(byteCount * feeSatsPerVB) + bumpedTx.totalFee; - feeSatsPerVB = fee / byteCount; // save this for later :D - const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); - bumpedPsbt.validateSignaturesOfAllInputs(); - bumpedPsbt.finalizeAllInputs(); + this.logger.info( + `Creating CPFP transaction with feeSatsPerVB=${feeSatsPerVB}, ` + + `cumulativeByteCount=${cumulativeByteCount}. ` + + `Original tx (${rawBumpedTx.getId()}): feeSatsPerVB=${bumpedTx.feeSatsPerVB} ` + + `cumulativeByteCount=${bumpedTx.cumulativeByteCount}.` + ); - const rawBumpedTx = bumpedPsbt.extractTransaction(); + const maxFeeBtc = 0.01; + if (fee >= maxFeeBtc * 1e8) { + throw new Error(`yeah, we're not paying over ${maxFeeBtc} for CPFP`); + } let changeOutput: any = null; let changeOutputVout: number = -1; @@ -526,8 +551,8 @@ export class BitcoinMultisig { derivationPaths: [derivationPath], noChange: false, isCpfp: true, + cumulativeByteCount, feeSatsPerVB, - totalFee: fee, }; if (signSelf) { ret = this.signTransaction(ret); @@ -650,8 +675,8 @@ export class BitcoinMultisig { derivationPaths: [...tx.derivationPaths], noChange: Boolean(tx.noChange), isCpfp: Boolean(tx.isCpfp), + cumulativeByteCount: tx.cumulativeByteCount, feeSatsPerVB: tx.feeSatsPerVB, - totalFee: tx.totalFee, } } From 53424524dcdbe616cd61790ad168909eb6ee3262 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Tue, 4 Oct 2022 03:16:00 +0300 Subject: [PATCH 03/15] CPFP validation (untested) --- packages/fastbtc-node/src/btc/multisig.ts | 108 +++++++++++++++++----- 1 file changed, 87 insertions(+), 21 deletions(-) diff --git a/packages/fastbtc-node/src/btc/multisig.ts b/packages/fastbtc-node/src/btc/multisig.ts index 4e07568..04bc2a1 100644 --- a/packages/fastbtc-node/src/btc/multisig.ts +++ b/packages/fastbtc-node/src/btc/multisig.ts @@ -2,7 +2,18 @@ * Bitcoin multisig signature logic, Bitcoin transaction sending and reading data from the Bitcoin network */ import {inject, injectable} from 'inversify'; -import {bip32, ECPair, Network, networks, Payment, payments, Psbt, script, address as bitcoinjsAddress} from "bitcoinjs-lib"; +import { + bip32, + ECPair, + Network, + networks, + Payment, + payments, + Psbt, + script, + address as bitcoinjsAddress, + Transaction as RawTransaction, +} from "bitcoinjs-lib"; import {normalizeKey, xprvToPublic} from './utils'; import getByteCount from './bytecount'; import BitcoinNodeWrapper, {IBitcoinNodeWrapper} from './nodewrapper'; @@ -13,6 +24,7 @@ import Logger from '../logger'; import {setsEqual} from "../utils/sets"; import {TYPES} from "../stats"; import {StatsD} from "hot-shots"; +import {Output} from 'bitcoinjs-lib/types/transaction'; export interface PartiallySignedBitcoinTransaction { @@ -39,6 +51,10 @@ export interface CPFPOpts { signSelf?: boolean; } +export class CPFPValidationError extends Error { + isValidationError = true; +} + // https://developer.bitcoin.org/reference/rpc/gettransaction.html // this is only partially reflected here because we don't need everything export interface BitcoinRPCGetTransactionResponse { @@ -455,7 +471,7 @@ export class BitcoinMultisig { ): Promise { const { feeMultiplier = 2, // TODO: experiment with this - signSelf = true, + signSelf = false, } = (opts ?? {}); if (!this.changePayment.address || !this.changePayment.redeem) { throw new Error("no change address"); @@ -508,23 +524,7 @@ export class BitcoinMultisig { throw new Error(`yeah, we're not paying over ${maxFeeBtc} for CPFP`); } - let changeOutput: any = null; - let changeOutputVout: number = -1; - const derivationPath = this.keyDerivationPath; // probably correct - - for (let i = 0; i < rawBumpedTx.outs.length; i++) { - const output = rawBumpedTx.outs[i]; - try { - const address = bitcoinjsAddress.fromOutputScript(output.script, this.network); - if (address === this.changePayment.address) { - changeOutput = output; - changeOutputVout = i; - break; - } - } catch (e) { - // ignore - } - } + const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(rawBumpedTx); if (!changeOutput) { throw new Error("bumpedTx has no change output"); } @@ -544,6 +544,7 @@ export class BitcoinMultisig { address: this.changePayment.address, value: changeOutput.value - fee, }); + const derivationPath = this.keyDerivationPath; // probably correct let ret: PartiallySignedBitcoinTransaction = { serializedTransaction: cpfpPsbt.toBase64(), signedPublicKeys: [], @@ -560,6 +561,70 @@ export class BitcoinMultisig { return ret; } + validatePartiallySignedCpfpTransaction( + bumpedTx: PartiallySignedBitcoinTransaction, + cpfpTx: PartiallySignedBitcoinTransaction, + ): void { + if (!cpfpTx.isCpfp) { + throw new CPFPValidationError("cpfpTx is not a CPFP transaction"); + } + if (!this.changePayment.address || !this.changePayment.redeem) { + throw new Error("no change address -- cannot validate"); + } + const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); + const cpfpPsbt = Psbt.fromBase64(cpfpTx.serializedTransaction, { network: this.network }); + const rawBumpedTx = bumpedPsbt.extractTransaction(); + const rawCpfpTx = cpfpPsbt.extractTransaction(); + const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(rawBumpedTx); + if (!changeOutput) { + throw new CPFPValidationError("bumpedTx has no change output"); + } + if (rawCpfpTx.ins.length !== 1) { + throw new CPFPValidationError("cpfpTx must have exactly one input"); + } + if (rawCpfpTx.outs.length !== 1) { + throw new CPFPValidationError("cpfpTx must have exactly one output"); + } + + const cpfpInput = rawCpfpTx.ins[0]; + const cpfpInputHash = cpfpInput.hash.toString('hex'); + const rawBumpedTxHash = rawBumpedTx.getHash().toString('hex'); + if (cpfpInputHash !== rawBumpedTxHash) { + throw new CPFPValidationError( + `cpfpTx input hash ${cpfpInputHash} does not match bumpedTx hash ${rawBumpedTxHash}` + ); + } + if (cpfpInput.index !== changeOutputVout) { + throw new CPFPValidationError( + `cpfpTx input index ${cpfpInput.index} does not match change output vout ${changeOutputVout}` + ); + } + // TODO: could maybe validate the witnessScript and nonWitnessUtxo + + const cpfpOutput = rawCpfpTx.outs[0]; + const cpfpOutputAddress = bitcoinjsAddress.fromOutputScript(cpfpOutput.script, this.network); + if (cpfpOutputAddress !== this.changePayment.address) { + throw new CPFPValidationError( + `cpfpTx output address ${cpfpOutputAddress} does not match change address ${this.changePayment.address}` + ); + } + } + + private getChangeOutputAndVout(rawTx: RawTransaction): [Output|undefined, number] { + for (let i = 0; i < rawTx.outs.length; i++) { + const output = rawTx.outs[i]; + try { + const address = bitcoinjsAddress.fromOutputScript(output.script, this.network); + if (address === this.changePayment.address) { + return [ output, i ]; + } + } catch (e) { + // ignore + } + } + return [ undefined, -1 ]; + } + getTransactionTransfers(tx: PartiallySignedBitcoinTransaction): BtcTransfer[] { const psbtUnserialized = Psbt.fromBase64(tx.serializedTransaction, {network: this.network}); let end; @@ -573,8 +638,9 @@ export class BitcoinMultisig { `The partial no-change transaction does not have enough outputs, ` + `should have at least 2 outputs, has ${transferLength + 1}`); } - } - else { + } else if (tx.isCpfp) { + throw new Error('this method is not implemented for CPFP transactions') + } else { end = -1; transferLength = psbtUnserialized.txOutputs.length - 2; if (transferLength < 1) { From 22482c37f88446c9729433a6ee9d1f4febfead4e Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Tue, 4 Oct 2022 05:19:37 +0300 Subject: [PATCH 04/15] Multisig refactoring --- packages/fastbtc-node/src/btc/multisig.ts | 39 +++++++++++------------ 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/packages/fastbtc-node/src/btc/multisig.ts b/packages/fastbtc-node/src/btc/multisig.ts index 04bc2a1..dc42957 100644 --- a/packages/fastbtc-node/src/btc/multisig.ts +++ b/packages/fastbtc-node/src/btc/multisig.ts @@ -13,6 +13,7 @@ import { script, address as bitcoinjsAddress, Transaction as RawTransaction, + PsbtTxOutput, } from "bitcoinjs-lib"; import {normalizeKey, xprvToPublic} from './utils'; import getByteCount from './bytecount'; @@ -24,7 +25,6 @@ import Logger from '../logger'; import {setsEqual} from "../utils/sets"; import {TYPES} from "../stats"; import {StatsD} from "hot-shots"; -import {Output} from 'bitcoinjs-lib/types/transaction'; export interface PartiallySignedBitcoinTransaction { @@ -524,7 +524,7 @@ export class BitcoinMultisig { throw new Error(`yeah, we're not paying over ${maxFeeBtc} for CPFP`); } - const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(rawBumpedTx); + const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(bumpedPsbt); if (!changeOutput) { throw new Error("bumpedTx has no change output"); } @@ -571,22 +571,24 @@ export class BitcoinMultisig { if (!this.changePayment.address || !this.changePayment.redeem) { throw new Error("no change address -- cannot validate"); } - const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); const cpfpPsbt = Psbt.fromBase64(cpfpTx.serializedTransaction, { network: this.network }); + const bumpedPsbt = Psbt.fromBase64(bumpedTx.serializedTransaction, { network: this.network }); + bumpedPsbt.validateSignaturesOfAllInputs(); + bumpedPsbt.finalizeAllInputs(); + const rawBumpedTx = bumpedPsbt.extractTransaction(); - const rawCpfpTx = cpfpPsbt.extractTransaction(); - const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(rawBumpedTx); + const [ changeOutput, changeOutputVout ] = this.getChangeOutputAndVout(bumpedPsbt); if (!changeOutput) { throw new CPFPValidationError("bumpedTx has no change output"); } - if (rawCpfpTx.ins.length !== 1) { + if (cpfpPsbt.txInputs.length !== 1) { throw new CPFPValidationError("cpfpTx must have exactly one input"); } - if (rawCpfpTx.outs.length !== 1) { + if (cpfpPsbt.txOutputs.length !== 1) { throw new CPFPValidationError("cpfpTx must have exactly one output"); } - const cpfpInput = rawCpfpTx.ins[0]; + const cpfpInput = cpfpPsbt.txInputs[0]; const cpfpInputHash = cpfpInput.hash.toString('hex'); const rawBumpedTxHash = rawBumpedTx.getHash().toString('hex'); if (cpfpInputHash !== rawBumpedTxHash) { @@ -601,7 +603,7 @@ export class BitcoinMultisig { } // TODO: could maybe validate the witnessScript and nonWitnessUtxo - const cpfpOutput = rawCpfpTx.outs[0]; + const cpfpOutput = cpfpPsbt.txOutputs[0]; const cpfpOutputAddress = bitcoinjsAddress.fromOutputScript(cpfpOutput.script, this.network); if (cpfpOutputAddress !== this.changePayment.address) { throw new CPFPValidationError( @@ -610,19 +612,14 @@ export class BitcoinMultisig { } } - private getChangeOutputAndVout(rawTx: RawTransaction): [Output|undefined, number] { - for (let i = 0; i < rawTx.outs.length; i++) { - const output = rawTx.outs[i]; - try { - const address = bitcoinjsAddress.fromOutputScript(output.script, this.network); - if (address === this.changePayment.address) { - return [ output, i ]; - } - } catch (e) { - // ignore + private getChangeOutputAndVout(psbt: Psbt): [PsbtTxOutput|undefined, number] { + for (let i = 0; i < psbt.txOutputs.length; i++) { + const output = psbt.txOutputs[i]; + if (output.address === this.changePayment.address) { + return [output, i]; } } - return [ undefined, -1 ]; + return [undefined, -1]; } getTransactionTransfers(tx: PartiallySignedBitcoinTransaction): BtcTransfer[] { @@ -746,7 +743,7 @@ export class BitcoinMultisig { } } - async combine(txs: PartiallySignedBitcoinTransaction[]): Promise { + combine(txs: PartiallySignedBitcoinTransaction[]): PartiallySignedBitcoinTransaction { if (! txs.length) { throw new Error('Cannot combine zero transactions'); } From c1678051c6ab90680b79c799466187ab6c4b903e Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 04:27:10 +0200 Subject: [PATCH 05/15] CPFP checkpoint --- packages/fastbtc-contracts/hardhat.config.ts | 35 +++- packages/fastbtc-node/src/config.ts | 2 + packages/fastbtc-node/src/core/cpfp.ts | 188 +++++++++++++++++++ packages/fastbtc-node/src/core/node.ts | 8 +- packages/fastbtc-node/src/core/transfers.ts | 95 +++++++++- packages/fastbtc-node/src/utils/copy.ts | 3 + 6 files changed, 321 insertions(+), 10 deletions(-) create mode 100644 packages/fastbtc-node/src/core/cpfp.ts create mode 100644 packages/fastbtc-node/src/utils/copy.ts diff --git a/packages/fastbtc-contracts/hardhat.config.ts b/packages/fastbtc-contracts/hardhat.config.ts index ce5a67c..fffa5c4 100644 --- a/packages/fastbtc-contracts/hardhat.config.ts +++ b/packages/fastbtc-contracts/hardhat.config.ts @@ -84,6 +84,39 @@ task("show-transfer", "Show transfer details") }); + +task("ispaused", "Check if the bridge is paused") + .addPositionalParam('btcAddressOrTransferId') + .addOptionalPositionalParam('nonce') + .addOptionalParam("bridgeAddress", "FastBTCBridge contract address (if empty, use deployment)") + .setAction(async ({ btcAddressOrTransferId, nonce, bridgeAddress }, hre) => { + const contract = await hre.ethers.getContractAt( + 'FastBTCBridge', + await getDeploymentAddress(bridgeAddress, hre, 'FastBTCBridge'), + ); + + let transferId; + if (nonce === undefined) { + console.log('Nonce not given, treat', btcAddressOrTransferId, 'as transferId'); + transferId = btcAddressOrTransferId; + } else { + console.log('Nonce given, treat', btcAddressOrTransferId, 'as btcAddress'); + transferId = await contract.getTransferId(btcAddressOrTransferId, nonce); + } + + console.log('transferId', transferId); + + const transfer = await contract.getTransferByTransferId(transferId); + for (let [key, value] of transfer.entries()) { + console.log( + key, + BigNumber.isBigNumber(value) ? value.toString() : value + ); + } + console.log(transfer); + + }); + task("free-money", "Sends free money to address") .addPositionalParam("address", "Address to send free money to") .addPositionalParam("rbtcAmount", "RBTC amount to send", "1.0") @@ -276,7 +309,7 @@ task('roles', 'Manage roles') await getDeploymentAddress(undefined, hre, 'FastBTCAccessControl'), signer, ); - + console.log('fastBTCBridge address', await getDeploymentAddress(undefined, hre, 'FastBTCBridge')); console.log(`${action} role ${role}`, account ? `for ${account}` : ''); const roleHash = await accessControl[`ROLE_${role}`](); console.log('role hash:', roleHash); diff --git a/packages/fastbtc-node/src/config.ts b/packages/fastbtc-node/src/config.ts index 37a6625..81c72a8 100644 --- a/packages/fastbtc-node/src/config.ts +++ b/packages/fastbtc-node/src/config.ts @@ -30,6 +30,7 @@ export interface Config { btcRpcUrl: string; btcRpcUsername: string; btcKeyDerivationPath: string; + btcWaitedBlocksBeforeCpfp: number; statsdUrl?: string; secrets: () => ConfigSecrets; replenisherConfig: ReplenisherConfig|undefined; @@ -154,6 +155,7 @@ export const envConfigProviderFactory = async ( btcRpcUrl: env.FASTBTC_BTC_RPC_URL!, btcRpcUsername: env.FASTBTC_BTC_RPC_USERNAME ?? '', btcKeyDerivationPath: env.FASTBTC_BTC_KEY_DERIVATION_PATH ?? 'm/0/0/0', + btcWaitedBlocksBeforeCpfp: parseInt(env.FASTBTC_BTC_WAITED_BLOCKS_BEFORE_CPFP ?? '5'), statsdUrl: env.FASTBTC_STATSD_URL, secrets: () => ( { diff --git a/packages/fastbtc-node/src/core/cpfp.ts b/packages/fastbtc-node/src/core/cpfp.ts new file mode 100644 index 0000000..42e218a --- /dev/null +++ b/packages/fastbtc-node/src/core/cpfp.ts @@ -0,0 +1,188 @@ +import {inject, injectable} from 'inversify'; +import Logger from '../logger'; +import {Network} from '../p2p/network'; +import {BitcoinMultisig, CPFPValidationError, PartiallySignedBitcoinTransaction} from '../btc/multisig'; +import {BitcoinTransferService, TransferBatch, TransferBatchDTO, TransferBatchValidator} from './transfers'; +import {Config} from '../config'; +import {setExtend, setIntersection} from '../utils/sets'; +import {sleep} from '../utils'; +import {MessageUnion} from 'ataraxia'; + +export interface CPFPBumperConfig { + numRequiredSigners: number; +} + +interface RequestCPFPSignatureMessage { + transferBatchDto: TransferBatchDTO; + cpfpTransaction: PartiallySignedBitcoinTransaction + requestId: number; +} +interface CPFPSignatureResponseMessage { + cpfpTransaction: PartiallySignedBitcoinTransaction + requestId: number; +} +interface CPFPBumperMessage { + 'fastbtc:cpfp-bumper:request-signature': RequestCPFPSignatureMessage; + 'fastbtc:cpfp-bumper:signature-response': CPFPSignatureResponseMessage; +} + + +/** + * Service for bumping transactions with CPFP + */ +@injectable() +export class CPFPBumper { + readonly MAX_SIGNATURE_WAIT_TIME_MS = 1000 * 60 * 2; + readonly SLEEP_TIME_MS = 1000; + + private logger = new Logger('cpfp-bumper') + private signatureRequestId = 0; + + constructor( + @inject(Config) private config: CPFPBumperConfig, + @inject(Network) private network: Network, + @inject(BitcoinTransferService) private bitcoinTransferService: BitcoinTransferService, + @inject(TransferBatchValidator) private transferBatchValidator: TransferBatchValidator, + @inject(BitcoinMultisig) private btcMultisig: BitcoinMultisig, + ) { + network.onMessage(this.onMessage); + } + + public async addCpfpTransaction(transferBatch: TransferBatch): Promise { + await this.transferBatchValidator.validateForAddingCpfpTransaction(transferBatch); + + const bumpedTransaction = transferBatch.signedBtcTransaction; + if (!bumpedTransaction) { + throw new Error('Cannot add CPFP to a transfer batch without a signed transaction'); + } + + const initialCpfpTransaction = await this.btcMultisig.createPartiallySignedCpfpTransaction(bumpedTransaction); + const signedCpfpTransaction = await this.requestCpfpSignatures(transferBatch, initialCpfpTransaction); + transferBatch = await this.bitcoinTransferService.addCpfpTransaction(transferBatch, signedCpfpTransaction); + return transferBatch; + } + + private async requestCpfpSignatures( + transferBatch: TransferBatch, + initialCpfpTransaction: PartiallySignedBitcoinTransaction + ): Promise { + if (initialCpfpTransaction.signedPublicKeys.length > 0) { + throw new Error('initialCpfpTransaction should not have any signatures'); + } + let signedCpfpTransaction = this.btcMultisig.signTransaction(initialCpfpTransaction); + const seenPublicKeys = new Set(signedCpfpTransaction.signedPublicKeys); + + const requestId = this.signatureRequestId++; + let gatheredPsbts: PartiallySignedBitcoinTransaction[] = []; + const listener = this.network.onMessage(async (msg) => { + if (msg.type !== 'fastbtc:cpfp-bumper:signature-response') { + return; + } + const { requestId: responseRequestId, cpfpTransaction } = msg.data; + if (responseRequestId !== requestId) { + return; + } + try { + this.validateCpfpTransaction(transferBatch, signedCpfpTransaction); + gatheredPsbts.push(cpfpTransaction); + } catch (e: any) { + if (e.isValidationError) { + this.logger.warn(`Invalid CPFP signature response: ${e.message}`); + } else { + this.logger.exception(e, 'Error processing CPFP signature response'); + } + } + }); + + const maxIterations = this.MAX_SIGNATURE_WAIT_TIME_MS / this.SLEEP_TIME_MS; + try { + for (let iteration = 0; iteration < maxIterations; iteration++) { + await this.network.broadcast('fastbtc:cpfp-bumper:request-signature', { + transferBatchDto: transferBatch.getDto(), + cpfpTransaction: initialCpfpTransaction, + requestId, + }); + await sleep(this.SLEEP_TIME_MS) + + const newPsbts = [...gatheredPsbts]; + gatheredPsbts = []; + for (const psbt of newPsbts) { + if (seenPublicKeys.size == this.config.numRequiredSigners) { + break; + } + + const seenIntersection = setIntersection(seenPublicKeys, new Set(psbt.signedPublicKeys)); + if (seenIntersection.size) { + this.logger.info(`public keys ${[...seenIntersection]} have already signed the CPFP tx`); + continue; + } + + setExtend(seenPublicKeys, psbt.signedPublicKeys); + signedCpfpTransaction = this.btcMultisig.combine([signedCpfpTransaction, psbt]); + } + + if (seenPublicKeys.size === signedCpfpTransaction.requiredSignatures) { + return signedCpfpTransaction; + } + } + throw new Error('Timed out waiting for CPFP signatures'); + } finally { + listener.unsubscribe(); + } + } + + private onMessage = async (message: MessageUnion) => { + try { + if (message.type === 'fastbtc:cpfp-bumper:request-signature') { + const {transferBatchDto, cpfpTransaction, requestId} = message.data; + if (cpfpTransaction.signedPublicKeys.indexOf(this.btcMultisig.getThisNodePublicKey()) === -1) { + this.logger.info('CPFP already signed by this node') + return; + } + + const transferBatch = await this.bitcoinTransferService.loadFromDto(transferBatchDto) + if (!transferBatch) { + this.logger.warn('TransferBatch not found'); + return; + } + + await this.transferBatchValidator.validateForSigningCpfpTransaction(transferBatch); + this.validateCpfpTransaction(transferBatch, cpfpTransaction); + + this.logger.info('Signing CPFP with requestId %s', requestId) + const signedCpfpTransaction = this.btcMultisig.signTransaction(cpfpTransaction); + await message.source.send('fastbtc:cpfp-bumper:signature-response', { + cpfpTransaction: signedCpfpTransaction, + requestId, + }); + } + } catch (e: any) { + if (e.isValidationError) { + this.logger.warn( + 'Validation error while processing CPFP message %s with data %s: %s', + message.type, + message.data, + e.message + ); + } else { + this.logger.exception( + e, + `Error processing CPFP message %s with data %s`, + message.type, + message.data + ); + } + } + } + + private validateCpfpTransaction( + transferBatch: TransferBatch, + cpfpTransaction: PartiallySignedBitcoinTransaction + ): void { + const bumpedTransaction = transferBatch.signedBtcTransaction; + if (!bumpedTransaction) { + throw new CPFPValidationError('no signed transaction'); + } + this.btcMultisig.validatePartiallySignedCpfpTransaction(bumpedTransaction, cpfpTransaction); + } +} diff --git a/packages/fastbtc-node/src/core/node.ts b/packages/fastbtc-node/src/core/node.ts index 0968910..3ac7c04 100644 --- a/packages/fastbtc-node/src/core/node.ts +++ b/packages/fastbtc-node/src/core/node.ts @@ -292,7 +292,13 @@ export class FastBTCNode { transferBatchDto: transferBatch.getDto(), } ); - await this.bitcoinTransferService.sendToBitcoin(transferBatch); + const wasMined = await this.bitcoinTransferService.sendToBitcoin(transferBatch); + if (!wasMined) { + this.logger.info( + 'TransferBatch was not sent in due time, initiating CPFP' + ); + this.statsd.increment('fastbtc.pegout.cpfp_initiated'); + } return; } diff --git a/packages/fastbtc-node/src/core/transfers.ts b/packages/fastbtc-node/src/core/transfers.ts index 72cbd4e..36534c7 100644 --- a/packages/fastbtc-node/src/core/transfers.ts +++ b/packages/fastbtc-node/src/core/transfers.ts @@ -14,6 +14,7 @@ import {sleep} from '../utils'; import {setExtend, setIntersection} from "../utils/sets"; import {toNumber} from '../rsk/utils'; import {Satoshis} from '../btc/types'; +import {deepcopy} from '../utils/copy'; // For lack of a better place, just have these here export const MAX_BTC_IN_BATCH = 5.0; @@ -34,6 +35,7 @@ export interface TransferBatchDTO { signedBtcTransaction?: PartiallySignedBitcoinTransaction; rskMinedSignatures: string[]; rskMinedSigners: string[]; + signedCpfpTransactions?: PartiallySignedBitcoinTransaction[]; } export interface TransferBatchEnvironment { @@ -60,6 +62,7 @@ export class TransferBatch { public signedBtcTransaction: PartiallySignedBitcoinTransaction|undefined, public rskMinedSignatures: string[], public rskMinedSigners: string[], + public signedCpfpTransactions?: PartiallySignedBitcoinTransaction[], ) { } @@ -81,6 +84,7 @@ export class TransferBatch { signedBtcTransaction: this.signedBtcTransaction, rskMinedSignatures: this.rskMinedSignatures, rskMinedSigners: this.rskMinedSigners, + signedCpfpTransactions: this.signedCpfpTransactions, } } @@ -304,6 +308,7 @@ export class BitcoinTransferService { dto.signedBtcTransaction, dto.rskMinedSignatures, dto.rskMinedSigners, + dto.signedCpfpTransactions, ); }); } @@ -530,6 +535,25 @@ export class BitcoinTransferService { return transferBatch; } + async addCpfpTransaction( + transferBatch: TransferBatch, + cpfpTransaction: PartiallySignedBitcoinTransaction + ): Promise { + await this.validator.validateForAddingCpfpTransaction(transferBatch); + if (!transferBatch.signedBtcTransaction) { + throw new Error('Cannot add cpfp transaction to unsigned transaction'); + } + this.btcMultisig.validatePartiallySignedCpfpTransaction(transferBatch.signedBtcTransaction, cpfpTransaction); + transferBatch = transferBatch.copy(); + if(!transferBatch.signedCpfpTransactions) { + transferBatch.signedCpfpTransactions = []; + } + transferBatch.signedCpfpTransactions.push(cpfpTransaction); + await this.validator.validateCpfpTransactions(transferBatch); + await this.updateStoredTransferBatch(transferBatch); + return transferBatch; + } + async markAsSendingInRsk(transferBatch: TransferBatch): Promise { if (!transferBatch.hasEnoughRskSendingSignatures()) { throw new Error('TransferBatch does not have enough signatures to be marked as sending'); @@ -615,12 +639,12 @@ export class BitcoinTransferService { return this.btcMultisig.signTransaction(transferBatch.initialBtcTransaction); } - async sendToBitcoin(transferBatch: TransferBatch): Promise { + async sendToBitcoin(transferBatch: TransferBatch): Promise { this.logger.info("Sending TransferBatch to bitcoin"); await this.validator.validateForSendingToBitcoin(transferBatch); if (transferBatch.isSentToBitcoin()) { this.logger.info("TransferBatch is already sent to bitcoin"); - return; + return true; } if (!transferBatch.signedBtcTransaction) { throw new Error("TransferBatch doesn't have signedBtcTransaction"); @@ -629,6 +653,7 @@ export class BitcoinTransferService { this.logger.info("TransferBatch successfully sent to bitcoin"); // This method should be idempotent, but we will still wait for the required number of confirmations. + // TODO: wait for N blocks, not N time const requiredConfirmations = this.config.btcRequiredConfirmations; const maxIterations = 200; const avgBlockTimeMs = 10 * 60 * 1000; @@ -639,10 +664,13 @@ export class BitcoinTransferService { const chainTx = await this.btcMultisig.getTransaction(transferBatch.bitcoinTransactionHash); const confirmations = chainTx ? chainTx.confirmations : 0; if (confirmations >= requiredConfirmations) { - break; + // mined + return true; } await sleep(sleepTimeMs); } + // not mined + return false; } async signRskMinedUpdate(transferBatch: TransferBatch): Promise<{signature: string, address: string}> { @@ -852,6 +880,32 @@ export class TransferBatchValidator { await this.validateTransferBatch(transferBatch, null, true); } + async validateForSigningCpfpTransaction(transferBatch: TransferBatch): Promise { + await this.validateForAddingCpfpTransaction(transferBatch); + } + + async validateForAddingCpfpTransaction(transferBatch: TransferBatch): Promise { + if ( + transferBatch.transfers.length == 0 || + //!transferBatch.hasEnoughRskSendingSignatures() || + !transferBatch.hasEnoughBitcoinSignatures() || + !transferBatch.isMarkedAsSendingInRsk() || + !transferBatch.signedBtcTransaction + ) { + throw new TransferBatchValidationError('TransferBatch is not sendable to bitcoin'); + } + if (transferBatch.isSentToBitcoin()) { + throw new TransferBatchValidationError('TransferBatch is already sent to bitcoin'); + } + if (transferBatch.signedCpfpTransactions?.length) { + // TODO: get rid of this when we add support for more than one CPFP transaction + throw new TransferBatchValidationError( + `Only one CPFP transaction is currently supported, got ${transferBatch.signedCpfpTransactions.length}` + ); + } + await this.validateTransferBatch(transferBatch, TransferStatus.Sending, true); + } + async validateCompleteTransferBatch(transferBatch: TransferBatch): Promise { if ( transferBatch.transfers.length == 0 || @@ -929,7 +983,7 @@ export class TransferBatchValidator { private async validateTransferBatch( transferBatch: TransferBatch, expectedStatus: TransferStatus|null, - requireSignedBtcTransaction: boolean + requireSignedBtcTransaction: boolean, ): Promise { if (!transferBatch.hasValidTransferState()) { throw new TransferBatchValidationError( @@ -946,6 +1000,7 @@ export class TransferBatchValidator { 'TransferBatch is missing signedBtcTransaction' ); } + await this.validateCpfpTransactions(transferBatch); } private async validateRskSignatures(transferBatch: TransferBatch): Promise { @@ -1076,6 +1131,34 @@ export class TransferBatchValidator { } } + async validateCpfpTransactions(transferBatch: TransferBatch) { + if (!transferBatch.signedCpfpTransactions?.length) { + return; + } + if (!transferBatch.signedBtcTransaction) { + throw new TransferBatchValidationError( + 'TransferBatch must have a signed BTC transaction to have CPFP transactions' + ); + } + if (transferBatch.signedCpfpTransactions.length > 1) { + // TODO: get rid of this when we add support for more than one CPFP transaction + throw new TransferBatchValidationError( + `Only one CPFP transaction is currently supported, got ${transferBatch.signedCpfpTransactions.length}` + ); + } + let bumpedTx = transferBatch.signedBtcTransaction; + for (const cpfp of transferBatch.signedCpfpTransactions) { + // TODO: validate signers + if (cpfp.signedPublicKeys.length !== cpfp.requiredSignatures) { + throw new TransferBatchValidationError( + `TransferBatch has a CPFP tx with ${cpfp.signedPublicKeys.length} signatures but requires ${cpfp.requiredSignatures}` + ); + } + await this.btcMultisig.validatePartiallySignedCpfpTransaction(bumpedTx, cpfp); + bumpedTx = cpfp; + } + } + private async fetchRskTransferInfo(btcPaymentAddress: string, nonce: number): Promise { const currentBlock = await this.ethersProvider.getBlockNumber(); const transferData = await this.fastBtcBridge.getTransfer(btcPaymentAddress, nonce); @@ -1135,7 +1218,3 @@ async function getUpdateHashForMined(fastBtcBridge: Contract, transferBatch: Tra TransferStatus.Mined ); } - -function deepcopy(thing: T): T { - return JSON.parse(JSON.stringify(thing)); -} diff --git a/packages/fastbtc-node/src/utils/copy.ts b/packages/fastbtc-node/src/utils/copy.ts new file mode 100644 index 0000000..5ba898b --- /dev/null +++ b/packages/fastbtc-node/src/utils/copy.ts @@ -0,0 +1,3 @@ +export function deepcopy(thing: T): T { + return JSON.parse(JSON.stringify(thing)); +} From 9543d20de044cf6692992e37e28a37bacff08aa6 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 05:22:00 +0200 Subject: [PATCH 06/15] Fix DI mishap --- packages/fastbtc-node/src/core/cpfp.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fastbtc-node/src/core/cpfp.ts b/packages/fastbtc-node/src/core/cpfp.ts index 42e218a..a22046e 100644 --- a/packages/fastbtc-node/src/core/cpfp.ts +++ b/packages/fastbtc-node/src/core/cpfp.ts @@ -1,6 +1,6 @@ import {inject, injectable} from 'inversify'; import Logger from '../logger'; -import {Network} from '../p2p/network'; +import {Network, P2PNetwork} from '../p2p/network'; import {BitcoinMultisig, CPFPValidationError, PartiallySignedBitcoinTransaction} from '../btc/multisig'; import {BitcoinTransferService, TransferBatch, TransferBatchDTO, TransferBatchValidator} from './transfers'; import {Config} from '../config'; @@ -40,7 +40,7 @@ export class CPFPBumper { constructor( @inject(Config) private config: CPFPBumperConfig, - @inject(Network) private network: Network, + @inject(P2PNetwork) private network: Network, @inject(BitcoinTransferService) private bitcoinTransferService: BitcoinTransferService, @inject(TransferBatchValidator) private transferBatchValidator: TransferBatchValidator, @inject(BitcoinMultisig) private btcMultisig: BitcoinMultisig, From f442add80911c42bcbb9d48421a03b76121d9fa9 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 05:23:07 +0200 Subject: [PATCH 07/15] Actualyl send the CPFP (maybe, untested) --- packages/fastbtc-node/src/core/index.ts | 2 + packages/fastbtc-node/src/core/node.ts | 3 ++ packages/fastbtc-node/src/core/transfers.ts | 43 ++++++++++++++++++--- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/packages/fastbtc-node/src/core/index.ts b/packages/fastbtc-node/src/core/index.ts index fc8e3ac..08d17d1 100644 --- a/packages/fastbtc-node/src/core/index.ts +++ b/packages/fastbtc-node/src/core/index.ts @@ -2,6 +2,7 @@ import {interfaces} from 'inversify'; import {FastBTCNode} from './node'; import {BitcoinTransferService, TransferBatchValidator} from './transfers'; import StatusChecker from './statuschecker'; +import {CPFPBumper} from './cpfp'; import Container = interfaces.Container; export function setupInversify(container: Container) { @@ -9,4 +10,5 @@ export function setupInversify(container: Container) { container.bind(BitcoinTransferService).toSelf().inSingletonScope(); container.bind(TransferBatchValidator).toSelf().inSingletonScope(); container.bind(StatusChecker).toSelf().inSingletonScope(); + container.bind(CPFPBumper).toSelf().inSingletonScope(); } diff --git a/packages/fastbtc-node/src/core/node.ts b/packages/fastbtc-node/src/core/node.ts index 3ac7c04..5477170 100644 --- a/packages/fastbtc-node/src/core/node.ts +++ b/packages/fastbtc-node/src/core/node.ts @@ -15,6 +15,7 @@ import {StatsD} from "hot-shots"; import {TYPES} from "../stats"; import StatusChecker from './statuschecker'; import {BitcoinReplenisher} from '../replenisher/replenisher'; +import {CPFPBumper} from './cpfp'; type FastBTCNodeConfig = Pick< Config, @@ -115,6 +116,7 @@ export class FastBTCNode { @inject(TYPES.StatsD) private statsd: StatsD, @inject(StatusChecker) private statusChecker: StatusChecker, @inject(BitcoinReplenisher) private replenisher: BitcoinReplenisher, + @inject(CPFPBumper) private cpfpBumper: CPFPBumper, ) { this.networkUtil = new NetworkUtil(network, this.logger); network.onNodeAvailable(this.onNodeAvailable); @@ -298,6 +300,7 @@ export class FastBTCNode { 'TransferBatch was not sent in due time, initiating CPFP' ); this.statsd.increment('fastbtc.pegout.cpfp_initiated'); + transferBatch = await this.cpfpBumper.addCpfpTransaction(transferBatch); } return; } diff --git a/packages/fastbtc-node/src/core/transfers.ts b/packages/fastbtc-node/src/core/transfers.ts index 36534c7..594227c 100644 --- a/packages/fastbtc-node/src/core/transfers.ts +++ b/packages/fastbtc-node/src/core/transfers.ts @@ -46,6 +46,7 @@ export interface TransferBatchEnvironment { requiredBitcoinConfirmations: number; requiredRskConfirmations: number; bitcoinOnChainTransaction?: BitcoinRPCGetTransactionResponse; + bitcoinOnChainCpfpTransactions: (BitcoinRPCGetTransactionResponse|undefined)[]; } /** @@ -192,6 +193,14 @@ export class TransferBatch { if (!chainTx) { return false; } + for (let cpfpTx of this.environment.bitcoinOnChainCpfpTransactions) { + if (!cpfpTx) { + return false; + } + if (cpfpTx.confirmations < this.environment.requiredBitcoinConfirmations) { + return false; + } + } return chainTx.confirmations >= this.environment.requiredBitcoinConfirmations; } @@ -298,8 +307,12 @@ export class BitcoinTransferService { transfers.push(transfer); } + const cpfpTxHashes = dto.signedCpfpTransactions?.map( + tx => this.btcMultisig.getBitcoinTransactionHash(tx) + ) ?? []; + return new TransferBatch( - await this.getTransferBatchEnvironment(dto.bitcoinTransactionHash), + await this.getTransferBatchEnvironment(dto.bitcoinTransactionHash, cpfpTxHashes), transfers, dto.rskSendingSignatures, dto.rskSendingSigners, @@ -651,6 +664,12 @@ export class BitcoinTransferService { } await this.btcMultisig.submitTransaction(transferBatch.signedBtcTransaction); this.logger.info("TransferBatch successfully sent to bitcoin"); + if (transferBatch.signedCpfpTransactions) { + for (const cpfpTransaction of transferBatch.signedCpfpTransactions) { + await this.btcMultisig.submitTransaction(cpfpTransaction); + this.logger.info("CPFP transaction successfully sent to bitcoin"); + } + } // This method should be idempotent, but we will still wait for the required number of confirmations. // TODO: wait for N blocks, not N time @@ -664,8 +683,18 @@ export class BitcoinTransferService { const chainTx = await this.btcMultisig.getTransaction(transferBatch.bitcoinTransactionHash); const confirmations = chainTx ? chainTx.confirmations : 0; if (confirmations >= requiredConfirmations) { - // mined - return true; + let isConfirmed = true; + if (transferBatch.signedCpfpTransactions) { + for (const cpfpPsbt of transferBatch.signedCpfpTransactions) { + const cpfpTxHash = this.btcMultisig.getBitcoinTransactionHash(cpfpPsbt); + const cpfpTx = await this.btcMultisig.getTransaction(cpfpTxHash); + if (!cpfpTx || cpfpTx.confirmations < requiredConfirmations) { + isConfirmed = false; + break; + } + } + } + return isConfirmed; } await sleep(sleepTimeMs); } @@ -691,7 +720,7 @@ export class BitcoinTransferService { initialSignedBtcTransaction ); return new TransferBatch( - await this.getTransferBatchEnvironment(bitcoinTxHash), // could also null here since it is not in blockchain + await this.getTransferBatchEnvironment(bitcoinTxHash, []), // could also null here since it is not in blockchain transfers, [], [], @@ -703,15 +732,19 @@ export class BitcoinTransferService { ); } - private async getTransferBatchEnvironment(bitcoinTransactionHash: string|null): Promise { + private async getTransferBatchEnvironment(bitcoinTransactionHash: string|null, cpfpTransactionHashes: string[]): Promise { const currentBlockNumber = await this.ethersProvider.getBlockNumber(); let bitcoinOnChainTransaction = undefined; if (bitcoinTransactionHash) { bitcoinOnChainTransaction = await this.btcMultisig.getTransaction(bitcoinTransactionHash); } + const bitcoinOnChainCpfpTransactions = await Promise.all( + cpfpTransactionHashes.map(h => this.btcMultisig.getTransaction(h)) + ); return { currentBlockNumber, bitcoinOnChainTransaction, + bitcoinOnChainCpfpTransactions, requiredBitcoinConfirmations: this.config.btcRequiredConfirmations, requiredRskConfirmations: this.config.rskRequiredConfirmations, numRequiredSigners: this.config.numRequiredSigners, From be0226802b51b7f19edc58c776100fb161e45c6b Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 13:58:03 +0200 Subject: [PATCH 08/15] CPFP integration tests --- Makefile | 3 ++ README.md | 29 +++++++++++++++++++ docker-compose-regtest.yml | 4 +++ .../bitcoind-regtest/docker-entrypoint.sh | 18 +++++++++++- 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 47941eb..aecf130 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,9 @@ run-demo-regtest-replenisher-very-small-coins: packages/fastbtc-node/version.jso .PHONY: run-demo-regtest-replenisher-limits run-demo-regtest-replenisher-limits: packages/fastbtc-node/version.json @export TEST_REPLENISHER_LIMITS=true && make run-demo-regtest +.PHONY: run-demo-regtest-cpfp +run-demo-regtest-cpfp: packages/fastbtc-node/version.json + @export TEST_CPFP=true && make run-demo-regtest .PHONY: show-node-logs show-node-logs: diff --git a/README.md b/README.md index 0887875..2aff532 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,35 @@ $ make run-demo-regtest-replenisher-limits $ make test-transfers-big-amounts ``` +Observe the output, quit with Ctrl-C if wanted (though it quits automatically on success). +You should see the lines: +``` +TEST_CPFP is true, only sleeping for 100 ms +``` +and +``` +CPFP transaction successfully sent to bitcoin +``` +in the output. + +This test will also take ~10 minutes to run. + +#### Automatic bumping of slow transfers with CPFP + +This test tests the case when a bitcoin transfer is slow and requires a CPFP (child-pays-for-parent) +transaction to speed it up. + +This is notoriously hard to test in a regtest environment. The test case will validate +that the code for creating a CPFP transaction works, but it will not actually test that +it will actually bump the parent transaction. + +``` +# In one tab: +$ make run-demo-regtest-cpfp +# In another tab +$ make test-transfers +``` + Observe the output, quit with Ctrl-C if wanted (though it quits automatically on success). ### Advanced details diff --git a/docker-compose-regtest.yml b/docker-compose-regtest.yml index 582a1e8..5c77b9c 100644 --- a/docker-compose-regtest.yml +++ b/docker-compose-regtest.yml @@ -29,6 +29,7 @@ services: # Extra test vars - TEST_VERY_SMALL_REPLENISHER_COINS - TEST_REPLENISHER_LIMITS + - TEST_CPFP ports: - 18443:18443 @@ -55,6 +56,7 @@ services: - FASTBTC_RSK_RPC_URL=http://hardhat:8545 - FASTBTC_BTC_RPC_URL=http://bitcoin-regtest:18443/wallet/multisig - FASTBTC_REPLENISHER_RPC_URL=http://bitcoin-regtest:18443/wallet/replenisher + - TEST_CPFP depends_on: - bitcoin-regtest - hardhat @@ -64,6 +66,7 @@ services: - FASTBTC_RSK_RPC_URL=http://hardhat:8545 - FASTBTC_BTC_RPC_URL=http://bitcoin-regtest:18443/wallet/multisig - FASTBTC_REPLENISHER_RPC_URL=http://bitcoin-regtest:18443/wallet/replenisher + - TEST_CPFP depends_on: - bitcoin-regtest - hardhat @@ -81,6 +84,7 @@ services: - FASTBTC_RSK_RPC_URL=http://hardhat:8545 - FASTBTC_BTC_RPC_URL=http://bitcoin-regtest:18443/wallet/multisig - FASTBTC_REPLENISHER_RPC_URL=http://bitcoin-regtest:18443/wallet/replenisher + - TEST_CPFP depends_on: - bitcoin-regtest - hardhat diff --git a/integration_test/bitcoind-regtest/docker-entrypoint.sh b/integration_test/bitcoind-regtest/docker-entrypoint.sh index 1d15502..22b6207 100755 --- a/integration_test/bitcoind-regtest/docker-entrypoint.sh +++ b/integration_test/bitcoind-regtest/docker-entrypoint.sh @@ -60,11 +60,15 @@ echo "done" # triggered (as long as the number matches the one configured in the backend) -- and the rest to the replenisher # wallet. This is meant to test the case where a new TransferBatch cannot be created because the multisig doesn't # have enough funds, but the replenisher doesn't trigger either because the balance is over the threshold. +# - If TEST_CPFP is true, we wait a crapton of time between mining new blocks. This is very slow, but +# it enables us to actually send a CPFP transaction and test that the code does not fail. +# Caveat: It does not actually test that the CPFP transaction does a CPFP. # # Ugh. echo "Test settings:" echo "TEST_VERY_SMALL_REPLENISHER_COINS=$TEST_VERY_SMALL_REPLENISHER_COINS" echo "TEST_REPLENISHER_LIMITS=$TEST_REPLENISHER_LIMITS" +echo "TEST_CPFP=$TEST_CPFP" # We need a temporary address for both of these cases, because we need to send amounts smaller than the block reward. if [[ "$TEST_VERY_SMALL_REPLENISHER_COINS" = "true" || "$TEST_REPLENISHER_LIMITS" = "true" ]] @@ -107,6 +111,12 @@ then echo "Sending 5.5 BTC to the multisig (should be just over the threshold)..." bitcoin-cli -rpcwallet=temporary sendtoaddress "$MULTISIG_ADDRESS" 5.5 > /dev/null else + if [[ "$TEST_CPFP" = "true" ]] + then + echo "Generating 101 blocks (sending balance to multisig because we are testing CPFP)..." + bitcoin-cli -rpcwallet=replenisher generatetoaddress 101 "$MULTISIG_ADDRESS" > /dev/null + fi + echo "Generating 101+ blocks (sending balance to replenisher wallet, not directly to multisig)..." echo "Init replenisher funds" for i in $(bitcoin-cli deriveaddresses "$REPLENISHER_SOURCE_DESCRIPTOR" '[5,10]'|cut -f 2 -d '"'|grep bc) @@ -147,7 +157,13 @@ do # sending to replenisher here, not multisig bitcoin-cli -rpcwallet=replenisher generatetoaddress 1 "$i" > /dev/null fi - sleep 1 + + if [[ "$TEST_CPFP" = "true" ]] + then + sleep 600 + else + sleep 1 + fi done done From 8eead6b126cbdd40d9ecb939c0fc5062abb46b97 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 13:58:40 +0200 Subject: [PATCH 09/15] CPFP integration testing functionality in the actual CPFP code --- packages/fastbtc-node/src/core/transfers.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/fastbtc-node/src/core/transfers.ts b/packages/fastbtc-node/src/core/transfers.ts index 594227c..5d256e2 100644 --- a/packages/fastbtc-node/src/core/transfers.ts +++ b/packages/fastbtc-node/src/core/transfers.ts @@ -677,7 +677,14 @@ export class BitcoinTransferService { const maxIterations = 200; const avgBlockTimeMs = 10 * 60 * 1000; const overheadMultiplier = 2; - const sleepTimeMs = Math.round((avgBlockTimeMs * requiredConfirmations * overheadMultiplier) / maxIterations); + let sleepTimeMs: number; + if (process.env.TEST_CPFP === 'true') { + // hack for testing cpfp + this.logger.info("TEST_CPFP is true, only sleeping for 100 ms"); + sleepTimeMs = 100; + } else { + sleepTimeMs = Math.round((avgBlockTimeMs * requiredConfirmations * overheadMultiplier) / maxIterations); + } this.logger.info(`Waiting for ${requiredConfirmations} confirmations`); for (let i = 0; i < maxIterations; i++) { const chainTx = await this.btcMultisig.getTransaction(transferBatch.bitcoinTransactionHash); From 11b8705d6ed5c592bb9906d98211bc1c8facaa6d Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 13:59:00 +0200 Subject: [PATCH 10/15] Fix rest of the CPFP bugs --- packages/fastbtc-node/src/btc/multisig.ts | 3 +++ packages/fastbtc-node/src/core/cpfp.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/fastbtc-node/src/btc/multisig.ts b/packages/fastbtc-node/src/btc/multisig.ts index dc42957..795d94b 100644 --- a/packages/fastbtc-node/src/btc/multisig.ts +++ b/packages/fastbtc-node/src/btc/multisig.ts @@ -775,6 +775,9 @@ export class BitcoinMultisig { requiredSignatures: tx.requiredSignatures, derivationPaths: [...tx.derivationPaths], noChange: Boolean(tx.noChange), + isCpfp: Boolean(tx.isCpfp), + cumulativeByteCount: tx.cumulativeByteCount, + feeSatsPerVB: tx.feeSatsPerVB, } } return result; diff --git a/packages/fastbtc-node/src/core/cpfp.ts b/packages/fastbtc-node/src/core/cpfp.ts index a22046e..2c9f695 100644 --- a/packages/fastbtc-node/src/core/cpfp.ts +++ b/packages/fastbtc-node/src/core/cpfp.ts @@ -135,7 +135,7 @@ export class CPFPBumper { try { if (message.type === 'fastbtc:cpfp-bumper:request-signature') { const {transferBatchDto, cpfpTransaction, requestId} = message.data; - if (cpfpTransaction.signedPublicKeys.indexOf(this.btcMultisig.getThisNodePublicKey()) === -1) { + if (cpfpTransaction.signedPublicKeys.indexOf(this.btcMultisig.getThisNodePublicKey()) !== -1) { this.logger.info('CPFP already signed by this node') return; } From 042871a803c189d5cf84e98b16d81a54598f938b Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 14:18:46 +0200 Subject: [PATCH 11/15] Hacky make target to show initiator logs --- Makefile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index aecf130..b3beaa1 100644 --- a/Makefile +++ b/Makefile @@ -25,6 +25,13 @@ run-demo-regtest-cpfp: packages/fastbtc-node/version.json show-node-logs: @docker-compose -f docker-compose-base.yml -f docker-compose-regtest.yml logs -f node1 node2 node3 +# This is very hacky :P We grep for a message only sent by the initiator and parse the node id from there +.PHONY: show-initiator-logs +show-initiator-logs: + docker-compose -f docker-compose-base.yml -f docker-compose-regtest.yml logs -f $$( \ + docker-compose -f docker-compose-base.yml -f docker-compose-regtest.yml logs --no-color \ + | grep -e 'stored batches in total' | tail -1 | cut -d_ -f 1) + .PHONY: build-regtest-bitcoin build-regtest-bitcoin: @(cd integration_test/bitcoind-regtest \ From 6e22d0827920102d9c59e51bb6bcaa14f42b2f44 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 15:35:49 +0200 Subject: [PATCH 12/15] "Only" wait 5min between btc blocks with TEST_CPFP --- integration_test/bitcoind-regtest/docker-entrypoint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test/bitcoind-regtest/docker-entrypoint.sh b/integration_test/bitcoind-regtest/docker-entrypoint.sh index 22b6207..d5b8c26 100755 --- a/integration_test/bitcoind-regtest/docker-entrypoint.sh +++ b/integration_test/bitcoind-regtest/docker-entrypoint.sh @@ -160,7 +160,7 @@ do if [[ "$TEST_CPFP" = "true" ]] then - sleep 600 + sleep 300 else sleep 1 fi From 338d1eff1ffff0aaf93c802785d9f454ffc4f04e Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 15:36:23 +0200 Subject: [PATCH 13/15] Fix CPFP onMessage handler Turns out calling unsubscribe will unsubscribe *every* onMessage handler!? --- packages/fastbtc-node/src/core/cpfp.ts | 20 ++++++++++++++++---- packages/fastbtc-node/src/core/node.ts | 11 ++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/fastbtc-node/src/core/cpfp.ts b/packages/fastbtc-node/src/core/cpfp.ts index 2c9f695..a800b62 100644 --- a/packages/fastbtc-node/src/core/cpfp.ts +++ b/packages/fastbtc-node/src/core/cpfp.ts @@ -37,6 +37,7 @@ export class CPFPBumper { private logger = new Logger('cpfp-bumper') private signatureRequestId = 0; + private responseListeners: ((msg: MessageUnion) => void)[] = []; constructor( @inject(Config) private config: CPFPBumperConfig, @@ -74,7 +75,8 @@ export class CPFPBumper { const requestId = this.signatureRequestId++; let gatheredPsbts: PartiallySignedBitcoinTransaction[] = []; - const listener = this.network.onMessage(async (msg) => { + + const listener = async (msg: MessageUnion) => { if (msg.type !== 'fastbtc:cpfp-bumper:signature-response') { return; } @@ -82,6 +84,7 @@ export class CPFPBumper { if (responseRequestId !== requestId) { return; } + this.logger.info(`Received CPFP signature response from ${msg.source.id}`); try { this.validateCpfpTransaction(transferBatch, signedCpfpTransaction); gatheredPsbts.push(cpfpTransaction); @@ -92,7 +95,11 @@ export class CPFPBumper { this.logger.exception(e, 'Error processing CPFP signature response'); } } - }); + } + + // We don't use this.network.onMessage(listener) because I cannot find a way to remove that + // without removing EVERY onMessage listener -_- + this.responseListeners.push(listener); const maxIterations = this.MAX_SIGNATURE_WAIT_TIME_MS / this.SLEEP_TIME_MS; try { @@ -127,13 +134,18 @@ export class CPFPBumper { } throw new Error('Timed out waiting for CPFP signatures'); } finally { - listener.unsubscribe(); + // Remove the listener + this.responseListeners = this.responseListeners.filter(l => l !== listener); } } private onMessage = async (message: MessageUnion) => { try { - if (message.type === 'fastbtc:cpfp-bumper:request-signature') { + if (message.type === 'fastbtc:cpfp-bumper:signature-response') { + for (const listener of this.responseListeners) { + await listener(message); + } + } else if (message.type === 'fastbtc:cpfp-bumper:request-signature') { const {transferBatchDto, cpfpTransaction, requestId} = message.data; if (cpfpTransaction.signedPublicKeys.indexOf(this.btcMultisig.getThisNodePublicKey()) !== -1) { this.logger.info('CPFP already signed by this node') diff --git a/packages/fastbtc-node/src/core/node.ts b/packages/fastbtc-node/src/core/node.ts index 5477170..e1e7fba 100644 --- a/packages/fastbtc-node/src/core/node.ts +++ b/packages/fastbtc-node/src/core/node.ts @@ -568,7 +568,16 @@ export class FastBTCNode { return; } - await callback(transferBatch, message); + try { + await callback(transferBatch, message); + } catch (e: any) { + if (e.isValidationError) { + this.logger.warn(`Validation error: ${e.message}`); + } else { + this.logger.exception(e, 'Error processing a message from the initiator'); + } + throw e; + } } onRskSendingSignatureResponse = async (data: RSKSendingSignatureResponseMessage, source: Node) => { From 573e745156ef0969bae814700ed7611753e33122 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 15:39:10 +0200 Subject: [PATCH 14/15] Move these lines to the right place in the README --- README.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2aff532..1779b11 100644 --- a/README.md +++ b/README.md @@ -149,17 +149,6 @@ $ make test-transfers-big-amounts ``` Observe the output, quit with Ctrl-C if wanted (though it quits automatically on success). -You should see the lines: -``` -TEST_CPFP is true, only sleeping for 100 ms -``` -and -``` -CPFP transaction successfully sent to bitcoin -``` -in the output. - -This test will also take ~10 minutes to run. #### Automatic bumping of slow transfers with CPFP @@ -179,6 +168,19 @@ $ make test-transfers Observe the output, quit with Ctrl-C if wanted (though it quits automatically on success). +You should see the lines: +``` +TEST_CPFP is true, only sleeping for 100 ms +``` +and +``` +CPFP transaction successfully sent to bitcoin +``` +in the output. + +This test will also take ~10 minutes to run. + + ### Advanced details The test setup (launched with `make run-demo-regtest`) will expose the Hardhat RPC server at `http://localhost:18545` From 4e7f5f8499d85198ce7729ff0ef3b7a163372af9 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 2 Nov 2022 15:44:03 +0200 Subject: [PATCH 15/15] Fix comment --- packages/fastbtc-node/src/core/transfers.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/fastbtc-node/src/core/transfers.ts b/packages/fastbtc-node/src/core/transfers.ts index 5d256e2..ff53531 100644 --- a/packages/fastbtc-node/src/core/transfers.ts +++ b/packages/fastbtc-node/src/core/transfers.ts @@ -1188,7 +1188,9 @@ export class TransferBatchValidator { } let bumpedTx = transferBatch.signedBtcTransaction; for (const cpfp of transferBatch.signedCpfpTransactions) { - // TODO: validate signers + // We should maybe validate that signed public keys match signers... But 1) it doesn't look like they are + // validated for the "normal" BTC transactions either and 2) any federator can put anything in the + // signed public keys as it's not derived from the PSBT if (cpfp.signedPublicKeys.length !== cpfp.requiredSignatures) { throw new TransferBatchValidationError( `TransferBatch has a CPFP tx with ${cpfp.signedPublicKeys.length} signatures but requires ${cpfp.requiredSignatures}`