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: allow passing gasPrice to getTransactionCost #3608

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .changeset/clever-mirrors-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@fuel-ts/account": patch
"@fuel-ts/program": patch
---

feat: allow passing `gasPrice` to `getTransactionCost`
3 changes: 2 additions & 1 deletion packages/account/src/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ export class Account extends AbstractAccount implements WithAddress {
*/
async getTransactionCost(
transactionRequestLike: TransactionRequestLike,
{ signatureCallback, quantities = [] }: TransactionCostParams = {}
{ signatureCallback, quantities = [], gasPrice }: TransactionCostParams = {}
): Promise<TransactionCost> {
const txRequestClone = clone(transactionRequestify(transactionRequestLike));
const baseAssetId = await this.provider.getBaseAssetId();
Expand Down Expand Up @@ -603,6 +603,7 @@ export class Account extends AbstractAccount implements WithAddress {

const txCost = await this.provider.getTransactionCost(txRequestClone, {
signatureCallback,
gasPrice,
});

return {
Expand Down
54 changes: 43 additions & 11 deletions packages/account/src/providers/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,30 @@ export type TransactionCostParams = EstimateTransactionParams & {
* @returns A promise that resolves to the signed transaction request.
*/
signatureCallback?: (request: ScriptTransactionRequest) => Promise<ScriptTransactionRequest>;

/**
* The gas price to use for the transaction.
*/
gasPrice?: BN;
};

export type EstimateTxDependenciesParams = {
/**
* The gas price to use for the transaction.
*/
gasPrice?: BN;
};

export type EstimateTxGasAndFeeParams = {
/**
* The transaction request to estimate the gas and fee for.
*/
transactionRequest: TransactionRequest;

/**
* The gas price to use for the transaction.
*/
gasPrice?: BN;
};

/**
Expand Down Expand Up @@ -971,10 +995,12 @@ export default class Provider {
* `addVariableOutputs` is called on the transaction.
*
* @param transactionRequest - The transaction request object.
* @param gasPrice - The gas price to use for the transaction, if not provided it will be fetched.
* @returns A promise that resolves to the estimate transaction dependencies.
*/
async estimateTxDependencies(
transactionRequest: TransactionRequest
transactionRequest: TransactionRequest,
{ gasPrice: gasPriceParam }: EstimateTxDependenciesParams = {}
Comment on lines +1002 to +1003
Copy link
Member Author

Choose a reason for hiding this comment

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

Thoughts on breaking change and having a single object parameter?

): Promise<EstimateTxDependenciesReturns> {
if (isTransactionTypeCreate(transactionRequest)) {
return {
Expand All @@ -991,13 +1017,15 @@ export default class Provider {

await this.validateTransaction(transactionRequest);

const gasPrice = gasPriceParam ?? (await this.estimateGasPrice(10));

for (let attempt = 0; attempt < MAX_RETRIES; attempt++) {
const {
dryRun: [{ receipts: rawReceipts, status }],
} = await this.operations.dryRun({
encodedTransactions: [hexlify(transactionRequest.toTransactionBytes())],
utxoValidation: false,
gasPrice: '0',
gasPrice: gasPrice.toString(),
});

receipts = rawReceipts.map(processGqlReceipt);
Expand All @@ -1019,7 +1047,7 @@ export default class Provider {

const { maxFee } = await this.estimateTxGasAndFee({
transactionRequest,
gasPrice: bn(0),
gasPrice,
});

// eslint-disable-next-line no-param-reassign
Expand Down Expand Up @@ -1186,12 +1214,12 @@ export default class Provider {

/**
* Estimates the transaction gas and fee based on the provided transaction request.
* @param transactionRequest - The transaction request object.
* @param params - The parameters for estimating the transaction gas and fee.
* @returns An object containing the estimated minimum gas, minimum fee, maximum gas, and maximum fee.
*/
async estimateTxGasAndFee(params: { transactionRequest: TransactionRequest; gasPrice?: BN }) {
const { transactionRequest } = params;
let { gasPrice } = params;
async estimateTxGasAndFee(params: EstimateTxGasAndFeeParams) {
const { transactionRequest, gasPrice: gasPriceParam } = params;
let gasPrice = gasPriceParam;

await this.autoRefetchConfigs();

Expand Down Expand Up @@ -1307,7 +1335,7 @@ export default class Provider {
*/
async getTransactionCost(
transactionRequestLike: TransactionRequestLike,
{ signatureCallback }: TransactionCostParams = {}
{ signatureCallback, gasPrice: gasPriceParam }: TransactionCostParams = {}
): Promise<Omit<TransactionCost, 'requiredQuantities'>> {
const txRequestClone = clone(transactionRequestify(transactionRequestLike));
const updateMaxFee = txRequestClone.maxFee.eq(0);
Expand All @@ -1329,12 +1357,16 @@ export default class Provider {
await this.estimatePredicates(signedRequest);
txRequestClone.updatePredicateGasUsed(signedRequest.inputs);

const gasPrice = gasPriceParam ?? (await this.estimateGasPrice(10));

/**
* Calculate minGas and maxGas based on the real transaction
*/
// eslint-disable-next-line prefer-const
let { maxFee, maxGas, minFee, minGas, gasPrice, gasLimit } = await this.estimateTxGasAndFee({
let { maxFee, maxGas, minFee, minGas, gasLimit } = await this.estimateTxGasAndFee({
// Fetches and returns a gas price
transactionRequest: signedRequest,
gasPrice,
});

let receipts: TransactionResultReceipt[] = [];
Expand All @@ -1351,7 +1383,7 @@ export default class Provider {
}

({ receipts, missingContractIds, outputVariables, dryRunStatus } =
await this.estimateTxDependencies(txRequestClone));
await this.estimateTxDependencies(txRequestClone, { gasPrice }));

if (dryRunStatus && 'reason' in dryRunStatus) {
throw this.extractDryRunError(txRequestClone, receipts, dryRunStatus);
Expand All @@ -1363,7 +1395,7 @@ export default class Provider {
gasUsed = bn(pristineGasUsed.muln(GAS_USED_MODIFIER)).max(maxGasPerTx.sub(minGas));
txRequestClone.gasLimit = gasUsed;

({ maxFee, maxGas, minFee, minGas, gasPrice } = await this.estimateTxGasAndFee({
({ maxFee, maxGas, minFee, minGas } = await this.estimateTxGasAndFee({
transactionRequest: txRequestClone,
gasPrice,
}));
Expand Down
228 changes: 152 additions & 76 deletions packages/fuel-gauge/src/fee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,158 @@ describe('Fee', () => {
});
});

it('should not run estimateGasPrice in between estimateTxDependencies dry run attempts', async () => {
using launched = await launchTestNode({
contractsConfigs: [
{
factory: MultiTokenContractFactory,
},
],
});

const {
contracts: [contract],
wallets: [wallet],
provider,
} = launched;

const assetId = getMintedAssetId(contract.id.toB256(), SUB_ID);

// Minting coins first
const mintCall = await contract.functions.mint_coins(SUB_ID, 10_000).call();
await mintCall.waitForResult();

const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');
const dryRun = vi.spyOn(provider.operations, 'dryRun');

/**
* Sway transfer without adding `OutputVariable` which will result in
* 2 dry runs at the `Provider.estimateTxDependencies` method:
* - 1st dry run will fail due to missing `OutputVariable`
* - 2nd dry run will succeed
*/
const transferCall = await contract.functions
.transfer_to_address({ bits: wallet.address.toB256() }, { bits: assetId }, 10_000)
.call();

await transferCall.waitForResult();

expect(estimateGasPrice).toHaveBeenCalledOnce();
expect(dryRun).toHaveBeenCalledTimes(2);
});

it('should ensure estimateGasPrice runs only once when funding a transaction.', async () => {
const amountPerCoin = 100;

using launched = await launchTestNode({
walletsConfig: {
amountPerCoin, // Funding with multiple UTXOs so the fee will change after funding the TX.
coinsPerAsset: 250,
},
contractsConfigs: [
{
factory: MultiTokenContractFactory,
},
],
});

const {
wallets: [wallet],
provider,
} = launched;

const fund = vi.spyOn(wallet, 'fund');
const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');

const tx = await wallet.transfer(
wallet.address,
amountPerCoin * 20,
await provider.getBaseAssetId()
);
const { isStatusSuccess } = await tx.waitForResult();

expect(fund).toHaveBeenCalledOnce();
expect(estimateGasPrice).toHaveBeenCalledOnce();

expect(isStatusSuccess).toBeTruthy();
});

it('ensures estimateGasPrice runs only once when getting transaction cost [w/ gas price]', async () => {
using launched = await launchTestNode({
contractsConfigs: [
{
factory: CallTestContractFactory,
},
],
});

const {
contracts: [contract],
provider,
wallets: [wallet],
} = launched;

const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');

const txRequest = await contract.functions.foo(10).getTransactionRequest();
const cost = await wallet.getTransactionCost(txRequest);

expect(cost.gasUsed.toNumber()).toBeGreaterThan(0);
expect(estimateGasPrice).toHaveBeenCalledOnce();
});

it('ensures estimateGasPrice runs twice when getting transaction cost with estimate gas and fee [w/o gas price]', async () => {
using launched = await launchTestNode({
contractsConfigs: [
{
factory: CallTestContractFactory,
},
],
});

const {
contracts: [contract],
provider,
wallets: [wallet],
} = launched;

const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');

const txRequest = await contract.functions.foo(10).getTransactionRequest();
const { gasPrice } = await provider.estimateTxGasAndFee({ transactionRequest: txRequest });
const { gasUsed } = await wallet.getTransactionCost(txRequest);

expect(estimateGasPrice).toHaveBeenCalledTimes(2);
expect(gasPrice.toNumber()).toBeGreaterThan(0);
expect(gasUsed.toNumber()).toBeGreaterThan(0);
});

it('ensures estimateGasPrice runs only once when getting transaction cost with estimate gas and fee', async () => {
using launched = await launchTestNode({
contractsConfigs: [
{
factory: CallTestContractFactory,
},
],
});

const {
contracts: [contract],
provider,
wallets: [wallet],
} = launched;

const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');

const txRequest = await contract.functions.foo(10).getTransactionRequest();
const { gasPrice } = await provider.estimateTxGasAndFee({ transactionRequest: txRequest });
const { gasUsed } = await wallet.getTransactionCost(txRequest, { gasPrice });

expect(gasPrice.toNumber()).toBeGreaterThan(0);
expect(gasUsed.toNumber()).toBeGreaterThan(0);
expect(estimateGasPrice).toHaveBeenCalledOnce();
});

describe('Estimation with Message containing data within TX request inputs', () => {
// Message with data and amount
const testMessage1 = new TestMessage({
Expand Down Expand Up @@ -437,81 +589,5 @@ describe('Fee', () => {

expect(cost.dryRunStatus?.type).toBe('DryRunSuccessStatus');
});

it('should not run estimateGasPrice in between estimateTxDependencies dry run attempts', async () => {
using launched = await launchTestNode({
contractsConfigs: [
{
factory: MultiTokenContractFactory,
},
],
});

const {
contracts: [contract],
wallets: [wallet],
provider,
} = launched;

const assetId = getMintedAssetId(contract.id.toB256(), SUB_ID);

// Minting coins first
const mintCall = await contract.functions.mint_coins(SUB_ID, 10_000).call();
await mintCall.waitForResult();

const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');
const dryRun = vi.spyOn(provider.operations, 'dryRun');

/**
* Sway transfer without adding `OutputVariable` which will result in
* 2 dry runs at the `Provider.estimateTxDependencies` method:
* - 1st dry run will fail due to missing `OutputVariable`
* - 2nd dry run will succeed
*/
const transferCall = await contract.functions
.transfer_to_address({ bits: wallet.address.toB256() }, { bits: assetId }, 10_000)
.call();

await transferCall.waitForResult();

expect(estimateGasPrice).toHaveBeenCalledOnce();
expect(dryRun).toHaveBeenCalledTimes(2);
});

it('should ensure estimateGasPrice runs only once when funding a transaction.', async () => {
const amountPerCoin = 100;

using launched = await launchTestNode({
walletsConfig: {
amountPerCoin, // Funding with multiple UTXOs so the fee will change after funding the TX.
coinsPerAsset: 250,
},
contractsConfigs: [
{
factory: MultiTokenContractFactory,
},
],
});

const {
wallets: [wallet],
provider,
} = launched;

const fund = vi.spyOn(wallet, 'fund');
const estimateGasPrice = vi.spyOn(provider, 'estimateGasPrice');

const tx = await wallet.transfer(
wallet.address,
amountPerCoin * 20,
await provider.getBaseAssetId()
);
const { isStatusSuccess } = await tx.waitForResult();

expect(fund).toHaveBeenCalledOnce();
expect(estimateGasPrice).toHaveBeenCalledOnce();

expect(isStatusSuccess).toBeTruthy();
});
});
});
Loading
Loading