Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Upgrade num to 0.2, use enum-primitive-derive #11636

Merged
merged 1 commit into from
Jun 7, 2020
Merged

Conversation

vorot93
Copy link

@vorot93 vorot93 commented Apr 18, 2020

This was highlighted during the AllCoreDevs call. num 0.1 is too slow and thus blocks the proposed repricing of ModExp precompile. enum-primitive is unmaintained, so switched to enum-primitive-derive which is based on num 0.2.

ethcore/sync/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

PR description is lacking: why are we doing this? Why is it worth to merge a git dependency?

@vorot93
Copy link
Author

vorot93 commented Apr 20, 2020

@dvdplm sorry, added reasoning to PR description.

@vorot93 vorot93 requested a review from dvdplm April 20, 2020 08:54
@dvdplm
Copy link
Collaborator

dvdplm commented Apr 20, 2020

num 0.1 is too slow

So this PR is a performance win? Benches?

Is EIP 2565 being considered for the Berlin HF?

@vorot93
Copy link
Author

vorot93 commented May 2, 2020

@dvdplm
Copy link
Collaborator

dvdplm commented May 19, 2020

Performance data for EIP-2565:
https://docs.google.com/spreadsheets/d/1zqGQwj2d0qGt3Gue8QZ4Lf-4KGt575SwGgx8Opfay2c/edit?usp=sharing

I have a hard time understanding the numbers there without further context.

EIP 2565 makes sense to me, although I'm not 100% clear on what the gain is, but in general no opposition to optimizing gas prices to enable advanced crypto.

This PR does not implement 2565 though right? So is this a prequel do further work?

@vorot93
Copy link
Author

vorot93 commented Jun 6, 2020

Updated to newly released enum-primitive-derive 0.2

@vorot93 vorot93 requested review from niklasad1 and dvdplm and removed request for dvdplm June 6, 2020 12:36
@vorot93 vorot93 added the A0-pleasereview 🤓 Pull request needs code review. label Jun 6, 2020
@dvdplm dvdplm merged commit 596d011 into master Jun 7, 2020
@dvdplm dvdplm deleted the vorot93/num-0.2 branch June 7, 2020 09:45
dvdplm added a commit that referenced this pull request Jun 30, 2020
* master:
  Update parity-crypto dependency (#11791)
  ethcore/private-tcx: fix deadlock caused by conflicting lock order (#11764)
  Feature/publish draft release ci (#11786)
  Trigger custom docker build when adding ci-build-docker to the commit (#11782)
  Fix alpine docker image entry point (#11749)
  Use BigUint::modpow for ModExp precompile (#11772)
  Add YOLO-1 testnet (#11757)
  Upgrade num to 0.2, use enum-primitive-derive (#11636)
  Update Simple Subroutines to the latest spec  (#11731)
  update secret-store reference (#11761)
  Implementation of EIP 2537 (#11707)
  Add missing forks to fork ID (#11747)
  Use nightly tag for master branch (#11732)
dvdplm added a commit that referenced this pull request Jun 30, 2020
…ereum/openethereum into dp/chore/upgrade-to-rocksdb-0.14

* 'dp/chore/upgrade-to-rocksdb-0.14' of github.com:openethereum/openethereum:
  Update parity-crypto dependency (#11791)
  ethcore/private-tcx: fix deadlock caused by conflicting lock order (#11764)
  Feature/publish draft release ci (#11786)
  Trigger custom docker build when adding ci-build-docker to the commit (#11782)
  Fix alpine docker image entry point (#11749)
  Use BigUint::modpow for ModExp precompile (#11772)
  Add YOLO-1 testnet (#11757)
  Upgrade num to 0.2, use enum-primitive-derive (#11636)
  Update Simple Subroutines to the latest spec  (#11731)
  update secret-store reference (#11761)
  Implementation of EIP 2537 (#11707)
  Add missing forks to fork ID (#11747)
  Use nightly tag for master branch (#11732)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants