feat: isClassDeclared createBulkInvocations, fix cairo0 test

[tabaktoni]

Motivation and Resolution

Fix #1209

• Implement TransactionType Fix from chore: revert companion pattern except constants #1193 to be able to use TS Invocations properly

Usage related changes

New
Test if class is already declared from ContractClassIdentifier

provider.isClassDeclared(contractClassIdentifier: ContractClassIdentifier, blockIdentifier?: BlockIdentifier)

Build bulk invocations with auto-detect declared class and declare order sort
provider.createBulkInvocations(invocations: Invocations)

provider.prepareInvocations(invocations: Invocations)

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

[penovicp]

Two minor nitpicks, in general lgtm

[penovicp]

I would suggest renaming the method, I feel like users might make a mistake and think it's similar to accountInvocationsFactory(). Admittedly I haven't come up with a surefire alternative, maybe something like sanitizeInvocations().

[penovicp]

Could be filtered for instances of LibraryError and re-throw otherwise.

[PhilippeR26]

Just little naming concerns.

[PhilippeR26]

Why an object containing the classHash? Seems complicated.
I think that the easiest way for the user is something like this :

export type ContractClassIdentifier = DeclareContractPayload | BigNumberish;

[tabaktoni]

Because it would be ambiguous.
Class can be identified by classHash or by contract, in case of BigNumberish it can be both and non-specific.

[PhilippeR26]

For me, a class is only identified by its hash, and nothing else.

[PhilippeR26]

And if you have the address of an instance of a class, you have already the answer of the
question is classDeclared.
It's more than weird to ask if a parent is existing if you have already one of its children....

[tabaktoni]

And if you have the address of an instance of a class, you have already the answer of the question is classDeclared. It's more than weird to ask if a parent is existing if you have already one of its children....

Agree, and this does not contain an address. What address are you referring to?

[tabaktoni]

It does not deal with Contract instance nor addresses, just with Contract Class aka. definition classHash

ContractClassIdentifier is

{
  contract: CompiledContract | string;
  classHash?: string;
  casm?: CompiledSierraCasm;
  compiledClassHash?: string;
};

In this case it covers cases for:

1. identifying class from its source code by computing classHash from contract
2. optional zero computing and classHash is already provided
3. less computing by providing optional params for classHash

or 
{ classHash: string }

This case cover providing non-optional classHash

So that in the end, you can get classHash which is in essence ContractClassIdentifier.

Changing it to

export type ContractClassIdentifier = DeclareContractPayload | BigNumberish;

In this case, BigNumberish represents classHash but in the source, we used mixed cases sometimes string sometimes BigNumberish.

Solutions:
1.
Another case is that we set | { classHash: BigNumberish }.
This case is less ambiguous and I think we can do it.
2. create type ClassHashIdentifier: BigNumberish
and than do:

export type ContractClassIdentifier = DeclareContractPayload | ClassHashIdentifier;

[tabaktoni]

For me, a class is only identified by its hash, and nothing else.

Yes but you can provide that classHash or you can provide source code and we can compute classHash from it.
So both source code and classHash are identifiers

[PhilippeR26]

I agree with @penovicp ; all these public functions with bulk in their names are not understood by the users.
Such names are OK for internal functions, but are problematic for basic users usage.
We have already this problem with getEstimateFeeBulk & parseFeeEstimateBulkResponse.
My proposal : buildInvocations

[tabaktoni]

prepareBulkInvocations ?
prepareInvocations ?

[PhilippeR26]

OK for me.

[github-actions]

🎉 This issue has been resolved in version 6.14.0 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