From f2407dcc45e2f75ad43e419127fc73e6078d27f4 Mon Sep 17 00:00:00 2001 From: Daniel Bate Date: Wed, 8 Jan 2025 17:14:26 +0000 Subject: [PATCH] feat!: remove redundant gas price call for tx summary (#3559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: update chagneset * feat: remove redundant gas price call for tx summary * chore: fix cyclic dep and status tests * chore: is breaking * chore: breaking changeset * feat: pass down totalFee * Revert "feat: pass down totalFee" This reverts commit 36c61909dc4fd9c5b880f61d35c1fdf9b7e1bf55. * feat: remove totalFee from assemble tx summary * chore: lint * chore: mitigate test flakiness Co-authored-by: Nedim Salkić * chore: lint --------- Co-authored-by: Nedim Salkić --- .changeset/short-bears-fry.md | 5 ++ .../transaction-response.ts | 7 ++- .../assemble-transaction-summary.test.ts | 9 ++-- .../assemble-transaction-summary.ts | 27 +++++----- .../calculate-tx-fee-for-summary.ts | 10 ---- .../get-transaction-summary.ts | 7 ++- .../transaction-summary/status.test.ts | 19 ++++++- .../providers/transaction-summary/status.ts | 14 ++++- .../src/transaction-response.test.ts | 54 +++++++++++++++++++ 9 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 .changeset/short-bears-fry.md diff --git a/.changeset/short-bears-fry.md b/.changeset/short-bears-fry.md new file mode 100644 index 00000000000..5e6ea8d7f51 --- /dev/null +++ b/.changeset/short-bears-fry.md @@ -0,0 +1,5 @@ +--- +"@fuel-ts/account": minor +--- + +feat!: remove redundant gas price call for tx summary diff --git a/packages/account/src/providers/transaction-response/transaction-response.ts b/packages/account/src/providers/transaction-response/transaction-response.ts index 1b02a5cec38..64c106c672b 100644 --- a/packages/account/src/providers/transaction-response/transaction-response.ts +++ b/packages/account/src/providers/transaction-response/transaction-response.ts @@ -36,6 +36,7 @@ import type Provider from '../provider'; import type { JsonAbisFromAllCalls, TransactionRequest } from '../transaction-request'; import { assembleTransactionSummary } from '../transaction-summary/assemble-transaction-summary'; import { processGqlReceipt } from '../transaction-summary/receipt'; +import { getTotalFeeFromStatus } from '../transaction-summary/status'; import type { TransactionSummary, GqlTransaction, AbiMap } from '../transaction-summary/types'; import { extractTxError } from '../utils'; @@ -299,7 +300,11 @@ export class TransactionResponse { const { gasPerByte, gasPriceFactor, gasCosts, maxGasPerTx } = await this.provider.getGasConfig(); - const gasPrice = await this.provider.getLatestGasPrice(); + + // If we have the total fee, we do not need to refetch the gas price + const totalFee = getTotalFeeFromStatus(this.status ?? this.gqlTransaction?.status); + const gasPrice = totalFee ? bn(0) : await this.provider.getLatestGasPrice(); + const maxInputs = (await this.provider.getChain()).consensusParameters.txParameters.maxInputs; const baseAssetId = await this.provider.getBaseAssetId(); diff --git a/packages/account/src/providers/transaction-summary/assemble-transaction-summary.test.ts b/packages/account/src/providers/transaction-summary/assemble-transaction-summary.test.ts index ede6df68606..d4ebb2f27a4 100644 --- a/packages/account/src/providers/transaction-summary/assemble-transaction-summary.test.ts +++ b/packages/account/src/providers/transaction-summary/assemble-transaction-summary.test.ts @@ -56,7 +56,8 @@ describe('TransactionSummary', () => { const runTest = ( status: GraphqlTransactionStatus, expected: Record, - baseAssetId: string + baseAssetId: string, + calculateTransactionFeeCalls = 1 ) => { const { calculateTransactionFee } = mockCalculateTransactionFee(); @@ -77,7 +78,7 @@ describe('TransactionSummary', () => { }); expect(transactionSummary).toMatchObject(expected); - expect(calculateTransactionFee).toHaveBeenCalledTimes(1); + expect(calculateTransactionFee).toHaveBeenCalledTimes(calculateTransactionFeeCalls); }; it('should assemble transaction summary just fine (SUCCESS)', async () => { @@ -104,7 +105,7 @@ describe('TransactionSummary', () => { type: expect.any(String), }; - runTest(MOCK_SUCCESS_STATUS, expected, await provider.getBaseAssetId()); + runTest(MOCK_SUCCESS_STATUS, expected, await provider.getBaseAssetId(), 0); }); it('should assemble transaction summary just fine (FAILURE)', async () => { @@ -131,7 +132,7 @@ describe('TransactionSummary', () => { type: expect.any(String), }; - runTest(MOCK_FAILURE_STATUS, expected, await provider.getBaseAssetId()); + runTest(MOCK_FAILURE_STATUS, expected, await provider.getBaseAssetId(), 0); }); it('should assemble transaction summary just fine (SUBMITTED)', async () => { diff --git a/packages/account/src/providers/transaction-summary/assemble-transaction-summary.ts b/packages/account/src/providers/transaction-summary/assemble-transaction-summary.ts index a4c34921117..99785b4bd2a 100644 --- a/packages/account/src/providers/transaction-summary/assemble-transaction-summary.ts +++ b/packages/account/src/providers/transaction-summary/assemble-transaction-summary.ts @@ -79,20 +79,21 @@ export function assembleTransactionSummary( const { isStatusFailure, isStatusPending, isStatusSuccess, blockId, status, time, totalFee } = processGraphqlStatus(gqlTransactionStatus); - const fee = calculateTXFeeForSummary({ - totalFee, - gasPrice, - rawPayload, - tip, - consensusParameters: { - gasCosts, - maxGasPerTx, - feeParams: { - gasPerByte, - gasPriceFactor, + const fee = + totalFee ?? + calculateTXFeeForSummary({ + gasPrice, + rawPayload, + tip, + consensusParameters: { + gasCosts, + maxGasPerTx, + feeParams: { + gasPerByte, + gasPriceFactor, + }, }, - }, - }); + }); const mintedAssets = extractMintedAssetsFromReceipts(receipts); const burnedAssets = extractBurnedAssetsFromReceipts(receipts); diff --git a/packages/account/src/providers/transaction-summary/calculate-tx-fee-for-summary.ts b/packages/account/src/providers/transaction-summary/calculate-tx-fee-for-summary.ts index 8a8c6bb5cfe..1ec23f7b749 100644 --- a/packages/account/src/providers/transaction-summary/calculate-tx-fee-for-summary.ts +++ b/packages/account/src/providers/transaction-summary/calculate-tx-fee-for-summary.ts @@ -25,7 +25,6 @@ export type CalculateTXFeeForSummaryParams = { gasPrice: BN; rawPayload: string; tip: BN; - totalFee?: BN; consensusParameters: Pick & { feeParams: FeeParams; maxGasPerTx: BN; @@ -37,18 +36,9 @@ export const calculateTXFeeForSummary = (params: CalculateTXFeeForSummaryParams) gasPrice, rawPayload, tip, - totalFee, consensusParameters: { gasCosts, feeParams, maxGasPerTx }, } = params; - /** - * If totalFee is provided it means that the TX was already processed and we could extract the fee - * from its status - */ - if (totalFee) { - return totalFee; - } - const gasPerByte = bn(feeParams.gasPerByte); const gasPriceFactor = bn(feeParams.gasPriceFactor); diff --git a/packages/account/src/providers/transaction-summary/get-transaction-summary.ts b/packages/account/src/providers/transaction-summary/get-transaction-summary.ts index 7c8fa136757..a495f41dae8 100644 --- a/packages/account/src/providers/transaction-summary/get-transaction-summary.ts +++ b/packages/account/src/providers/transaction-summary/get-transaction-summary.ts @@ -15,8 +15,8 @@ import { validatePaginationArgs } from '../utils/validate-pagination-args'; import { assembleTransactionSummary } from './assemble-transaction-summary'; import { processGqlReceipt } from './receipt'; +import { getTotalFeeFromStatus } from './status'; import type { AbiMap, TransactionSummary } from './types'; - /** @hidden */ export interface GetTransactionSummaryParams { id: string; @@ -61,7 +61,10 @@ export async function getTransactionSummary( }, } = await provider.getChain(); - const gasPrice = await provider.getLatestGasPrice(); + // If we have the total fee, we do not need to refetch the gas price + const totalFee = getTotalFeeFromStatus(gqlTransaction.status); + const gasPrice = totalFee ? bn(0) : await provider.getLatestGasPrice(); + const baseAssetId = await provider.getBaseAssetId(); const transactionInfo = assembleTransactionSummary({ diff --git a/packages/account/src/providers/transaction-summary/status.test.ts b/packages/account/src/providers/transaction-summary/status.test.ts index 100f73ae199..95c3687039d 100644 --- a/packages/account/src/providers/transaction-summary/status.test.ts +++ b/packages/account/src/providers/transaction-summary/status.test.ts @@ -1,3 +1,5 @@ +import { bn } from '@fuel-ts/math'; + import { MOCK_FAILURE_STATUS, MOCK_SQUEEZEDOUT_STATUS, @@ -5,7 +7,7 @@ import { MOCK_SUCCESS_STATUS, } from '../../../test/fixtures/transaction-summary'; -import { getTransactionStatusName, processGraphqlStatus } from './status'; +import { getTotalFeeFromStatus, getTransactionStatusName, processGraphqlStatus } from './status'; import type { GqlTransactionStatusesNames, GraphqlTransactionStatus } from './types'; import { TransactionStatus } from './types'; @@ -46,6 +48,8 @@ describe('status', () => { blockIdType: 'string', status: TransactionStatus.success, timeType: 'string', + totalFee: bn(1000), + totalGas: bn(1000), }, }, { @@ -58,6 +62,8 @@ describe('status', () => { blockIdType: 'string', status: TransactionStatus.failure, timeType: 'string', + totalFee: bn(1000), + totalGas: bn(1000), }, }, { @@ -95,6 +101,8 @@ describe('status', () => { blockId, status: resultStatus, time, + totalFee, + totalGas, } = processGraphqlStatus(status); expect(isStatusFailure).toBe(expected.isStatusFailure); @@ -103,6 +111,15 @@ describe('status', () => { expect(typeof blockId).toBe(expected.blockIdType); expect(resultStatus).toBe(expected.status); expect(typeof time).toBe(expected.timeType); + expect(totalFee).toStrictEqual(expected.totalFee); + expect(totalGas).toStrictEqual(expected.totalGas); + }); + }); + + statuses.forEach(({ name, status, expected }) => { + it(`should ensure getTotalFeeFromStatus works fine for ${name}`, () => { + const totalFee = getTotalFeeFromStatus(status); + expect(totalFee).toStrictEqual(expected.totalFee); }); }); }); diff --git a/packages/account/src/providers/transaction-summary/status.ts b/packages/account/src/providers/transaction-summary/status.ts index d919c6efb50..4d9d5831e28 100644 --- a/packages/account/src/providers/transaction-summary/status.ts +++ b/packages/account/src/providers/transaction-summary/status.ts @@ -1,6 +1,8 @@ import { ErrorCode, FuelError } from '@fuel-ts/errors'; -import { bn, type BN } from '@fuel-ts/math'; +import type { BN } from '@fuel-ts/math'; +import { bn } from '@fuel-ts/math'; +import { TransactionStatus } from './types'; import type { BlockId, GqlTransactionStatusesNames, @@ -8,7 +10,6 @@ import type { Time, TransactionSummary, } from './types'; -import { TransactionStatus } from './types'; /** @hidden */ export const getTransactionStatusName = (gqlStatus: GqlTransactionStatusesNames) => { @@ -87,3 +88,12 @@ export const processGraphqlStatus = (gqlTransactionStatus?: GraphqlTransactionSt return processedGraphqlStatus; }; + +/** + * Returns the total fee from the transaction status. + * + * @param status - The transaction status. + * @returns The total fee from the transaction status or undefined. + */ +export const getTotalFeeFromStatus = (status?: GraphqlTransactionStatus): BN | undefined => + status && 'totalFee' in status ? bn(status.totalFee) : undefined; diff --git a/packages/fuel-gauge/src/transaction-response.test.ts b/packages/fuel-gauge/src/transaction-response.test.ts index 4ca32f8c800..d4fb20427bf 100644 --- a/packages/fuel-gauge/src/transaction-response.test.ts +++ b/packages/fuel-gauge/src/transaction-response.test.ts @@ -306,4 +306,58 @@ describe('TransactionResponse', () => { ); } ); + + it('builds response and awaits result [uses fee from status]', async () => { + using launched = await launchTestNode(); + + const { + provider, + wallets: [genesisWallet], + } = launched; + + const getLatestGasPriceSpy = vi.spyOn(provider, 'getLatestGasPrice'); + + const request = new ScriptTransactionRequest(); + request.addCoinOutput(Wallet.generate(), 100, await provider.getBaseAssetId()); + await request.autoCost(genesisWallet); + + const tx = await genesisWallet.sendTransaction(request); + const result = await tx.waitForResult(); + + // fee is used from the success status, latest gas price not needed + expect(getLatestGasPriceSpy).toHaveBeenCalledTimes(0); + expect(result.fee.toNumber()).toBeGreaterThan(0); + expect(result.id).toBe(tx.id); + }); + + it('builds response and assembles result [fetches gas price then uses fee]', async () => { + using launched = await launchTestNode({ + nodeOptions: { + args: ['--poa-instant', 'false', '--poa-interval-period', '2sec'], + }, + }); + + const { + provider, + wallets: [genesisWallet], + } = launched; + + const getLatestGasPriceSpy = vi.spyOn(provider, 'getLatestGasPrice'); + + const request = new ScriptTransactionRequest(); + request.addCoinOutput(Wallet.generate(), 100, await provider.getBaseAssetId()); + await request.autoCost(genesisWallet); + + const tx = await genesisWallet.sendTransaction(request); + const result = await tx.assembleResult(); + + // tx has not settled so response will fetch the gas price + expect(getLatestGasPriceSpy).toHaveBeenCalledTimes(1); + expect(result.id).toBe(tx.id); + + const finalisedResult = await tx.waitForResult(); + expect(finalisedResult.fee.toNumber()).toBeGreaterThan(0); + expect(getLatestGasPriceSpy).toHaveBeenCalledTimes(1); + expect(finalisedResult.id).toBe(tx.id); + }); });