fix: consolidate ResourceBounds sum (#456)

* fix: add consolidate fee on resource bound * chore: fix test for consolidate fee
Consensys · Dec 10, 2024 · 85ea8ba · 85ea8ba
1 parent 46770ee
commit 85ea8ba
Show file tree

Hide file tree

Showing 14 changed files with 387 additions and 132 deletions.
diff --git a/packages/starknet-snap/src/__tests__/helper.ts b/packages/starknet-snap/src/__tests__/helper.ts
@@ -468,18 +468,16 @@ export function generateTransactionRequests({
         },
       ],
       includeDeploy: false,
-      resourceBounds: [
-        {
-          l1_gas: {
-            max_amount: '0',
-            max_price_per_unit: '0',
-          },
-          l2_gas: {
-            max_amount: '0',
-            max_price_per_unit: '0',
-          },
+      resourceBounds: {
+        l1_gas: {
+          max_amount: '0',
+          max_price_per_unit: '0',
         },
-      ],
+        l2_gas: {
+          max_amount: '0',
+          max_price_per_unit: '0',
+        },
+      },
     });
   }
 

diff --git a/packages/starknet-snap/src/rpcs/__tests__/helper.ts b/packages/starknet-snap/src/rpcs/__tests__/helper.ts
@@ -3,9 +3,11 @@ import type { constants } from 'starknet';
 
 import type { StarknetAccount } from '../../__tests__/helper';
 import { generateAccounts, generateRandomValue } from '../../__tests__/helper';
+import { FeeTokenUnit } from '../../types/snapApi';
 import type { SnapState } from '../../types/snapState';
 import * as snapUiUtils from '../../ui/utils';
 import { getExplorerUrl, shortenAddress, toJson } from '../../utils';
+import { mockEstimateFeeBulkResponse } from '../../utils/__tests__/helper';
 import * as snapHelper from '../../utils/snap';
 import * as snapUtils from '../../utils/snapUtils';
 import * as starknetUtils from '../../utils/starknetUtils';
