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

Made ecrecover implementation trait public #11188

Merged
merged 2 commits into from
Oct 22, 2019
Merged

Made ecrecover implementation trait public #11188

merged 2 commits into from
Oct 22, 2019

Conversation

tolbrino
Copy link
Contributor

Making the trait public enables its use in FFIs to integrate with the ecrecover functionality.

In particular this has been done in aeternity/ecrecover

@parity-cla-bot
Copy link

It looks like @tolbrino hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@tolbrino
Copy link
Contributor Author

[clabot:check]

@parity-cla-bot
Copy link

It looks like @tolbrino signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@tolbrino tolbrino mentioned this pull request Oct 22, 2019
Closed
@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 22, 2019
@niklasad1 niklasad1 added this to the 2.7 milestone Oct 22, 2019
niklasad1
niklasad1 suggested changes Oct 22, 2019
View reviewed changes
Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your PR

I see no problem by making Implementation trait and EcRecover public but it makes the code inconsistent i.e, all other builtin contract types are not public.

Thus, either we export all types or none

@niklasad1 niklasad1 added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 22, 2019
@tolbrino
Copy link
Contributor Author

That sounds reasonable. I will adapt the PR.

@tolbrino
Make all builtin contract types public
02eb78b 
This ensure the API is consistent in terms of visibility.
@dvdplm dvdplm requested a review from debris October 22, 2019 11:28
@niklasad1 niklasad1 added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Oct 22, 2019
niklasad1
niklasad1 approved these changes Oct 22, 2019
View reviewed changes
Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM

dvdplm
dvdplm approved these changes Oct 22, 2019
View reviewed changes
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.

lgtm.

@dvdplm dvdplm merged commit 81ca599 into openethereum:master Oct 22, 2019
@dvdplm dvdplm added B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 labels Oct 22, 2019
@dvdplm
Copy link
Collaborator

dvdplm commented Oct 22, 2019

@s3krit Not 100% sure this backports cleanly and if it doesn't it's probably not a big deal.

