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

Signing with ledger throws an empty error #2650

Closed
1 of 4 tasks
Wondertan opened this issue Oct 10, 2023 · 5 comments · Fixed by #2668 or celestiaorg/cosmos-sdk#349
Closed
1 of 4 tasks

Signing with ledger throws an empty error #2650

Wondertan opened this issue Oct 10, 2023 · 5 comments · Fixed by #2668 or celestiaorg/cosmos-sdk#349
Assignees
Labels
bug Something isn't working

Comments

@Wondertan
Copy link
Member

Wondertan commented Oct 10, 2023

Summary of Bug

When attempting to sign any TX with ledger hardware wallet an empty error is thrown with a second delay.

❯ celestia-appd tx sign tx-test.json --from ledger-test   --chain-id="mocha-4" --offline -s 0 -a 0 --ledger
Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.
Error:

I tried playing around with various options and flags - the result is always the same

Version

v1.0.0

Steps to Reproduce

  • Have working ledger and ensure its connected and unlocked
  • Add ledger key via celestia-appd keys add <name> --ledger
    • This prompts perm on ledger and this works fine
  • Generate any test tx with --generate-only
  • Sign transaction
    celestia-appd tx sign tx-test.json --from <name> --chain-id="mocha-4" --offline -s 0 -a 0 --ledger

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@Wondertan Wondertan added the bug Something isn't working label Oct 10, 2023
@evan-forbes
Copy link
Member

This appears to be due to a recent ledger firmware update, as I could not replicate the bug until I tried with an updated ledger X running 2.2.2

@rootulp
Copy link
Collaborator

rootulp commented Oct 12, 2023

Notes

  • Based on this comment we need to bump ledger-cosmos-go to v0.13.1 in celestiaorg/cosmos-sdk and then in celestia-app

  • ledger-cosmos-go v0.13.0 contains a poorly documented breaking change

    $ make build
    go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=sim -X github.com/cosmos/cosmos-sdk/version.AppName=simd -X github.com/cosmos/cosmos-sdk/version.Version=0.46.13-128-g3273d71f4c -X github.com/cosmos/cosmos-sdk/version.Commit=3273d71f4c20a34b4258bdc57e8281c83e8ef0f7 -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" -X github.com/tendermint/tendermint/version.TMCoreSemVer=v1.26.2-tm-v0.34.28 -w -s' -trimpath -o /Users/rootulp/git/rootulp/celestia/cosmos-sdk/build/ ./...
    # github.com/cosmos/cosmos-sdk/crypto/ledger
    crypto/ledger/ledger_real.go:18:10: cannot use device (variable of type *ledger_cosmos_go.LedgerCosmos) as SECP256K1 value in return statement: *ledger_cosmos_go.LedgerCosmos does not implement SECP256K1 (wrong type for method SignSECP256K1)
      	  have SignSECP256K1([]uint32, []byte, byte) ([]byte, error)
      	  want SignSECP256K1([]uint32, []byte) ([]byte, error)
    make: *** [build] Error 1
    
  • Relevant commit in upstream cosmos/cosmos-sdk which has not been applied to the v0.46.x release branch. I think we'll need to apply some portion of that commit to celestiaorg/cosmos-sdk.

@rootulp rootulp self-assigned this Oct 12, 2023
@rootulp
Copy link
Collaborator

rootulp commented Oct 12, 2023

Works after bumping cosmos-ledger-go in celestiaorg/cosmos-sdk and overriding celestia-app to use that cosmos-sdk.

Tested with some steps from gist and then:

$ celestia-appd tx sign ~/Downloads/tx-test.json --from ledger-1 --chain-id private --ledger --home $CELESTIA_APP_HOME --offline --sequence 0 --account-number 0
Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.
Error: please open Cosmos app on the Ledger device - error: Error code: 6b0c
$ celestia-appd tx sign ~/Downloads/tx-test.json --from ledger-1 --chain-id private --ledger --home $CELESTIA_APP_HOME --offline --sequence 0 --account-number 0
Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.
{"body":{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"celestia1xdc39xr027r78w4eeq6e7e35glgza9ka9rsds2","to_address":"celestia10enzsgdw8wgnsx8w25w5km07wxh8pa6cmeklsg","amount":[{"denom":"utia","amount":"100000"}]}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[{"public_key":{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A+u46124WB1YIjLA+Ufriy5aQzcvdM4bily/Y32INEGV"},"mode_info":{"single":{"mode":"SIGN_MODE_LEGACY_AMINO_JSON"}},"sequence":"0"}],"fee":{"amount":[{"denom":"utia","amount":"21000"}],"gas_limit":"210000","payer":"","granter":""},"tip":null},"signatures":["16Ty6LIwBa4zJfDioWLQ+MwUTCHJL22W2HafSJWc1rV9QwwG2cwZNdBc+pnnFAzQ0eQ7ffHPkKYJm3faItPLjA=="]}

@rootulp
Copy link
Collaborator

rootulp commented Oct 12, 2023

Nvm I observed working behavior on celestia-app v1.x branch too (in other words without my cosmos-sdk modifications) so I suspect this is b/c my Ledger isn't on the buggy version yet.

My test Ledger was on Ledger firmware 2.0.1 and Ledger Cosmos App v2.18.0. I updated to Ledger Live 2.69.0 and then Ledger firmware 2.2.1 and then Ledger firmware 2.2.2 and Ledger Cosmos App v2.34.12. Was able to repro the broken behavior

$ celestia-appd tx sign ~/Downloads/tx-test.json --from ledger-1 --chain-id private --ledger --home $CELESTIA_APP_HOME --offline --sequence 0 --account-number 0
Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.
Error:

@rootulp
Copy link
Collaborator

rootulp commented Oct 12, 2023

Re-verified the fix works

$ make build && make install
--> Updating go.mod
--> Installing celestia-appd
$ celestia-appd tx bank send --chain-id private $CELESTIA_LEDGER $CELESTIA_VALIDATOR 100000utia --fees 21000utia --generate-only > ~/Downloads/tx-test.json
$ celestia-appd tx sign ~/Downloads/tx-test.json --from ledger-1 --chain-id private --ledger --home $CELESTIA_APP_HOME --offline --sequence 0 --account-number 0
Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.
{"body":{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"celestia1xdc39xr027r78w4eeq6e7e35glgza9ka9rsds2","to_address":"celestia1mm9wqztvknazdmtm8veuzcjjh2mux3h3hc65rv","amount":[{"denom":"utia","amount":"100000"}]}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[{"public_key":{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A+u46124WB1YIjLA+Ufriy5aQzcvdM4bily/Y32INEGV"},"mode_info":{"single":{"mode":"SIGN_MODE_LEGACY_AMINO_JSON"}},"sequence":"0"}],"fee":{"amount":[{"denom":"utia","amount":"21000"}],"gas_limit":"210000","payer":"","granter":""},"tip":null},"signatures":["cbBwJf7f9mZwJ8GYyAZk3Mgyw2udxFJx4vwtgkjTcrFmM5QRyQGGENq80N/m9pOtqHF6dME1Olxahdy+GYA/+g=="]}

rootulp added a commit that referenced this issue Oct 13, 2023
## Overview

Closes #2650 by upgrading to celestiaorg/cosmos-sdk
[v1.18.2-sdk-v0.46.14](https://github.com/celestiaorg/cosmos-sdk/releases/tag/v1.18.2-sdk-v0.46.14)

## Checklist

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords

---------

Co-authored-by: Rootul Patel <[email protected]>
mergify bot pushed a commit that referenced this issue Oct 13, 2023
## Overview

Closes #2650 by upgrading to celestiaorg/cosmos-sdk
[v1.18.2-sdk-v0.46.14](https://github.com/celestiaorg/cosmos-sdk/releases/tag/v1.18.2-sdk-v0.46.14)

## Checklist

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords

---------

Co-authored-by: Rootul Patel <[email protected]>
(cherry picked from commit c0e1c22)

# Conflicts:
#	go.mod
#	go.sum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants