chore: refine execute txn (#339)

* chore: refine execute txn * fix: lint error * fix: transaction version v2 does not apply (#340) * fix: transaction version v2 does not apply * chore: lint + prettier * chore: fix default version * fix: disable state changes in account creation --------- Co-authored-by: stanleyyuen <[email protected]> * fix: txn error * fix: tx default version should be set by starknet.js * chore: update updateAccountAsDeploy --------- Co-authored-by: khanti42 <[email protected]>
Consensys · Aug 30, 2024 · c1a51a4 · c1a51a4
1 parent 925b39e
commit c1a51a4
Show file tree

Hide file tree

Showing 19 changed files with 353 additions and 1,026 deletions.
diff --git a/packages/starknet-snap/src/__tests__/fixture/callsExamples.json b/packages/starknet-snap/src/__tests__/fixture/callsExamples.json
@@ -9,7 +9,7 @@
     ],
     "details": {
       "nonce": "0x1",
-      "version": "0x2",
+      "version": "0x1",
       "maxFee": "1000000000000000"
     },
     "hash": "0x042f5e546b2e55eb6b1b735f15fbfbd7621fc01ea7c96dcf87928ac27f054adb"
@@ -24,7 +24,7 @@
     ],
     "details": {
       "nonce": "0x2",
-      "version": "0x2",
+      "version": "0x1",
       "maxFee": "1000000000000000"
     },
     "hash": "0x06385d46da9fbed4a5798298b17df069ac5f786e4c9f8f6b81c665540aea245a"

diff --git a/packages/starknet-snap/src/estimateFee.ts b/packages/starknet-snap/src/estimateFee.ts
@@ -38,7 +38,6 @@ export async function estimateFee(params: ApiParamsWithKeyDeriver) {
     );
     const { senderAddress } = requestParamsObj;
     const network = getNetworkFromChainId(state, requestParamsObj.chainId);
-    const { transactionVersion } = requestParamsObj;
 
     if (
       !contractAddress ||
@@ -117,7 +116,6 @@ export async function estimateFee(params: ApiParamsWithKeyDeriver) {
       senderAddress,
       senderPrivateKey,
       bulkTransactions,
-      transactionVersion,
     );
     logger.log(
       `estimateFee:\nestimateFeeBulk estimateBulkFeeResp: ${toJson(

diff --git a/packages/starknet-snap/src/estimateFees.ts b/packages/starknet-snap/src/estimateFees.ts
@@ -32,7 +32,6 @@ export async function estimateFees(params: ApiParamsWithKeyDeriver) {
       senderAddress,
       senderPrivateKey,
       requestParamsObj.invocations,
-      requestParamsObj.transactionVersion,
       requestParamsObj.invocationDetails,
     );
 

diff --git a/packages/starknet-snap/src/executeTxn.ts b/packages/starknet-snap/src/executeTxn.ts
@@ -8,11 +8,7 @@ import type {
   ApiParamsWithKeyDeriver,
   ExecuteTxnRequestParams,
 } from './types/snapApi';
-import {
-  ACCOUNT_CLASS_HASH,
-  CAIRO_VERSION,
-  TRANSACTION_VERSION,
-} from './utils/constants';
+import { ACCOUNT_CLASS_HASH, CAIRO_VERSION } from './utils/constants';
 import { logger } from './utils/logger';
 import {
   getNetworkFromChainId,
@@ -75,7 +71,6 @@ export async function executeTxn(params: ApiParamsWithKeyDeriver) {
       senderAddress,
       senderPrivateKey,
       bulkTransactions,
-      TRANSACTION_VERSION,
       requestParamsObj.invocationsDetails
         ? requestParamsObj.invocationsDetails
         : undefined,
@@ -147,10 +142,9 @@ export async function executeTxn(params: ApiParamsWithKeyDeriver) {
       senderAddress,
       senderPrivateKey,
       requestParamsObj.txnInvocation,
-      undefined,
+      requestParamsObj.abis,
       { maxFee, nonce: nonceSendTransaction },
       CAIRO_VERSION,
-      requestParamsObj.abis,
     );
   } catch (error) {
     logger.error(`Problem found:`, error);

diff --git a/packages/starknet-snap/src/rpcs/estimateFee.test.ts b/packages/starknet-snap/src/rpcs/estimateFee.test.ts
@@ -74,7 +74,7 @@ describe('estimateFee', () => {
         includeDeploy: false,
         chainId,
         address: account.address,
-        version: constants.TRANSACTION_VERSION.V2,
+        version: constants.TRANSACTION_VERSION.V1,
       });
 
     const result = await estimateFee.execute(request);
@@ -86,7 +86,7 @@ describe('estimateFee', () => {
       account.publicKey,
       request.invocations,
       {
-        version: constants.TRANSACTION_VERSION.V2,
+        version: constants.TRANSACTION_VERSION.V1,
       },
     );
     expect(result).toStrictEqual(estimateBulkFeeRespMock);

diff --git a/packages/starknet-snap/src/rpcs/estimateFee.ts b/packages/starknet-snap/src/rpcs/estimateFee.ts
@@ -1,5 +1,4 @@
 import type { Json } from '@metamask/snaps-sdk';
-import type { UniversalDetails } from 'starknet';
 import type { Infer } from 'superstruct';
 import { object, string, assign, boolean, optional, enums } from 'superstruct';
 
@@ -69,7 +68,7 @@ export class EstimateFeeRpc extends AccountRpcController<
       this.account.privateKey,
       this.account.publicKey,
       invocations,
-      details as UniversalDetails,
+      details,
     );
 
     return estimateFeeResp;

diff --git a/packages/starknet-snap/src/rpcs/executeTxn.test.ts b/packages/starknet-snap/src/rpcs/executeTxn.test.ts
@@ -2,15 +2,12 @@ import {
   InvalidParamsError,
   UserRejectedRequestError,
 } from '@metamask/snaps-sdk';
-import type { UniversalDetails, Call } from 'starknet';
+import type { UniversalDetails, Call, InvokeFunctionResponse } from 'starknet';
 import { constants } from 'starknet';
 
 import callsExamples from '../__tests__/fixture/callsExamples.json'; // Assuming you have a similar fixture
 import type { FeeTokenUnit } from '../types/snapApi';
-import {
-  STARKNET_SEPOLIA_TESTNET_NETWORK,
-  TRANSACTION_VERSION,
-} from '../utils/constants';
+import { STARKNET_SEPOLIA_TESTNET_NETWORK } from '../utils/constants';
 import * as starknetUtils from '../utils/starknetUtils';
 import { executeTxn as executeTxnUtil } from '../utils/starknetUtils';
 import {
@@ -59,6 +56,7 @@ const prepareMockExecuteTxn = async (
     includeDeploy: !accountDeployed,
     unit: 'wei' as FeeTokenUnit,
   };
+
   const getEstimatedFeesSpy = jest.spyOn(starknetUtils, 'getEstimatedFees');
   getEstimatedFeesSpy.mockResolvedValue(getEstimatedFeesRepsMock);
 
@@ -67,8 +65,7 @@ const prepareMockExecuteTxn = async (
 
   const createAccountSpy = jest.spyOn(starknetUtils, 'createAccount');
   createAccountSpy.mockResolvedValue({
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    transaction_hash:
+    transactionHash:
       '0x07f901c023bac6c874691244c4c2332c6825b916fb68d240c807c6156db84fd3',
     address: account.address,
   });
@@ -88,9 +85,8 @@ const prepareMockExecuteTxn = async (
 
 describe('ExecuteTxn', () => {
   let callsExample: any;
-  const transactionVersion = TRANSACTION_VERSION;
 
-  it('executes transaction correctly when account is deployed', async () => {
+  it('executes transaction correctly if the account is deployed', async () => {
     callsExample = callsExamples[0];
     const {
       account,
@@ -114,18 +110,17 @@ describe('ExecuteTxn', () => {
       account.address,
       account.privateKey,
       request.calls,
-      transactionVersion,
-      expect.objectContaining({
-        maxFee: getEstimatedFeesRepsMock.suggestedMaxFee,
-        nonce: undefined,
-      }),
       undefined,
+      {
+        ...callsExample.details,
+        maxFee: getEstimatedFeesRepsMock.suggestedMaxFee,
+      },
     );
     expect(getEstimatedFeesSpy).toHaveBeenCalled();
     expect(createAccountSpy).not.toHaveBeenCalled();
   });
 
-  it('creates an account if the account is not deployed and execute transaction', async () => {
+  it('creates an account if the account is not deployed and execute the transaction', async () => {
     callsExample = callsExamples[1];
     const {
       account,
@@ -151,13 +146,12 @@ describe('ExecuteTxn', () => {
       account.address,
       account.privateKey,
       callsExample.calls,
-      transactionVersion,
+      undefined,
       {
         ...callsExample.details,
         maxFee: getEstimatedFeesRepsMock.suggestedMaxFee,
         nonce: 1,
       },
-      undefined,
     );
   });
 
@@ -176,47 +170,20 @@ describe('ExecuteTxn', () => {
     );
   });
 
-  it('throws error if calls are empty', async () => {
-    callsExample = Object.assign({}, callsExamples[1]);
-    const { request } = await prepareMockExecuteTxn(
+  it('throws `Failed to execute transaction` when the transaction hash is not returned from executeTxnUtil', async () => {
+    callsExample = callsExamples[1];
+    const { request, executeTxnUtilSpy } = await prepareMockExecuteTxn(
       callsExample.hash,
       callsExample.calls,
       callsExample.details,
       true,
     );
-    request.calls = [];
-
-    await expect(executeTxn.execute(request)).rejects.toThrow(
-      'Calls cannot be empty',
+    executeTxnUtilSpy.mockResolvedValue(
+      {} as unknown as InvokeFunctionResponse,
     );
-  });
 
-  it.each([
-    {
-      case: 'executeTxnUtil response is undefined',
-      executeTxnUtilResp: {},
-    },
-    {
-      case: 'executeTxnUtil response.transaction_hash is undefined',
-      executeTxnUtilResp: {
-        // eslint-disable-next-line @typescript-eslint/naming-convention
-        transaction_hash: undefined,
-      },
-    },
-  ])(
-    'throws `Error` when $case',
-    async ({ executeTxnUtilResp }: { executeTxnUtilResp: unknown }) => {
-      callsExample = callsExamples[1];
-      const { request, executeTxnUtilSpy } = await prepareMockExecuteTxn(
-        callsExample.hash,
-        callsExample.calls,
-        callsExample.details,
-        true,
-      );
-      executeTxnUtilSpy.mockResolvedValue(executeTxnUtilResp as any);
-      await expect(executeTxn.execute(request)).rejects.toThrow(Error);
-    },
-  );
+    await expect(executeTxn.execute(request)).rejects.toThrow(Error);
+  });
 
   it('throws `InvalidParamsError` when request parameter is not correct', async () => {
     await expect(