Skip to content

Commit

Permalink
fix: fee estimation error messages (#1479)
Browse files Browse the repository at this point in the history
- Closes #1457

---

| 📷  Demo |
| --- |
| <img width="462" alt="Screenshot 2024-09-17 at 11 46 36"
src="https://github.com/user-attachments/assets/c3ee6819-bfae-473b-840d-855f0a176101">
|
  • Loading branch information
helciofranco authored Sep 18, 2024
1 parent 9506c6e commit 14e6385
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 143 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilly-planets-hide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fuels-wallet": minor
---

Improve how error messages are displayed/parsed during fee estimation.
5 changes: 5 additions & 0 deletions .changeset/giant-horses-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fuels-wallet": minor
---

Display fees options even when there are tx simulation errors.
5 changes: 5 additions & 0 deletions .changeset/swift-bugs-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fuels-wallet": patch
---

Allow dApps to pass account owner with `0x` address.
8 changes: 7 additions & 1 deletion examples/cra-dapp/src/Connected.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ import {
} from '@fuels/react';

import { DEVNET_NETWORK_URL, TESTNET_NETWORK_URL, bn } from 'fuels';
import { useState } from 'react';
import './App.css';

export function Connected() {
const [loading, setLoading] = useState(false);

const { fuel } = useFuel();
const { disconnect } = useDisconnect();
const { wallet } = useWallet();
Expand Down Expand Up @@ -60,6 +63,7 @@ export function Connected() {
<button
type="button"
onClick={async () => {
setLoading(true);
const txn = await wallet?.createTransfer(
'0xed73857a06ba2a706700e4e69e59f63a012ae6663a54309043e8fdc690bed926',
bn(100),
Expand All @@ -74,10 +78,12 @@ export function Connected() {
console.log(result);
} catch (e) {
console.error(e);
} finally {
setLoading(false);
}
}}
>
Send transaction with default fees
{loading ? 'Loading...' : 'Send transaction with default fees'}
</button>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export class BackgroundService {
'accounts',
'connect',
'network',
'networks',
'disconnect',
'signMessage',
'sendTransaction',
Expand Down Expand Up @@ -302,13 +303,22 @@ export class BackgroundService {
);
}

const { address, provider, transaction } = input;

const popupService = await PopUpService.open(
origin,
Pages.requestTransaction(),
this.communicationProtocol
);

// We need to forward bech32 addresses to the popup, regardless if we receive a b256 here
// our database is storing fuel addresses
const bech32Address = Address.fromDynamicInput(address).toString();

const signedMessage = await popupService.sendTransaction({
...input,
address: bech32Address,
provider,
transaction,
origin,
title,
favIconUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ const selectors = {
errors(state: TransactionRequestState) {
if (!state.context.errors) return {};
const simulateTxErrors = state.context.errors?.simulateTxErrors;
const hasSimulateTxErrors = Boolean(
Object.keys(simulateTxErrors || {}).length
);
const hasSimulateTxErrors = Boolean(simulateTxErrors);
const txApproveError = state.context.errors?.txApproveError;
return { txApproveError, simulateTxErrors, hasSimulateTxErrors };
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,45 +23,15 @@ import {
import { TxFeeOptions } from '../TxFeeOptions/TxFeeOptions';

const ErrorHeader = ({ errors }: { errors?: GroupedErrors }) => {
const errorMessages = useMemo(() => {
const messages = [];
if (errors) {
if (errors.InsufficientInputAmount || errors.NotEnoughCoins) {
messages.push('Not enough funds');
}

// biome-ignore lint: will not be a large array
Object.keys(errors).forEach((key: string) => {
if (key === 'InsufficientInputAmount' || key === 'NotEnoughCoins') {
return;
}

let errorMessage = `${key}: `;
try {
errorMessage += JSON.stringify(errors[key]);
} catch (_) {
errorMessage += errors[key];
}
messages.push(errorMessage);
});
}

return messages;
}, [errors]);

return (
<Alert status="error" css={styles.alert} aria-label="Transaction Error">
<Alert.Description as="div">
{errorMessages.map((message) => (
<Box
key={message}
css={{
wordBreak: 'break-word',
}}
>
{message}
</Box>
))}
<Alert.Description
as="div"
css={{
wordBreak: 'break-word',
}}
>
{errors}
</Alert.Description>
</Alert>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Box } from '@fuel-ui/react';
import { Alert, Box } from '@fuel-ui/react';
import type { AssetData } from '@fuel-wallet/types';
import type { Operation, TransactionStatus } from 'fuels';
import type { Maybe } from '~/systems/Core';
Expand All @@ -18,6 +18,16 @@ export function TxOperations({
assets,
isLoading,
}: TxOperationsProps) {
if (operations?.length === 0) {
return (
<Alert status="error">
<Alert.Description>
No operations found in this transaction
</Alert.Description>
</Alert>
);
}

return (
<Box.Stack gap="$4">
{operations?.map((operation, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('TxApprove', () => {
shouldShowTxSimulated: true,
shouldShowTxExecuted: false,
shouldShowActions: true,
simulateTxErrors: mockTxResult,
simulateTxErrors: 'Unknown error',
txSummarySimulated: mockTxResult,
approveStatus: jest.fn().mockReturnValue(TransactionStatus.success),
handlers: {
Expand Down Expand Up @@ -168,15 +168,13 @@ describe('TxApprove', () => {
setup(
{
errors: {
simulateTxErrors: {
InsufficientInputAmount: true,
},
simulateTxErrors: 'Insufficient Input Amount',
},
},
{},
{ status: TxRequestStatus.failed, result: true }
);
expect(screen.getByText('Not enough funds')).toBeDefined();
expect(screen.getByText('Insufficient Input Amount')).toBeDefined();
});

it('does not show the approve button show actions is false', () => {
Expand Down
44 changes: 21 additions & 23 deletions packages/app/src/systems/Transaction/services/transaction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import type { TransactionRequest, WalletLocked } from 'fuels';
import {
Address,
type BN,
ErrorCode,
FuelError,
TransactionResponse,
TransactionStatus,
assembleTransactionSummary,
Expand All @@ -19,7 +21,7 @@ import { createProvider } from '@fuel-wallet/connections';
import { AccountService } from '~/systems/Account/services/account';
import { NetworkService } from '~/systems/Network/services/network';
import type { Transaction } from '../types';
import { getAbiMap, getGroupedErrors } from '../utils';
import { type GroupedErrors, getAbiMap, getErrorMessage } from '../utils';
import { getCurrentTips } from '../utils/fee';

export type TxInputs = {
Expand Down Expand Up @@ -206,16 +208,11 @@ export class TxService {
minGasLimit: customFee?.gasUsed,
txSummary: {
...txSummary,
// if customFee was chosen, we override the txSummary fee with the customFee
fee: feeAdaptedToSdkDiff,
gasUsed: txSummary.gasUsed,
// fee: customFee?.txCost?.maxFee || feeAdaptedToSdkDiff,
// gasUsed: customFee?.txCost?.gasUsed || txSummary.gasUsed,
},
};

// biome-ignore lint/suspicious/noExplicitAny: allow any
} catch (e: any) {
} catch (e) {
const { gasPerByte, gasPriceFactor, gasCosts, maxGasPerTx } =
provider.getGasConfig();
const consensusParameters = provider.getChain().consensusParameters;
Expand All @@ -228,9 +225,8 @@ export class TxService {
inputs: transaction.inputs,
});

const errorsToParse =
e.name === 'FuelError' ? [{ message: e.message }] : e.response?.errors;
const simulateTxErrors = getGroupedErrors(errorsToParse);
const simulateTxErrors: GroupedErrors =
e instanceof FuelError ? getErrorMessage(e) : 'Unknown error';

const gasPrice = await provider.getLatestGasPrice();
const baseAssetId = provider.getBaseAssetId();
Expand All @@ -251,9 +247,14 @@ export class TxService {
txSummary.isStatusFailure = true;
txSummary.status = TransactionStatus.failure;

// Fallback to the values from the transactionRequest
if ('gasLimit' in transactionRequest) {
txSummary.gasUsed = transactionRequest.gasLimit;
}

return {
baseFee: undefined,
minGasLimit: undefined,
baseFee: txSummary.fee.add(1),
minGasLimit: txSummary.gasUsed,
txSummary,
simulateTxErrors,
};
Expand Down Expand Up @@ -361,16 +362,10 @@ export class TxService {
} catch (e) {
attempts += 1;

// @TODO: Waiting to match with FuelError type and ErrorCode enum from "fuels"
// These types are not exported from "fuels" package, but they exists in the "@fuels-ts/errors"
if (
e instanceof Error &&
'toObject' in e &&
typeof e.toObject === 'function'
) {
const error: { code: string } = e.toObject();
if (e instanceof FuelError) {
const error = e.toObject();

if (error.code === 'gas-limit-too-low') {
if (error.code === ErrorCode.GAS_LIMIT_TOO_LOW) {
throw e;
}
}
Expand All @@ -386,7 +381,7 @@ export class TxService {
};
}

static async computeCustomFee({
private static async computeCustomFee({
wallet,
transactionRequest,
}: TxInputs['computeCustomFee']) {
Expand All @@ -399,7 +394,10 @@ export class TxService {

// funding the transaction with the required quantities (the maxFee might have changed)
await wallet.fund(transactionRequest, {
...txCost,
estimatedPredicates: txCost.estimatedPredicates,
addedSignatures: txCost.addedSignatures,
gasPrice: txCost.gasPrice,
updateMaxFee: txCost.updateMaxFee,
requiredQuantities: [],
});

Expand Down
Loading

0 comments on commit 14e6385

Please sign in to comment.