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

gaiacli: Improve the error message for invalid passphrases #1498

Closed
wants to merge 2 commits into from

Conversation

ValarDragon
Copy link
Contributor

Ref #1497

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG.md
  • Updated Gaia/Examples
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)

@codecov
Copy link

codecov bot commented Jul 2, 2018

Codecov Report

Merging #1498 into develop will increase coverage by 0.07%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1498      +/-   ##
===========================================
+ Coverage     64.4%   64.48%   +0.07%     
===========================================
  Files          118      118              
  Lines         6496     6493       -3     
===========================================
+ Hits          4184     4187       +3     
+ Misses        2062     2050      -12     
- Partials       250      256       +6

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Can we just change this error in the keybase itself? That code should be in the SDK now.

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Jul 2, 2018

This error is in the armor file, for the actual symmetric encryption/decryption so its still in tendermint. I'm not sure this is the error message we want to be using for tendermint usecases though. (tendermint/tendermint#1821)

I think the better solution though is to eventually have an error table which maps from error key to error message, which everyone uses. (Would also make multi-language support easier in the future)

@cwgoes
Copy link
Contributor

cwgoes commented Jul 5, 2018

I'm not a fan of checking and rewriting an error message like this; it will just introduce more work in the future when we change it upstream. Can we submit a PR to Tendermint to use the error message we want instead (or use an error table, as you mention)?

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.

2 participants