Investigate if we should keep Declare cheatcode

[cptartur]

software-mansion/protostar#2170

@MaksymilianDemitraszek suggested that we should remove declare cheatcode entirely, and implicitly declare all contracts from the project when running a test runner.

This might be a good approach, but there are some open questions:

• how should declaring work if the user doesn't know the class hash
• how to actually get the class hash of declared contract
• what in the case where user doesn't want to have their contract declared automatically for whatever testing reasons

[glihm]

Interesting point @cptartur and @MaksymilianDemitraszek.

I would also tend to say that, declare can be implicit since it's not a starknet syscall and it's not a logic someone can implement in a contract. On-chain, we cannot test for something declared or not and only class_hash is used, no contract names. It just panic if we provide an undeclared class hash. Which can be tested easily by providing a random value for a class hash.

Also, the hard limitation I see with declare is that it implies a limitation to 31 chars, but Cairo does not have this limitation and contracts name may be longer than that.

As you mentioned @cptartur, what then can be the better UX to get this class hash back from code. As we will again have the 31 chars limitation (until long strins).

But waiting the long string, we can have a get_class_hash_from_name('MyContract') equivalent, and/or directly:

// One can pass a valid class hash, or a short string with cairo contract name
// and we lookup first if the short string match a contract name?
// This avoid having an other field contract_name to init...
let prepared = PreparedContract {
    class_hash: 'MyContract',
    constructor_calldata: @ArrayTrait::new()
};

let contract_address = deploy(prepared).unwrap();
assert(get_class_hash_from_name('MyContract') == get_class_hash(contract_address));

This removes the declare, but is this better at UX level? I don't know. :/

An other approach, do you think we can use a search/replace in the code to inject the class hash where a use may require it? I have to review the exact path of starknet foundry handles the files, but if we load them as string, this can be a solution.

But this then introduces an other semantic and I'm not sure if it's the way to go.

Here is an example of what it can look like:

let class_hash = __SNFORGE_CLASS_HASH__(MyContract);
// ...

[MaksymilianDemitraszek]

This removes the declare, but is this better at UX level? I don't know. :/

After we introduce state forking somebody may be willing to deploy contracts that they may not have deployed themselves, IMO it is better to keep it consistent.

I thought about a UX using a compiler plugin and dispatchers so class hash can be extracted from dispatcher static field

deploy(ContractADispatcher::CLASS_HASH)

[cptartur]

With the changes introduced in #433 declare becomes more important as it now returns a structure with methods for deploying and calculating address. How can this integrate with deploying contracts from forked state I'm not sure currently, but this will have to be discussed.

This removes the declare, but is this better at UX level? I don't know. :/

I'm not sure about this UX wise. Since short strings get evaluated to felt values anyways, I'm not sure if it would be possible to distinguish if user passed a name of the contract or the value. This could be done with adding separate constructing methods to the PreparedContract but then the UX gets even more complicated.

An other approach, do you think we can use a search/replace in the code to inject the class hash where a use may require it? I have to review the exact path of starknet foundry handles the files, but if we load them as string, this can be a solution.

The files are compiled to sierra by scarb at an very early stage and then we are dealing with the compiled artefacts. Preprocessing like that would be possible but it would have to be added as an step before scarb compilation.