Crypto primitives removed from ethkey #11174

grbIzl · 2019-10-16T19:40:03Z

It's a continuation of https://github.com/paritytech/parity-common/pull/210/files and this code uses this PR as source for parity crypto. As soon as this code will be reviewed, the PR in parity crypto will be moved to merged and new corresponding version will be published. After that, the proper version for parity crypto will be inserted.

Content of this PR:

Crypto primitives removed from ethkey crate
All crates, that used them, reworked in order to use these primitives from parity-crypto crate

Please note, that most of the changes are straightforward replacements of includes (from ethkey to parity-crypto) and corresponding changes of API usages (like usages of renamed methods). It has two exceptions, where changes were not straightforward:

Currently ethstore uses pretty weird scheme of ethkey re-export (ethkey crate included as _ethkey and included into other files via additional ethkey.rs). See here:
https://github.com/paritytech/parity-ethereum/pull/11174/files#diff-24ae21ec5c620e499b3380a7c69e172fL18 I've removed this and moved additional exports from ethkey.rs into ethstore lib.rs file. Hopefully, that simplifies the code
I had to call serialization methods in secret store more explicitly: https://github.com/paritytech/parity-ethereum/pull/11174/files#diff-2371ec7e676494334e9154656f32b190R95 because we removed ToHex trait implementation for Secret. Cc @svyatonik

dvdplm

Looks real nice, good job. A few nits and imports like use some_crate::{AnItem} should be use some_crate:AnItem.

It's a big PR and mostly changes imports. If there is anything particular you'd like reviewers to look at it'd be great if you pointed out those areas in the description so we don't loose them in the noise.

accounts/ethkey/src/prefix.rs

accounts/ethstore/src/account/crypto.rs

accounts/ethstore/src/account/safe_account.rs

ethcore/engines/clique/src/tests.rs

secret-store/Cargo.toml

secret-store/src/key_server_cluster/jobs/signing_job_ecdsa.rs

grbIzl · 2019-10-17T08:05:58Z

Looks real nice, good job. A few nits and imports like use some_crate::{AnItem} should be use some_crate:AnItem.

Actually I did this because of @niklasad1 requirements. He always asks to put the brackets in such cases :-)

It's a big PR and mostly changes imports. If there is anything particular you'd like reviewers to look at it'd be great if you pointed out those areas in the description so we don't loose them in the noise.

Yes, I will. I just need to achieve the green status on it first.

niklasad1 · 2019-10-17T08:22:44Z

Actually I did this because of @niklasad1 requirements. He always asks to put the brackets in such cases :-)

Haha, that was really funny but actually I think you got it backward because I usually do this: #10056 (comment)

grbIzl · 2019-10-17T08:27:49Z

Haha, that was really funny but actually I think you got it backward because I usually do this: #10056 (comment)

My bad (((

dvdplm

lgtm.

* 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)

* master: Type annotation for next_key() matching of json filter options (#11192) Crypto primitives removed from ethkey (#11174)

* 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)

* 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) ...

grbIzl added M4-core ⛓ Core client code / Rust. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 16, 2019

dvdplm suggested changes Oct 17, 2019

View reviewed changes

grbIzl added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 18, 2019

grbIzl requested a review from dvdplm October 18, 2019 09:57

dvdplm approved these changes Oct 21, 2019

View reviewed changes

svyatonik approved these changes Oct 22, 2019

View reviewed changes

svyatonik added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 22, 2019

niklasad1 mentioned this pull request Oct 22, 2019

Move ethkey crypto utils to parity crypto crate paritytech/parity-common#210

Merged

niklasad1 added this to the 2.7 milestone Oct 23, 2019

grbIzl added 15 commits October 23, 2019 11:41

Crypto utils removed from ethkey

c24414f

Fix ethkey lib

765a2d9

Switch ethsore to new crypto

2817f9a

Accounts crate fixed

d4c06f8

Secret store crate switched to new crypto

824cf92

Ethcore builtin fixed

7d38b66

Accounts crate fixed

3f10de5

Ethcore crate fixed

265968a

Util network fixed

efad258

Util network-devp2p fixed

903317a

Private tx fixed

3d00913

Ethcore sync fixed

3909613

Secret store fixed

77781fb

Rpc fixed

1463b5c

Parity fixed

7486d65

grbIzl added 15 commits October 23, 2019 11:41

Ethkey cli fixed

0d6ab9f

Local store fixed

84b292a

Ethcore blockchain fixed

aba2fb6

Cargo.lock pushed; doc comment added for reversed nonce

fb92dbd

Ethstore tests fixed

612880e

Ethstore cli fixed

9908bd6

Miner fixed

46a7547

Snapshot tests are fixed

cff97d1

Single brackets removed

a92327a

Machine fixed

51e2206

Verification fixed

117a0f9

Executive state fixed

022472a

More single brackets removed

9e31fb3

Update version of parity-crypto

4209bc6

Use published version 0.4.2 of parity-crypto

bb7ac3e

grbIzl force-pushed the SwitchFromEthKeyToCrypto branch from 564739f to bb7ac3e Compare October 23, 2019 09:48

New test in tx_filter fixed

0c0e9e5

grbIzl removed the A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. label Oct 23, 2019

grbIzl merged commit 834585d into master Oct 23, 2019

grbIzl deleted the SwitchFromEthKeyToCrypto branch October 23, 2019 11:03

dvdplm added a commit that referenced this pull request Oct 24, 2019

Merge branch 'master' into dp/chore/ensure-we-ignore-old-snapshots

c543a46

* master: Type annotation for next_key() matching of json filter options (#11192) Crypto primitives removed from ethkey (#11174)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crypto primitives removed from ethkey #11174

Crypto primitives removed from ethkey #11174

grbIzl commented Oct 16, 2019 •

edited

Loading

dvdplm left a comment

grbIzl commented Oct 17, 2019

niklasad1 commented Oct 17, 2019 •

edited

Loading

grbIzl commented Oct 17, 2019

dvdplm left a comment

Crypto primitives removed from ethkey #11174

Crypto primitives removed from ethkey #11174

Conversation

grbIzl commented Oct 16, 2019 • edited Loading

dvdplm left a comment

Choose a reason for hiding this comment

grbIzl commented Oct 17, 2019

niklasad1 commented Oct 17, 2019 • edited Loading

grbIzl commented Oct 17, 2019

dvdplm left a comment

Choose a reason for hiding this comment

grbIzl commented Oct 16, 2019 •

edited

Loading

niklasad1 commented Oct 17, 2019 •

edited

Loading