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: Error string in decryptPrivKey. Use errors.As in IsErrWrongPassword. #1289

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

jefft0
Copy link
Contributor

@jefft0 jefft0 commented Oct 24, 2023

This PR fixes bugs with error handling:

…s.As in IsErrWrongPassword. See the PR.

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
@github-actions github-actions bot added the 📦 🌐 tendermint v2 Issues or PRs tm2 related label Oct 24, 2023
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a3bdd2b) 47.89% compared to head (409df81) 47.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1289      +/-   ##
==========================================
+ Coverage   47.89%   47.90%   +0.01%     
==========================================
  Files         372      372              
  Lines       62990    62990              
==========================================
+ Hits        30166    30175       +9     
+ Misses      30363    30354       -9     
  Partials     2461     2461              
Files Coverage Δ
tm2/pkg/crypto/keys/armor/armor.go 53.75% <100.00%> (+2.50%) ⬆️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jefft0 jefft0 changed the title fix check in decryptPrivKey. Use errors.As in IsErrWrongPassword. fix: Error string in decryptPrivKey. Use errors.As in IsErrWrongPassword. Oct 24, 2023
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Thank you for the fix 🙏

@jefft0 jefft0 removed the request for review from jaekwon October 25, 2023 07:05
@jefft0
Copy link
Contributor Author

jefft0 commented Oct 25, 2023

Thanks for the approval, @zivkovicmilos . Does this PR need more approvals before merging?

@zivkovicmilos
Copy link
Member

Thanks for the approval, @zivkovicmilos . Does this PR need more approvals before merging?

We try to keep at least a minimum of 2 reviewers per PR, even for small changes like this. I've pinged @ajnavarro to take a look as well, so we should have it merged in soon 🚀

@thehowl thehowl merged commit f39cc46 into gnolang:master Oct 26, 2023
177 of 178 checks passed
gfanton pushed a commit to gfanton/gno that referenced this pull request Nov 9, 2023
…ord. (gnolang#1289)

This PR fixes bugs with error handling:
* `decryptPrivKey` checks if `DecryptSymmetric` [returns the error
"Ciphertext decryption
failed"](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/keys/armor/armor.go#L131)
and converts it to `ErrWrongPassword`. The problem is that
`DecryptSymmetric` [returns "ciphertext decryption
failed"](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/xsalsa20symmetric/symmetric.go#L53C27-L53C55)
(spelled differently). This PR fixes the string in the error check.
* `IsErrWrongPassword` checks if the [error type is
`keybaseError`](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/keys/keyerror/errors.go#L75C24-L75C36)
. But the error can be wrapped as it is [in
`signAndBroadcastTxCommit`](https://github.com/gnolang/gno/blob/60e05e83f57558843c0808f78500b6a51b2a22c1/gno.land/pkg/gnoclient/client_txs.go#L104).
Therefore, instead of a simple error type check, this PR updates
`IsErrWrongPassword` (and `IsErrKeyNotFound`) to use `errors.As` to
check the error type (which unwraps the error if needed).

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
moul pushed a commit to moul/gno that referenced this pull request Nov 14, 2023
…ord. (gnolang#1289)

This PR fixes bugs with error handling:
* `decryptPrivKey` checks if `DecryptSymmetric` [returns the error
"Ciphertext decryption
failed"](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/keys/armor/armor.go#L131)
and converts it to `ErrWrongPassword`. The problem is that
`DecryptSymmetric` [returns "ciphertext decryption
failed"](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/xsalsa20symmetric/symmetric.go#L53C27-L53C55)
(spelled differently). This PR fixes the string in the error check.
* `IsErrWrongPassword` checks if the [error type is
`keybaseError`](https://github.com/gnolang/gno/blob/a3bdd2bb25b76b17176c3c59a1ce2522f8a75e53/tm2/pkg/crypto/keys/keyerror/errors.go#L75C24-L75C36)
. But the error can be wrapped as it is [in
`signAndBroadcastTxCommit`](https://github.com/gnolang/gno/blob/60e05e83f57558843c0808f78500b6a51b2a22c1/gno.land/pkg/gnoclient/client_txs.go#L104).
Therefore, instead of a simple error type check, this PR updates
`IsErrWrongPassword` (and `IsErrKeyNotFound`) to use `errors.As` to
check the error type (which unwraps the error if needed).

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related
Projects
Status: Done
Status: No status
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants