Skip to content
This repository has been archived by the owner on Jul 15, 2022. It is now read-only.

LL-8531 Fix Bitcoin RBF logic #1826

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,7 @@ describe("testing estimateMaxSpendable", () => {

it("should estimate max spendable correctly", async () => {
await wallet.syncAccount(account);
let maxSpendable = await wallet.estimateAccountMaxSpendable(
account,
0,
[],
true
);
let maxSpendable = await wallet.estimateAccountMaxSpendable(account, 0, []);
const balance = 109088;
expect(maxSpendable.toNumber()).toEqual(balance);
const maxSpendableExcludeUtxo = await wallet.estimateAccountMaxSpendable(
Expand All @@ -43,16 +38,14 @@ describe("testing estimateMaxSpendable", () => {
hash: "f80246be50064bb254d2cad82fb0d4ce7768582b99c113694e72411f8032fd7a",
outputIndex: 0,
},
],
true
]
);
expect(maxSpendableExcludeUtxo.toNumber()).toEqual(balance - 1000);
let feesPerByte = 100;
maxSpendable = await wallet.estimateAccountMaxSpendable(
account,
feesPerByte,
[],
true
[]
);
expect(maxSpendable.toNumber()).toEqual(
balance -
Expand All @@ -69,8 +62,7 @@ describe("testing estimateMaxSpendable", () => {
maxSpendable = await wallet.estimateAccountMaxSpendable(
account,
feesPerByte,
[],
true
[]
);
expect(maxSpendable.toNumber()).toEqual(0);
}, 60000);
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/bridge/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const createTransaction = (): Transaction => ({
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
});
Expand Down
6 changes: 3 additions & 3 deletions src/families/bitcoin/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ const getCacheKeyForCalculateFees = ({
}) =>
`${a.id}_${a.blockHeight || 0}_${t.amount.toString()}_${String(
t.useAllAmount
)}_${t.recipient}_${t.feePerByte ? t.feePerByte.toString() : ""}_${
t.utxoStrategy.pickUnconfirmedRBF ? 1 : 0
}_${t.utxoStrategy.strategy}_${String(t.rbf)}_${t.utxoStrategy.excludeUTXOs
)}_${t.recipient}_${t.feePerByte ? t.feePerByte.toString() : ""}_${0}_${
t.utxoStrategy.strategy
}_${String(t.rbf)}_${t.utxoStrategy.excludeUTXOs
.map(({ hash, outputIndex }) => `${hash}@${outputIndex}`)
.join("+")}`;

Expand Down
6 changes: 0 additions & 6 deletions src/families/bitcoin/cli-transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ const options = [
type: String,
desc: "how much fee per byte",
},
{
name: "pickUnconfirmedRBF",
type: Boolean,
desc: "also pick unconfirmed replaceable txs",
},
{
name: "excludeUTXO",
alias: "E",
Expand Down Expand Up @@ -52,7 +47,6 @@ function inferTransactions(
rbf: opts.rbf || false,
utxoStrategy: {
strategy: bitcoinPickingStrategy[opts["bitcoin-pick-strategy"]] || 0,
pickUnconfirmedRBF: opts.pickUnconfirmedRBF || false,
excludeUTXOs: (opts.excludeUTXO || []).map((str) => {
const [hash, index] = str.split("@");
invariant(
Expand Down
3 changes: 0 additions & 3 deletions src/families/bitcoin/datasets/bitcoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -73,7 +72,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -99,7 +97,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand Down
3 changes: 0 additions & 3 deletions src/families/bitcoin/datasets/digibyte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -69,7 +68,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -92,7 +90,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand Down
3 changes: 0 additions & 3 deletions src/families/bitcoin/datasets/litecoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -70,7 +69,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand All @@ -93,7 +91,6 @@ const dataset: CurrenciesData<Transaction> = {
rbf: false,
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
}),
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/js-buildTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export const buildTransaction = async (
walletAccount,
transaction.feePerByte.toNumber(), //!\ wallet-btc handles fees as JS number
transaction.utxoStrategy.excludeUTXOs,
transaction.utxoStrategy.pickUnconfirmedRBF,
[transaction.recipient]
);
log("btcwallet", "building transaction", transaction);
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/js-createTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const createTransaction = (): Transaction => {
amount: new BigNumber(0),
utxoStrategy: {
strategy: 0,
pickUnconfirmedRBF: false,
excludeUTXOs: [],
},
recipient: "",
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/js-estimateMaxSpendable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const estimateMaxSpendable = async ({
walletAccount,
feePerByte.toNumber(), //!\ wallet-btc handles fees as JS number
transaction?.utxoStrategy?.excludeUTXOs || [],
transaction?.utxoStrategy?.pickUnconfirmedRBF || false,
transaction ? [transaction.recipient] : []
);
return maxSpendable.lt(0) ? new BigNumber(0) : maxSpendable;
Expand Down
9 changes: 6 additions & 3 deletions src/families/bitcoin/js-synchronisation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,18 @@ const toWalletNetwork = (currencyId: string): "testnet" | "mainnet" => {
};

// Map wallet-btc's Output to LL's BitcoinOutput
const fromWalletUtxo = (utxo: WalletOutput): BitcoinOutput => {
const fromWalletUtxo = (
utxo: WalletOutput,
changeAddresses: Set<string>
): BitcoinOutput => {
return {
hash: utxo.output_hash,
outputIndex: utxo.output_index,
blockHeight: utxo.block_height,
address: utxo.address,
value: new BigNumber(utxo.value),
rbf: utxo.rbf,
isChange: false, // wallet-btc limitation: doesn't provide it
isChange: changeAddresses.has(utxo.address),
};
};

Expand Down Expand Up @@ -349,7 +352,7 @@ const getAccountShape: GetAccountShape = async (info) => {
const newUniqueOperations = deduplicateOperations(newOperations);
const operations = mergeOps(oldOperations, newUniqueOperations);
const rawUtxos = await wallet.getAccountUnspentUtxos(walletAccount);
const utxos = rawUtxos.map(fromWalletUtxo);
const utxos = rawUtxos.map((utxo) => fromWalletUtxo(utxo, changeAddresses));
return {
id: accountId,
xpub,
Expand Down
13 changes: 4 additions & 9 deletions src/families/bitcoin/logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const isValidRecipient = async (params: {
type UTXOStatus =
| {
excluded: true;
reason: "pickUnconfirmedRBF" | "pickPendingNonRBF" | "userExclusion";
reason: "pickPendingUtxo" | "userExclusion";
}
| {
excluded: false;
Expand All @@ -77,16 +77,11 @@ export const getUTXOStatus = (
utxo: BitcoinOutput,
utxoStrategy: UtxoStrategy
): UTXOStatus => {
if (!utxoStrategy.pickUnconfirmedRBF && utxo.rbf && !utxo.blockHeight) {
if (!utxo.blockHeight && !utxo.isChange) {
// exclude pending and not change utxo
return {
excluded: true,
reason: "pickUnconfirmedRBF",
};
}
if (!utxo.rbf && !utxo.blockHeight) {
return {
excluded: true,
reason: "pickPendingNonRBF",
reason: "pickPendingUtxo",
};
}
if (
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ const bitcoinLikeMutations = ({
{
utxoStrategy: {
...transaction.utxoStrategy,
pickUnconfirmedRBF: true,
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions src/families/bitcoin/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const formatNetworkInfo = (

export const formatTransaction = (t: Transaction, account: Account): string => {
const n = getEnv("DEBUG_UTXO_DISPLAY");
const { excludeUTXOs, strategy, pickUnconfirmedRBF } = t.utxoStrategy;
const { excludeUTXOs, strategy } = t.utxoStrategy;
const displayAll = excludeUTXOs.length <= n;
return `
SEND ${
Expand All @@ -99,7 +99,7 @@ ${[
Object.keys(bitcoinPickingStrategy).find(
(k) => bitcoinPickingStrategy[k] === strategy
),
pickUnconfirmedRBF && "pick-unconfirmed",
"pick-unconfirmed",
t.rbf && "RBF-enabled",
]
.filter(Boolean)
Expand Down
1 change: 0 additions & 1 deletion src/families/bitcoin/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ export type BitcoinPickingStrategy =
typeof bitcoinPickingStrategy[keyof typeof bitcoinPickingStrategy];
export type UtxoStrategy = {
strategy: BitcoinPickingStrategy;
pickUnconfirmedRBF: boolean;
excludeUTXOs: Array<{
hash: string;
outputIndex: number;
Expand Down
10 changes: 8 additions & 2 deletions src/families/bitcoin/wallet-btc/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ class BitcoinLikeWallet {
account: Account,
feePerByte: number,
excludeUTXOs: Array<{ hash: string; outputIndex: number }>,
pickUnconfirmedRBF: boolean,
outputAddresses: string[] = []
) {
const addresses = await account.xpub.getXpubAddresses();
const changeAddresses = (await account.xpub.getAccountAddresses(1)).map(
(item) => item.address
);
const utxos = flatten(
await Promise.all(
addresses.map((address) =>
Expand All @@ -144,7 +146,11 @@ class BitcoinLikeWallet {
excludeUtxo.outputIndex === utxo.output_index
)
) {
if ((pickUnconfirmedRBF && utxo.rbf) || utxo.block_height !== null) {
// we can use either non pending utxo or change utxo
if (
changeAddresses.includes(utxo.address) ||
utxo.block_height !== null
) {
usableUtxoCount++;
balance = balance.plus(utxo.value);
}
Expand Down