Precompiles and Keccak256 repricing #2666
Conversation
Make even with master
Make even
|
Didn't we agree on ACD#84 to split these: ethereum/pm#162 (comment) ? Also this seems to conflate even more EIPs into one, including EIP-1352 which has been discussed separately. Furthermore I don't see too much benefit duplicating EIP-2046, when you could be added to it to reflect your work. |
|
Here proposal is much more radical than EIP-2046: I can set EIP-1352 as a requirement instead instead of talking about it explicitly. Otherwise it's all merged into one EIP to reflect that all these changes should (at least in my mind) happen simultaneously to have logical correctness. |
|
It was also mentioned multiple times that the figure in 2046 is not final and should be adjusted based on benchmarking. |
|
Do you mind to update it to zero and I'll add it as a requirement instead of replacement? |
|
I can update stuff next week (trying to wind down now). I think it could help to have perhaps a semi-ad-hoc chat early next week, with some others interested in this topic, to get aligned, as it seems the ACD format doesn't really facilitates a good discussion. I'm sure it would be possible to resolve these small conflicts. |
shamatar
changed the title
Merge origin master into branch
…/EIPs into precompile_repricing
shamatar
changed the title
shamatar
changed the title
|
Error handling was removed for now |
|
Note: I agree with @axic that this EIP should really be split up into one EIP per repricing so they can be easily discussed independently, rather than people having to agree on all or nothing. We don't currently have a hard rule against monolithic EIPs though so I'll stick with my approval for now, but I'll wait to see if the author would prefer to split this up before merging. @shamatar If you want to merge this without splitting it up, let me know and I'll click the button. |
|
More EIPs is more scary than less EIPs, so I'll keep it as it is for now. Let me refactor with your suggestions in mind and it will be ready to merge. Since I'll be pushing further for this EIP and 2046 it may be good to return back to having this one supersede the 2046. |
It should be the opposite. You are much more likely to see things go through with more EIPs because people cannot hamstring the entire set of changes by only complaining about one thing. e.g., someone has some problem with the way you calculated SHA256 costs, so the entire EIP gets stuck while you debate about the merits of your strategy vs theirs. If they were separate EIPs then such a debate would only block SHA256, but not all of the other changes. |
|
There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
|
Waiting for a next dev call to discuss inclusion. |
|
@MicahZoltu Last two commits should make it much more readable and simple |
There was a problem hiding this comment.
The only thing that must be fixed prior to merging as a draft is that all opcodes must include their numeric code in parenthesis after as described here: https://eips.ethereum.org/EIPS/eip-1#special-requirements-for-core-eips. All of the other feedback should probably be addressed/considered prior to moving this to Review, but isn't a blocker for Draft.
|
Should be in better shape now |
Main points: - Reprice sha256 and Ripemd160 precompiles after ethereum#2046 changes and to better reflect current performance and internal structures of the computations performed - Reprice Keccak256 native function and reflect internal structures of the computations performed
Main points:
Edits: