fix: the RPC starkNet_executeTxn storing incorrect state data if sing…

…le calls argument was given
Consensys · Oct 10, 2024 · 78f8573 · 78f8573
1 parent d0384bf
commit 78f8573
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 17 deletions.
diff --git a/packages/starknet-snap/src/__tests__/fixture/callsExamples.json b/packages/starknet-snap/src/__tests__/fixture/callsExamples.json
@@ -1,5 +1,5 @@
-[
-  {
+{
+  "multipleCalls": {
     "calls": [
       {
         "contractAddress": "0x00b28a089e7fb83debee4607b6334d687918644796b47d9e9e38ea8213833137",
@@ -14,19 +14,17 @@
     },
     "hash": "0x042f5e546b2e55eb6b1b735f15fbfbd7621fc01ea7c96dcf87928ac27f054adb"
   },
-  {
-    "calls": [
-      {
-        "contractAddress": "0x00b28a089e7fb83debee4607b6334d687918644796b47d9e9e38ea8213833137",
-        "entrypoint": "functionName",
-        "calldata": ["1", "1"]
-      }
-    ],
+  "singleCall": {
+    "calls": {
+      "contractAddress": "0x00b28a089e7fb83debee4607b6334d687918644796b47d9e9e38ea8213833137",
+      "entrypoint": "functionName",
+      "calldata": ["1", "1"]
+    },
     "details": {
       "nonce": "0x2",
       "version": "0x1",
       "maxFee": "1000000000000000"
     },
     "hash": "0x06385d46da9fbed4a5798298b17df069ac5f786e4c9f8f6b81c665540aea245a"
   }
-]
+}
diff --git a/packages/starknet-snap/src/rpcs/executeTxn.test.ts b/packages/starknet-snap/src/rpcs/executeTxn.test.ts
@@ -18,6 +18,7 @@ import {
 } from './__tests__/helper';
 import type { ExecuteTxnParams } from './executeTxn';
 import { executeTxn } from './executeTxn';
+import { Transaction } from '../types/snapState';
 
 jest.mock('../utils/snap');
 jest.mock('../utils/logger');
@@ -76,6 +77,7 @@ const prepareMockExecuteTxn = async (
 
   return {
     network: state.networks[0],
+    transactions: state.transactions,
     account,
     request,
     confirmDialogSpy,
@@ -90,8 +92,8 @@ const prepareMockExecuteTxn = async (
 describe('ExecuteTxn', () => {
   let callsExample: any;
 
-  it('executes transaction correctly if the account is deployed', async () => {
-    callsExample = callsExamples[0];
+  it('executes transaction correctly if the account is deployed and calls is $testCaseTitle', async () => {
+    callsExample = callsExamples.multipleCalls;
     const {
       account,
       createAccountSpy,
@@ -126,10 +128,61 @@ describe('ExecuteTxn', () => {
     expect(createAccountSpy).not.toHaveBeenCalled();
   });
 
+  it.each([
+    {
+      callsExampleKey: 'multipleCalls',
+      testCaseTitle: 'an array of call object',
+    },
+    {
+      callsExampleKey: 'singleCall',
+      testCaseTitle: 'a call object',
+    },
+  ])(
+    'executes transaction correctly if calls is $testCaseTitle',
+    async ({ callsExampleKey }: { callsExampleKey: string }) => {
+      callsExample = callsExamples[callsExampleKey];
+      const {
+        account,
+        transactions,
+        createAccountSpy,
+        executeTxnRespMock,
+        getEstimatedFeesSpy,
+        getEstimatedFeesRepsMock,
+        request,
+      } = await prepareMockExecuteTxn(
+        callsExample.hash,
+        callsExample.calls,
+        callsExample.details,
+        true,
+      );
+
+      const result = await executeTxn.execute(request);
+
+      expect(result).toStrictEqual(executeTxnRespMock);
+      expect(executeTxnUtil).toHaveBeenCalledWith(
+        STARKNET_SEPOLIA_TESTNET_NETWORK,
+        account.address,
+        account.privateKey,
+        request.calls,
+        undefined,
+        {
+          ...callsExample.details,
+          maxFee: getEstimatedFeesRepsMock.suggestedMaxFee,
+          resourceBounds:
+            getEstimatedFeesRepsMock.estimateResults[0].resourceBounds,
+        },
+      );
+      expect(getEstimatedFeesSpy).toHaveBeenCalled();
+      expect(createAccountSpy).not.toHaveBeenCalled();
+      expect(transactions.length).toBe(1);
+      expect((transactions[0] as Transaction).txnHash).toBe(callsExample.hash);
+    },
+  );
+
   it.each([constants.TRANSACTION_VERSION.V1, constants.TRANSACTION_VERSION.V3])(
     'creates an account and execute the transaction with nonce 1 with transaction version %s if the account is not deployed',
     async (transactionVersion) => {
-      callsExample = callsExamples[1];
+      callsExample = callsExamples.multipleCalls;
       const {
         account,
         createAccountSpy,
@@ -180,7 +233,7 @@ describe('ExecuteTxn', () => {
   );
 
   it('throws UserRejectedOpError if user cancels execution', async () => {
-    callsExample = callsExamples[1];
+    callsExample = callsExamples.multipleCalls;
     const { request, confirmDialogSpy } = await prepareMockExecuteTxn(
       callsExample.hash,
       callsExample.calls,
@@ -195,7 +248,7 @@ describe('ExecuteTxn', () => {
   });
 
   it('throws `Failed to execute transaction` when the transaction hash is not returned from executeTxnUtil', async () => {
-    callsExample = callsExamples[1];
+    callsExample = callsExamples.multipleCalls;
     const { request, executeTxnUtilSpy } = await prepareMockExecuteTxn(
       callsExample.hash,
       callsExample.calls,

diff --git a/packages/starknet-snap/src/rpcs/executeTxn.ts b/packages/starknet-snap/src/rpcs/executeTxn.ts
@@ -176,8 +176,13 @@ export class ExecuteTxnRpc extends AccountRpcController<
       throw new Error('Failed to execute transaction');
     }
 
+    // Since the RPC supports the `calls` parameter either as a single `call` object or an array of `call` objects,
+    // and the current state data structure does not yet support multiple `call` objects in a single transaction,
+    // we need to convert `calls` into a single `call` object as a temporary fix.
+    const call = Array.isArray(calls) ? calls[0] : calls;
+
     await this.txnStateManager.addTransaction(
-      this.createInvokeTxn(address, executeTxnResp.transaction_hash, calls[0]),
+      this.createInvokeTxn(address, executeTxnResp.transaction_hash, call),
     );
 
     return executeTxnResp;