Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: CASM runner deploy_syscall doesn't support external call to deployer in constructor #4488

Closed
enitrat opened this issue Nov 27, 2023 · 0 comments · Fixed by #4489
Closed
Labels
bug Something isn't working

Comments

@enitrat
Copy link
Contributor

enitrat commented Nov 27, 2023

Bug Report

Cairo version:
2.4.0-rc4

Current behavior:
The deploy function in the CASM runner in charge of deploying a contract first starts by executing the constructor of the contract, and only then registers the address of the contract in the starknet state.
This causes a problem when a Factory contract deploys a Child contract, and this Child needs to access a method from the Factory contract as an external call, as this will fail with a CONTRACT_NOT_DEPLOYED error.

Expected behavior:
The contract should be registered in starknet state before the execution of the constructor, so that Child contracts can call methods from their Factory.

Steps to reproduce:

Related code:

This is the part in the runner that is responsible for executing the constructor and setting the contract address

 // Call constructor if it exists.
        let (res_data_start, res_data_end) = if let Some(constructor) = &contract_info.constructor {
            let old_addrs = self
                .starknet_state
                .open_caller_context((deployed_contract_address.clone(), deployer_address));
            let res = self.call_entry_point(gas_counter, runner, constructor, calldata, vm);
            self.starknet_state.close_caller_context(old_addrs);
            match res {
                Ok(value) => value,
                Err(mut revert_reason) => {
                    fail_syscall!(revert_reason, b"CONSTRUCTOR_FAILED");
                }
            }
            
             // Set the class hash of the deployed contract before executing the constructor,
        // as the constructor could make an external call to this address.
        self.starknet_state
            .deployed_contracts
            .insert(deployed_contract_address.clone(), class_hash);

Reproducible example:

use core::result::ResultTrait;
use starknet::deploy_syscall;
use debug::PrintTrait;

#[starknet::interface]
trait IFactory<TContractState> {
    fn test(self: @TContractState) -> felt252;
}

#[starknet::interface]
trait IEmptyContract<TContractState> {
    fn initialize(ref self: TContractState) -> felt252;
}

#[starknet::contract]
mod Factory {
    use core::traits::TryInto;
    use starknet::{ClassHash, deploy_syscall};
    use array::ArrayTrait;
    use super::{IEmptyContractDispatcher, IEmptyContractDispatcherTrait};
    use debug::PrintTrait;
    #[storage]
    struct Storage {}

    #[constructor]
    fn constructor(ref self: ContractState, child_hash: ClassHash) {
        let calldata = array![];
        let (address, _) = deploy_syscall(child_hash.try_into().unwrap(), 0, calldata.span(), false)
            .expect('child deploy failed');
        // The child contract is deployed, but the following line fails without propagating an error.
        let res = IEmptyContractDispatcher { contract_address: address }.initialize();
        res.print();
    }

    #[abi(embed_v0)]
    impl FactoryImpl of super::IFactory<ContractState> {
        fn test(self: @ContractState) -> felt252 {
            'test'
        }
    }
}

#[starknet::contract]
mod EmptyContract {
    use starknet::account::{Call, AccountContract};
    use starknet::get_caller_address;
    use super::{IFactoryDispatcher, IFactoryDispatcherTrait};
    use debug::PrintTrait;

    #[storage]
    struct Storage {}

    #[abi(embed_v0)]
    impl EmptyContractImpl of super::IEmptyContract<ContractState> {
        fn initialize(ref self: ContractState) -> felt252 {
            let deployer_address = get_caller_address();
            // This fails, but secretly: nothing happens and no error is returned.
            // but the following line is not executed.
            let res = IFactoryDispatcher { contract_address: deployer_address }.test();
            'This line is not printed'.print();
            res
        }
    }
}

#[cfg(test)]
#[test]
fn test_deploy_in_construct() {
    let calldata = array![EmptyContract::TEST_CLASS_HASH.try_into().unwrap()];
    deploy_syscall(Factory::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant