Merge pull request #885 from starknet-io/develop-to-beta

Develop to beta

Author: penovicp

CHANGELOG.md

@@ -1,3 +1,11 @@
+## [5.24.5](https://github.com/starknet-io/starknet.js/compare/v5.24.4...v5.24.5) (2023-12-10)
+
+### Bug Fixes
+
+- apply bound for contract address from hash calculation ([6d8c291](https://github.com/starknet-io/starknet.js/commit/6d8c291bce130d7b00ae6d81aff071c4986f04af))
+- **Calldata.compile:** do not split long `entrypoint` names before calling `getSelectorFromName` ([89715da](https://github.com/starknet-io/starknet.js/commit/89715da3fdb4b497cc5771eb83a88460007740b6))
+- prioritize error states in waitForTransaction evaluation ([ac54404](https://github.com/starknet-io/starknet.js/commit/ac544045e2079b68042d850a09b203fc5536c0d0))
+
 # [6.0.0-beta.4](https://github.com/starknet-io/starknet.js/compare/v6.0.0-beta.3...v6.0.0-beta.4) (2023-12-08)
 
 ### Bug Fixes

__tests__/config/fixtures.ts

@@ -57,11 +57,8 @@ export function getTestProvider(isProvider: boolean = true): ProviderInterface |
   if (process.env.IS_LOCALHOST_DEVNET === 'true') {
     // accelerate the tests when running locally
     const originalWaitForTransaction = provider.waitForTransaction.bind(provider);
-    provider.waitForTransaction = (
-      txHash: string,
-      { retryInterval }: waitForTransactionOptions = {}
-    ) => {
-      return originalWaitForTransaction(txHash, { retryInterval: retryInterval || 1000 });
+    provider.waitForTransaction = (txHash: string, options: waitForTransactionOptions = {}) => {
+      return originalWaitForTransaction(txHash, { retryInterval: 1000, ...options });
     };
   }
 

__tests__/rpcProvider.test.ts

@@ -1,8 +1,16 @@
 import { getStarkKey, utils } from '@scure/starknet';
 
-import { Account, Contract, GetBlockResponse, stark } from '../src';
+import {
+  Account,
+  CallData,
+  Contract,
+  GetBlockResponse,
+  RPC,
+  TransactionExecutionStatus,
+  stark,
+  waitForTransactionOptions,
+} from '../src';
 import { StarknetChainId } from '../src/constants';
-import { CallData } from '../src/utils/calldata';
 import { felt, uint256 } from '../src/utils/calldata/cairo';
 import { toHexString } from '../src/utils/num';
 import {
@@ -103,6 +111,66 @@ describeIfRpc('RPCProvider', () => {
     });
   });
 
+  describe('waitForTransaction', () => {
+    const receipt = {};
+    const transactionStatusSpy = jest.spyOn(rpcProvider as any, 'getTransactionStatus');
+    const transactionReceiptSpy = jest.spyOn(rpcProvider as any, 'getTransactionReceipt');
+
+    const generateOptions = (o: waitForTransactionOptions) => ({ retryInterval: 10, ...o });
+    const generateTransactionStatus = (
+      finality_status: RPC.SPEC.TXN_STATUS,
+      execution_status?: RPC.SPEC.TXN_EXECUTION_STATUS
+    ): RPC.TransactionStatus => ({
+      finality_status,
+      execution_status,
+    });
+    const response = {
+      successful: generateTransactionStatus('ACCEPTED_ON_L1', 'SUCCEEDED'),
+      reverted: generateTransactionStatus('ACCEPTED_ON_L2', 'REVERTED'),
+      rejected: generateTransactionStatus('REJECTED'),
+    };
+
+    beforeAll(() => {
+      transactionStatusSpy.mockResolvedValue(null);
+      transactionReceiptSpy.mockResolvedValue(receipt);
+    });
+
+    afterAll(() => {
+      transactionStatusSpy.mockRestore();
+      transactionReceiptSpy.mockRestore();
+    });
+
+    test('successful - default', async () => {
+      transactionStatusSpy.mockResolvedValueOnce(response.successful);
+      await expect(rpcProvider.waitForTransaction(0)).resolves.toBe(receipt);
+    });
+
+    test('reverted - default', async () => {
+      transactionStatusSpy.mockResolvedValueOnce(response.reverted);
+      await expect(rpcProvider.waitForTransaction(0)).resolves.toBe(receipt);
+    });
+
+    test('rejected - default', async () => {
+      transactionStatusSpy.mockResolvedValueOnce(response.rejected);
+      await expect(rpcProvider.waitForTransaction(0)).rejects.toThrow(
+        `${undefined}: ${RPC.ETransactionStatus.REJECTED}`
+      );
+    });
+
+    test('reverted - as error state', async () => {
+      transactionStatusSpy.mockResolvedValueOnce(response.reverted);
+      const options = generateOptions({ errorStates: [TransactionExecutionStatus.REVERTED] });
+      await expect(rpcProvider.waitForTransaction(0, options)).rejects.toThrow(
+        `${RPC.ETransactionExecutionStatus.REVERTED}: ${RPC.ETransactionStatus.ACCEPTED_ON_L2}`
+      );
+    });
+
+    test('no error state; timed-out', async () => {
+      const options = generateOptions({ errorStates: [] });
+      await expect(rpcProvider.waitForTransaction(0, options)).rejects.toThrow(/timed-out/);
+    });
+  });
+
   describe('RPC methods', () => {
     let latestBlock: GetBlockResponse;
 

__tests__/utils/address.test.ts

@@ -1,3 +1,4 @@
+import { constants, num } from '../../src';
 import {
   addAddressPadding,
   getChecksumAddress,
@@ -17,6 +18,11 @@ describe('validateAndParseAddress', () => {
 
     return expect(validateAndParseAddress(addr)).toEqual(`${addAddressPadding(addr)}`);
   });
+
+  test('should fail for out of bound address', () => {
+    const addr = num.toHex(constants.ADDR_BOUND + 1n);
+    expect(() => validateAndParseAddress(addr)).toThrow(/^Message not signable/);
+  });
 });
 
 describe('address checksums', () => {

__tests__/utils/utils.test.ts

@@ -1,3 +1,5 @@
+import * as starkCurve from '@scure/starknet';
+
 import { constants, ec, hash, num, stark } from '../../src';
 import { Block } from '../../src/utils/provider';
 
@@ -78,18 +80,15 @@ describe('estimatedFeeToMaxFee()', () => {
 });
 
 describe('calculateContractAddressFromHash()', () => {
-  // This test just show how to use calculateContractAddressFromHash for new devs
-
+  const ethAddress = '0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7';
+  const daiAddress = '0x03e85bfbb8e2a42b7bead9e88e9a1b19dbccf661471061807292120462396ec9';
+  const factoryAddress = '0x249827618A01858A72B7D04339C47195A324D20D6037033DFE2829F98AFF4FC';
+  const classHash = '0x55187E68C60664A947048E0C9E5322F9BF55F7D435ECDCF17ED75724E77368F';
+  // Any type of salt can be used. It depends on the dApp what kind of salt it wants to use.
+  const salt = ec.starkCurve.pedersen(ethAddress, daiAddress);
+
+  // This test just shows how to use calculateContractAddressFromHash for new devs
   test('calculated contract address should match the snapshot', () => {
-    const ethAddress = '0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7';
-
-    const daiAddress = '0x03e85bfbb8e2a42b7bead9e88e9a1b19dbccf661471061807292120462396ec9';
-    const factoryAddress = '0x249827618A01858A72B7D04339C47195A324D20D6037033DFE2829F98AFF4FC';
-    const classHash = '0x55187E68C60664A947048E0C9E5322F9BF55F7D435ECDCF17ED75724E77368F';
-
-    // Any type of salt can be used. It depends on the dApp what kind of salt it wants to use.
-    const salt = ec.starkCurve.pedersen(ethAddress, daiAddress);
-
     const res = hash.calculateContractAddressFromHash(
       salt,
       classHash,
@@ -101,6 +100,20 @@ describe('calculateContractAddressFromHash()', () => {
       `"0x36dc8dcb3440596472ddde11facacc45d0cd250df764ae7c3d1a360c853c324"`
     );
   });
+
+  test('output should be bound', () => {
+    const starkCurveSpy = jest.spyOn(starkCurve, 'pedersen');
+    starkCurveSpy.mockReturnValue(num.toHex(constants.ADDR_BOUND + 1n));
+    const res = hash.calculateContractAddressFromHash(
+      salt,
+      classHash,
+      [ethAddress, daiAddress, factoryAddress],
+      factoryAddress
+    );
+    expect(starkCurveSpy).toHaveBeenCalled();
+    expect(BigInt(res)).toBeLessThan(constants.ADDR_BOUND);
+    starkCurveSpy.mockRestore();
+  });
 });
 
 describe('new Block()', () => {

src/channel/rpc_0_6.ts

@@ -240,7 +240,9 @@ export class RpcChannel {
     const retryInterval = options?.retryInterval ?? 5000;
     const errorStates: any = options?.errorStates ?? [
       RPC.ETransactionStatus.REJECTED,
-      RPC.ETransactionExecutionStatus.REVERTED,
+      // TODO: commented out to preserve the long-standing behavior of "reverted" not being treated as an error by default
+      // should decide which behavior to keep in the future
+      // RPC.ETransactionExecutionStatus.REVERTED,
     ];
     const successStates: any = options?.successStates ?? [
       RPC.ETransactionExecutionStatus.SUCCEEDED,
@@ -266,14 +268,17 @@ export class RpcChannel {
           throw error;
         }
 
-        if (successStates.includes(executionStatus) || successStates.includes(finalityStatus)) {
-          onchain = true;
-        } else if (errorStates.includes(executionStatus) || errorStates.includes(finalityStatus)) {
+        if (errorStates.includes(executionStatus) || errorStates.includes(finalityStatus)) {
           const message = `${executionStatus}: ${finalityStatus}`;
           const error = new Error(message) as Error & { response: RPC.TransactionStatus };
           error.response = txStatus;
           isErrorState = true;
           throw error;
+        } else if (
+          successStates.includes(executionStatus) ||
+          successStates.includes(finalityStatus)
+        ) {
+          onchain = true;
         }
       } catch (error) {
         if (error instanceof Error && isErrorState) {

src/constants.ts

@@ -16,9 +16,12 @@ export { ETransactionVersion as TRANSACTION_VERSION };
 
 export const ZERO = 0n;
 export const MASK_250 = 2n ** 250n - 1n; // 2 ** 250 - 1
-export const MASK_251 = 2n ** 251n;
 export const API_VERSION = ZERO;
 
+// based on: https://github.com/starkware-libs/cairo-lang/blob/v0.12.3/src/starkware/starknet/common/storage.cairo#L3
+export const MAX_STORAGE_ITEM_SIZE = 256n;
+export const ADDR_BOUND = 2n ** 251n - MAX_STORAGE_ITEM_SIZE;
+
 export enum BaseUrl {
   SN_MAIN = 'https://alpha-mainnet.starknet.io',
   SN_GOERLI = 'https://alpha4.starknet.io',

src/utils/address.ts

@@ -1,7 +1,7 @@
 /* eslint-disable no-bitwise */
 import { hexToBytes } from '@noble/curves/abstract/utils';
 
-import { MASK_251, ZERO } from '../constants';
+import { ADDR_BOUND, ZERO } from '../constants';
 import { BigNumberish } from '../types';
 import { addHexPrefix, removeHexPrefix } from './encode';
 import { keccakBn } from './hash';
@@ -12,7 +12,7 @@ export function addAddressPadding(address: BigNumberish): string {
 }
 
 export function validateAndParseAddress(address: BigNumberish): string {
-  assertInRange(address, ZERO, MASK_251, 'Starknet Address');
+  assertInRange(address, ZERO, ADDR_BOUND - 1n, 'Starknet Address');
 
   const result = addAddressPadding(address);
 

src/utils/calldata/index.ts

@@ -164,8 +164,8 @@ export class CallData {
         const oe = Array.isArray(o) ? [o.length.toString(), ...o] : o;
         return Object.entries(oe).flatMap(([k, v]) => {
           let value = v;
-          if (isLongText(value)) value = splitLongString(value);
           if (k === 'entrypoint') value = getSelectorFromName(value);
+          else if (isLongText(value)) value = splitLongString(value);
           const kk = Array.isArray(oe) && k === '0' ? '$$len' : k;
           if (isBigInt(value)) return [[`${prefix}${kk}`, felt(value)]];
           if (Object(value) === value) {

src/utils/hash/classHash.ts

@@ -4,7 +4,7 @@
 
 import { poseidonHashMany } from '@scure/starknet';
 
-import { API_VERSION } from '../../constants';
+import { ADDR_BOUND, API_VERSION } from '../../constants';
 import {
   BigNumberish,
   Builtins,
@@ -49,13 +49,14 @@ export function calculateContractAddressFromHash(
 
   const CONTRACT_ADDRESS_PREFIX = felt('0x535441524b4e45545f434f4e54524143545f41444452455353'); // Equivalent to 'STARKNET_CONTRACT_ADDRESS'
 
-  return computeHashOnElements([
+  const hash = computeHashOnElements([
     CONTRACT_ADDRESS_PREFIX,
     deployerAddress,
     salt,
     classHash,
     constructorCalldataHash,
   ]);
+  return toHex(BigInt(hash) % ADDR_BOUND);
 }
 
 function nullSkipReplacer(key: string, value: any) {