feat(btc): support full RBF

ShookLyngs commented 4 months ago

Changes

Add sendRbf() and createSendRbfBuilder() APIs to support Full-RBF of BTC transactions
Add excludeUtxos, skipInputsValidation options in the sendUtxos() API to support the RBF feature
Add onlyProvableUtxos option in the sendRgbppUtxos() API for future update supports
Add changeIndex in the return type of the BTC Builder APIs

Related issues

Resolves #82
Resolves #94

Reference

https://bitcoinops.org/en/topics/replace-by-fee/

New APIs

The sendRbf() and createSendRbfBuilder() APIs are design for constructing BTC Full-RBF transactions.

interface SendRbfProps {
  from: string;
  txHex: string;
  source: DataSource;
  feeRate?: number;
  fromPubkey?: string;
  changeIndex?: number;
  changeAddress?: string;
  minUtxoSatoshi?: number;
  onlyConfirmedUtxos?: boolean;
  requireValidOutputsValue?: boolean;
  requireGreaterFeeAndRate?: boolean;

  // EXPERIMENTAL: the below props are unstable and can be altered at any time
  inputsPubkey?: Record<string, string>; // Record<address, pubkey>
}

declare function createSendRbfBuilder(props: SendRbfProps): Promise<{
  builder: TxBuilder;
  fee: number;
  feeRate: number;
  changeIndex: number;
}>;

declare function sendRbf(props: SendRbfProps): Promise<bitcoin.Psbt>;

txHex

The hex of the original transaction.

changeIndex

Optional, default to undefined.

By default, all the inputs and outputs from the original transaction remains the same in the RBF transaction, but if you wish to charge fee from or return change satoshi to the change output of the original transaction, you can specify the option.

requireValidOutputsValue

Optional, default to false.

Require the value of each output in the RBF transaction to be greater or equals to minUtxoSatoshi.

requireGreaterFeeAndRate

Optional, default to false.

Require the fee rate and the fee amount of the RBF transaction to be greater than the original transaction.

inputsPubkey

[EXPERIMENTAL] Optional, default to undefined.

A Map of addresses and public keys. It's useful when the original transaction contains multi-origin P2TR inputs. This option is currently unstable and can be updated, also note that in you won't need it most cases.

New options

skipInputsValidation

[EXPERIMENTAL] Optional, default to undefined in sendUtxos(), default to true in sendRbf().

Previously, when the onlyConfirmedUtxos is true, all inputs will be validated and an error will be thrown if any input is unconfirmed. With the skipInputsValidation option, you can skip validation of the props.inputs, while still apply the onlyConfirmedUtxos rule to the satoshi collector.

Which means if onlyConfirmedUtxos = true and skipInputsValidation = true:

UTXOs passed from the props.inputs option can be unconfirmed UTXOs
Only confirmed UTXOs will be collected during the pay fee process

In the sendRbf() API, this option is always set to true, allowing transaction chains to be RBF.

excludeUtxos

Optional, default to undefined.

This option is a list of UTXO[] to be excluded from the satoshi collection process when paying fee.

