-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
EIP-1344: Update uint256 field size to uint64 #2263
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good.
Basically you mentioned all of us, who involved Chainid tests here: |
When merge? |
So how would we do a reference test to validate this? or is it validatable via reference test? |
Since it is possible to sign with with a chain ID > 64 bits (up to a value of ~255 bits IIRC), you could have an EIP-155 transaction validation that uses 65 bits and it should fail validation in some way. The opcode should also revert if the value of chain ID is > 64 bits. |
Nope. No revertals for this opcode. It simply should not be possible to configure the node with a chainid above
So yes, that would obviously never equal the |
This suggestions seems a arbitrary, not fully defined and late to me.
Imho the only meaningful way forward at this point is to keep the EIP as is, chain id being a 256 bit number, and if we think that it should be 64, then open a new EIP that redefines it and updates it throughout the Ethereum spec (e.g. signature schemes too) and not just one EIP. |
To be fair this isn't something proposed last week, but has been under discussion since chaind was proposed and the current discussion stems from a month ago. The problem with 256-bit chainid is that clients won't be able to handle it, unless their transaction encoding works on unrestricted bignums (and not just 256-bit bignums). I really feel like this hardfork is rushed for no reason and dismissing every useful proposal just to be "done with the hard fork". |
They need to handle that even now, since EIP155 already introduced this. So this PR limiting it to 64 bits makes it a weird cornercase that's not in line with EIP155. And this EIP not fixing EIP155 means that the whole fix is moot, because clients still need to handle large V-s properly. |
|
I am an un-involved observer, I have nothing to do with the hard-fork planning nor this EIP, and perhaps no claim to make the following comment, but I do have 39.25 years of experience with software development, so I'll go for it. There are a few things that are quite obvious: (1) if 63 bits would be safer, then it should be 63 bits; (2) the spec should be explicit about this size of the item; (3) if you're going to change the spec, then there MUST be testing; and (4) this feels a lot like a change to an EIP without testing weeks for deployment. What's the purpose of the EIP 1 process if it doesn't apply when we need it to not apply (clarifications included)? |
So, without this change merged, the EIP is still well-formed enough to deploy as is, as it is consistent with any currently known network that uses EIP-155 (all of which use less than 64/256 bits). Perhaps the greater issue is that @axic identified a scenario with handling EIP-155 (and EIP-1344 by extension) where chain ID being unlimited in size would cause a logical error that could be disruptive to how RLP encoding is performed on transactions. This modification (which should also be made to EIP-155) should actually be a follow-up EIP that can be implemented outside of the hardfork process since the mainnet and every other known network would not run into this scenario, but we want to make sure to bound this size so the issue is never encountered. I will withdraw this amended change and communicate to the client teams that have implemented this update so far to revert back to EIP-1344 returning a |
Got no response on this, so I am going to move ahead with the suggestion from @karalabe that this change proposal be dropped in favor of the implicit assumption from EIP-155 that chain ID is potentially "limitless" in size. This PR will therefore be dropped in favor of #2294, which would create this limit explicitly. The original wording of EIP-1344 is still accurate and required because the EVM will return a 256-bit field in response to calling the |
Per Discussion: https://ethereum-magicians.org/t/eip-1344-add-chain-id-opcode/1131/109