Update IACCOUNT_ID to match the interface for Cairo 1

[sgc-code]

🧐 Motivation

With Cairo 1, there are some changes to the IAccount interface. To be used with ERC-165. The main change is the use of snake_case for the method names.

📝 Details

The old IACCOUNT_ID was calculated here
#449 (comment)

I suggest the following change

interface IAccount {

    struct Call {
        address to;
        bytes4 selector;
        bytes data;
    }

    function supports_interface(bytes4) external;
    function is_valid_signature(bytes32, bytes32[] calldata) external;
    function __execute__(Call[] calldata) external;
    function __validate__(Call[] calldata) external;
    function __validate_declare__(bytes32) external;
}

contract Selector {

    // returns 0xab3059ac
    function calculateSelector() public pure returns (bytes4) {
        IAccount i;
        return i.supports_interface.selector^
        i.is_valid_signature.selector ^
        i.__execute__.selector ^
        i.__validate__.selector ^
        i.__validate_declare__.selector;
    }
}

There is also __validate_deploy__ but i see that it was not included in the previous version, not sure if there was a reason to exclude it

[andrew-fleming]

Good suggestion! And if I recall correctly, __validate_deploy__ wasn't included in the account scope when the id was discussed, but I think that should be included as well. @martriay @juniset thoughts?

[martriay]

There's two reasons to not include it:

1. __validate_deploy__ is not required for a contract to be considered a valid account, so it doesn't need to be part of the standard interface
2. moreover, such function is meant to be called by the OS and not other contracts

[martriay]

Let's make sure to document this whenever this is settled.

[ericnordelo]

Shouldn't we be using Cairo types for generating the Id instead of Solidity types? The rationale for using Solidity types is that is easier to compute using remix or there is something else?

Wouldn't something like this be more appropriate?

interface IAccount {

    struct Call {
        ContractAddress, u32, Array<felt252>
    }

    function supports_interface(u32);
    function is_valid_signature(felt252, Array<felt252>);
    function __execute__(Array<Call>) external;
    function __validate__(Array<Call>) external;
    function __validate_declare__(felt252) external;
}

Being translated into these signatures:

functions = [
    'supports_interface(u32)',
    'is_valid_signature(felt252,Array<felt252>)',
    '__execute__(Array<(ContractAddress,u32,Array<felt252>)>)',
    '__validate__(Array<(ContractAddress,u32,Array<felt252>)>)',
    '__validate_declare__(felt252)'
]

And we can use this Python script for getting the final interface_id:

# pip install pysha3
import sha3

keccak_256 = sha3.keccak_256
functions = [
    'supports_interface(u32)',
    'is_valid_signature(felt252,Array<felt252>)',
    '__execute__(Array<(ContractAddress,u32,Array<felt252>)>)',
    '__validate__(Array<(ContractAddress,u32,Array<felt252>)>)',
    '__validate_declare__(felt252)'
]

def get_id(fn_signature):
    id = keccak_256(fn_signature.encode()).hexdigest()[0:8]
    print(fn_signature, id)
    return id

def main():
    interface_id = 0x0
    for fn in functions:
        interface_id ^= int(get_id(fn), 16)
    print(hex(interface_id))

if __name__ == '__main__':
    main()

Which gives us: 0x15fd598f

I think it may be worth even defining a SIP-165 following the same structure of the ERC, to define if we should use u256 when is present, or a (felt252,felt252) tuple for example. This is an important enough subject to push for consistency IMO because is not just the accounts, are all the interfaces.

[ericnordelo]

Apart from if we use Solidity or Cairo types, how should we coordinate these interface migrations @martriay? Should I tag @juniset from Argent and someone specific from Braavos too?

After this change current ERC721 contracts won't be able to send NFTs to new Accounts, if the latter doesn't implement the IERC721_RECEIVER interface (or adds the old ID as well), should we recommend the usage of this interface in Accounts, given that we will potentially update the Account interface again until a proper standard is achieved?

[martriay]

The rationale for using Solidity types is that is easier to compute using remix or there is something else?

The rationale was to follow ERC165.

I think it may be worth even defining a SIP-165 following the same structure of the ERC

I'm not sure whether a SNIP* can be non-sequential. I guess we could work on such a standard, not entirely convinced about the benefits.

