Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Devp2p-ethereum-cryptography-dependency #1947

Merged
merged 14 commits into from
Jun 18, 2022
Merged

Devp2p-ethereum-cryptography-dependency #1947

merged 14 commits into from
Jun 18, 2022

Conversation

ScottyPoi
Copy link
Contributor

@ScottyPoi ScottyPoi commented Jun 8, 2022 •
edited
Loading

Resubmitting on updated master.
Fixes for #1934

Removing secp256k1 and keccak dependencies in favor of ethereum-cryptography

  • secp256k1 removal pretty straight forward. Most of the functions were indentical.

  • ecdh method required some changes to the hash function to make ecdhX() work

  • keccak class used in rlpx/mac.ts is not exposed by ethereum-cryptography

    • Talking to PaulM about a fix, possibly on the ethereum-cryptography side.

  • Used @noble/hashes import for Keccak in rlpx/mac.ts

  • Replaced hi-base32 dependency with @scure/base from @paulmillr

@ScottyPoi ScottyPoi added PR state: WIP dependencies Pull requests that update a dependency file package: devp2p target: master Work to be done towards master branch labels Jun 8, 2022
@codecov
Copy link

codecov bot commented Jun 8, 2022 •
edited
Loading

Codecov Report

Merging #1947 (19f5268) into master (2bd7ae6) will increase coverage by 0.00%.
The diff coverage is 96.29%.

Impacted file tree graph

Flag Coverage Δ
block 86.15% <ø> (ø)
blockchain 81.56% <ø> (ø)
client 78.34% <ø> (ø)
common 95.01% <ø> (ø)
devp2p 82.84% <96.29%> (+0.06%) ⬆️
ethash 90.81% <ø> (ø)
statemanager 84.55% <ø> (ø)
trie 79.97% <ø> (ø)
tx 92.17% <ø> (ø)
util 87.03% <ø> (ø)
vm 80.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@acolytec3 acolytec3 force-pushed the devp2p-ethereum-cryptography-dependency branch from 9398133 to 4713cb0 Compare June 12, 2022 02:07
paulmillr
paulmillr requested changes Jun 15, 2022
View reviewed changes
packages/devp2p/src/util.ts Outdated Show resolved Hide resolved
packages/devp2p/src/dns/enr.ts Outdated Show resolved Hide resolved
packages/devp2p/src/rlpx/mac.ts Outdated Show resolved Hide resolved
@ScottyPoi ScottyPoi requested a review from paulmillr June 16, 2022 05:18
@ScottyPoi ScottyPoi mentioned this pull request Jun 17, 2022
Closed
@ScottyPoi ScottyPoi merged commit ddee04d into master Jun 18, 2022
@holgerd77 holgerd77 mentioned this pull request Jun 28, 2022
Closed
@holgerd77 holgerd77 deleted the devp2p-ethereum-cryptography-dependency branch June 28, 2022 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulmillr paulmillr paulmillr approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file package: devp2p PR state: merge ready target: master Work to be done towards master branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ScottyPoi @paulmillr @acolytec3