Open sorpaas opened 5 years ago
The format of this proposal is missing a few suggested paragraphs from the [eip-X.md](the https://github.com/ethereum/EIPs/blob/master/eip-X.md) template. Please go over that and add the paragraphs here possible.
If I understand Alternative Design correctly, it's only init code that has prefix, but not the deployed code. Maybe it's worth to clarify this.
Contract Execution VM version used in contract execution is determined via calling address (I_a in yellow paper).
This sentence is not quite clear. Does it mean that it's determined by the version
field of the executed contract? (I guess the edge case here is DELEGATECALL
& CALLCODE
, better to clarify which version is used, caller or callee)
Does it mean that it's determined by the version field of the executed contract?
Yes. I used I_a
in yellow paper to specify that. I think it covers the case for DELEGATECALL
/CALLCODE
. However, it's indeed debatable whether use another variable would be better (for example, use the version field of the code contract).
A note regarding upgradable contract and account versioning (posted on Gitter as well) -- my current conclusion is that upgradable contracts are not affected at all by EIP-1702 variant 1 (or variant 2)!
Just did another update to EIP-1702 -- I'm making the version of the execution frame always the same as the account code's version. That is, when fetching code from an account in state, we always fetch the version field of that account together, and "associate" the version with the code. This should be easier to implement (we just need to change all definition of code
into (code, version)
tuple), and it should provide more sensible results for DELEGATECALL/CALLCODE.
Is it possible to remove the variants that are not to be implemented?
Previously we also discussed (most regarding state rent) of how we should order additional account RLP fields. Now I'm thinking, maybe we can just use 1702's version
fields to order all other additional fields -- read version
field first, and then interpret all fields after version
as defined by that particular version. This will not only allow us to add new fields, but also allow us to remove fields if necessary!
I added this to 1702 spec. Note that this will not require any change for current 1702 implementations, but I think it may help us implement things like state rent in the future.
You removed Alternative Design, but it's still mentioned in Rationale.
I've been implementing this in aleth (https://github.com/ethereum/aleth/pull/5640), and one edge case to be aware of is creating with empty init code (both with creation transaction and CREATE/CREATE2). When init code is empty, creation ends with a new account with empty code and nonce = 1. I've decided to leave it with version 0 in this case, regardless of parent's version and LATEST_VERSION
, because other accounts with no code - EOAs - don't have version.
Let me know if I'm not right, and better to clarify it in EIP (and this definitely should go to the test cases)
Another nit for the spec: maybe it's worth pointing out in Contract Execution
section, that contracts with version 0 are executed with Petersburg VM.
You removed Alternative Design, but it's still mentioned in Rationale.
Fixed in https://github.com/ethereum/EIPs/pull/2152. Thanks!
When init code is empty, creation ends with a new account with empty code and nonce = 1.
@gumb0 What do you think if in this case, we keep the version of the account to still be the parent account version? I feel this may be more consistent:
codeHash
but with different account versions already. And I feel the situation better falls in this category.codeHash
just like any other code -- it's expected to spin up the required VM of the particular version and attempt to execute the "code". (Of course, in case if code is empty, there're many shortcuts we can take.)Another nit for the spec: maybe it's worth pointing out in Contract Execution section, that contracts with version 0 are executed with Petersburg VM.
I have small worries that people may still want to apply EVM features to version 0
in Istanbul, until we're all used to account versioning. So maybe we want to finalize this in Istanbul meta EIP but not in EIP-1702.
I think I don't mind either way, it's either slight confusion of having empty code accounts with different versions, or slight confusion of family of contracts not always producing their version.
Version 0 takes less space to store account, but I guess it shouldn't be significant difference here, as it's a rare case anyway.
It might become important to always produce strictly the same version, if at some point your idea about parsing additional fields based on version field is implemented. (but to me that idea feels like conflating "version of code executor" and "version of account format", so I'm not completely sure it's good)
Version 0 takes less space to store account, but I guess it shouldn't be significant difference here, as it's a rare case anyway.
Yeah agreed. I think it won't make much difference in storage size as it's only a corner case.
It might become important to always produce strictly the same version, if at some point your idea about parsing additional fields based on version field is implemented.
I think this may be a good reason to keep empty-code account (note it's not empty account!) of the parent account's version. Consider if we implement state rent via some additional fields with versions, then by using this scheme we can make sure those new empty-code account are garbage collected. However, if we set empty-code account to version zero, then this won't work.
Conflating "version of code executor" and "version of account format"
I don't think this is actually a bad thing. In the contrary, for some EIPs this is strictly needed. Consider EIP-2027, where we keep a contract size counter as an additional field in the account -- the code executor must support contract size counter (by slightly changing behavior of SSTORE) to make the spec work. It's nearly impossible to completely separate them out.
So if it's okay for you, maybe let's keep the current behavior (empty-code account has the same version of parent account)!
EIP-1702 Account Versioning is a bad idea at this time and it should not be in Istanbul.
a) The only relevant motivation is preparing for emergency last minute hard forks. b) Other use cases don't exist.
If eWASM launches in 2030, it will be better to deploy this dependency at that time.
This reminds me back in 2013 when we were made our wedding gift registry at Bed Bath and Beyond. The salesperson was trying to sell us a mixer. We told him we don't make cakes. Then he said "well you plan to have kids right? so when your kids go to school you can make cakes for the rest of the class." We said "okay so we get married next year, have a kid and they're in first grade in the year 2021 and then I'll be thinking 'wow I wish a had a vintage 2013 model mixer'".
A minor comment wrt the extension for Contract Creation Transaction scheme is that it would be nice to make the version
field optional here as well, and interpret no version as the default 0
version. This would yield a lot more backwards capability. Maybe this is already intended behaviour. If so it needs to be clarified in the specification.
@MrChico it's there
Check the RLP list length, if it is 4, then set account version to 0, and do not parse any additional fields.
@gumb0 Sorry, my comment was referring to the extension for creating transactions. I have updated it for clarification
@MrChico You're right! I edited the 43-VERTXN spec so that version
field is optional.
Hi @sorpaas I read this a long time ago, and once more today. I thought this time I would ask some questions.
Thanks!
Discussion thread for account versioning: