Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
vgonkivs committed Jul 3, 2024
1 parent dbf3679 commit bece04c
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 65 deletions.
2 changes: 1 addition & 1 deletion cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Options passed on start override configuration options only on start and are not
// TODO @renaynay: Include option for setting custom `userInput` parameter with
// implementation of https://github.com/celestiaorg/celestia-node/issues/415.
encConf := encoding.MakeConfig(app.ModuleEncodingRegisters...)
ring, err := keyring.New(app.Name, cfg.State.KeyringBackend, keysPath, os.Stdin, encConf.Codec)
ring, err := keyring.New(app.Name, cfg.State.DefaultBackendName, keysPath, os.Stdin, encConf.Codec)
if err != nil {
return err
}
Expand Down
16 changes: 8 additions & 8 deletions docs/adr/adr-009-public-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,15 @@ NetworkHead(ctx context.Context) (*header.ExtendedHeader, error)
ctx context.Context,
nID namespace.ID,
data []byte,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)
// Transfer sends the given amount of coins from default wallet of the node
// to the given account address.
Transfer(
ctx context.Context,
to types.Address,
amount types.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)

// StateModule also provides StakingModule
Expand All @@ -282,23 +282,23 @@ yet.
ctx context.Context,
delAddr state.ValAddress,
amount state.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)
// BeginRedelegate sends a user's delegated tokens to a new validator for redelegation.
BeginRedelegate(
ctx context.Context,
srcValAddr,
dstValAddr state.ValAddress,
amount state.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)
// Undelegate undelegates a user's delegated tokens, unbonding them from the
// current validator.
Undelegate(
ctx context.Context,
delAddr state.ValAddress,
amount state.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)

// CancelUnbondingDelegation cancels a user's pending undelegation from a
Expand All @@ -308,7 +308,7 @@ yet.
valAddr state.ValAddress,
amount types.Int,
height types.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)

