feat: snip29 safety checks (#1403)

* fix: add safeExecutePaymaster(to verify)

* test: add tests suite for safeExecutePaymaster

* fix: refactor, add comments, add interfaces function

* fix: merge executePaymaster & safeExecutePaymaster

* fix: refactor 'assertCallsAreStrictlyEqual' function & move it to utils

* refactor: move safety checks to utils

* fix: remove paymaster flow from standard 'execute' call

* fix: remove paymaster flow from standard 'execute' call

* fix: assert gasToken address in assertPaymasterTransactionSafety

* fix: types were not correct for typedData calls

Author: 0xlny

__tests__/accountPaymaster.test.ts

@@ -1,32 +1,55 @@
-import { Account, Signature, Call, PaymasterDetails, PaymasterRpc } from '../src';
+import { OutsideCallV2, OutsideExecutionTypedDataV2 } from 'starknet-types-08';
+import { Account, Signature, Call, PaymasterDetails, OutsideExecutionVersion } from '../src';
+import { getSelectorFromName } from '../src/utils/hash';
 
 jest.mock('../src/paymaster/rpc');
 
 describe('Account - Paymaster integration', () => {
+  let account: Account | null = null;
   const mockBuildTransaction = jest.fn();
+  const mockMaliciousBuildTransactionChangeToken = jest.fn();
+  const mockMaliciousBuildTransactionChangeFees = jest.fn();
+  const mockMaliciousBuildTransactionAddedCalls = jest.fn();
   const mockExecuteTransaction = jest.fn();
+  const mockGetSnip9Version = jest.fn();
   const mockSignMessage = jest.fn();
 
-  const fakeTypedData = {
+  const fakeSignature: Signature = ['0x1', '0x2'];
+  const originalCalls: Call[] = [
+    { contractAddress: '0x123', entrypoint: 'transfer', calldata: [] },
+  ];
+
+  const originalCallsAsOutsideCalls: OutsideCallV2[] = [
+    {
+      To: '0x123',
+      Selector: getSelectorFromName('transfer'),
+      Calldata: [],
+    },
+  ];
+
+  const typedData: OutsideExecutionTypedDataV2 = {
     types: {},
     domain: {},
     primaryType: '',
     message: {
-      caller: '0xcaller',
-      nonce: '0xnonce',
-      execute_after: '0x1',
-      execute_before: '0x2',
-      calls_len: '0x0',
-      calls: [],
+      Caller: '0xcaller',
+      Nonce: '0xnonce',
+      'Execute After': '0x1',
+      'Execute Before': '0x2',
+      Calls: [
+        ...originalCallsAsOutsideCalls,
+        {
+          To: '0x456',
+          Selector: getSelectorFromName('transfer'),
+          Calldata: ['0xcaller', '1200', '0'],
+        },
+      ],
     },
   };
 
-  const fakeSignature: Signature = ['0x1', '0x2'];
-  const calls: Call[] = [{ contractAddress: '0x123', entrypoint: 'transfer', calldata: [] }];
-
   const paymasterResponse = {
     type: 'invoke',
-    typed_data: fakeTypedData,
+    typed_data: typedData,
     parameters: {
       version: '0x1',
       feeMode: { mode: 'default', gasToken: '0x456' },
@@ -40,41 +63,109 @@ describe('Account - Paymaster integration', () => {
     },
   };
 
-  const mockPaymaster = () =>
-    ({
-      buildTransaction: mockBuildTransaction,
-      executeTransaction: mockExecuteTransaction,
-    }) as unknown as PaymasterRpc;
-
-  const setupAccount = () =>
-    new Account(
-      {},
-      '0xabc',
-      { signMessage: mockSignMessage.mockResolvedValue(fakeSignature) } as any,
-      undefined,
-      undefined,
-      mockPaymaster()
-    );
+  const maliciousTypedDataChangeToken: OutsideExecutionTypedDataV2 = {
+    ...typedData,
+    message: {
+      ...typedData.message,
+      Calls: [
+        ...originalCallsAsOutsideCalls,
+        {
+          To: '0x4567',
+          Selector: getSelectorFromName('transfer'),
+          Calldata: ['0xcaller', '1200', '0'],
+        },
+      ],
+    },
+  };
+
+  const maliciousTypedDataChangeFees: OutsideExecutionTypedDataV2 = {
+    ...typedData,
+    message: {
+      ...typedData.message,
+      Calls: [
+        ...originalCallsAsOutsideCalls,
+        {
+          To: '0x456',
+          Selector: getSelectorFromName('transfer'),
+          Calldata: ['0xcaller', '13000', '0'],
+        },
+      ],
+    },
+  };
+
+  const maliciousTypedDataAddedCalls: OutsideExecutionTypedDataV2 = {
+    ...typedData,
+    message: {
+      ...typedData.message,
+      Calls: [
+        ...originalCallsAsOutsideCalls,
+        {
+          To: '0x456',
+          Selector: getSelectorFromName('transfer'),
+          Calldata: ['0xcaller', '13000', '0'],
+        },
+        {
+          To: '0x456',
+          Selector: getSelectorFromName('transfer'),
+          Calldata: ['0xcaller', '13000', '0'],
+        },
+      ],
+    },
+  };
+
+  const maliciousPaymasterResponseChangeToken = {
+    ...paymasterResponse,
+    typed_data: maliciousTypedDataChangeToken,
+  };
+  const maliciousPaymasterResponseChangeFees = {
+    ...paymasterResponse,
+    typed_data: maliciousTypedDataChangeFees,
+  };
+  const maliciousPaymasterResponseAddedCalls = {
+    ...paymasterResponse,
+    typed_data: maliciousTypedDataAddedCalls,
+  };
+
+  const getAccount = () => {
+    if (!account) {
+      account = new Account(
+        {},
+        '0xabc',
+        { signMessage: mockSignMessage.mockResolvedValue(fakeSignature) } as any,
+        undefined,
+        undefined,
+        undefined
+      );
+      // account object is instanciate in the constructor, we need to mock the paymaster methods after paymaster object is instanciate
+      account.paymaster.buildTransaction = mockBuildTransaction;
+      account.paymaster.executeTransaction = mockExecuteTransaction;
+    }
+    return account;
+  };
 
   beforeEach(() => {
     jest.clearAllMocks();
-    (PaymasterRpc as jest.Mock).mockImplementation(mockPaymaster);
+    jest.spyOn(getAccount(), 'getSnip9Version').mockImplementation(mockGetSnip9Version);
     mockBuildTransaction.mockResolvedValue(paymasterResponse);
+    mockMaliciousBuildTransactionChangeToken.mockResolvedValue(
+      maliciousPaymasterResponseChangeToken
+    );
+    mockMaliciousBuildTransactionChangeFees.mockResolvedValue(maliciousPaymasterResponseChangeFees);
+    mockMaliciousBuildTransactionAddedCalls.mockResolvedValue(maliciousPaymasterResponseAddedCalls);
     mockExecuteTransaction.mockResolvedValue({ transaction_hash: '0x123' });
+    mockGetSnip9Version.mockResolvedValue(OutsideExecutionVersion.V2);
   });
 
   describe('estimatePaymasterTransactionFee', () => {
     it('should return estimated transaction fee from paymaster', async () => {
-      const account = setupAccount();
-
-      const result = await account.estimatePaymasterTransactionFee(calls, {
+      const result = await getAccount().estimatePaymasterTransactionFee(originalCalls, {
         feeMode: { mode: 'default', gasToken: '0x456' },
       });
 
       expect(mockBuildTransaction).toHaveBeenCalledWith(
         {
           type: 'invoke',
-          invoke: { userAddress: '0xabc', calls },
+          invoke: { userAddress: '0xabc', calls: originalCalls },
         },
         {
           version: '0x1',
@@ -88,33 +179,21 @@ describe('Account - Paymaster integration', () => {
   });
 
   describe('executePaymasterTransaction', () => {
-    it('should throw if token price exceeds maxPriceInStrk', async () => {
-      const account = setupAccount();
-      const details: PaymasterDetails = {
-        feeMode: { mode: 'default', gasToken: '0x456' },
-      };
-
-      await expect(account.executePaymasterTransaction(calls, details, 100n)).rejects.toThrow(
-        'Gas token price is too high'
-      );
-    });
-
-    it('should sign and execute transaction via paymaster', async () => {
-      const account = setupAccount();
+    it('should sign and execute transaction via paymaster without checking gas fees', async () => {
       const details: PaymasterDetails = {
         feeMode: { mode: 'default', gasToken: '0x456' },
       };
 
-      const result = await account.executePaymasterTransaction(calls, details);
+      const result = await getAccount().executePaymasterTransaction(originalCalls, details);
 
       expect(mockBuildTransaction).toHaveBeenCalledTimes(1);
-      expect(mockSignMessage).toHaveBeenCalledWith(fakeTypedData, '0xabc');
+      expect(mockSignMessage).toHaveBeenCalledWith(typedData, '0xabc');
       expect(mockExecuteTransaction).toHaveBeenCalledWith(
         {
           type: 'invoke',
           invoke: {
             userAddress: '0xabc',
-            typedData: fakeTypedData,
+            typedData,
             signature: ['0x1', '0x2'],
           },
         },
@@ -126,5 +205,73 @@ describe('Account - Paymaster integration', () => {
       );
       expect(result).toEqual({ transaction_hash: '0x123' });
     });
+
+    it('should sign and execute transaction via paymaster', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'default', gasToken: '0x456' },
+      };
+
+      const result = await getAccount().executePaymasterTransaction(
+        originalCalls,
+        details,
+        '0x123456'
+      );
+      expect(result).toEqual({ transaction_hash: '0x123' });
+    });
+
+    it('should not throw if token price exceeds maxPriceInGasToken but transaction is sponsored', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'sponsored' },
+      };
+      const result = await getAccount().executePaymasterTransaction(
+        originalCalls,
+        details,
+        '0x123'
+      );
+      expect(result).toEqual({
+        transaction_hash: '0x123',
+      });
+    });
+
+    it('should throw if token price exceeds maxPriceInGasToken', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'default', gasToken: '0x456' },
+      };
+
+      await expect(
+        getAccount().executePaymasterTransaction(originalCalls, details, '0x123')
+      ).rejects.toThrow('Gas token price is too high');
+    });
+
+    it('should throw if Gas token value is not equal to the provided gas fees', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'default', gasToken: '0x456' },
+      };
+      getAccount().paymaster.buildTransaction = mockMaliciousBuildTransactionChangeFees;
+      await expect(
+        getAccount().executePaymasterTransaction(originalCalls, details, '0x123456')
+      ).rejects.toThrow('Gas token value is not equal to the provided gas fees');
+    });
+
+    it('should throw if Gas token address is not equal to the provided gas token', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'default', gasToken: '0x456' },
+      };
+      getAccount().paymaster.buildTransaction = mockMaliciousBuildTransactionChangeToken;
+      await expect(
+        getAccount().executePaymasterTransaction(originalCalls, details, '0x123456')
+      ).rejects.toThrow('Gas token address is not equal to the provided gas token');
+    });
+
+    it('should throw if provided calls are not strictly equal to the returned calls', async () => {
+      const details: PaymasterDetails = {
+        feeMode: { mode: 'default', gasToken: '0x456' },
+      };
+      getAccount().paymaster.buildTransaction = mockMaliciousBuildTransactionAddedCalls;
+
+      await expect(
+        getAccount().executePaymasterTransaction(originalCalls, details, '0x123456')
+      ).rejects.toThrow('Provided calls are not strictly equal to the returned calls');
+    });
   });
 });