Skip to content

Commit

Permalink
fix: In Keybase GetByAddress, change generic error to NewErrKeyNotFou…
Browse files Browse the repository at this point in the history
…nd (#1316)

The Keybase method `GetByName` [returns the specific
error](https://github.com/gnolang/gno/blob/199cd29584d44812a0aec3606bbff37a320c609a/tm2/pkg/crypto/keys/keybase.go#L184)
`ErrKeyNotFound` if the key is not found. This is very nice because the
GnoMobile API wraps this with a gRPC function and we use
`keyerror.IsErrKeyNotFound` to check the error type and convert it to
the gRPC equivalent. But the Keybase method `GetByAddress` just [returns
a generic
error](https://github.com/gnolang/gno/blob/199cd29584d44812a0aec3606bbff37a320c609a/tm2/pkg/crypto/keys/keybase.go#L192)
if the key isn't found. (I assume that it is not intentional to restrict
this to a generic error.) This means that the gRPC interface must search
the error string for "not found", which is unreliable. We would prefer
to use `keyerror.IsErrKeyNotFound`. Furthermore, `GetByNameOrAddress`
[can return an error from
either](https://github.com/gnolang/gno/blob/199cd29584d44812a0aec3606bbff37a320c609a/tm2/pkg/crypto/keys/keybase.go#L175-L177)
`GetByName` or `GetByAddress`. It is preferable to return just one error
type for key not found.

This pull request updates `GetByAddress` to use
`keyerror.NewErrKeyNotFound` with the same error message.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
  • Loading branch information
jefft0 committed Nov 9, 2023
1 parent e803c45 commit da05213
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
2 changes: 1 addition & 1 deletion tm2/pkg/crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (kb dbKeybase) GetByName(name string) (Info, error) {
func (kb dbKeybase) GetByAddress(address crypto.Address) (Info, error) {
ik := kb.db.Get(addrKey(address))
if len(ik) == 0 {
return nil, fmt.Errorf("key with address %s not found", address)
return nil, keyerror.NewErrKeyNotFound(fmt.Sprintf("key with address %s not found", address))
}
bs := kb.db.Get(ik)
return readInfo(bs)
Expand Down
2 changes: 2 additions & 0 deletions tm2/pkg/crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/crypto/ed25519"
"github.com/gnolang/gno/tm2/pkg/crypto/keys/keyerror"
)

func TestCreateAccountInvalidMnemonic(t *testing.T) {
Expand Down Expand Up @@ -107,6 +108,7 @@ func TestKeyManagement(t *testing.T) {
require.NoError(t, err)
_, err = cstore.GetByAddress(addr)
require.NotNil(t, err)
require.True(t, keyerror.IsErrKeyNotFound(err))

// list shows them in order
keyS, err := cstore.List()
Expand Down

0 comments on commit da05213

Please sign in to comment.