-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to force the selection of utxos #4
base: main
Are you sure you want to change the base?
Conversation
This proposal to integrate a For instance, in a scenario I am interested in, I have time-unlocked UTXOs that I prefer to be spent first (prioritized) but without the necessity to force spend all of them. A While your use case is valid and necessay, I am considering ways to implement it without overcomplicating the codebase as more use cases appear (see the 3 possible flags just as an example). I suggest a focused approach on each use case by designing separate algos. Implementing CPFP would require further modifications and additional code anyway if I am not wrong. It requires calculating the effective fee rate for the entire transaction chain, involving the vsize and fee of previous transactions. This would mean augmenting the utxos array with more data beyond Even with your proposal, determining the right fee rate to achieve the desired effective fee rate presents a challenge, as it depends on the utxos that will be selected, which isn't predetermined. While I was thinking in your proposal, I conceptualized a possible implementation for CPFP and, the way I see it, I would need having the txId, vsize and fee of the parent txs anyway and all the algos would require even more changes: export function CPFP({
utxos,
parentUtxos,
targets,
remainder,
effectiveFeeRate
}: {
/** utxos of regular confirmed txs - pass them in the order that you prefer
* being added */
utxos: Array<OutputWithValue>;
/** utxos corresponding to unconfirmed txs.
* txId is passed to cluster utxos which belong to the same unconfirmed tx.
*/
parentUtxos: Array<
OutputWithValue & { vsize: number; fee: number; txId: string }
>;
/** targets can be an empty array */
targets: Array<OutputWithValue>;
remainder: OutputInstance;
/** the effective fee rate is the desired ratio between the sum of all the fees
* involved / the sum of the sizes of all the the txs involved
* including the unconfirmed txs and the new tx that will be created
*/
effectiveFeeRate: number;
}) {
//selectedUtxos = parentUtxos
//Compute the totalVsize of parent unconfirmed txs (only one per unconfirmed txId, not per parentUtxo)
//Compute the totalFee of parent txs (one per unconfirmed txId, not per parentUtxo)
//totalFee = f(totalFee, feeForTargets /*if any*/)
//totalVsize += vsizeOfNewTxWithParentUtxosAndTargets (targets, if any)
//totalVsize += vsizeIncreaseForRemainder (if necessary)
//while (totalFee / totalVSize < effectiveFeeRate)
// pick a candidateUtxo
// selectedUtxos.push(candidateUtxo)
// totalFee = f(totalFee, feeForCandidateUtxo)
// totalFee = f(totalFee, feeForRemained /*if necessary*/)
// totalVsize += vsizeIncreaseForCandidateUtxo
// totalFee += vsizeIncreaseForRemainer (if necessary)
//Return selectedUtxos
} This approach focuses on selecting UTXOs based on the effective fee rate while considering the total size and fee of the parent transactions. Do you think a more focused solution would better meet your needs? I'm not outright rejecting the idea of implementing CPFP, but I am considering how to add this feature without overly complicating the codebase as we address increasing use cases. |
Using this PR, I've managed to do a CPFP: get my wallet UTXOs, set I don't think implementing CPFP in this library makes much sense as most of the calculation etc can be done before / after the coinselection and while building the transaction. What's only needed for this to work well is to be able to properly select UTXOs to pay for fees while force selecting some UTXOs. It feels like if we start adding algos for everything like CPFP, we'll get too many quite quickly. So, even if that complicates a bit the codebase, maybe we can come up with meaningful flags that can be useful in various situations, without having to write distinctive algos? Btw, for my example above, allowing the targets to be empty would save some steps (adding a dummy output then removing it and adding back the sats). The thing is that all the algos for now are based on the target value to reach. Maybe we could say that if the targets are empty, then the target value should be the fees to pay, and that would change based on the const targetFeeRate = await getFeeRate()
const actualFeeRate = 2 * targetFeeRate - previousFeeRate
// selectUtxos is just a wrapper that converts descriptor string to Output and sums the selected utxos amount to inAmount
const selectionResult = selectUtxos(config)({
utxos: [
{
descriptor: anchorDescriptor,
value: transaction.outs[anchorOutputIndex]!.value,
txHex: transaction.toHex(),
vout: anchorOutputIndex,
utxo: `${transaction.getId()}:${anchorOutputIndex}`,
forceSelection: true,
},
...spendableUtxos,
],
targets: [
{
descriptor: dummyDescriptor('wpkh'),
value: 1000,
},
],
remainderDescriptor: cpfpChangeDescriptor,
feeRate: actualFeeRate,
})
const cpfpChangeOutput = new Output({
// For now, we send back to the anchor wallet change. Can be changed.
descriptor: cpfpChangeDescriptor,
network: config.network,
})
const psbt = new Psbt({ network: config.network })
const finalizers: Array<
ReturnType<InstanceType<typeof Output>['updatePsbtAsInput']>
> = []
for (const utxo of selectionResult.utxos) {
finalizers.push(
utxo.output.updatePsbtAsInput({
psbt,
txHex: utxo.txHex,
vout: utxo.vout,
})
)
}
cpfpChangeOutput.updatePsbtAsOutput({
psbt,
value:
selectionResult.inAmount -
selectionResult.fee +
// We calculated the fee for two outputs but we keep only a single change so we need to add back the fee for one output
(actualFeeRate * cpfpChangeOutput.outputWeight()) / 4,
}) |
This point is exactly what I was trying to address in my last message. I spent quite some time re-reading your initial issue, reviewing your PR, and conceptualizing a possible implementation for CPFP that might suit your needs. Imagine you need to unstick a previous transaction that was a large multi-sig or had a very large witness. For instance, a parent transaction of 4000 bytes stuck at 10 sats/b, and you need to bump it to 100 sats/b. According to your approach, you would choose your utxos based on this fee rate: In your particular use case, your proposal might work because the sizes of the parent and child transactions are similar. However, when implementing a feature like this, it's essential to consider a broader range of scenarios, ensuring that all cases are adequately covered. In essence, for creating a CPFP tx it's necessary to consider the parent transaction's size and fee for each utxo you need to unstick, ensuring it's done once per parent transaction (not per utxo). Hence, a solution like the one I proposed might be more appropriate.
I still believe that a properly implemented CPFP requires a more focused approach, involving more flags or optional pareams in the coinselect algorithms beyond My concern is that as more use cases arise, we might end up adding an increasing number of flags and optional parameters to each utxo, leading to a complex codebase filled with if-else statements. Please take some time to carefully review my proposed solution and assess whether it meets your needs. I'm open to the possibility that I might have missed some aspects, and I'm more than willing to discuss this further if you'd like. |
Hey! Sorry for the long delay, I was working on something else and kinda forgot to answer. You're right, my solution seems to be working in my specific case but really is not generic enough. Thanks for the explanation. If you think that a separate method for CPFP makes sense, I can give a shot at implementing it based on what you proposed. |
I added a
forceSelection
property to theutxos
argument ofcoinselect
and the algorithmsaddUntilReach
andavoidChange
. This can be useful if we want to use some base UTXOs and if necessary complete with a selection (e.g. when spending an anchor output). The changes aren't significant, we just need to check before the loop if the forced UTXOs already cover the targets value + fee, in that case we return directly. That duplicates a bit of code so let me know if there is a better way.Tests are passing, I added some to cover this new behavior.
Another thing that could be useful in the case of an anchor output spending scenario is to be able to have an empty targets array. In that case, it should just select UTXOs to pay for the fees without any precise targets value. This may be in a follow-up PR.