@@ -146,13 +148,14 @@ export function prepareRenderDisplayPrivateKeyAlertUI() {
 
 /**
  *
+ * @param result
  */
-export function prepareConfirmDialogInteractiveUI() {
+export function prepareConfirmDialogInteractiveUI(result = true) {
   const confirmDialogSpy = jest.spyOn(
     snapHelper,
     'createInteractiveConfirmDialog',
   );
-  confirmDialogSpy.mockResolvedValue(true);
+  confirmDialogSpy.mockResolvedValue(result);
   return {
     confirmDialogSpy,
   };
@@ -230,3 +233,39 @@ export function generateRandomFee(
     ? randomFee.toString(10)
     : BigNumber.from(randomFee).toString();
 }
+
+/**
+ *
+ * @param options0
+ * @param options0.includeDeploy
+ * @param options0.unit
+ */
+export function mockGetEstimatedFeesResponse({
+  includeDeploy = false,
+  unit = FeeTokenUnit.ETH,
+}: {
+  includeDeploy?: boolean;
+  unit?: FeeTokenUnit;
+}) {
+  const {
+    consolidatedFees: { suggestedMaxFee, overallFee, resourceBounds },
+    estimateFeesResponse,
+  } = mockEstimateFeeBulkResponse();
+
+  const getEstimatedFeesResponse = {
+    suggestedMaxFee,
+    overallFee,
+    unit,
+    includeDeploy,
+    estimateResults: estimateFeesResponse,
+    resourceBounds,
+  };
+
+  const getEstimatedFeesSpy = jest.spyOn(starknetUtils, 'getEstimatedFees');
+  getEstimatedFeesSpy.mockResolvedValue(getEstimatedFeesResponse);
+
+  return {
+    getEstimatedFeesSpy,
+    getEstimatedFeesResponse,
+  };
+}
diff --git a/packages/starknet-snap/src/rpcs/estimate-fee.test.ts b/packages/starknet-snap/src/rpcs/estimate-fee.test.ts
@@ -2,13 +2,15 @@ import type { Invocations } from 'starknet';
 import { constants, TransactionType } from 'starknet';
 import type { Infer } from 'superstruct';
 
-import { generateEstimateFeesResponse } from '../__tests__/helper';
-import { FeeTokenUnit } from '../types/snapApi';
+import callsExamples from '../__tests__/fixture/callsExamples.json';
 import { STARKNET_SEPOLIA_TESTNET_NETWORK } from '../utils/constants';
 import { InvalidRequestParamsError } from '../utils/exceptions';
-import * as starknetUtils from '../utils/starknetUtils';
 import type { TxVersionStruct } from '../utils/superstruct';
-import { mockAccount, prepareMockAccount } from './__tests__/helper';
+import {
+  mockAccount,
+  mockGetEstimatedFeesResponse,
+  prepareMockAccount,
+} from './__tests__/helper';
 import { estimateFee } from './estimate-fee';
 import type { EstimateFeeParams } from './estimate-fee';
 
@@ -29,12 +31,7 @@ const prepareMockEstimateFee = ({
   const invocations: Invocations = [
     {
       type: TransactionType.INVOKE,
-      payload: {
-        contractAddress:
-          '0x00b28a089e7fb83debee4607b6334d687918644796b47d9e9e38ea8213833137',
-        entrypoint: 'functionName',
-        calldata: ['1', '1'],
-      },
+      payload: callsExamples.singleCall.calls,
     },
   ];
 
@@ -45,20 +42,13 @@ const prepareMockEstimateFee = ({
     details: { version },
   } as unknown as EstimateFeeParams;
 
-  const estimateResults = generateEstimateFeesResponse();
-
-  const estimateBulkFeeRespMock = {
-    suggestedMaxFee: BigInt(1000000000000000).toString(10),
-    overallFee: BigInt(1500000000000000).toString(10),
-    unit: FeeTokenUnit.ETH,
-    includeDeploy,
-    estimateResults,
+  return {
+    invocations,
+    request,
+    ...mockGetEstimatedFeesResponse({
+      includeDeploy,
+    }),
   };
-
-  const getEstimatedFeesSpy = jest.spyOn(starknetUtils, 'getEstimatedFees');
-  getEstimatedFeesSpy.mockResolvedValue(estimateBulkFeeRespMock);
-
-  return { estimateBulkFeeRespMock, invocations, request, getEstimatedFeesSpy };
 };
 
 describe('estimateFee', () => {
@@ -73,13 +63,21 @@ describe('estimateFee', () => {
     const chainId = constants.StarknetChainId.SN_SEPOLIA;
     const account = await mockAccount(chainId);
     prepareMockAccount(account, state);
-    const { request, getEstimatedFeesSpy, estimateBulkFeeRespMock } =
-      prepareMockEstimateFee({
-        includeDeploy: false,
-        chainId,
-        address: account.address,
-        version: constants.TRANSACTION_VERSION.V1,
-      });
+    const {
+      request,
+      getEstimatedFeesSpy,
+      getEstimatedFeesResponse: {
+        includeDeploy,
+        overallFee,
+        suggestedMaxFee,
+        unit,
+      },
+    } = prepareMockEstimateFee({
+      includeDeploy: false,
+      chainId,
+      address: account.address,
+      version: constants.TRANSACTION_VERSION.V1,
+    });
 
     const result = await estimateFee.execute(request);
 
@@ -94,10 +92,10 @@ describe('estimateFee', () => {
       },
     );
     expect(result).toStrictEqual({
-      includeDeploy: estimateBulkFeeRespMock.includeDeploy,
-      overallFee: estimateBulkFeeRespMock.overallFee,
-      suggestedMaxFee: estimateBulkFeeRespMock.suggestedMaxFee,
-      unit: estimateBulkFeeRespMock.unit,
+      includeDeploy,
+      overallFee,
+      suggestedMaxFee,
+      unit,
     });
   });
 

diff --git a/packages/starknet-snap/src/rpcs/execute-txn.test.ts b/packages/starknet-snap/src/rpcs/execute-txn.test.ts
@@ -61,6 +61,7 @@ const prepareMockExecuteTxn = async (
     includeDeploy: !accountDeployed,
     unit: 'wei' as FeeTokenUnit,
     estimateResults,
+    resourceBounds: estimateResults[0].resourceBounds,
   };
 
   const getEstimatedFeesSpy = jest.spyOn(starknetUtils, 'getEstimatedFees');

diff --git a/packages/starknet-snap/src/rpcs/execute-txn.ts b/packages/starknet-snap/src/rpcs/execute-txn.ts
@@ -116,7 +116,7 @@ export class ExecuteTxnRpc extends AccountRpcController<
       const { privateKey, publicKey } = this.account;
       const callsArray = Array.isArray(calls) ? calls : [calls];
 
-      const { includeDeploy, suggestedMaxFee, estimateResults } =
+      const { includeDeploy, suggestedMaxFee, resourceBounds } =
         await getEstimatedFees(
           this.network,
           address,
@@ -156,7 +156,7 @@ export class ExecuteTxnRpc extends AccountRpcController<
         addressIndex: this.account.addressIndex,
         maxFee: suggestedMaxFee,
         calls: formattedCalls,
-        resourceBounds: estimateResults.map((result) => result.resourceBounds),
+        resourceBounds,
         selectedFeeToken:
           version === constants.TRANSACTION_VERSION.V3
             ? FeeToken.STRK
@@ -210,10 +210,7 @@ export class ExecuteTxnRpc extends AccountRpcController<
         // TODO: we may also need to increment the nonce base on the input, if the account is not deployed
         nonce: accountDeployed ? details?.nonce : 1,
         maxFee: updatedRequest.maxFee,
-        resourceBounds:
-          updatedRequest.resourceBounds[
-            updatedRequest.resourceBounds.length - 1
-          ],
+        resourceBounds: updatedRequest.resourceBounds,
         version:
           updatedRequest.selectedFeeToken === FeeToken.STRK
             ? constants.TRANSACTION_VERSION.V3

diff --git a/packages/starknet-snap/src/state/request-state-manager.ts b/packages/starknet-snap/src/state/request-state-manager.ts
@@ -32,7 +32,9 @@ export class TransactionRequestStateManager extends StateManager<TransactionRequ
     // This is the only field that can be updated
     dataInState.maxFee = data.maxFee;
     dataInState.selectedFeeToken = data.selectedFeeToken;
-    dataInState.resourceBounds = [...data.resourceBounds];
+    dataInState.resourceBounds = {
+      ...data.resourceBounds,
+    };
   }
 
   /**

diff --git a/packages/starknet-snap/src/types/snapState.ts b/packages/starknet-snap/src/types/snapState.ts
@@ -32,7 +32,15 @@ export type FormattedCallData = {
   tokenTransferData?: TokenTransferData;
 };
 
-type ResourceBounds = Pick<EstimateFee, 'resourceBounds'>['resourceBounds'];
+export type ResourceBounds = Pick<
+  EstimateFee,
+  'resourceBounds'
+>['resourceBounds'];
+
+export type ResourceBoundsInBigInt = {
+  l1_gas: { max_amount: bigint; max_price_per_unit: bigint };
+  l2_gas: { max_amount: bigint; max_price_per_unit: bigint };
+};
 
 export type TransactionRequest = {
   id: string;
@@ -44,7 +52,7 @@ export type TransactionRequest = {
   networkName: string;
   maxFee: string;
   calls: FormattedCallData[];
-  resourceBounds: ResourceBounds[];
+  resourceBounds: ResourceBounds;
   selectedFeeToken: string;
   includeDeploy: boolean;
 };

diff --git a/packages/starknet-snap/src/ui/controllers/user-input-event-controller.test.ts b/packages/starknet-snap/src/ui/controllers/user-input-event-controller.test.ts
@@ -20,6 +20,7 @@ import {
   STARKNET_TESTNET_NETWORK,
   STRK_SEPOLIA_TESTNET,
 } from '../../utils/constants';
+import { ConsolidateFees } from '../../utils/fee';
 import * as keyPairUtils from '../../utils/keyPair';
 import * as StarknetUtils from '../../utils/starknetUtils';
 import * as UiUtils from '../utils';
@@ -91,16 +92,16 @@ describe('UserInputEventController', () => {
   const mockEstimateFee = (feeToken: FeeToken) => {
     const getEstimatedFeesSpy = jest.spyOn(StarknetUtils, 'getEstimatedFees');
     const mockEstimateFeeResponse = generateEstimateFeesResponse();
-    const concatedFee = StarknetUtils.addFeesFromAllTransactions(
-      mockEstimateFeeResponse,
-    );
+    const consolidateFeesObj = new ConsolidateFees(mockEstimateFeeResponse);
+    const consolidatedFees = consolidateFeesObj.serializate();
 
     const mockGetEstimatedFeesResponse = {
-      suggestedMaxFee: concatedFee.suggestedMaxFee.toString(10),
-      overallFee: concatedFee.overall_fee.toString(10),
+      suggestedMaxFee: consolidatedFees.suggestedMaxFee,
+      overallFee: consolidatedFees.overallFee,
       unit: FeeTokenUnit[feeToken],
       includeDeploy: true,
       estimateResults: mockEstimateFeeResponse,
+      resourceBounds: consolidatedFees.resourceBounds,
     };
 
     getEstimatedFeesSpy.mockResolvedValue(mockGetEstimatedFeesResponse);
@@ -350,7 +351,7 @@ describe('UserInputEventController', () => {
         maxFee: transactionRequest.maxFee,
         selectedFeeToken: transactionRequest.selectedFeeToken,
         includeDeploy: transactionRequest.includeDeploy,
-        resourceBounds: [...transactionRequest.resourceBounds],
+        resourceBounds: transactionRequest.resourceBounds,
       };
 
       const event = generateInputEvent({

diff --git a/packages/starknet-snap/src/ui/controllers/user-input-event-controller.ts b/packages/starknet-snap/src/ui/controllers/user-input-event-controller.ts
@@ -138,7 +138,9 @@ export class UserInputEventController {
       maxFee: request.maxFee,
       selectedFeeToken: request.selectedFeeToken,
       includeDeploy: request.includeDeploy,
-      resourceBounds: [...request.resourceBounds],
+      resourceBounds: {
+        ...request.resourceBounds,
+      },
     };
   }
 
@@ -156,7 +158,7 @@ export class UserInputEventController {
 
       const requestTxnVersion = this.feeTokenToTransactionVersion(feeToken);
 
-      const { includeDeploy, suggestedMaxFee, estimateResults } =
+      const { includeDeploy, suggestedMaxFee, resourceBounds } =
         await getEstimatedFees(
           network,
           signer,
@@ -196,9 +198,7 @@ export class UserInputEventController {
       request.maxFee = suggestedMaxFee;
       request.selectedFeeToken = feeToken;
       request.includeDeploy = includeDeploy;
-      request.resourceBounds = estimateResults.map(
-        (result) => result.resourceBounds,
-      );
+      request.resourceBounds = resourceBounds;
 
       await updateExecuteTxnFlow(this.eventId, request);
       await this.reqStateMgr.upsertTransactionRequest(request);

diff --git a/packages/starknet-snap/src/utils/__tests__/helper.ts b/packages/starknet-snap/src/utils/__tests__/helper.ts
@@ -0,0 +1,22 @@
+import { generateEstimateFeesResponse } from '../../__tests__/helper';
+import { ConsolidateFees } from '../fee';
+import * as starknetUtils from '../starknetUtils';
+
+/**
+ *
+ */
+export function mockEstimateFeeBulkResponse() {
+  const estimateFeesResponse = generateEstimateFeesResponse();
+
+  const consolidatedFeesObj = new ConsolidateFees(estimateFeesResponse);
+  const consolidatedFees = consolidatedFeesObj.serializate();
+
+  const estimateBulkFeeSpy = jest.spyOn(starknetUtils, 'estimateFeeBulk');
+  estimateBulkFeeSpy.mockResolvedValue(estimateFeesResponse);
+
+  return {
+    estimateBulkFeeSpy,
+    estimateFeesResponse,
+    consolidatedFees,
+  };
+}