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.

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

crypto/hd: add 'm/' prefix to hd path #7977

Merged
merged 7 commits into from
Nov 20, 2020

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Nov 18, 2020

The original issue was filed against Ethermint:
cosmos/ethermint#605
Thanks: @xiangjianmeng for the original bug report.

From: #7970
Closes: #7966


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Nov 18, 2020

Codecov Report

Merging #7977 (cbb8ff4) into launchpad/backports (7b152b3) will increase coverage by 0.01%.
The diff coverage is 96.00%.

@@                   Coverage Diff                   @@
##           launchpad/backports    #7977      +/-   ##
=======================================================
+ Coverage                50.28%   50.29%   +0.01%     
=======================================================
  Files                      338      338              
  Lines                    17560    17570      +10     
=======================================================
+ Hits                      8830     8837       +7     
- Misses                    7937     7940       +3     
  Partials                   793      793              

@lgtm-com
Copy link

lgtm-com bot commented Nov 18, 2020

This pull request fixes 1 alert when merging 186a352 into 7b152b3 - view on LGTM.com

fixed alerts:

  • 1 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Nov 18, 2020

This pull request fixes 1 alert when merging 323988f into 36ff5ca - view on LGTM.com

fixed alerts:

  • 1 for Incorrect conversion between integer types

CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 179 to 181
// INVALID
// INVALID
//
Copy link
Contributor

Choose a reason for hiding this comment

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

@fedekunze this was a testable example, you turned it into a standard test case (ExampleSomeBIP32TestVecs() -> TestBIP32Vecs(t *testing.T)). Mind elaborating why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what's the purpose of a testable example?

@lgtm-com
Copy link

lgtm-com bot commented Nov 19, 2020

This pull request fixes 1 alert when merging 52b9730 into 69f6ec2 - view on LGTM.com

fixed alerts:

  • 1 for Incorrect conversion between integer types

@alessio alessio changed the title launchpad: backport 7970 crypto/hd: add 'm/' prefix to hd path Nov 19, 2020
@odeke-em
Copy link
Collaborator

odeke-em commented Nov 19, 2020 via email

@fedekunze fedekunze requested a review from alessio November 19, 2020 14:26
Alessio Treglia added 2 commits November 19, 2020 16:20
Testable examples were accidentally converted into tests.
@lgtm-com
Copy link

lgtm-com bot commented Nov 19, 2020

This pull request fixes 1 alert when merging 65fcb82 into 69f6ec2 - view on LGTM.com

fixed alerts:

  • 1 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Nov 19, 2020

This pull request fixes 1 alert when merging cbb8ff4 into 69f6ec2 - view on LGTM.com

fixed alerts:

  • 1 for Incorrect conversion between integer types

@alessio
Copy link
Contributor

alessio commented Nov 19, 2020

@ethanfrey @clevinson your approval is required here

Copy link
Contributor

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Ack

@alessio alessio merged commit 5be42d9 into launchpad/backports Nov 20, 2020
@alessio alessio deleted the fedekunze/launchpad-bip44 branch November 20, 2020 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants