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

Harry/wallet init #1190

Merged
merged 20 commits into from
Oct 20, 2023
Merged

Harry/wallet init #1190

merged 20 commits into from
Oct 20, 2023

Conversation

Cosmos-Harry
Copy link
Contributor

  • Added a new flag --restore-all
  • Without the flag, the args work normally but with the flag chain_name is not needed and the order has changed
  • closes initialize wallets command #1030

Copy link
Member

@jtieri jtieri left a comment

Choose a reason for hiding this comment

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

I had a few comments but nothing major, changes mostly look good. This will be a nice feature to have!

One request I do have is can we get a test case added for this?
There are some existing tests in keys_test.go, perhaps create a new test case based on the existing test

relayer/cmd/keys_test.go

Lines 38 to 78 in cb4708b

func TestKeysRestore_Delete(t *testing.T) {
t.Parallel()
sys := relayertest.NewSystem(t)
_ = sys.MustRun(t, "config", "init")
slip44 := 118
sys.MustAddChain(t, "testChain", cmd.ProviderConfigWrapper{
Type: "cosmos",
Value: cosmos.CosmosProviderConfig{
AccountPrefix: "cosmos",
ChainID: "testcosmos",
KeyringBackend: "test",
Timeout: "10s",
Slip44: &slip44,
},
})
// Restore a key with mnemonic to the chain.
res := sys.MustRun(t, "keys", "restore", "testChain", "default", relayertest.ZeroMnemonic)
require.Equal(t, res.Stdout.String(), relayertest.ZeroCosmosAddr+"\n")
require.Empty(t, res.Stderr.String())
// Restored key must show up in list.
res = sys.MustRun(t, "keys", "list", "testChain")
require.Equal(t, res.Stdout.String(), "key(default) -> "+relayertest.ZeroCosmosAddr+"\n")
require.Empty(t, res.Stderr.String())
// Deleting the key must succeed.
res = sys.MustRun(t, "keys", "delete", "testChain", "default", "-y")
require.Empty(t, res.Stdout.String())
require.Equal(t, res.Stderr.String(), "key default deleted\n")
// Listing the keys again gives the no keys warning.
res = sys.MustRun(t, "keys", "list", "testChain")
require.Empty(t, res.Stdout.String())
require.Contains(t, res.Stderr.String(), "no keys found for chain testChain")
}

cmd/keys.go Outdated Show resolved Hide resolved
cmd/keys.go Outdated Show resolved Hide resolved
cmd/keys.go Show resolved Hide resolved
cmd/keys.go Outdated Show resolved Hide resolved
@Cosmos-Harry
Copy link
Contributor Author

I had a few comments but nothing major, changes mostly look good. This will be a nice feature to have!

One request I do have is can we get a test case added for this? There are some existing tests in keys_test.go, perhaps create a new test case based on the existing test

relayer/cmd/keys_test.go

Lines 38 to 78 in cb4708b

func TestKeysRestore_Delete(t *testing.T) {
t.Parallel()
sys := relayertest.NewSystem(t)
_ = sys.MustRun(t, "config", "init")
slip44 := 118
sys.MustAddChain(t, "testChain", cmd.ProviderConfigWrapper{
Type: "cosmos",
Value: cosmos.CosmosProviderConfig{
AccountPrefix: "cosmos",
ChainID: "testcosmos",
KeyringBackend: "test",
Timeout: "10s",
Slip44: &slip44,
},
})
// Restore a key with mnemonic to the chain.
res := sys.MustRun(t, "keys", "restore", "testChain", "default", relayertest.ZeroMnemonic)
require.Equal(t, res.Stdout.String(), relayertest.ZeroCosmosAddr+"\n")
require.Empty(t, res.Stderr.String())
// Restored key must show up in list.
res = sys.MustRun(t, "keys", "list", "testChain")
require.Equal(t, res.Stdout.String(), "key(default) -> "+relayertest.ZeroCosmosAddr+"\n")
require.Empty(t, res.Stderr.String())
// Deleting the key must succeed.
res = sys.MustRun(t, "keys", "delete", "testChain", "default", "-y")
require.Empty(t, res.Stdout.String())
require.Equal(t, res.Stderr.String(), "key default deleted\n")
// Listing the keys again gives the no keys warning.
res = sys.MustRun(t, "keys", "list", "testChain")
require.Empty(t, res.Stdout.String())
require.Contains(t, res.Stderr.String(), "no keys found for chain testChain")
}

I have made the suggested changes, please let me know if they are correct, thank you!

Copy link
Member

@jtieri jtieri left a comment

Choose a reason for hiding this comment

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

looks good! thanks for the contribution 🙂

@jtieri jtieri merged commit d0deac5 into cosmos:main Oct 20, 2023
18 checks passed
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.

initialize wallets command
2 participants