renlord pushed a commit to renlord/parity-ethereum that referenced this pull request Oct 22, 2019
@tolbrino @renlord
Made ecrecover implementation trait public (openethereum#11188)
30144ef 
* Made ecrecover Implementation trait public

* Make all builtin contract types public

This ensure the API is consistent in terms of visibility.
@tolbrino tolbrino deleted the tb-pubtrait-ecrecover branch October 23, 2019 08:01
@s3krit
Copy link
Contributor

s3krit commented Oct 23, 2019

Ok thanks @dvdplm. I'll tentatively add it to the list and check it out.

dvdplm added a commit that referenced this pull request Oct 24, 2019
@dvdplm
Merge branch 'master' into dp/fix/debug-snapshot-start
99d5b61 
* master:
  Type annotation for next_key() matching of json filter options (#11192)
  Crypto primitives removed from ethkey (#11174)
  Made ecrecover implementation trait public (#11188)
  Remove unused macro_use. (#11191)
  [dependencies]: jsonrpc `14.0.1` (#11183)
  [receipt]: add `sender` & `receiver` to `RichReceipts` (#11179)
  [dependencies] bump rand 0.7 (#11022)
  [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
  TxPermissions ver 3: gas price & data (#11170)
  [ethash] chainspec validate `ecip1017EraRounds` non-zero (#11123)
  util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
  ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
  Aura: Report malice on sibling blocks from the same validator (#11160)
dvdplm added a commit that referenced this pull request Oct 24, 2019
@dvdplm
Merge branch 'master' into dp/chore/put-snapshot-progress-behind-rwlock
01cf083 
* master:
  Pause pruning while snapshotting (#11178)
  Type annotation for next_key() matching of json filter options (#11192)
  Crypto primitives removed from ethkey (#11174)
  Made ecrecover implementation trait public (#11188)
  Remove unused macro_use. (#11191)
  [dependencies]: jsonrpc `14.0.1` (#11183)
  [receipt]: add `sender` & `receiver` to `RichReceipts` (#11179)
  [dependencies] bump rand 0.7 (#11022)
  [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
  TxPermissions ver 3: gas price & data (#11170)
  [ethash] chainspec validate `ecip1017EraRounds` non-zero (#11123)
  util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
  ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
  Aura: Report malice on sibling blocks from the same validator (#11160)
This was referenced Nov 5, 2019
Merged
Merged
niklasad1 pushed a commit that referenced this pull request Nov 5, 2019
@tolbrino @niklasad1
Made ecrecover implementation trait public (#11188)
7f32ef0 
* Made ecrecover Implementation trait public

* Make all builtin contract types public

This ensure the API is consistent in terms of visibility.
dvdplm added a commit that referenced this pull request Nov 6, 2019
@dvdplm
Merge branch 'master' into dp/chore/add-bench-for-to_fat_rlps
89cdb3b 
* master: (21 commits)
  ropsten #6631425 foundation #8798209 (#11201)
  Update list of bootnodes for xDai chain (#11236)
  ethcore/res: add mordor testnet configuration (#11200)
  [chain specs]: activate `Istanbul` on mainnet (#11228)
  [builtin]: support `multiple prices and activations` in chain spec (#11039)
  Insert explicit warning into the panic hook (#11225)
  Snapshot restoration overhaul (#11219)
  Fix docker centos build (#11226)
  retry on gitlab system failures (#11222)
  Update bootnodes. (#11203)
  Use provided usd-per-eth value if an endpoint is specified (#11209)
  Use a lock instead of atomics for snapshot Progress (#11197)
  [informant]: `MillisecondDuration` -> `as_millis()` (#11211)
  Step duration map configuration parameter ported from the POA Network fork (#10902)
  Upgrade jsonrpc to latest (#11206)
  [export hardcoded sync]: use debug for `H256` (#11204)
  Pause pruning while snapshotting (#11178)
  Type annotation for next_key() matching of json filter options (#11192)
  Crypto primitives removed from ethkey (#11174)
  Made ecrecover implementation trait public (#11188)
  ...
dvdplm pushed a commit that referenced this pull request Nov 6, 2019
@tolbrino @dvdplm
Made ecrecover implementation trait public (#11188)
fca912e 
* Made ecrecover Implementation trait public

* Make all builtin contract types public

This ensure the API is consistent in terms of visibility.
s3krit pushed a commit that referenced this pull request Nov 11, 2019
@soc1c @s3krit
v2.5.10 stable (#11239)
f3cdd7b 
* ropsten #6631425 foundation #8798209 (#11201)
* [stable] builtin, istanbul and mordor testnet backports (#11234)
  * ethcore-builtin (#10850)
  * [builtin]: support `multiple prices and activations` in chain spec (#11039)
  * [chain specs]: activate `Istanbul` on mainnet (#11228)
  * ethcore/res: add mordor testnet configuration (#11200)
* Update list of bootnodes for xDai chain (#11236)
* ethcore: remove `test-helper feat` from build (#11047)
* Secret store: fix Instant::now() related race in net_keep_alive (#11155) (#11159)
* [stable]: backport #10691 and #10683 (#11143)
  * Fix compiler warning (that will become an error) (#10683)
  * Refactor Clique stepping (#10691)
* Add Constantinople eips to the dev (instant_seal) config (#10809)
* Add cargo-remote dir to .gitignore (?)
* Insert explicit warning into the panic hook (#11225)
* Fix docker centos build (#11226)
* Update MIX bootnodes. (#11203)
* Use provided usd-per-eth value if an endpoint is specified (#11209)
* Add new line after writing block to hex file. (#10984)
* Type annotation for next_key() matching of json filter options (#11192) (but no `FilterOption` in 2.5 so…)
* Upgrade jsonrpc to latest (#11206)
* [CI] check evmbin build (#11096)
* Correct EIP-712 encoding (#11092)
* [client]: Fix for incorrectly dropped consensus messages (#11086)
* Fix block detail updating (#11015)
* Switching sccache from local to Redis (#10971)
* Made ecrecover implementation trait public (#11188)
* [dependencies]: jsonrpc `14.0.1` (#11183)
* [receipt]: add `sender` & `receiver` to `RichReceipts` (#11179)
* [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
* util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
* ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
* Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)
* Cleanup stratum a bit (#11161)
* Upgrade to jsonrpc v14 (#11151)
* SecretStore: expose restore_key_public in HTTP API (#10241)
s3krit pushed a commit that referenced this pull request Nov 11, 2019
@soc1c @s3krit
v2.6.5 beta (#11240)
bc90ba2 
* [CI] check evmbin build (#11096)
* Correct EIP-712 encoding (#11092)
* [client]: Fix for incorrectly dropped consensus messages (#11082) (#11086)
* Update hardcoded headers (foundation, classic, kovan, xdai, ewc, ...) (#11053)
* Add cargo-remote dir to .gitignore (?)
* Update light client headers: ropsten 6631425 foundation 8798209 (#11201)
* Update list of bootnodes for xDai chain (#11236)
* ethcore/res: add mordor testnet configuration (#11200)
* [chain specs]: activate Istanbul on mainnet (#11228)
* [builtin]: support multiple prices and activations in chain spec (#11039)
* [receipt]: add sender & receiver to RichReceipts (#11179)
* [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
* Made ecrecover implementation trait public (#11188)
* Fix docker centos build (#11226)
* Update MIX bootnodes. (#11203)
* Insert explicit warning into the panic hook (#11225)
* Use provided usd-per-eth value if an endpoint is specified (#11209)
* Cleanup stratum a bit (#11161)
* Add Constantinople EIPs to the dev (instant_seal) config (#10809) (already backported)
* util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
* ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
* Type annotation for next_key() matching of json filter options (#11192)
* Upgrade jsonrpc to latest (#11206)
* [dependencies]: jsonrpc 14.0.1 (#11183)
* Upgrade to jsonrpc v14 (#11151)
* Switching sccache from local to Redis (#10971)
* Snapshot restoration overhaul (#11219)
* Add new line after writing block to hex file. (#10984)
* Pause pruning while snapshotting (#11178)
* Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)
* Fix block detail updating (#11015)
* Make InstantSeal Instant again #11186
* Filter out some bad ropsten warp snapshots (#11247)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dvdplm dvdplm dvdplm approved these changes

@niklasad1 niklasad1 niklasad1 approved these changes

@debris debris Awaiting requested review from debris

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
6 participants
@tolbrino @parity-cla-bot @dvdplm @s3krit @niklasad1 @jsnewby