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

Refactor: dependencies dedup (such as keccaak, bip, etc) #139

Merged
merged 29 commits into from
Mar 28, 2024
Merged

Refactor: dependencies dedup (such as keccaak, bip, etc) #139

merged 29 commits into from
Mar 28, 2024

Conversation

nicolasbrugneaux
Copy link
Contributor

@nicolasbrugneaux nicolasbrugneaux commented Feb 13, 2024 •
edited
Loading

This PR aims to remove duplicate imports (tiny-keccak, eth-lib, ethereum-utils, etc..) all used to do the same hashing or close to it. Fortunately regressions tests are present and should make this trustworthy

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 2a28bac

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@nicolasbrugneaux nicolasbrugneaux changed the base branch from master to elliptic-more-like-byelliptic February 13, 2024 17:27
@socket-security Socket Security
Copy link

socket-security bot commented Feb 13, 2024 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@scure/[email protected] None +2 2.22 MB paulmillr
npm/@scure/[email protected] None 0 374 kB paulmillr

🚮 Removed packages: npm/@types/[email protected], npm/[email protected]

View full report↗︎

@nicolasbrugneaux nicolasbrugneaux self-assigned this Feb 14, 2024
@nicolasbrugneaux nicolasbrugneaux mentioned this pull request Feb 14, 2024
Closed
@shazarre
Copy link
Contributor

as discussed dropping bip32 in favor of @scure/bip32 is because it's part of a suite of audited crypto libraries

@nicolasbrugneaux please share the suite

@nicolasbrugneaux
Copy link
Contributor Author

Here's the aforementioned suite: https://paulmillr.com/noble/#scure

@nicolasbrugneaux nicolasbrugneaux marked this pull request as ready for review February 23, 2024 16:32
@nicolasbrugneaux nicolasbrugneaux requested a review from a team as a code owner February 23, 2024 16:32
@nicolasbrugneaux nicolasbrugneaux changed the title WIP: dependencies dedup (such as keccaak, bip, etc) Refactor: dependencies dedup (such as keccaak, bip, etc) Feb 23, 2024
shazarre
shazarre reviewed Feb 26, 2024
View reviewed changes
Copy link
Contributor

@shazarre shazarre left a comment

Choose a reason for hiding this comment

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

amazing effort! 🚀

as discussed generally: we need to make sure this is tested properly

packages/sdk/wallets/wallet-local/src/local-signer.ts Show resolved Hide resolved
packages/sdk/cryptographic-utils/src/bls.ts Outdated Show resolved Hide resolved
packages/sdk/wallets/wallet-base/src/signing-utils.ts Outdated Show resolved Hide resolved
aaronmgdr
aaronmgdr previously requested changes Mar 27, 2024
View reviewed changes
Copy link
Member

@aaronmgdr aaronmgdr left a comment

Choose a reason for hiding this comment

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

merge conflict artifacts

packages/docs/sdk/docs/wallet-base/modules/signing_utils.md Outdated Show resolved Hide resolved
@aaronmgdr aaronmgdr self-requested a review March 27, 2024 08:49
@aaronmgdr aaronmgdr dismissed their stale review March 27, 2024 08:49

already handled

@nicolasbrugneaux nicolasbrugneaux merged commit 1c6d202 into elliptic-more-like-byelliptic Mar 28, 2024
13 checks passed
@nicolasbrugneaux nicolasbrugneaux deleted the chore/bip-to-scure branch March 28, 2024 08:51
nicolasbrugneaux added a commit that referenced this pull request Apr 2, 2024
@nicolasbrugneaux
elliptic -> @noble/* (#59)
38b2631 
* chore: add snapshot

* wip: first attempt, the ecies file is not done

* wip

* fix: ecies module works

* fix: ???

* chore: remove unused concatkdf?

* fix: types

* fix: wallets

* fix: wallet-gcp

* fix: regression tests

* chore: add a note

* chore: add changeset

* fix: test

* fix: web3 resolution

* Refactor: dependencies dedup (such as keccaak, bip, etc) (#139)

* test: add bls regression tests

* refactor: more deps gone

* refactor: types

* refactor: getWordList

* refactor: more deps gone

* fix: utils

* fix: keccak hash

* fix: various buffer fixes

* fix: force keccak to consider string as bytes

* chore: update docs

* fix: type

* refactor: rlp -> @ethereumjs/rlp

* refactor: eth-lib removal wip

* refactor: eth-lib removal

* chore: regenerate docs

* fix: ensure no 0x-prefixed string is passed to hextobytes

* fix: update snapshot

* fix: ensure r,s,v are 0x prefixed

* chore: regenerate docs

* test: bls

* test: some more signing-utils tests

* chore: regenerate docs

* chore: regenerate docs

* fix: web3 resolution

* fix: ledger SignMessage

* chore: add note

* chore: regenerate docs

* chore: remove comment

* chore: changesets

* chore: changesets wording

* chore: changesets wording

* chore: remove comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shazarre shazarre shazarre left review comments

@aaronmgdr aaronmgdr Awaiting requested review from aaronmgdr

Assignees

@nicolasbrugneaux nicolasbrugneaux

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicolasbrugneaux @shazarre @aaronmgdr