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

make operationType order more intuitive #141

Open
z0r0z opened this issue Jul 7, 2022 · 4 comments
Open

make operationType order more intuitive #141

z0r0z opened this issue Jul 7, 2022 · 4 comments

Comments

@z0r0z
Copy link
Contributor

z0r0z commented Jul 7, 2022

operationType ordering could be more intuitive:

replacing:

image

with:

0 for call
1 for delegatecall
2 for staticcall
3 for create
4 for create2

@YamenMerhi
Copy link
Collaborator

Hey @z0r0z
I see your point on making the calls Operations first, then the create Operations.
But also in our case, it's from the most to less used operations, there could be an idea to order them by the number of EIP introduced. 0 for CALL, 1 for CREATE, 2 for DELEGATECALL (EIP-7), 3 for STATICCALL (EIP-214), and 4 for CREATE2 (EIP-1014).

But I guess having them ordered by the most to less used is more intuitive than ordering them by the EIP introduced.
The only thing that I am concerned about is having delegatecall as operation number 1, specially knowing the effect of delegatecall and how dangerous it is.

Let's discuss 🚀 @z0r0z @CJ42 @skimaharvey

@z0r0z
Copy link
Contributor Author

z0r0z commented Aug 2, 2022

thanks @YamenMerhi Is it the case that create would be used more often than delegate call for a smart account?

@CJ42
Copy link
Collaborator

CJ42 commented Aug 16, 2022

@z0r0z It's very more likely that a smart contract based account will be use to deploy more contracts, probably first using CREATE, and then as an alternative option via CREATE2.

STATICCALL and DELEGATECALL are placed at the end here, as we believe they are less likely to be used, but we still want to allow the possibility.

Firstly because STATICCALL is mainly used for getter functions, so you can call the getter function yourself directly from the Dapp interface or a contract Cheaper, no need to craft a payload to return something, if you can call the read-only function directly. But you could have a getter function protected by an onlyOwner modifier, where the owner is the smart contract based account. An example where it could be used.

For DELEGATECALL, we put it at the end for safety mainly. We believe it will be rarely used, and we discourage it mainly, unless you really know what you are doing.

@CJ42
Copy link
Collaborator

CJ42 commented Apr 22, 2023

I would quite like to re-open this discussion as it might be worth thinking if this ordering make sense.

We also put 2 for CREATE2 because of "2" in the opcode.

@z0r0z what do your prefer in the order you suggested? Why do you find the proposed new ordering more intuitive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants