-
Notifications
You must be signed in to change notification settings - Fork 67
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
Comments
Hey @z0r0z But I guess having them ordered by the most to less used is more intuitive than ordering them by the EIP introduced. Let's discuss 🚀 @z0r0z @CJ42 @skimaharvey |
thanks @YamenMerhi Is it the case that create would be used more often than delegate call for a smart account? |
@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. |
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? |
operationType ordering could be more intuitive:
replacing:
with:
0
forcall
1
fordelegatecall
2
forstaticcall
3
forcreate
4
forcreate2
The text was updated successfully, but these errors were encountered: