feat!: remove redundant gas price call for tx summary (#3559)

* 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 36c6190. * feat: remove totalFee from assemble tx summary * chore: lint * chore: mitigate test flakiness Co-authored-by: Nedim Salkić <[email protected]> * chore: lint --------- Co-authored-by: Nedim Salkić <[email protected]>
FuelLabs · Jan 8, 2025 · f2407dc · f2407dc
1 parent bd9b102
commit f2407dc
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 33 deletions.
diff --git a/.changeset/short-bears-fry.md 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
@@ -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
@@ -56,7 +56,8 @@ describe('TransactionSummary', () => {
   const runTest = (
     status: GraphqlTransactionStatus,
     expected: Record<string, unknown>,
-    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
@@ -79,20 +79,21 @@ export function assembleTransactionSummary<TTransactionType = void>(
   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
@@ -25,7 +25,6 @@ export type CalculateTXFeeForSummaryParams = {
   gasPrice: BN;
   rawPayload: string;
   tip: BN;
-  totalFee?: BN;
   consensusParameters: Pick<ConsensusParameters, 'gasCosts'> & {
     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
@@ -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<TTransactionType = void>(
     },
   } = 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<TTransactionType>({

diff --git a/packages/account/src/providers/transaction-summary/status.test.ts b/packages/account/src/providers/transaction-summary/status.test.ts
@@ -1,11 +1,13 @@
+import { bn } from '@fuel-ts/math';
+
 import {
   MOCK_FAILURE_STATUS,
   MOCK_SQUEEZEDOUT_STATUS,
   MOCK_SUBMITTED_STATUS,
   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
@@ -1,14 +1,15 @@
 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,
   GraphqlTransactionStatus,
   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
@@ -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);
+  });
 });