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

Ensure '0x' prefix exists (web3-eth-accounts) #3316

Merged
merged 5 commits into from
Jan 21, 2020
Merged

Ensure '0x' prefix exists (web3-eth-accounts) #3316

merged 5 commits into from
Jan 21, 2020

Conversation

nivida
Copy link
Contributor

@nivida nivida commented Jan 20, 2020 •
edited
Loading

Description

This PR ensures the '0x' prefix is always existing on a method call of the Accounts module.

Fixes #3041

Type of change

  • Enhancement

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success and extended the tests if necessary.
  • I ran npm run build-all and tested the resulting file/'s from dist folder in a browser.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have tested my code on the live network.

@nivida nivida added Enhancement Includes improvements or optimizations 1.x 1.0 related issues labels Jan 20, 2020
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 85.445% when pulling 49793cf on issue/3041 into bc27652 on 1.x.

@nivida nivida requested a review from cgewecke January 20, 2020 12:11
@nivida nivida added the Review Needed Maintainer(s) need to review label Jan 20, 2020
@nivida nivida marked this pull request as ready for review January 20, 2020 12:11
cgewecke
cgewecke approved these changes Jan 21, 2020
View reviewed changes
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

LGTM! 💯

@nivida nivida removed the Review Needed Maintainer(s) need to review label Jan 21, 2020
@nivida nivida merged commit 7386be2 into 1.x Jan 21, 2020
@DalderupMaurice
Copy link

Out of curiousity, is there a reason why the implementation differs from the v2 branch?
https://github.com/ethereum/web3.js/pull/2929/files#diff-0150e6b63e1674be31f24602a91ea0f5R126

This would result in an extra breaking change going from v1 to v2

Either way, thanks for adding it in!

@nivida nivida deleted the issue/3041 branch January 21, 2020 09:24
@nivida
Copy link
Contributor Author

nivida commented Jan 21, 2020 •
edited
Loading

@DalderupMaurice Oh thanks for commenting here! This is true, the implemented solution in 2.0 is the more advanced solution. This shouldn't introduce an extra breaking change but I will open a PR against 1.x asap with the solution implemented in 2.x thanks!

@cgewecke cgewecke mentioned this pull request Apr 22, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request Sep 21, 2022
Open
@adamlaska adamlaska mentioned this pull request Oct 2, 2022
Open
@Woodpile37 Woodpile37 mentioned this pull request Aug 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgewecke cgewecke cgewecke approved these changes

Assignees
No one assigned
Labels
1.x 1.0 related issues Enhancement Includes improvements or optimizations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

web3.eth.accounts.privateKeyToAccount accepts private key without "0x" prefix but returns incorrect result
4 participants
@nivida @coveralls @DalderupMaurice @cgewecke