Skip to content

Commit

Permalink
chore: Add Keybase HasByNameOrAddress, HasByName and HasByAddress (gn…
Browse files Browse the repository at this point in the history
…olang#1313)

The Keybase [interface
has](https://github.com/gnolang/gno/blob/7105d00e10209003ea41fbae7a4d7c463c4167c3/tm2/pkg/crypto/keys/types.go#L16C2-L18)
`GetByNameOrAddress`, `GetByName` and `GetByAddress`. If an application
simply wants to check if the key is in the key base, it must use one of
these and check for an error, as is done [in the gnokey command
line](https://github.com/gnolang/gno/blob/7105d00e10209003ea41fbae7a4d7c463c4167c3/tm2/pkg/crypto/keys/client/add.go#L159-L160).

```
		_, err = kb.GetByName(name)
		if err == nil {
			// account exists, ask for user confirmation
			...
		}
```

A GnoMobile app also needs to check for the key. But it's expensive to
return the entire key info. The byte buffer must be copies multiple
times, in the Keybase and in the gRPC interface, and the key info
structure must be reformatted to Protobuf and then to the application's
language. And not preferable to generate errors to pass information in
normal program flow.

In the underlying `DB` interface, there is already a [Has
function](https://github.com/gnolang/gno/blob/7105d00e10209003ea41fbae7a4d7c463c4167c3/tm2/pkg/db/types.go#L13).
This PR updates the `Keybase` with the related functions
`HasByNameOrAddress`, `HasByName` and `HasByAddress` which use `Has` to
inexpensively return a simple bool. It also updates the gnokey command
line to use `HasByName` to be more clear:

```
		if has, err := kb.HasByName(name); err == nil && has {
			// account exists, ask for user confirmation
			...
		}
```

---------

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
  • Loading branch information
2 people authored and moul committed Nov 14, 2023
1 parent f91752d commit 85f7ac6
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 13 deletions.
3 changes: 1 addition & 2 deletions tm2/pkg/crypto/keys/client/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,7 @@ func execAdd(cfg *addCfg, args []string, io commands.IO) error {
return err
}

_, err = kb.GetByName(name)
if err == nil {
if has, err := kb.HasByName(name); err == nil && has {
// account exists, ask for user confirmation
response, err2 := io.GetConfirmation(fmt.Sprintf("Override the existing name %s", name))
if err2 != nil {
Expand Down
22 changes: 20 additions & 2 deletions tm2/pkg/crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,32 @@ func (kb dbKeybase) List() ([]Info, error) {
return res, nil
}

// HasByNameOrAddress checks if a key with the name or bech32 string address is in the keybase.
func (kb dbKeybase) HasByNameOrAddress(nameOrBech32 string) (bool, error) {
address, err := crypto.AddressFromBech32(nameOrBech32)
if err != nil {
return kb.HasByName(nameOrBech32)
}
return kb.HasByAddress(address)
}

// HasByName checks if a key with the name is in the keybase.
func (kb dbKeybase) HasByName(name string) (bool, error) {
return kb.db.Has(infoKey(name)), nil
}

// HasByAddress checks if a key with the address is in the keybase.
func (kb dbKeybase) HasByAddress(address crypto.Address) (bool, error) {
return kb.db.Has(addrKey(address)), nil
}

// Get returns the public information about one key.
func (kb dbKeybase) GetByNameOrAddress(nameOrBech32 string) (Info, error) {
addr, err := crypto.AddressFromBech32(nameOrBech32)
if err != nil {
return kb.GetByName(nameOrBech32)
} else {
return kb.GetByAddress(addr)
}
return kb.GetByAddress(addr)
}

func (kb dbKeybase) GetByName(name string) (Info, error) {
Expand Down
27 changes: 18 additions & 9 deletions tm2/pkg/crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ func TestKeyManagement(t *testing.T) {
assert.Empty(t, l)

// create some keys
_, err = cstore.GetByName(n1)
require.Error(t, err)
has, err := cstore.HasByName(n1)
require.NoError(t, err)
require.False(t, has)
i, err := cstore.CreateAccount(n1, mn1, bip39Passphrase, p1, 0, 0)
require.NoError(t, err)
require.Equal(t, n1, i.GetName())
Expand All @@ -100,10 +101,16 @@ func TestKeyManagement(t *testing.T) {
// we can get these keys
i2, err := cstore.GetByName(n2)
require.NoError(t, err)
_, err = cstore.GetByName(n3)
require.NotNil(t, err)
_, err = cstore.GetByAddress(toAddr(i2))
has, err = cstore.HasByName(n3)
require.NoError(t, err)
require.False(t, has)
has, err = cstore.HasByAddress(toAddr(i2))
require.NoError(t, err)
require.True(t, has)
// Also check with HasByNameOrAddress
has, err = cstore.HasByNameOrAddress(crypto.AddressToBech32(toAddr(i2)))
require.NoError(t, err)
require.True(t, has)
addr, err := crypto.AddressFromBech32("g1frtkxv37nq7arvyz5p0mtjqq7hwuvd4dnt892p")
require.NoError(t, err)
_, err = cstore.GetByAddress(addr)
Expand All @@ -127,8 +134,9 @@ func TestKeyManagement(t *testing.T) {
keyS, err = cstore.List()
require.NoError(t, err)
require.Equal(t, 1, len(keyS))
_, err = cstore.GetByName(n1)
require.Error(t, err)
has, err = cstore.HasByName(n1)
require.NoError(t, err)
require.False(t, has)

// create an offline key
o1 := "offline"
Expand Down Expand Up @@ -388,8 +396,9 @@ func TestSeedPhrase(t *testing.T) {
// now, let us delete this key
err = cstore.Delete(n1, p1, false)
require.Nil(t, err, "%+v", err)
_, err = cstore.GetByName(n1)
require.NotNil(t, err)
has, err := cstore.HasByName(n1)
require.NoError(t, err)
require.False(t, has)
}

func ExampleNew() {
Expand Down
30 changes: 30 additions & 0 deletions tm2/pkg/crypto/keys/lazy_keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,36 @@ func (lkb lazyKeybase) List() ([]Info, error) {
return NewDBKeybase(db).List()
}

func (lkb lazyKeybase) HasByNameOrAddress(nameOrBech32 string) (bool, error) {
db, err := db.NewDB(lkb.name, dbBackend, lkb.dir)
if err != nil {
return false, err
}
defer db.Close()

return NewDBKeybase(db).HasByNameOrAddress(nameOrBech32)
}

func (lkb lazyKeybase) HasByName(name string) (bool, error) {
db, err := db.NewDB(lkb.name, dbBackend, lkb.dir)
if err != nil {
return false, err
}
defer db.Close()

return NewDBKeybase(db).HasByName(name)
}

func (lkb lazyKeybase) HasByAddress(address crypto.Address) (bool, error) {
db, err := db.NewDB(lkb.name, dbBackend, lkb.dir)
if err != nil {
return false, err
}
defer db.Close()

return NewDBKeybase(db).HasByAddress(address)
}

func (lkb lazyKeybase) GetByNameOrAddress(nameOrBech32 string) (Info, error) {
db, err := db.NewDB(lkb.name, dbBackend, lkb.dir)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions tm2/pkg/crypto/keys/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import (
type Keybase interface {
// CRUD on the keystore
List() ([]Info, error)
HasByNameOrAddress(nameOrBech32 string) (bool, error)
HasByName(name string) (bool, error)
HasByAddress(address crypto.Address) (bool, error)
GetByNameOrAddress(nameOrBech32 string) (Info, error)
GetByName(name string) (Info, error)
GetByAddress(address crypto.Address) (Info, error)
Expand Down

0 comments on commit 85f7ac6

Please sign in to comment.