Repository: 0xs34n/starknet.js
Pull request #676 (2023-07-06 10:21:35)

Contract Factory
## Motivation and Resolution
BREAKING: contractFactory constructor signature
Update Contract Factory to work with Cairo 1

- Fix two failing tests

## Usage related changes
```diff
- new contractFactory(par1, par2, ...)
+ new contractFactory(obj)
```

## Checklist:

- [ ] Performed a self-review of the code
- [ ] Rebased to the last commit of the target branch (or merged it into my branch)
- [ ] Linked the issues which this PR resolves
- [ ] Documented the changes in code (API docs will be generated automatically)
- [ ] Updated the tests
- [ ] All tests are passing



Patch:
diff --git a/src/account/default.ts b/src/account/default.ts
index d9039f09..6f81513d 100644
--- a/src/account/default.ts
+++ b/src/account/default.ts
@@ -37,7 +37,6 @@ import {
   Signature,
   SimulateTransactionDetails,
   SimulateTransactionResponse,
-  TransactionStatus,
   TransactionType,
   TypedData,
   UniversalDeployerContractPayload,
@@ -399,9 +398,7 @@ export class Account extends Provider implements AccountInterface {
     details?: InvocationsDetails | undefined
   ): Promise<DeployContractUDCResponse> {
     const deployTx = await this.deploy(payload, details);
-    const txReceipt = await this.waitForTransaction(deployTx.transaction_hash, {
-      successStates: [TransactionStatus.ACCEPTED_ON_L2],
-    });
+    const txReceipt = await this.waitForTransaction(deployTx.transaction_hash);
     return parseUDCEvent(txReceipt);
   }
 
@@ -412,9 +409,7 @@ export class Account extends Provider implements AccountInterface {
     const { constructorCalldata, salt, unique } = payload;
     let declare = await this.declareIfNot(payload, details);
     if (declare.transaction_hash !== '') {
-      const tx = await this.waitForTransaction(declare.transaction_hash, {
-        successStates: [TransactionStatus.ACCEPTED_ON_L2],
-      });
+      const tx = await this.waitForTransaction(declare.transaction_hash);
       declare = { ...declare, ...tx };
     }
     const deploy = await this.deployContract(
diff --git a/src/account/interface.ts b/src/account/interface.ts
index b899e292..ab5c043d 100644
--- a/src/account/interface.ts
+++ b/src/account/interface.ts
@@ -210,9 +210,11 @@ export abstract class AccountInterface extends ProviderInterface {
    * Internal wait for L2 transaction, do not support multicall
    * Method will pass even if contract is already declared (internal using DeclareIfNot)
    *
-   * @param  containing
+   * @param payload
    * - contract: compiled contract code
-   * - classHash: computed class hash of compiled contract
+   * - [casm=cairo1]: CairoAssembly | undefined;
+   * - [compiledClassHash]: string | undefined;
+   * - [classHash]: computed class hash of compiled contract
    * - [constructorCalldata] contract constructor calldata
    * - [salt=pseudorandom] deploy address salt
    * - [unique=true] ensure unique salt
diff --git a/src/contract/contractFactory.ts b/src/contract/contractFactory.ts
index d92d9c38..40928653 100644
--- a/src/contract/contractFactory.ts
+++ b/src/contract/contractFactory.ts
@@ -1,35 +1,61 @@
 import { AccountInterface } from '../account';
-import { Abi, ArgsOrCalldataWithOptions, CompiledContract, ValidateType } from '../types';
+import {
+  Abi,
+  ArgsOrCalldataWithOptions,
+  CairoAssembly,
+  CompiledContract,
+  ValidateType,
+} from '../types';
 import assert from '../utils/assert';
 import { CallData } from '../utils/calldata';
 import { Contract, getCalldata, splitArgsAndOptions } from './default';
 
+export type ContractFactoryParams = {
+  compiledContract: CompiledContract;
+  account: any;
+  casm?: CairoAssembly;
+  classHash?: string;
+  compiledClassHash?: string;
+  abi?: Abi;
+};
+
 export class ContractFactory {
+  compiledContract: CompiledContract;
+
+  account: AccountInterface;
+
   abi: Abi;
 
-  compiledContract: CompiledContract;
+  classHash?: string;
 
-  classHash: string;
+  casm?: CairoAssembly;
 
-  account: AccountInterface;
+  compiledClassHash?: string;
 
   private CallData: CallData;
 
-  constructor(
-    compiledContract: CompiledContract,
-    classHash: string,
-    account: AccountInterface,
-    abi: Abi = compiledContract.abi // abi can be different from the deployed contract ie for proxy contracts
-  ) {
-    this.abi = abi;
-    this.compiledContract = compiledContract;
-    this.account = account;
-    this.classHash = classHash;
-    this.CallData = new CallData(abi);
+  /**
+   * @param params CFParams
+   *  - compiledContract: CompiledContract;
+   *  - account: AccountInterface;
+   *  - casm?: CairoAssembly;
+   *  - classHash?: string;
+   *  - compiledClassHash?: string;
+   *  - abi?: Abi;
+   */
+  constructor(params: ContractFactoryParams) {
+    this.compiledContract = params.compiledContract;
+    this.account = params.account;
+    this.casm = params.casm;
+    this.abi = params.abi ?? params.compiledContract.abi;
+    this.classHash = params.classHash;
+    this.compiledClassHash = params.compiledClassHash;
+    this.CallData = new CallData(this.abi);
   }
 
   /**
    * Deploys contract and returns new instance of the Contract
+   * If contract is not declared it will first declare it, and then deploy
    *
    * @param args - Array of the constructor arguments for deployment
    * @param options (optional) Object - parseRequest, parseResponse, addressSalt
@@ -52,6 +78,9 @@ export class ContractFactory {
       deploy: { contract_address, transaction_hash },
     } = await this.account.declareAndDeploy({
       contract: this.compiledContract,
+      casm: this.casm,
+      classHash: this.classHash,
+      compiledClassHash: this.compiledClassHash,
       constructorCalldata,
       salt: options.addressSalt,
     });
diff --git a/src/utils/calldata/index.ts b/src/utils/calldata/index.ts
index c32337f4..1b934d90 100644
--- a/src/utils/calldata/index.ts
+++ b/src/utils/calldata/index.ts
@@ -18,7 +18,7 @@ import { getSelectorFromName } from '../selector';
 import { isLongText, splitLongString } from '../shortString';
 import { felt, isLen } from './cairo';
 import formatter from './formatter';
-import { createAbiParser } from './parser';
+import { createAbiParser, isNoConstructorValid } from './parser';
 import { AbiParserInterface } from './parser/interface';
 import orderPropsByAbi from './propertyOrder';
 import { parseCalldataField } from './requestParser';
@@ -69,6 +69,10 @@ export class CallData {
         : abi.name === method && abi.type === 'function'
     ) as FunctionAbi;
 
+    if (isNoConstructorValid(method, args, abiMethod)) {
+      return;
+    }
+
     // validate arguments length
     const inputsLength = this.parser.methodInputsLength(abiMethod);
     if (args.length !== inputsLength) {
@@ -98,6 +102,10 @@ export class CallData {
   public compile(method: string, argsCalldata: RawArgs): Calldata {
     const abiMethod = this.abi.find((abi) => abi.name === method) as FunctionAbi;
 
+    if (isNoConstructorValid(method, argsCalldata, abiMethod)) {
+      return [];
+    }
+
     let args: RawArgsArray;
     if (Array.isArray(argsCalldata)) {
       args = argsCalldata;
diff --git a/src/utils/calldata/parser/index.ts b/src/utils/calldata/parser/index.ts
index 75614fe9..76a59937 100644
--- a/src/utils/calldata/parser/index.ts
+++ b/src/utils/calldata/parser/index.ts
@@ -1,4 +1,4 @@
-import { Abi } from '../../../types';
+import { Abi, FunctionAbi, RawArgs } from '../../../types';
 import { isCairo1Abi } from '../cairo';
 import { AbiParserInterface } from './interface';
 import { AbiParser1 } from './parser-0-1.1.0';
@@ -20,3 +20,12 @@ export function getAbiVersion(abi: Abi) {
   if (isCairo1Abi(abi)) return 1;
   return 0;
 }
+
+export function isNoConstructorValid(
+  method: string,
+  argsCalldata: RawArgs,
+  abiMethod?: FunctionAbi
+) {
+  // No constructor in abi and validly empty args
+  return method === 'constructor' && !abiMethod && !argsCalldata.length;
+}


Test patch:
diff --git a/__tests__/cairo1.test.ts b/__tests__/cairo1.test.ts
index 78206b2c..a347d27f 100644
--- a/__tests__/cairo1.test.ts
+++ b/__tests__/cairo1.test.ts
@@ -6,6 +6,7 @@ import {
   Calldata,
   CompiledSierra,
   Contract,
+  ContractFactory,
   DeclareDeployUDCResponse,
   RawArgsArray,
   RawArgsObject,
@@ -59,6 +60,16 @@ describeIfDevnet('Cairo 1 Devnet', () => {
       expect(cairo1Contract).toBeInstanceOf(Contract);
     });
 
+    test('ContractFactory on Cairo1', async () => {
+      const c1CFactory = new ContractFactory({
+        compiledContract: compiledHelloSierra,
+        casm: compiledHelloSierraCasm,
+        account,
+      });
+      const cfContract = await c1CFactory.deploy();
+      expect(cfContract).toBeInstanceOf(Contract);
+    });
+
     xtest('validate TS for redeclare - skip testing', async () => {
       const cc0 = await account.getClassAt(dd.deploy.address);
       const cc0_1 = await account.getClassByHash(toHex(dd.declare.class_hash));
diff --git a/__tests__/cairo1v2.test.ts b/__tests__/cairo1v2.test.ts
index ade604b4..cb12dd80 100644
--- a/__tests__/cairo1v2.test.ts
+++ b/__tests__/cairo1v2.test.ts
@@ -290,8 +290,8 @@ describe('Cairo 1 Devnet', () => {
         [1, 2],
         [3, 4],
       ]);
-      const tx1 = await cairo1Contract.array2d_ex(cd);
       await account.waitForTransaction(tx.transaction_hash);
+      const tx1 = await cairo1Contract.array2d_ex(cd);
       await account.waitForTransaction(tx1.transaction_hash);
 
       const result0 = await cairo1Contract.array2d_felt([
diff --git a/__tests__/contract.test.ts b/__tests__/contract.test.ts
index 03cff4ae..660108bf 100644
--- a/__tests__/contract.test.ts
+++ b/__tests__/contract.test.ts
@@ -214,12 +214,12 @@ describe('contract module', () => {
       });
     });
     test('deployment of new contract', async () => {
-      const factory = new ContractFactory(compiledErc20, classHash, account);
+      const factory = new ContractFactory({ compiledContract: compiledErc20, classHash, account });
       const erc20 = await factory.deploy('Token', 'ERC20', wallet);
       expect(erc20).toBeInstanceOf(Contract);
     });
     test('wait for deployment transaction', async () => {
-      const factory = new ContractFactory(compiledErc20, classHash, account);
+      const factory = new ContractFactory({ compiledContract: compiledErc20, classHash, account });
       const contract = await factory.deploy(
         CallData.compile({
           name: encodeShortString('Token'),
@@ -230,7 +230,7 @@ describe('contract module', () => {
       expect(contract.deployed()).resolves.not.toThrow();
     });
     test('attach new contract', async () => {
-      const factory = new ContractFactory(compiledErc20, classHash, account);
+      const factory = new ContractFactory({ compiledContract: compiledErc20, classHash, account });
       const erc20 = factory.attach(erc20Address);
       expect(erc20).toBeInstanceOf(Contract);
     });
@@ -245,7 +245,7 @@ describe('Complex interaction', () => {
   let factory: ContractFactory;
 
   beforeAll(async () => {
-    factory = new ContractFactory(compiledErc20Echo, classHash, account);
+    factory = new ContractFactory({ compiledContract: compiledErc20Echo, classHash, account });
     erc20Echo20Contract = await factory.deploy(
       'Token',
       'ERC20',
diff --git a/__tests__/sequencerProvider.test.ts b/__tests__/sequencerProvider.test.ts
index 5ba2c4a6..060936fb 100644
--- a/__tests__/sequencerProvider.test.ts
+++ b/__tests__/sequencerProvider.test.ts
@@ -125,15 +125,12 @@ describeIfSequencer('SequencerProvider', () => {
     });
 
     test('estimate message fee', async () => {
-      const estimation = await sequencerProvider.estimateMessageFee(
-        {
-          from_address: L1_ADDRESS,
-          to_address: l1l2ContractAddress,
-          entry_point_selector: 'deposit',
-          payload: ['556', '123'],
-        },
-        'latest'
-      );
+      const estimation = await sequencerProvider.estimateMessageFee({
+        from_address: L1_ADDRESS,
+        to_address: l1l2ContractAddress,
+        entry_point_selector: 'deposit',
+        payload: ['556', '123'],
+      });
       expect(estimation).toEqual(
         expect.objectContaining({
           overall_fee: expect.anything(),