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 4 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
7 changes: 3 additions & 4 deletions src/families/bitcoin/bridge/libcore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ const receive = makeAccountBridgeReceive({
const getCacheKey = (a, t) =>
`${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 All @@ -61,7 +61,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/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),
path: "",
};
};
Expand Down Expand Up @@ -350,7 +353,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
2 changes: 1 addition & 1 deletion src/families/bitcoin/libcore-buildTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { isValidRecipient } from "../../libcore/isValidRecipient";
import { bigNumberToLibcoreAmount } from "../../libcore/buildBigNumber";
import type { Core, CoreCurrency, CoreAccount } from "../../libcore/types";
import type { CoreBitcoinLikeTransaction, Transaction } from "./types";
import { getUTXOStatus } from "./transaction";
import { getUTXOStatus } from "./logic";
import { promiseAllBatched } from "../../promise";
import { parseBitcoinUTXO, perCoinLogic } from "./transaction";

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 @@ -79,7 +79,7 @@ export function isChangeOutput(output: BitcoinOutput): boolean {
type UTXOStatus =
| {
excluded: true;
reason: "pickUnconfirmedRBF" | "pickPendingNonRBF" | "userExclusion";
reason: "pickPendingUtxo" | "userExclusion";
}
| {
excluded: false;
Expand All @@ -88,16 +88,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
6 changes: 3 additions & 3 deletions src/families/bitcoin/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ const genericTest = ({
// verify that no utxo that was supposed to be exploded were used
expect(
utxosPicked.filter(
(u) => getUTXOStatus(u, transaction.utxoStrategy).excluded
(u: BitcoinOutput) =>
u.blockHeight && getUTXOStatus(u, transaction.utxoStrategy).excluded
)
).toEqual([]);
};
Expand Down Expand Up @@ -243,7 +244,6 @@ const bitcoinLikeMutations = ({
{
utxoStrategy: {
...transaction.utxoStrategy,
pickUnconfirmedRBF: true,
},
},
{
Expand All @@ -256,7 +256,7 @@ const bitcoinLikeMutations = ({
test: ({ account }) => {
expect(
account.bitcoinResources?.utxos
.filter((u) => u.blockHeight) // Exclude pending UTXOs
.filter((u) => u.blockHeight && u.blockHeight < account.blockHeight) // Exclude pending UTXOs and the Utxos just written into new block
.reduce((p, c) => p.plus(c.value), new BigNumber(0))
.toString()
).toBe("0");
Expand Down
39 changes: 2 additions & 37 deletions src/families/bitcoin/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type {
FeeItems,
FeeItemsRaw,
BitcoinOutput,
UtxoStrategy,
CoreBitcoinLikeOutput,
CoreBitcoinLikeInput,
BitcoinInput,
Expand Down Expand Up @@ -90,7 +89,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 @@ -109,7 +108,7 @@ ${[
Object.keys(bitcoinPickingStrategy).find(
(k) => bitcoinPickingStrategy[k] === strategy
),
pickUnconfirmedRBF && "pick-unconfirmed",
"pick-unconfirmed",
t.rbf && "RBF-enabled",
]
.filter(Boolean)
Expand Down Expand Up @@ -200,14 +199,6 @@ export const perCoinLogic: Record<
}),
},
};
export type UTXOStatus =
| {
excluded: true;
reason: "pickUnconfirmedRBF" | "userExclusion";
}
| {
excluded: false;
};
export async function parseBitcoinInput(
input: CoreBitcoinLikeInput
): Promise<BitcoinInput> {
Expand Down Expand Up @@ -267,32 +258,6 @@ export async function parseBitcoinUTXO(
utxo.rbf = await output.isReplaceable();
return utxo;
}
export function getUTXOStatus(
utxo: BitcoinOutput,
utxoStrategy: UtxoStrategy
): UTXOStatus {
if (!utxoStrategy.pickUnconfirmedRBF && utxo.rbf && !utxo.blockHeight) {
return {
excluded: true,
reason: "pickUnconfirmedRBF",
};
}

if (
utxoStrategy.excludeUTXOs.some(
(u) => u.hash === utxo.hash && u.outputIndex === utxo.outputIndex
)
) {
return {
excluded: true,
reason: "userExclusion",
};
}

return {
excluded: false,
};
}
export function isChangeOutput(output: BitcoinOutput): boolean {
if (!output.path) return false;
const p = output.path.split("/");
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 @@ -134,7 +134,6 @@ export type BitcoinPickingStrategy =
typeof bitcoinPickingStrategy[keyof typeof bitcoinPickingStrategy];
export type UtxoStrategy = {
strategy: BitcoinPickingStrategy;
pickUnconfirmedRBF: boolean;
excludeUTXOs: Array<{
hash: string;
outputIndex: number;
Expand Down
Loading