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

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

Merged
merged 17 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changeset/short-bears-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fuel-ts/account": patch
danielbate marked this conversation as resolved.
Show resolved Hide resolved
---

feat: remove redundant gas price call for tx summary
danielbate marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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 { processGraphqlStatus } from '../transaction-summary/status';
import type { TransactionSummary, GqlTransaction, AbiMap } from '../transaction-summary/types';
import { extractTxError } from '../utils';

Expand Down Expand Up @@ -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 } = processGraphqlStatus(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();

Expand All @@ -317,6 +322,7 @@ export class TransactionResponse {
maxGasPerTx,
gasPrice,
baseAssetId,
totalFee,
});

return transactionSummary;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ describe('TransactionSummary', () => {
const runTest = (
status: GraphqlTransactionStatus,
expected: Record<string, unknown>,
baseAssetId: string
baseAssetId: string,
calculateTransactionFeeCalls = 1
) => {
const { calculateTransactionFee } = mockCalculateTransactionFee();

Expand All @@ -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 () => {
Expand All @@ -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);
danielbate marked this conversation as resolved.
Show resolved Hide resolved
});

it('should assemble transaction summary just fine (FAILURE)', async () => {
Expand All @@ -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);
danielbate marked this conversation as resolved.
Show resolved Hide resolved
});

it('should assemble transaction summary just fine (SUBMITTED)', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface AssembleTransactionSummaryParams {
maxGasPerTx: BN;
gasPrice: BN;
baseAssetId: string;
totalFee?: BN;
}

/** @hidden */
Expand All @@ -55,6 +56,7 @@ export function assembleTransactionSummary<TTransactionType = void>(
maxGasPerTx,
gasPrice,
baseAssetId,
totalFee,
danielbate marked this conversation as resolved.
Show resolved Hide resolved
} = params;

const gasUsed = getGasUsedFromReceipts(receipts);
Expand All @@ -76,23 +78,32 @@ export function assembleTransactionSummary<TTransactionType = void>(

const tip = bn(transaction.policies?.find((policy) => policy.type === PolicyType.Tip)?.data);

const { isStatusFailure, isStatusPending, isStatusSuccess, blockId, status, time, totalFee } =
processGraphqlStatus(gqlTransactionStatus);

const fee = calculateTXFeeForSummary({
totalFee,
gasPrice,
rawPayload,
tip,
consensusParameters: {
gasCosts,
maxGasPerTx,
feeParams: {
gasPerByte,
gasPriceFactor,
const {
isStatusFailure,
isStatusPending,
isStatusSuccess,
blockId,
status,
time,
totalFee: totalFeeFromStatus,
} = processGraphqlStatus(gqlTransactionStatus);

const fee =
totalFee ??
totalFeeFromStatus ??
calculateTXFeeForSummary({
gasPrice,
rawPayload,
tip,
consensusParameters: {
gasCosts,
maxGasPerTx,
feeParams: {
gasPerByte,
gasPriceFactor,
},
danielbate marked this conversation as resolved.
Show resolved Hide resolved
},
},
});
});

const mintedAssets = extractMintedAssetsFromReceipts(receipts);
const burnedAssets = extractBurnedAssetsFromReceipts(receipts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export type CalculateTXFeeForSummaryParams = {
gasPrice: BN;
rawPayload: string;
tip: BN;
totalFee?: BN;
consensusParameters: Pick<ConsensusParameters, 'gasCosts'> & {
feeParams: FeeParams;
maxGasPerTx: BN;
Expand All @@ -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;
}

danielbate marked this conversation as resolved.
Show resolved Hide resolved
const gasPerByte = bn(feeParams.gasPerByte);
const gasPriceFactor = bn(feeParams.gasPriceFactor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { TransactionCoder } from '@fuel-ts/transactions';
import { arrayify } from '@fuel-ts/utils';

import { processGraphqlStatus } from '../..';

Check failure on line 6 in packages/account/src/providers/transaction-summary/get-transaction-summary.ts

View workflow job for this annotation

GitHub Actions / Lint

Dependency cycle via ./providers:9=>./transaction-summary:11
import type {
GqlGetTransactionsByOwnerQueryVariables,
GqlReceiptFragment,
Expand Down Expand Up @@ -61,7 +62,10 @@
},
} = 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 } = processGraphqlStatus(gqlTransaction.status);
const gasPrice = totalFee ? bn(0) : await provider.getLatestGasPrice();

const baseAssetId = await provider.getBaseAssetId();

const transactionInfo = assembleTransactionSummary<TTransactionType>({
Expand All @@ -78,6 +82,7 @@
maxGasPerTx,
gasPrice,
baseAssetId,
totalFee,
});

return {
Expand Down
50 changes: 50 additions & 0 deletions packages/fuel-gauge/src/transaction-response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,4 +306,54 @@ 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();
danielbate marked this conversation as resolved.
Show resolved Hide resolved

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);
});
});
Loading