Apart from if we use Solidity or Cairo types, how should we coordinate these interface migrations @martriay? Should I tag @juniset from Argent and someone specific from Braavos too?

We can always start discussions in here and tag relevant people, but ultimately the place for network-wide discussion should be the Shaman's forum, where SNIPs also live. Note that @sgc-code works for Argent.

or adds the old ID as well

What it was discussed and implemented before is for tokens to support old account IDs, although as you point out it's not entirely future proof. I think adding the 721 or 1155 receiver interfaces incurs in a similar issue since accounts won't be able to handle new/different standards.

All in all I think introspection is important for accounts and other contracts to work with, so I would lean towards improving that. In the early days it didn't look as a great idea, but after achieving some maturity (especially after regenesis) it might make sense for Accounts to be interface retrocompatible.

Is this a good time to make such commitment?

[ericnordelo]

The rationale was to follow ERC165.

Do we have a defined mapping between Solidity and Cairo types for this?

I guess we could work on such a standard, not entirely convinced about the benefits.

If a Cairo developer tries to use ERC165 with his Cairo interfaces, today he could find the specification on how to generate the interface_id? Is there a place where he could find how to map the Cairo types to Solidity that he can be sure is the standard way for Starknet? If the answer is no I think a standard would help, and we could avoid mapping to Solidity using Cairo types directly with something like the previous proposal.

I'm not sure whether a SNIP* can be non-sequential.

We could use a different number of course.

All in all I think introspection is important for accounts and other contracts to work with, so I would lean towards improving that. In the early days it didn't look as a great idea, but after achieving some maturity (especially after regenesis) it might make sense for Accounts to be interface retrocompatible.

Is this a good time to make such commitment?

I think we may need to make a distinction between this interface we are defining and the term Account. Even if OZ, Argent, and Braavos follow this interface, other parties can implement valid accounts without the is_valid_signature function for example, so it feels a bit weird to me that we are defining what is the interface of an Account in a network where we have AA by default, with functions that could be optional. Maybe StandardAccount could be a better terminology for this interface, or is Straknet enforcing our interface for everyone in the near future? Interested in your thoughts @martriay @juniset.

After this, I think making Account retrocompatible sounds good, but this won't avoid changes in the interface id by potentially adding new methods, or changing functions signature. Is part of what you have in mind supporting the previous ids by default?

[Amxx]

Small note, I you go with the solidity style for function signature, than rather than doing the xor manually, you should use

interface IAccount {
    struct Call {
        address to;
        bytes4 selector;
        bytes data;
    }
    function supports_interface(bytes4) external;
    function is_valid_signature(bytes32, bytes32[] calldata) external;
    function __execute__(Call[] calldata) external;
    function __validate__(Call[] calldata) external;
    function __validate_declare__(bytes32) external;
}

contract Selector {
    // returns 0xab3059ac
    bytes4 public constant interfaceId = type(IAccount).interfaceId;
}

[Amxx]

After this change current ERC721 contracts won't be able to send NFTs to new Accounts, if the latter doesn't implement the IERC721_RECEIVER interface

Its more complexe than that.

• If the receiver is an account, and exposes the ERC165_ACCOUNT_ID, then you can safe transfer an ERC-721 to that account
• If the receiver is an account that does not expose ERC165_ACCOUNT_ID, but that exposes IERC721_RECEIVER_ID, then you can also safe transfer an NFT to that account
• If neither of these is true, then safe transfers will fail, but you will still be able to send an NFT to the address you want using the "normal" transfer.

[Amxx]

One thing to consider is wether we want

function __execute__(Array<Call>) external;

and

function __execute__(Span<Call>) external;

to produce the same id or not. If you use the "solidity array" syntax you won't be able to distinguish them.

[ericnordelo]

There are some things to consider for sure, and that's why I think a SNIP would help, but the most important thing to me is to settle on some standardization for interfaces (ERC-165 alone is not enough unless we have a strict mapping between types). And this is not just for Accounts ofc, is a wider issue. After that, I think we can settle on the Standard Account interface, because calling it just the Account interface looks like stating that account contracts not following this interface are not valid accounts, and this is not true, right?

[ericnordelo]

I created this post on the forum about defining this interface id standard for Starknet, feedback and comments are appreciated. We can continue the discussion about the Standard Account interface here.