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

fix: signing with Ledger throws an empty error #349

Merged

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Oct 12, 2023

Bumps to ledger-cosmos-go v.0.13.1 to fix celestiaorg/celestia-app#2650. Unblocks celestiaorg/celestia-app#2668

Notes for reviewers

Testing

Verified this fixes the issue here

@rootulp rootulp self-assigned this Oct 12, 2023
@rootulp rootulp changed the base branch from main to release/v0.46.x-celestia October 12, 2023 15:48
@rootulp rootulp requested a review from cmwaters October 12, 2023 15:49
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

nice, thanks for handling this

@codecov-commenter
Copy link

Codecov Report

Merging #349 (f21fce9) into release/v0.46.x-celestia (7babe18) will decrease coverage by 0.01%.
Report is 2 commits behind head on release/v0.46.x-celestia.
The diff coverage is 42.85%.

❗ Current head f21fce9 differs from pull request most recent head 78463a8. Consider uploading reports for the commit 78463a8 to get more accurate results

Impacted file tree graph

@@                     Coverage Diff                      @@
##           release/v0.46.x-celestia     #349      +/-   ##
============================================================
- Coverage                     65.57%   65.57%   -0.01%     
============================================================
  Files                           666      666              
  Lines                         71177    71188      +11     
============================================================
+ Hits                          46677    46679       +2     
- Misses                        21916    21925       +9     
  Partials                       2584     2584              
Files Coverage Δ
crypto/keyring/keyring.go 63.04% <100.00%> (ø)
crypto/ledger/ledger_mock.go 60.00% <100.00%> (ø)
crypto/ledger/ledger_secp256k1.go 53.37% <33.33%> (-2.76%) ⬇️

... and 2 files with indirect coverage changes

@rootulp rootulp merged commit c226e8b into celestiaorg:release/v0.46.x-celestia Oct 12, 2023
33 checks passed
@julienrbrt
Copy link

julienrbrt commented Oct 12, 2023

I am late here, but FWIW we got Zondax to create https://github.com/cosmos/ledger-cosmos-go/releases/tag/v0.12.3 with the fix as well.
We will bump to that version in mainline v0.46 and v0.47

@rootulp
Copy link
Collaborator Author

rootulp commented Oct 12, 2023

Thanks for the info! If cosmos/cosmos-sdk v0.46.x gets bumped to ledger-cosmos-go v0.12.3 then we can revert this PR and pull that change into celestiaorg/cosmos-sdk. I'll stay tuned for a cosmos-sdk v0.46.x release with it

@rootulp rootulp deleted the rp/bump-ledger-cosmos-go branch October 12, 2023 18:59
@liamsi
Copy link
Member

liamsi commented Oct 15, 2023

cosmos#18112

cmwaters pushed a commit that referenced this pull request Nov 13, 2023
* fix: bump ledger-cosmos-go

* feat: update cosmos-sdk code for ledger-cosmos-go

Inspired by cosmos#14661
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.

Signing with ledger throws an empty error
5 participants