// QueryDelegation retrieves the delegation information between a delegator
Expand Down Expand Up @@ -399,15 +399,15 @@ type BankModule interface {
SubmitPayForBlob(
ctx context.Context,
blobs []*state.Blob,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)
// Transfer sends the given amount of coins from default wallet of the node
// to the given account address.
Transfer(
ctx context.Context,
to state.AccAddress,
amount state.Int,
options *state.TxConfig,
config *state.TxConfig,
) (*state.TxResponse, error)
}
```
Expand Down
6 changes: 3 additions & 3 deletions nodebuilder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestUpdateConfig(t *testing.T) {
// ensure this config field is now set after updating the config
require.Equal(t, newCfg.Share.PeerManagerParams, cfg.Share.PeerManagerParams)
// ensure old custom values were not changed
require.Equal(t, "thisshouldnthavechanged", cfg.State.KeyringKeyName)
require.Equal(t, "thisshouldnthavechanged", cfg.State.DefaultKeyName)
require.Equal(t, "7979", cfg.RPC.Port)
require.True(t, cfg.Gateway.Enabled)
}
Expand All @@ -65,8 +65,8 @@ var outdatedConfig = `
GRPCPort = "0"
[State]
KeyringKeyName = "thisshouldnthavechanged"
KeyringBackend = "test"
DefaultKeyName = "thisshouldnthavechanged"
DefaultBackendName = "test"
[P2P]
ListenAddresses = ["/ip4/0.0.0.0/udp/2121/quic-v1", "/ip6/::/udp/2121/quic-v1", "/ip4/0.0.0.0/tcp/2121",
Expand Down
6 changes: 3 additions & 3 deletions nodebuilder/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ func initDir(path string) error {
func generateKeys(cfg Config, ksPath string) error {
encConf := encoding.MakeConfig(app.ModuleEncodingRegisters...)

if cfg.State.KeyringBackend == keyring.BackendTest {
if cfg.State.DefaultBackendName == keyring.BackendTest {
log.Warn("Detected plaintext keyring backend. For elevated security properties, consider using" +
" the `file` keyring backend.")
}
ring, err := keyring.New(app.Name, cfg.State.KeyringBackend, ksPath, os.Stdin, encConf.Codec)
ring, err := keyring.New(app.Name, cfg.State.DefaultBackendName, ksPath, os.Stdin, encConf.Codec)
if err != nil {
return err
}
Expand Down Expand Up @@ -228,6 +228,6 @@ func generateKeys(cfg Config, ksPath string) error {
// generateNewKey generates and returns a new key on the given keyring called
// "my_celes_key".
func generateNewKey(ring keyring.Keyring) (*keyring.Record, string, error) {
return ring.NewMnemonic(state.DefaultAccountName, keyring.English, sdk.GetConfig().GetFullBIP44Path(),
return ring.NewMnemonic(state.DefaultKeyName, keyring.English, sdk.GetConfig().GetFullBIP44Path(),
keyring.DefaultBIP39Passphrase, hd.Secp256k1)
}
4 changes: 2 additions & 2 deletions nodebuilder/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestInit_generateNewKey(t *testing.T) {
cfg := DefaultConfig(node.Bridge)

encConf := encoding.MakeConfig(app.ModuleEncodingRegisters...)
ring, err := keyring.New(app.Name, cfg.State.KeyringBackend, t.TempDir(), os.Stdin, encConf.Codec)
ring, err := keyring.New(app.Name, cfg.State.DefaultBackendName, t.TempDir(), os.Stdin, encConf.Codec)
require.NoError(t, err)

originalKey, mn, err := generateNewKey(ring)
Expand All @@ -93,7 +93,7 @@ func TestInit_generateNewKey(t *testing.T) {
assert.Contains(t, addr.String(), "celestia")

// ensure account is recoverable from mnemonic
ring2, err := keyring.New(app.Name, cfg.State.KeyringBackend, t.TempDir(), os.Stdin, encConf.Codec)
ring2, err := keyring.New(app.Name, cfg.State.DefaultBackendName, t.TempDir(), os.Stdin, encConf.Codec)
require.NoError(t, err)
duplicateKey, err := ring2.NewAccount("test", mn, keyring.DefaultBIP39Passphrase, sdk.GetConfig().GetFullBIP44Path(),
hd.Secp256k1)
Expand Down
10 changes: 5 additions & 5 deletions nodebuilder/state/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ import (
"github.com/cosmos/cosmos-sdk/crypto/keyring"
)

var defaultKeyringBackend = keyring.BackendTest
var defaultBackendName = keyring.BackendTest

// Config contains configuration parameters for constructing
// the node's keyring signer.
type Config struct {
KeyringKeyName string
KeyringBackend string
DefaultKeyName string
DefaultBackendName string
}

func DefaultConfig() Config {
return Config{
KeyringKeyName: DefaultAccountName,
KeyringBackend: defaultKeyringBackend,
DefaultKeyName: DefaultKeyName,
DefaultBackendName: defaultBackendName,
}
}

Expand Down
12 changes: 6 additions & 6 deletions nodebuilder/state/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ var (
func Flags() *flag.FlagSet {
flags := &flag.FlagSet{}

flags.String(keyringKeyNameFlag, DefaultAccountName,
flags.String(keyringKeyNameFlag, DefaultKeyName,
fmt.Sprintf("Directs node's keyring signer to use the key prefixed with the "+
"given string. Default is %s", DefaultAccountName))
flags.String(keyringBackendFlag, defaultKeyringBackend,
"given string. Default is %s", DefaultKeyName))
flags.String(keyringBackendFlag, defaultBackendName,
fmt.Sprintf("Directs node's keyring signer to use the given "+
"backend. Default is %s.", defaultKeyringBackend))
"backend. Default is %s.", defaultBackendName))
return flags
}

// ParseFlags parses State flags from the given cmd and saves them to the passed config.
func ParseFlags(cmd *cobra.Command, cfg *Config) {
cfg.KeyringKeyName = cmd.Flag(keyringKeyNameFlag).Value.String()
cfg.KeyringBackend = cmd.Flag(keyringBackendFlag).Value.String()
cfg.DefaultKeyName = cmd.Flag(keyringKeyNameFlag).Value.String()
cfg.DefaultBackendName = cmd.Flag(keyringBackendFlag).Value.String()
}
9 changes: 6 additions & 3 deletions nodebuilder/state/keyring.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package state

import (
"fmt"

kr "github.com/cosmos/cosmos-sdk/crypto/keyring"

"github.com/celestiaorg/celestia-node/libs/keystore"
)

const DefaultAccountName = "my_celes_key"
const DefaultKeyName = "my_celes_key"

type AccountName string

Expand All @@ -15,9 +17,10 @@ type AccountName string
// as having keyring-backend set to `file` prompts user for password.
func Keyring(cfg Config, ks keystore.Keystore) (kr.Keyring, AccountName, error) {
ring := ks.Keyring()
keyInfo, err := ring.Key(cfg.KeyringKeyName)
keyInfo, err := ring.Key(cfg.DefaultKeyName)
if err != nil {
log.Errorw("could not access key in keyring", "keyring.keyname", cfg.KeyringKeyName)
err = fmt.Errorf("can't get key: `%s` from the keystore: %w", cfg.DefaultKeyName, err)
log.Error(err)
return nil, "", err
}
return ring, AccountName(keyInfo.Name), nil
Expand Down
Loading

0 comments on commit bece04c

Please sign in to comment.