Skip to content

Add EIP-2565 support#138

Merged
artob merged 22 commits into
developfrom
feat/berlin/eip_2565
Jun 17, 2021
Merged

Add EIP-2565 support#138
artob merged 22 commits into
developfrom
feat/berlin/eip_2565

Conversation

@joshuajbouw
Copy link
Copy Markdown
Contributor

This is EIP 1 out of 4 for the Berlin hardfork.

For this one, it's the modexp gas cost changes. For more details, see the linked reference below.

Reference: https://eips.ethereum.org/EIPS/eip-2565

@joshuajbouw joshuajbouw self-assigned this Jun 11, 2021
@joshuajbouw joshuajbouw changed the base branch from master to develop June 11, 2021 06:14
@joshuajbouw joshuajbouw force-pushed the feat/berlin/eip_2565 branch from a377723 to 76e55d5 Compare June 11, 2021 10:07
@joshuajbouw joshuajbouw force-pushed the feat/berlin/eip_2565 branch from 5c4027c to 592eff2 Compare June 11, 2021 11:56
@joshuajbouw joshuajbouw marked this pull request as ready for review June 11, 2021 11:58
@joshuajbouw joshuajbouw requested a review from artob as a code owner June 11, 2021 11:58
@joshuajbouw joshuajbouw requested review from birchmd and mfornet June 11, 2021 11:58
Copy link
Copy Markdown
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Nice work!

Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Copy link
Copy Markdown
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, sorry, I found a small mistake, undoing my approve for now

Comment thread src/precompiles/modexp.rs Outdated
Copy link
Copy Markdown
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back to approval, sorry!

Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
@joshuajbouw joshuajbouw requested a review from birchmd June 14, 2021 15:00
@joshuajbouw joshuajbouw removed their assignment Jun 14, 2021
@joshuajbouw joshuajbouw changed the title Add EIP#2565 support Add EIP-2565 support Jun 14, 2021
Copy link
Copy Markdown
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing the new changes! There are still a couple issues with the implementation I think. Maybe we need tests that use really big values for the inputs (bigger than could fit in a U256) because I think the issues would only appear from very large inputs.

Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
joshuajbouw and others added 3 commits June 15, 2021 00:19
Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Michael Birch <michael@near.org>
@joshuajbouw joshuajbouw force-pushed the feat/berlin/eip_2565 branch from 5fdb031 to 58a913a Compare June 14, 2021 17:20
@joshuajbouw
Copy link
Copy Markdown
Contributor Author

@birchmd I'll finish the rest up once I get back from some sleep. Exhausted right now. Thanks for the review.

joshuajbouw and others added 4 commits June 16, 2021 01:49
Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Michael Birch <michael@near.org>
@joshuajbouw joshuajbouw requested a review from birchmd June 15, 2021 19:05
@joshuajbouw joshuajbouw mentioned this pull request Jun 15, 2021
4 tasks
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Comment thread src/precompiles/modexp.rs Outdated
Copy link
Copy Markdown
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made several changes around overflow handling as well wrote some functions for dealing with the slices to avoid index out of bounds panics (this behaviour is the same as geth). I also added some tests to make sure it all works. Should be good to go now.

066dca6

@joshuajbouw
Copy link
Copy Markdown
Contributor Author

Thanks for the clean up.

@artob artob assigned artob and unassigned birchmd and mfornet Jun 16, 2021
@artob artob added the C-enhancement Category: New feature or request label Jun 17, 2021
@artob artob requested a review from sept-en June 17, 2021 09:33
@artob
Copy link
Copy Markdown
Contributor

artob commented Jun 17, 2021

@joshuajbouw I'd want to merge this to develop, but need the merge conflicts fixed first

@artob artob added the P-high Pririoty: high label Jun 17, 2021
@joshuajbouw joshuajbouw force-pushed the feat/berlin/eip_2565 branch from 3ce740d to 4fcf44f Compare June 17, 2021 12:39
@artob artob merged commit 790c3f4 into develop Jun 17, 2021
@artob artob deleted the feat/berlin/eip_2565 branch June 17, 2021 12:50
artob added a commit that referenced this pull request Jun 18, 2021
* Implement EIP-2565 support. (#138)
* Fix CI debug pipeline. (#145)
* Enable `meta_call` only with feature flag. (#146)
* Add `mainnet`, `testnet`, and `betanet` feature flags. (#147)
* Prohibit static calls in exit precompiles. (#148)
* Fix `clippy::unnecessary_unwrap`. (#149)
* Fix all Clippy warnings. (#150)
* Create the `ExitToNear` receiver account if it does not exist. (#151)

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
Co-authored-by: Kirill <kirill@aurora.dev>
Co-authored-by: Marcelo Fornet <marcelo@aurora.dev>
Co-authored-by: Michael Birch <michael@aurora.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-enhancement Category: New feature or request P-high Pririoty: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants