Skip to content

Commit

Permalink
[feat/LIVE-8381]: add 10s timeout on estimate gas calls for (#4987)
Browse files Browse the repository at this point in the history
* feat: add 10s timeout on estimate gas calls for

* chore: add changeset

* feat: add comment and num formatting

* feat: add unit test
  • Loading branch information
kallen-ledger authored Nov 13, 2023
1 parent 47dcce5 commit e63205b
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 4 deletions.
9 changes: 9 additions & 0 deletions .changeset/silver-lamps-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@ledgerhq/errors": patch
"ledger-live-desktop": patch
"live-mobile": patch
"@ledgerhq/live-common": patch
"@ledgerhq/coin-evm": patch
---

add 10s timeout to estimate gas
3 changes: 3 additions & 0 deletions apps/ledger-live-desktop/static/i18n/en/app.json
Original file line number Diff line number Diff line change
Expand Up @@ -5457,6 +5457,9 @@
"FeeNotLoaded": {
"title": "Couldn't load fee rates. Please set manual fees"
},
"FeeNotLoadedSwap": {
"title": "Network fee estimation failed because of technical issues. Try again or change the assets."
},
"FeeRequired": {
"title": "Fees are required"
},
Expand Down
3 changes: 3 additions & 0 deletions apps/ledger-live-mobile/src/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@
"FeeNotLoaded": {
"title": "Could not load fee rates"
},
"FeeNotLoadedSwap": {
"title": "Network fee estimation failed because of technical issues. Try again or change the assets."
},
"FeeRequired": {
"title": "Fees are required"
},
Expand Down
16 changes: 16 additions & 0 deletions libs/coin-evm/src/__tests__/unit/api/node/ledger.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,22 @@ describe("EVM Family", () => {
}
});

it("should throw GasEstimationError if request throws ECONNABORTED", async () => {
jest
.spyOn(axios, "request")
.mockImplementation(() => Promise.reject({ code: "ECONNABORTED" }));

try {
await LEDGER_API.getGasEstimation(account, {} as any);
fail("Promise should have been rejected");
} catch (e) {
if (e instanceof AssertionError) {
throw e;
}
expect(e).toBeInstanceOf(GasEstimationError);
}
});

it("should return the expected payload", async () => {
jest.spyOn(axios, "request").mockImplementation(async ({ data: transaction }) => {
return (transaction as any)?.data?.length > 2
Expand Down
6 changes: 4 additions & 2 deletions libs/coin-evm/src/api/node/ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { NodeApi, isLedgerNodeConfig } from "./types";
import { getGasOptions } from "../gasTracker/ledger";
import { padHexString } from "../../logic";

export const LEDGER_TIMEOUT = 200; // 200ms between 2 calls
export const LEDGER_TIMEOUT = 10_000; // 10_000ms (10s) for network call timeout
export const LEDGER_TIME_BETWEEN_TRIES = 200; // 200ms between 2 calls
export const DEFAULT_RETRIES_API = 2;

export async function fetchWithRetries<T>(
Expand All @@ -35,7 +36,7 @@ export async function fetchWithRetries<T>(
} catch (e) {
if (retries) {
// wait the API timeout before trying again
await delay(LEDGER_TIMEOUT);
await delay(LEDGER_TIME_BETWEEN_TRIES);
// decrement with prefix here or it won't work
return fetchWithRetries<T>(params, --retries);
}
Expand Down Expand Up @@ -185,6 +186,7 @@ export const getGasEstimation: NodeApi["getGasEstimation"] = async (
estimated_gas_limit: string;
}>({
method: "POST",
timeout: LEDGER_TIMEOUT,
url: `${getEnv("EXPLORER")}/blockchain/v4/${node.explorerId}/tx/estimate-gas-limit`,
data: {
from: account.freshAddress, // should be necessary for some estimations
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { AmountRequired, NotEnoughGas, NotEnoughGasSwap } from "@ledgerhq/errors";
import {
AmountRequired,
FeeNotLoaded,
FeeNotLoadedSwap,
NotEnoughGas,
NotEnoughGasSwap,
} from "@ledgerhq/errors";
import { useMemo } from "react";
import {
SwapSelectorStateType,
Expand Down Expand Up @@ -73,6 +79,11 @@ export const useFromAmountStatusMessage = (
});
}

// convert to swap variation of error to display correct message to frontend.
if (relevantStatus instanceof FeeNotLoaded) {
return new FeeNotLoadedSwap();
}

return relevantStatus;
}, [statusEntries, currency, estimatedFees, transaction?.amount, account?.id, parentAccount?.id]);
};
Expand Down Expand Up @@ -122,7 +133,7 @@ export const useSwapTransaction = ({

const { account: toAccount } = toState;

const fromAmountError = useFromAmountStatusMessage(bridgeTransaction, ["amount"]);
const fromAmountError = useFromAmountStatusMessage(bridgeTransaction, ["amount", "gasLimit"]);
// treat the gasPrice error as a warning for swap.
const fromAmountWarning = useFromAmountStatusMessage(bridgeTransaction, ["gasPrice"]);

Expand Down
1 change: 1 addition & 0 deletions libs/ledgerjs/packages/errors/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export const WrongAppForCurrency = createCustomErrorClass("WrongAppForCurrency")
export const ETHAddressNonEIP = createCustomErrorClass("ETHAddressNonEIP");
export const CantScanQRCode = createCustomErrorClass("CantScanQRCode");
export const FeeNotLoaded = createCustomErrorClass("FeeNotLoaded");
export const FeeNotLoadedSwap = createCustomErrorClass("FeeNotLoadedSwap");
export const FeeRequired = createCustomErrorClass("FeeRequired");
export const FeeTooHigh = createCustomErrorClass("FeeTooHigh");
export const PendingOperation = createCustomErrorClass("PendingOperation");
Expand Down

1 comment on commit e63205b

@vercel
Copy link

@vercel vercel bot commented on e63205b Nov 13, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.