In the sendRbf() API, this option is used to exclude the outputs from the original transaction, preventing errors related to circular references (the RBF may uses outputs of the original transaction as the inputs if they're not excluded).

onlyProvableUtxos

[EXPERIMENTAL] Optional, default to true.

In the sendRgbppUtxos() API, this option is set to true by default, and it checks if each input's address is the same as the from address. If set to false, the address of the inputs can be different from the from address.

New return type

changeIndex

All BTC Builder APIs now return a new property changeIndex, which indicates the specific output in the generated transaction to be the change output. Note that the value of the changeIndex can be -1, meaning the transaction has no change output.

duanyytop commented 4 months ago

This draft PR doesn't look ready yet

ShookLyngs commented 4 months ago

This draft PR doesn't look ready yet

Yes, the PR is still in draft, and it's currently for API interface preview only. Some props can be modified during further tests, but the shape of it is relatively clear now.

Dawn-githup commented 3 months ago

Please help review 'full RBF' test analysis. https://www.notion.so/full-RBF-Test-Case-f198159a01f045748605b9249bc0c649 @ShookLyngs @Flouse @duanyytop

ShookLyngs commented 3 months ago

The PR is now ready for review/test, please update your status here @Dawn-githup, thanks.

Dawn-githup commented 3 months ago

todo list

Normal scenario:

[x] No introduction of new input to increase handling fees
[ ] Add new input and increase handling fee
[x] Specify the change output index

Abnormal scenario:

[x] Change output cannot be changed
[x] Rates are consistent
[ ] Unable to pay more fees
[x] Verify that there are no redundant inputs
[ ] Verify replacement transaction does not have paymaster
[x] Retry on failure

Boundary value scenario:

[x] Minimum rate and minimum amount
[x] Maximum rate and maximum amount
[x] Add feeRate bounds test

ShookLyngs commented 3 months ago

If we have an unconfirmed transaction chain:

TX_A: inputs=[O1], outputs=[O2], status=unconfirmed
TX_B: inputs=[O2], outputs=[O3], status=unconfirmed

And if we wish to construct an RBF transaction of the TX_A, let's call it TX_AR. When constructing the TX_AR, we should exclude O2 and O3 from being included in the inputs, preventing circular reference error since our goal is to replace TX_A and deduct its children (TX_B) from the blockchain. Kinda like the grandfather paradox.

Here's the question: how do we know that O3 should be excluded?

To address it, we may need to use this kind of API to help identify which transaction should be excluded in the RBF: https://mempool.space/docs/api/rest#get-transaction-outspends

The basic logic:

Given a transaction, where txid=A
Search the outspent status of A, and if any results found, record the txid of the results
Get transactions based on the above results, and and then repeat step 2 for all mentioned transactions
In the end, all recorded txid should be excluded when constructing an RBF transaction of A, avoiding circular referencing

What's more, things can get much more complicated if we're constructing an RBF of an RBF of a transaction. Another solution is to expose the excludeUtxos: BaseOutput[] option and leave it to the users. But obviously it's not a user-friendly solution.

On second thought, maybe just set onlyConfirmedUtxos=true and skipInputsValidation=true when construting RBF transactions, allowing the original inputs to be unconfirmed UTXOs, while only collecting confirmed UTXOs for paying fee.

Flouse commented 3 months ago

txHex

The hex of the original transaction.

changeIndex

Optional, default to undefined.

By default, all the inputs and outputs from the original transaction remains the same in the RBF transaction, but if you wish to charge fee from or return change satoshi to the change output of the original transaction, you can specify the option.

requireValidOutputsValue

Optional, default to false.

Require the value of each output in the RBF transaction to be greater or equals to minUtxoSatoshi.

requireGreaterFeeAndRate

Optional, default to false.

Require the fee rate and the fee amount of the RBF transaction to be greater than the original transaction.

inputsPubkey

[EXPERIMENTAL] Optional, default to undefined.

A Map of addresses and public keys. It's useful when the original transaction contains multi-origin P2TR inputs. This option is currently unstable and can be updated, also note that in you won't need it most cases.

What about adding these descriptions to the README or code comments?

Flouse commented 3 months ago

New options

skipInputsValidation

Previously, when the onlyConfirmedUtxos is true, all inputs will be validated and an error will be thrown if any input is unconfirmed. With the skipInputsValidation option, you can skip validation of the props.inputs, while still apply the onlyConfirmedUtxos rule to the satoshi collector.

Which means if onlyConfirmedUtxos = true and skipInputsValidation = true:

UTXOs passed from the props.inputs option can be unconfirmed UTXOs

Only confirmed UTXOs will be collected during the pay fee process

In the sendRbf() API, this option is always set to true, allowing transaction chains to be RBF.

excludeUtxos

This option is a list of UTXO[] to be excluded from the satoshi collection process when paying fee.

In the sendRbf() API, this option is used to exclude the outputs from the original transaction, preventing errors related to circular references (the RBF may uses outputs of the original transaction as the inputs if they're not excluded).

New return type

changeIndex

All BTC Builder APIs now return a new property changeIndex, which indicates the specific output in the generated transaction to be the change output. Note that the value of the changeIndex can be -1, meaning the transaction has no change output.

Same recommendation as above

ShookLyngs commented 3 months ago

What about adding these descriptions to the README or code comments?

I was thinking of maybe creating Markdown docs for each API, like this: packages/btc/docs/send-rbf.md. Adding comments to the props interface may also work; I'll try it in another PR.

ShookLyngs commented 2 months ago

Conflicts have been resolved, and the history commits are squashed into 8ce3593.

Additionally, all changes are documented in the changeset: https://github.com/ckb-cell/rgbpp-sdk/blob/08200c974ef336661723cc7556a003932babda9a/.changeset/spicy-cups-cheat.md

ckb-cell / rgbpp-sdk