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

chore: Check the signature returned by the ledger device against the public key in the keyring #14460

Merged
merged 6 commits into from
Jan 2, 2023
Merged

chore: Check the signature returned by the ledger device against the public key in the keyring #14460

merged 6 commits into from
Jan 2, 2023

Conversation

zmanian
Copy link
Member

@zmanian zmanian commented Jan 2, 2023

Check the signature returned by the ledger device against the public key in the keyring and provide a helpful error message if the signature is invalid

Description

Closes: #XXXX

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@zmanian
Check the signature returned by the ledger device against the public …
f9ec62f 
…key in the keyring

and provide a helpful error message if the signature is invalid
@zmanian zmanian requested a review from a team as a code owner January 2, 2023 00:11
@zmanian zmanian enabled auto-merge (squash) January 2, 2023 00:11
likhita-809
likhita-809 approved these changes Jan 2, 2023
View reviewed changes
Copy link
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

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

lgtm

@likhita-809 likhita-809 changed the title Check the signature returned by the ledger device against the public key in the keyring chore: Check the signature returned by the ledger device against the public key in the keyring Jan 2, 2023
atheeshp
atheeshp approved these changes Jan 2, 2023
View reviewed changes
Copy link
Contributor

@atheeshp atheeshp left a comment

Choose a reason for hiding this comment

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

lgtm, pending failing test.

zmanian and others added 2 commits January 2, 2023 11:04
@zmanian @facundomedica
Update crypto/keyring/keyring.go
5612dfe 
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
@zmanian
Merge branch 'main' into zaki/wrong_ledger_error
4b9f570
@sonarcloud
Copy link

sonarcloud bot commented Jan 2, 2023

[Cosmos SDK] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@zmanian zmanian merged commit 03196d7 into main Jan 2, 2023
@zmanian zmanian deleted the zaki/wrong_ledger_error branch January 2, 2023 18:35
@dtribble
Copy link

dtribble commented Jan 2, 2023

Huzzah! @JimLarson please note for integration in Agoric.

@julienrbrt
Copy link
Member

@Mergifyio backport release/v0.47.x

@mergify mergify bot mentioned this pull request Jan 2, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Jan 2, 2023

backport release/v0.47.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jan 2, 2023
@zmanian @mergify
chore: Check the signature returned by the ledger device against the …
e39a1fa 
…public key in the keyring (#14460)

Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
(cherry picked from commit 03196d7)
tac0turtle pushed a commit that referenced this pull request Jan 3, 2023
@mergify @zmanian @julienrbrt
chore: Check the signature returned by the ledger device against the …
58f81fa 
…public key in the keyring (backport #14460) (#14464)

Co-authored-by: Zaki Manian <zaki@iqlusion.io>
Co-authored-by: Julien Robert <julien@rbrt.fr>
This was referenced Mar 7, 2024
Closed
Merged
@rootulp
Copy link
Contributor

rootulp commented Mar 7, 2024

Out of curiosity, why wasn't this backported to v0.46.x? This PR happened in January 2023 and the v0.46.x release line was marked end of life ~10 months later in November 2023 (see v0.46.16).

@jaeseung-bae jaeseung-bae mentioned this pull request Mar 27, 2024
Merged
5 tasks
@mergify mergify bot mentioned this pull request Mar 27, 2024
Merged
5 tasks
@0xstepit 0xstepit mentioned this pull request Jul 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facundomedica facundomedica facundomedica left review comments

@tac0turtle tac0turtle tac0turtle approved these changes

@julienrbrt julienrbrt julienrbrt approved these changes

@atheeshp atheeshp atheeshp approved these changes

@likhita-809 likhita-809 likhita-809 approved these changes

Assignees

@tac0turtle tac0turtle

@likhita-809 likhita-809

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@zmanian @dtribble @julienrbrt @rootulp @facundomedica @tac0turtle @atheeshp @likhita-809