Add deployer preset

[martriay]

Fixes #384. This PR also centralizes IACCOUNT_ID across tests in tests/utils.py.

Current UniversalDeployerContract implementation results in address 0x414aa016992e868fa22e21bd104757e280c83ecb260fe9bccd1caee2f7f590e if deployed from zero.

EDIT: latest address is 0x041a78e741e5af2fec34b695679bc6891742439f7afb8484ecd7766661ad02bf

[frangio]

This is in the utils/constants directory. Is that intentional? The category seems wrong to me.

[martriay]

This is in the utils/constants directory. Is that intentional? The category seems wrong to me.

It was wrong, thanks.

[andrew-fleming]

Excellent PR! Left one tiny suggestion. Also, should we pass deployer documentation off until the next release?

[martriay]

I'm not sure. On one hand I wouldn't rush releasing this feature, on the other I'm uncertain of how documented it should be. I'm not sure if it's meant to be used for users either, I just thought having it in the library would make sense for standardization purposes, and probably aid tooling (i.e. the devnet taking this implementation to pre-deploy an UDC so it's available).

Thoughts?

[andrew-fleming]

Hm yeah, I agree. I suppose if later on we decide to include it or some mention of it, we can just point to the proposal in Shamans

[andrew-fleming]

Improvements look great, @martriay! Left a few tiny suggestions

[ericnordelo]

Looking good! I left a small suggestion and a question regarding reference handling.

[ericnordelo]

I think we don't need to increment ap here with tempvar when a reference is enough:

Suggested change

	tempvar pedersen_ptr = _pedersen;
	let pedersen_ptr = _pedersen;

Actually, the reference shouldn't even be necessary, from my understanding, if you rename _perdersen to pedersen_ptr rebinding the reference (because in both branches we should have [ap - 1]). But, for some reason, when doing this, the line emitting the event seems to revoke the reference. This is an expected behavior @bbrandtom ?

[ericnordelo]

I mean, this works (as I expect):

    if (unique == TRUE) {
        let (unique_salt) = hash2{hash_ptr=pedersen_ptr}(deployer, salt);
        _salt = unique_salt;
        from_zero = FALSE;
        tempvar pedersen_ptr = pedersen_ptr;
    } else {
        _salt = salt;
        from_zero = TRUE;
        tempvar pedersen_ptr = pedersen_ptr;
    }

    let (address) = deploy(
        class_hash=classHash,
        contract_address_salt=_salt,
        constructor_calldata_size=calldata_len,
        constructor_calldata=calldata,
        deploy_from_zero=from_zero,
    );

    return (address=address)

While this doesn't (I would expect it to work too):

    if (unique == TRUE) {
        let (unique_salt) = hash2{hash_ptr=pedersen_ptr}(deployer, salt);
        _salt = unique_salt;
        from_zero = FALSE;
        tempvar pedersen_ptr = pedersen_ptr;
    } else {
        _salt = salt;
        from_zero = TRUE;
        tempvar pedersen_ptr = pedersen_ptr;
    }

    let (address) = deploy(
        class_hash=classHash,
        contract_address_salt=_salt,
        constructor_calldata_size=calldata_len,
        constructor_calldata=calldata,
        deploy_from_zero=from_zero,
    );


    ContractDeployed.emit(
        address=address,
        deployer=deployer,
        unique=unique,
        classHash=classHash,
        calldata_len=calldata_len,
        calldata=calldata,
        salt=salt
    );

    return (address=address)

[martriay]

@ericnordelo what do you mean by it doesn't work? what exact error are you getting? and i'm not sure the point you're trying to make by removing the event emission

[ericnordelo]

The point is that the event emission is revoking the pedersen_ptr reference in the example I'm presenting when I think it shouldn't. And when you rebind the reference to the same ap reference, it works again (with the event). This looks like a potential compiler issue.

[ArielElp]

@ericnordelo What's the rebinding that works with the event? My guess is that the revocation happens due to an arbitrary change in ap caused by a function call. Usually (after 0.5.0 IIRC), such revocations are solved implicitly by adding a local variable definition prior to the call. However, here the function itself is generated by the compiler (emit is replaced by a code using the emit_event syscall) so it might be missed.

[ericnordelo]

Hi @ArielElp, the rebinding that works with the event is the one after the else block in this example:

    (...)
    if (unique == TRUE) {
        let (unique_salt) = hash2{hash_ptr=pedersen_ptr}(deployer, salt);
        _salt = unique_salt;
        from_zero = FALSE;
        tempvar pedersen_ptr = pedersen_ptr;
    } else {
        _salt = salt;
        from_zero = TRUE;
        tempvar pedersen_ptr = pedersen_ptr;
    }
    
    // This is the rebinding:
    let pedersen_ptr = pedersen_ptr;

    let (address) = deploy(
        class_hash=classHash,
        contract_address_salt=_salt,
        constructor_calldata_size=calldata_len,
        constructor_calldata=calldata,
        deploy_from_zero=from_zero,
    );

    ContractDeployed.emit(
        address=address,
        deployer=deployer,
        unique=unique,
        classHash=classHash,
        calldata_len=calldata_len,
        calldata=calldata,
        salt=salt
    );

    return (address=address)

If you remove that rebinding, the reference is revoked by the emit call. My question is, why is it not being revoked when we just rebind the reference from ap to itself? (the same reference to the exact ap location).

That rebinding to itself is avoiding the reference revocation in the emit call, and I think it should work without that line too, but is being revoked without it, what am I missing?