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(cmd): add notice when using --ledger with Ethereum HD path #1285

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

lumtis
Copy link
Member

@lumtis lumtis commented Oct 12, 2023

Description

Add a notice to quickly figure out the issue when using --ledger with default HD path.

I realized that injecting logic at runtime for an existing command is non-trivial without a fork or wrap (which we don't want for this fix).
I instead added a notice below the execution if the error is cannot set custom bip32 path with ledge

$zetacored keys add testledger --ledger --account 0
Error: cannot set custom bip32 path with ledger
Usage:
  <appd> keys add <name> [flags]

Flags:
      --account uint32           Account number for HD derivation (less than equal 2147483647)
      --algo string              Key signing algorithm to generate keys for (default "eth_secp256k1")
      --coin-type uint32         coin type number for HD derivation (default 118)
      --dry-run                  Perform action, but don't add key to local keystore
      --hd-path string           Manual HD Path derivation (overrides BIP44 config) (default "m/44'/60'/0'/0/0")
  -h, --help                     help for add
      --index uint32             Address index number for HD derivation (less than equal 2147483647)
  -i, --interactive              Interactively prompt user for BIP39 passphrase and mnemonic
      --ledger                   Store a local reference to a private key on a Ledger device
      --multisig strings         List of key names stored in keyring to construct a public legacy multisig key
      --multisig-threshold int   K out of N required signatures. For use in conjunction with --multisig (default 1)
      --no-backup                Don't print out seed phrase (if others are watching the terminal)
      --nosort                   Keys passed to --multisig are taken in the order they're supplied
      --pubkey string            Parse a public key in JSON format and saves key info to <name> file.
      --recover                  Provide seed phrase to recover existing key instead of creating

Global Flags:
      --keyring-backend string   Select keyring's backend (os|file|test) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --log_format string        The logging format (json|plain) (default "plain")
      --log_level string         The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --output string            Output format (text|json) (default "text")
      --trace                    print out full stack trace on errors

*****************************************************************************
* note: --ledger flag can't be used with Ethereum HD path (used by default)
* use --hd-path="" in the command to use Cosmos HD path
*****************************************************************************

Closes: #1284

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

Copy link
Contributor

@kevinssgh kevinssgh left a comment

Choose a reason for hiding this comment

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

Looks good

@lumtis lumtis merged commit 8a308fc into develop Oct 13, 2023
@lumtis lumtis deleted the fix/error-ledger branch October 13, 2023 16:37
ws4charlie added a commit that referenced this pull request Oct 13, 2023
#1292)

* refactor: change default mempool version in config (#1238)

* fix(`MsgWhitelistERC20`): set unique index for generate cctx (#1245)

* update index

* remove deprecated functions

* make generate

* add return value in message

* initialize test

* whitelist tests

* make generate

* fix(`observer`): remove error return in `IsAuthorized` (#1250)

* update function

* regenerate interfaces

* update for crosschain

* fix(`GetForeignCoinFromAsset`): Ethereum comparaison checksum/non-checksum format (#1261)

* fix error message

* compare with ETH address type

* tests

* goimport

---------

Co-authored-by: brewmaster012 <[email protected]>

* feat(`fungible`): add ability to update gas limit (#1260)

* add new field

* update message type

* message new logic

* fix: Blame index update (#1264)

* initial commit

* added queries and unit tests

* added cli

* fix parse error

* fix parse error 2

* fix lint and test errors

* ran make generate

* update index for keygen

* refactor query name

* refactor key calculation

* refactor lib name

* fix: feed sataoshi/B to zetacore and check actual outTx size (#1243)

* feed sataoshi/B to zetacore and check size limit

* removed fee cap

* replaced magic number 1000 with constant bytesPerKB

* put lowerbound, upperbound limit on sizeLimit

* use actual txSize for fee calculation

---------

Co-authored-by: charliec <[email protected]>

* fix: cherry pick all hotfix from v10.0.x (zero-amount, precision, etc.) (#1235)

* cherry pick all hotfix from v10.0.x

* adjusted code to for nosec

* adusted error handling and code comments according to PR review feedback

* cherry pick hotfix for bitcoin outbound performance and updated some log prints

* cherry pick mock mainnet hotfix for duplicate payment on nonce 0

---------

Co-authored-by: charliec <[email protected]>

* fix: register emissions grpc server (#1257)

* feat: Bitcoin block header and merkle proof (#1263)

* initiated bitcoin header and proof

* added smoke test for bitcoin merkle proof and RPC query

* make generate

* fix gosec and unit test

* Update common/headers_test.go

Co-authored-by: Lucas Bertrand <[email protected]>

* code adjustment according to feedback of PR review

* corrected a typo and added more comment to function

* fix gosec error

---------

Co-authored-by: charliec <[email protected]>
Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: brewmaster012 <[email protected]>

* fix: read gas limit from smart contract (#1277)

* read gas limit from smart contract

* add more checks for gas limit

* fix(`fungible`): add CLI command to query system contract (#1252)

* fix proto

* fix filename

* add cli query

* fix(`cmd`): add notice when using `--ledger` with Ethereum HD path (#1285)

* change comment

* add notice for ledger

* merge develop into inbound-tracker and unified proof verification

* fixed gosec errors

---------

Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: brewmaster012 <[email protected]>
Co-authored-by: kevinssgh <[email protected]>
Co-authored-by: charliec <[email protected]>
Co-authored-by: Tanmay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: Add informative error when --ledger is used with the EVM hd-path
3 participants