Skip to content
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

fix: tx calc unhandled errors, closes #4941 #5011

Merged
merged 1 commit into from
Mar 6, 2024
Merged
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 @@ -11,6 +11,12 @@ export interface DetermineUtxosForSpendArgs {
utxos: UtxoResponseItem[];
}

export class InsufficientFundsError extends Error {
constructor() {
super('Insufficient funds');
}
}

export function determineUtxosForSpendAll({
amount,
feeRate,
Expand Down Expand Up @@ -71,7 +77,7 @@ export function determineUtxosForSpend({
neededUtxos.push(utxo);
}

if (!sizeInfo) throw new Error('Transaction size must be defined');
if (!sizeInfo) throw new InsufficientFundsError();

const fee = Math.ceil(sizeInfo.txVBytes * feeRate);

Expand Down
41 changes: 5 additions & 36 deletions src/app/components/bitcoin-custom-fee/bitcoin-custom-fee-fiat.tsx
Original file line number Diff line number Diff line change
@@ -1,49 +1,18 @@
import { useMemo } from 'react';

import { useField } from 'formik';
import { Flex, styled } from 'leather-styles/jsx';

import { createMoney } from '@shared/models/money.model';

import { satToBtc } from '@app/common/money/unit-conversion';

import { useBitcoinCustomFee } from './hooks/use-bitcoin-custom-fee';

interface BitcoinCustomFeeFiatProps {
amount: number;
isSendingMax: boolean;
recipient: string;
feeInBtc: string;
fiatFeeValue: string;
}

export function BitcoinCustomFeeFiat({
amount,
isSendingMax,
recipient,
}: BitcoinCustomFeeFiatProps) {
const [field] = useField('feeRate');
const getCustomFeeValues = useBitcoinCustomFee({
amount: createMoney(amount, 'BTC'),
isSendingMax,
recipient,
});

const feeData = useMemo(() => {
const { fee, fiatFeeValue } = getCustomFeeValues(Number(field.value));
const feeInBtc = satToBtc(fee).toString();

return { fiatFeeValue, feeInBtc };
}, [getCustomFeeValues, field.value]);

const canShow = !feeData.feeInBtc.includes('e') && Number(field.value) > 0;
if (!canShow) return null;

export function BitcoinCustomFeeFiat({ feeInBtc, fiatFeeValue }: BitcoinCustomFeeFiatProps) {
return (
<Flex justifyContent="space-between">
<styled.span color="ink.text-subdued" textStyle="body.02">
{feeData.fiatFeeValue}
{fiatFeeValue}
</styled.span>
<styled.span color="ink.text-subdued" textStyle="body.02">
{feeData.feeInBtc} BTC
{feeInBtc} BTC
</styled.span>
</Flex>
);
Expand Down
110 changes: 110 additions & 0 deletions src/app/components/bitcoin-custom-fee/bitcoin-custom-fee-input.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { useState } from 'react';

import { useField } from 'formik';
import { Stack } from 'leather-styles/jsx';

import { createMoney } from '@shared/models/money.model';

import { useOnMount } from '@app/common/hooks/use-on-mount';
import { satToBtc } from '@app/common/money/unit-conversion';
import { InsufficientFundsError } from '@app/common/transactions/bitcoin/coinselect/local-coin-selection';
import { Input } from '@app/ui/components/input/input';

import { ErrorLabel } from '../error-label';
import { BitcoinCustomFeeFiat } from './bitcoin-custom-fee-fiat';
import { useBitcoinCustomFee } from './hooks/use-bitcoin-custom-fee';

interface Props {
onClick?(): void;
amount: number;
isSendingMax: boolean;
recipient: string;
hasInsufficientBalanceError: boolean;
errorMessage?: string;
setCustomFeeInitialValue?(value: string): void;
customFeeInitialValue: string;
}

const feeInputLabel = 'sats/vB';

export function BitcoinCustomFeeInput({
onClick,
amount,
isSendingMax,
recipient,
hasInsufficientBalanceError,
setCustomFeeInitialValue,
customFeeInitialValue,
}: Props) {
const [field] = useField('feeRate');

const [feeValue, setFeeValue] = useState<null | {
fee: number;
fiatFeeValue: string;
}>(null);

const getCustomFeeValues = useBitcoinCustomFee({
amount: createMoney(amount, 'BTC'),
isSendingMax,
recipient,
});
const [unknownError, setUnknownError] = useState(false);
const [customInsufficientBalanceError, setCustomInsufficientBalanceError] = useState(false);

const hasError = hasInsufficientBalanceError || unknownError || customInsufficientBalanceError;
const errorMessage =
hasInsufficientBalanceError || customInsufficientBalanceError
? 'Insufficient funds'
: 'Unknown error';

function processFeeValue(feeRate: string) {
try {
const feeValues = getCustomFeeValues(Number(feeRate));
setFeeValue(feeValues);

setUnknownError(false);
setCustomInsufficientBalanceError(false);
} catch (err) {
if (err instanceof InsufficientFundsError) {
return setCustomInsufficientBalanceError(true);
}

setUnknownError(true);
}
}

function onChange(e: React.ChangeEvent<HTMLInputElement>) {
const value = e.target.value;
setCustomFeeInitialValue?.(e.target.value);
processFeeValue(value);
}

useOnMount(() => {
processFeeValue(customFeeInitialValue);
});
Comment on lines +60 to +84
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Processing the fee on mount, then setting the values, like this seems kinda side-effecty. Okay for now as we're going to do fee work and refactor entirely, but I'm sure there's a nicer way to do this next time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I realise it could have been done way better

return (
<Stack gap="space.05">
<Stack>
<Input.Root hasError={hasError}>
<Input.Label>{feeInputLabel}</Input.Label>
<Input.Field
onClick={onClick}
{...field}
onChange={e => {
field.onChange(e);
onChange?.(e);
}}
/>
</Input.Root>
{hasError && <ErrorLabel>{errorMessage}</ErrorLabel>}
</Stack>

{!hasError && feeValue && (
<BitcoinCustomFeeFiat
feeInBtc={satToBtc(feeValue.fee).toString()}
fiatFeeValue={feeValue.fiatFeeValue}
/>
)}
</Stack>
);
}
51 changes: 10 additions & 41 deletions src/app/components/bitcoin-custom-fee/bitcoin-custom-fee.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Dispatch, SetStateAction, useCallback, useRef } from 'react';

import { Form, Formik, useField } from 'formik';
import { Form, Formik } from 'formik';
import { Stack, styled } from 'leather-styles/jsx';
import * as yup from 'yup';

Expand All @@ -9,41 +9,12 @@ import { createMoney } from '@shared/models/money.model';

import { openInNewTab } from '@app/common/utils/open-in-new-tab';
import { PreviewButton } from '@app/components/preview-button';
import { Input } from '@app/ui/components/input/input';
import { Link } from '@app/ui/components/link/link';

import { OnChooseFeeArgs } from '../bitcoin-fees-list/bitcoin-fees-list';
import { BitcoinCustomFeeFiat } from './bitcoin-custom-fee-fiat';
import { BitcoinCustomFeeInput } from './bitcoin-custom-fee-input';
import { useBitcoinCustomFee } from './hooks/use-bitcoin-custom-fee';

const feeInputLabel = 'sats/vB';

interface BitcoinCustomFeeInputProps {
hasInsufficientBalanceError: boolean;
onClick(): void;
onChange?(e: React.ChangeEvent<HTMLInputElement>): void;
}
function BitcoinCustomFeeInput({
hasInsufficientBalanceError,
onClick,
onChange,
}: BitcoinCustomFeeInputProps) {
const [field] = useField('feeRate');
return (
<Input.Root hasError={hasInsufficientBalanceError}>
<Input.Label>{feeInputLabel}</Input.Label>
<Input.Field
onClick={onClick}
{...field}
onChange={e => {
field.onChange(e);
onChange?.(e);
}}
/>
</Input.Root>
);
}

interface BitcoinCustomFeeProps {
amount: number;
customFeeInitialValue: string;
Expand All @@ -56,6 +27,7 @@ interface BitcoinCustomFeeProps {
setCustomFeeInitialValue: Dispatch<SetStateAction<string>>;
maxCustomFeeRate: number;
}

export function BitcoinCustomFee({
amount,
customFeeInitialValue,
Expand Down Expand Up @@ -123,20 +95,17 @@ export function BitcoinCustomFee({
</Link>
</styled.span>
<BitcoinCustomFeeInput
hasInsufficientBalanceError={hasInsufficientBalanceError}
amount={amount}
isSendingMax={isSendingMax}
onClick={async () => {
feeInputRef?.current?.focus();
feeInputRef.current?.focus();
await props.setValues({ ...props.values });
}}
onChange={e => setCustomFeeInitialValue((e.target as HTMLInputElement).value)}
customFeeInitialValue={customFeeInitialValue}
setCustomFeeInitialValue={setCustomFeeInitialValue}
recipient={recipient}
hasInsufficientBalanceError={hasInsufficientBalanceError}
/>
<Stack gap="space.01">
<BitcoinCustomFeeFiat
amount={amount}
isSendingMax={isSendingMax}
recipient={recipient}
/>
</Stack>
</Stack>
<PreviewButton isDisabled={!props.values.feeRate} text="Use custom fee" />
</Stack>
Expand Down
38 changes: 26 additions & 12 deletions src/app/components/bitcoin-fees-list/use-bitcoin-fees-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ function getFeeForList(
determineUtxosForFeeArgs: DetermineUtxosForSpendArgs,
isSendingMax?: boolean
) {
const { fee } = isSendingMax
? determineUtxosForSpendAll(determineUtxosForFeeArgs)
: determineUtxosForSpend(determineUtxosForFeeArgs);
return fee;
try {
const { fee } = isSendingMax
? determineUtxosForSpendAll(determineUtxosForFeeArgs)
: determineUtxosForSpend(determineUtxosForFeeArgs);
return fee;
} catch (error) {
return null;
}
}

interface UseBitcoinFeesListArgs {
Expand Down Expand Up @@ -75,36 +79,46 @@ export function useBitcoinFeesList({
feeRate: feeRates.hourFee.toNumber(),
};

const feesArr = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can spread values into this array instead?

    ...(highFeeValue ? [{
        label: BtcFeeType.High,
        value: highFeeValue,
        btcValue: formatMoneyPadded(createMoney(highFeeValue, 'BTC')),
        time: btcTxTimeMap.fastestFee,
        fiatValue: getFiatFeeValue(highFeeValue),
        feeRate: feeRates.fastestFee.toNumber(),
      }] : [])
  ]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it so to improve readability. don't really like such constructions even if they save several lines of code


const highFeeValue = getFeeForList(determineUtxosForHighFeeArgs, isSendingMax);
const standardFeeValue = getFeeForList(determineUtxosForStandardFeeArgs, isSendingMax);
const lowFeeValue = getFeeForList(determineUtxosForLowFeeArgs, isSendingMax);

return [
{
if (highFeeValue) {
feesArr.push({
label: BtcFeeType.High,
value: highFeeValue,
btcValue: formatMoneyPadded(createMoney(highFeeValue, 'BTC')),
time: btcTxTimeMap.fastestFee,
fiatValue: getFiatFeeValue(highFeeValue),
feeRate: feeRates.fastestFee.toNumber(),
},
{
});
}

if (standardFeeValue) {
feesArr.push({
label: BtcFeeType.Standard,
value: standardFeeValue,
btcValue: formatMoneyPadded(createMoney(standardFeeValue, 'BTC')),
time: btcTxTimeMap.halfHourFee,
fiatValue: getFiatFeeValue(standardFeeValue),
feeRate: feeRates.halfHourFee.toNumber(),
},
{
});
}

if (lowFeeValue) {
feesArr.push({
label: BtcFeeType.Low,
value: lowFeeValue,
btcValue: formatMoneyPadded(createMoney(lowFeeValue, 'BTC')),
time: btcTxTimeMap.hourFee,
fiatValue: getFiatFeeValue(lowFeeValue),
feeRate: feeRates.hourFee.toNumber(),
},
];
});
}

return feesArr;
}, [feeRates, utxos, isSendingMax, balance.amount, amount.amount, recipient, btcMarketData]);

return {
Expand Down
Loading
Loading