From e867c0a8f10ae285be7f90178a9c5d530beeb5a8 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Mon, 9 Oct 2023 12:31:43 +0200 Subject: [PATCH 01/16] Add in base gnokey derive command --- tm2/pkg/crypto/keys/client/derive.go | 134 ++++++++++++++++++++++ tm2/pkg/crypto/keys/client/derive_test.go | 98 ++++++++++++++++ tm2/pkg/crypto/keys/client/root.go | 1 + 3 files changed, 233 insertions(+) create mode 100644 tm2/pkg/crypto/keys/client/derive.go create mode 100644 tm2/pkg/crypto/keys/client/derive_test.go diff --git a/tm2/pkg/crypto/keys/client/derive.go b/tm2/pkg/crypto/keys/client/derive.go new file mode 100644 index 00000000000..cf4d6c1de7c --- /dev/null +++ b/tm2/pkg/crypto/keys/client/derive.go @@ -0,0 +1,134 @@ +package client + +import ( + "context" + "errors" + "flag" + "math" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto" + "github.com/gnolang/gno/tm2/pkg/crypto/bip39" + "github.com/gnolang/gno/tm2/pkg/crypto/hd" + "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" +) + +var ( + errInvalidMnemonic = errors.New("invalid bip39 mnemonic") + errInvalidNumAccounts = errors.New("invalid number of accounts") + errInvalidAccountIndex = errors.New("invalid account index") +) + +type deriveCfg struct { + mnemonic string + numAccounts uint64 + accountIndex uint64 +} + +// newDeriveCmd creates a new gnokey derive subcommand +func newDeriveCmd(io *commands.IO) *commands.Command { + cfg := &deriveCfg{} + + return commands.NewCommand( + commands.Metadata{ + Name: "derive", + ShortUsage: "derive [flags]", + ShortHelp: "Derives the account addresses from the specified mnemonic", + }, + cfg, + func(_ context.Context, _ []string) error { + return execDerive(cfg, io) + }, + ) +} + +func (c *deriveCfg) RegisterFlags(fs *flag.FlagSet) { + fs.StringVar( + &c.mnemonic, + "mnemonic", + "", + "the bip39 mnemonic", + ) + + fs.Uint64Var( + &c.numAccounts, + "num-accounts", + 10, + "the number of accounts to derive from the mnemonic", + ) + + fs.Uint64Var( + &c.accountIndex, + "account-index", + 0, + "the account index in the mnemonic", + ) +} + +func execDerive(cfg *deriveCfg, io *commands.IO) error { + // Make sure the number of accounts is valid + if cfg.numAccounts == 0 || !isUint32(cfg.numAccounts) { + return errInvalidNumAccounts + } + + // Make sure the account index is valid + if !isUint32(cfg.accountIndex) { + return errInvalidAccountIndex + } + + // Make sure the mnemonic is valid + if !bip39.IsMnemonicValid(cfg.mnemonic) { + return errInvalidMnemonic + } + + // Generate the accounts + accounts := generateAccounts( + cfg.mnemonic, + cfg.accountIndex, + cfg.numAccounts, + ) + + io.Printf("[Generated Accounts]\n\n") + io.Printf("Account Index: %d\n\n", cfg.accountIndex) + + // Print them out + for index, account := range accounts { + io.Printfln("%d. %s", index, account.String()) + } + + return nil +} + +// isUint32 verifies a uint64 value can be represented +// as a uint32 +func isUint32(value uint64) bool { + return value <= math.MaxUint32 +} + +// generateAccounts the accounts using the provided mnemonics +func generateAccounts(mnemonic string, accountIndex, numAccounts uint64) []crypto.Address { + addresses := make([]crypto.Address, numAccounts) + + // Generate the seed + seed := bip39.NewSeed(mnemonic, "") + + for i := uint64(0); i < numAccounts; i++ { + key := generateKeyFromSeed(seed, uint32(accountIndex), uint32(i)) + address := key.PubKey().Address() + + addresses[i] = address + } + + return addresses +} + +// generateKeyFromSeed generates a private key from +// the provided seed and index +func generateKeyFromSeed(seed []byte, account, index uint32) crypto.PrivKey { + pathParams := hd.NewFundraiserParams(account, crypto.CoinType, index) + + masterPriv, ch := hd.ComputeMastersFromSeed(seed) + derivedPriv, _ := hd.DerivePrivateKeyForPath(masterPriv, ch, pathParams.String()) + + return secp256k1.PrivKeySecp256k1(derivedPriv) +} diff --git a/tm2/pkg/crypto/keys/client/derive_test.go b/tm2/pkg/crypto/keys/client/derive_test.go new file mode 100644 index 00000000000..3ba9d525784 --- /dev/null +++ b/tm2/pkg/crypto/keys/client/derive_test.go @@ -0,0 +1,98 @@ +package client + +import ( + "bytes" + "math" + "testing" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto/bip39" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_execDerive(t *testing.T) { + t.Parallel() + + t.Run("invalid number of accounts, no accounts requested", func(t *testing.T) { + t.Parallel() + + cfg := &deriveCfg{ + numAccounts: 0, + } + + assert.ErrorIs(t, execDerive(cfg, nil), errInvalidNumAccounts) + }) + + t.Run("invalid number of accounts, > uint32", func(t *testing.T) { + t.Parallel() + + cfg := &deriveCfg{ + numAccounts: math.MaxUint32 + 1, // > uint32 + } + + assert.ErrorIs(t, execDerive(cfg, nil), errInvalidNumAccounts) + }) + + t.Run("invalid account index", func(t *testing.T) { + t.Parallel() + + cfg := &deriveCfg{ + numAccounts: 1, + accountIndex: math.MaxUint32 + 1, // > uint32 + } + + assert.ErrorIs(t, execDerive(cfg, nil), errInvalidAccountIndex) + }) + + t.Run("invalid mnemonic", func(t *testing.T) { + t.Parallel() + + cfg := &deriveCfg{ + numAccounts: 1, + accountIndex: 0, + mnemonic: "one two", + } + + assert.ErrorIs(t, execDerive(cfg, nil), errInvalidMnemonic) + }) + + t.Run("valid accounts generated", func(t *testing.T) { + t.Parallel() + + // Generate a dummy mnemonic + entropy, entropyErr := bip39.NewEntropy(mnemonicEntropySize) + require.NoError(t, entropyErr) + + mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) + require.NoError(t, mnemonicErr) + + cfg := &deriveCfg{ + numAccounts: 1, + accountIndex: 0, + mnemonic: mnemonic, + } + + // Create a test IO so we can capture output + mockOut := bytes.NewBufferString("") + + testIO := commands.NewTestIO() + testIO.SetOut(commands.WriteNopCloser(mockOut)) + + require.NoError(t, execDerive(cfg, testIO)) + + // Grab the output + deriveOutput := mockOut.String() + + // Verify the addresses are derived correctly + expectedAccounts := generateAccounts( + mnemonic, + cfg.accountIndex, + cfg.numAccounts, + ) + + for _, expectedAccount := range expectedAccounts { + assert.Contains(t, deriveOutput, expectedAccount.String()) + } + }) +} diff --git a/tm2/pkg/crypto/keys/client/root.go b/tm2/pkg/crypto/keys/client/root.go index 550dd408b77..d117d39b46c 100644 --- a/tm2/pkg/crypto/keys/client/root.go +++ b/tm2/pkg/crypto/keys/client/root.go @@ -46,6 +46,7 @@ func NewRootCmd(io *commands.IO) *commands.Command { newQueryCmd(cfg, io), newBroadcastCmd(cfg, io), newMakeTxCmd(cfg, io), + newDeriveCmd(io), ) return cmd From d9afdbb6dced9952c29509ef62f5d119414c69b3 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Mon, 9 Oct 2023 14:47:49 +0200 Subject: [PATCH 02/16] Add gnokey derive full command test --- .../integration/testdata/gnokey_derive.txtar | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 gno.land/pkg/integration/testdata/gnokey_derive.txtar diff --git a/gno.land/pkg/integration/testdata/gnokey_derive.txtar b/gno.land/pkg/integration/testdata/gnokey_derive.txtar new file mode 100644 index 00000000000..6d4ad6fcb50 --- /dev/null +++ b/gno.land/pkg/integration/testdata/gnokey_derive.txtar @@ -0,0 +1,47 @@ +# Verifies the gnokey derive subcommand outputs + +# Run the derivation subcommand on an example mnemonic +gnokey derive -mnemonic 'coyote brisk wagon cable quit opera mesh replace response scheme jelly adult income avoid box mass coil crash sort float salad dolphin blouse humor' +cmp stdout gnokey-derive-valid.stdout.golden +cmp stderr gnokey-derive-valid.stderr.golden + +# Run the derivation subcommand with an invalid mnemonic +! gnokey derive -mnemonic "one two three" +cmp stdout gnokey-derive-invalid-mnemonic.stdout.golden +cmp stderr gnokey-derive-invalid-mnemonic.stderr.golden + +# Run the derivation subcommand with an invalid account index (>uint32) +! gnokey derive --account-index 4294967296 +cmp stdout gnokey-derive-invalid-account-index.stdout.golden +cmp stderr gnokey-derive-invalid-account-index.stderr.golden + +# Run the derivation subcommand with an invalid number of accounts (>uint32) +! gnokey derive --num-accounts 4294967296 +cmp stdout gnokey-derive-invalid-num-accounts.stdout.golden +cmp stderr gnokey-derive-invalid-num-accounts.stderr.golden + +-- gnokey-derive-valid.stdout.golden -- +[Generated Accounts] + +Account Index: 0 + +0. g1laeayqanrlqlrm3rfgfx9fa5zstct3mcxehe5s +1. g197zj3t30ag9p0szcqpq53stprh8vnvtu9xtw4w +2. g1zhrdmurv3yqwpct7s09rgk0l3yj6kgtyf7hd2f +3. g1ml8awz3pt0xamvawszsg0l4p2g7u93d356ypar +4. g1tq27aeelggyaa8cracgyq9pvzwleq82gtulx96 +5. g1u6ut7k2lz4tuhm73vt4cdfraj6uaja7xl9rz8s +6. g19fke4tu3nz04sxc342mqfgwhedjfllpmplva3s +7. g1g54regr9ce5yg8yzmhylpkwwg34fw0k9upk2vh +8. g1wmgd638xnnly7jagmnd8vvkvag74j0ykkkr0ut +9. g174t9ts3g5gx8a8u2wkx2xdppa8ymsfv7nm42j7 +-- gnokey-derive-valid.stderr.golden -- +-- gnokey-derive-invalid-account-index.stdout.golden -- +-- gnokey-derive-invalid-account-index.stderr.golden -- +"gnokey" error: invalid account index +-- gnokey-derive-invalid-num-accounts.stdout.golden -- +-- gnokey-derive-invalid-num-accounts.stderr.golden -- +"gnokey" error: invalid number of accounts +-- gnokey-derive-invalid-mnemonic.stdout.golden -- +-- gnokey-derive-invalid-mnemonic.stderr.golden -- +"gnokey" error: invalid bip39 mnemonic \ No newline at end of file From a73bb49e121a1e05e3cf0a88f846273764634520 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Mon, 9 Oct 2023 15:01:44 +0200 Subject: [PATCH 03/16] Change title output --- gno.land/pkg/integration/testdata/gnokey_derive.txtar | 2 +- tm2/pkg/crypto/keys/client/derive.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gno.land/pkg/integration/testdata/gnokey_derive.txtar b/gno.land/pkg/integration/testdata/gnokey_derive.txtar index 6d4ad6fcb50..0c040581313 100644 --- a/gno.land/pkg/integration/testdata/gnokey_derive.txtar +++ b/gno.land/pkg/integration/testdata/gnokey_derive.txtar @@ -21,7 +21,7 @@ cmp stdout gnokey-derive-invalid-num-accounts.stdout.golden cmp stderr gnokey-derive-invalid-num-accounts.stderr.golden -- gnokey-derive-valid.stdout.golden -- -[Generated Accounts] +[Derived Accounts] Account Index: 0 diff --git a/tm2/pkg/crypto/keys/client/derive.go b/tm2/pkg/crypto/keys/client/derive.go index cf4d6c1de7c..5ffe2cae55b 100644 --- a/tm2/pkg/crypto/keys/client/derive.go +++ b/tm2/pkg/crypto/keys/client/derive.go @@ -88,7 +88,7 @@ func execDerive(cfg *deriveCfg, io *commands.IO) error { cfg.numAccounts, ) - io.Printf("[Generated Accounts]\n\n") + io.Printf("[Derived Accounts]\n\n") io.Printf("Account Index: %d\n\n", cfg.accountIndex) // Print them out From 313773bcf0af2635fe3b315a3e99c1f96f24305a Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Wed, 11 Oct 2023 12:54:49 +0200 Subject: [PATCH 04/16] Add unit test that covers the command init --- tm2/pkg/crypto/keys/client/derive_test.go | 100 ++++++++++++++-------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/tm2/pkg/crypto/keys/client/derive_test.go b/tm2/pkg/crypto/keys/client/derive_test.go index 3ba9d525784..e8b8914c5ce 100644 --- a/tm2/pkg/crypto/keys/client/derive_test.go +++ b/tm2/pkg/crypto/keys/client/derive_test.go @@ -2,6 +2,8 @@ package client import ( "bytes" + "context" + "fmt" "math" "testing" @@ -11,7 +13,20 @@ import ( "github.com/stretchr/testify/require" ) -func Test_execDerive(t *testing.T) { +// generateTestMnemonic generates a random mnemonic +func generateTestMnemonic(t *testing.T) string { + t.Helper() + + entropy, entropyErr := bip39.NewEntropy(256) + require.NoError(t, entropyErr) + + mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) + require.NoError(t, mnemonicErr) + + return mnemonic +} + +func TestDerive_InvalidDerive(t *testing.T) { t.Parallel() t.Run("invalid number of accounts, no accounts requested", func(t *testing.T) { @@ -56,43 +71,52 @@ func Test_execDerive(t *testing.T) { assert.ErrorIs(t, execDerive(cfg, nil), errInvalidMnemonic) }) +} - t.Run("valid accounts generated", func(t *testing.T) { - t.Parallel() - - // Generate a dummy mnemonic - entropy, entropyErr := bip39.NewEntropy(mnemonicEntropySize) - require.NoError(t, entropyErr) - - mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) - require.NoError(t, mnemonicErr) - - cfg := &deriveCfg{ - numAccounts: 1, - accountIndex: 0, - mnemonic: mnemonic, - } - - // Create a test IO so we can capture output - mockOut := bytes.NewBufferString("") - - testIO := commands.NewTestIO() - testIO.SetOut(commands.WriteNopCloser(mockOut)) - - require.NoError(t, execDerive(cfg, testIO)) - - // Grab the output - deriveOutput := mockOut.String() - - // Verify the addresses are derived correctly - expectedAccounts := generateAccounts( - mnemonic, - cfg.accountIndex, - cfg.numAccounts, - ) +func TestDerive_ValidDerive(t *testing.T) { + t.Parallel() - for _, expectedAccount := range expectedAccounts { - assert.Contains(t, deriveOutput, expectedAccount.String()) - } - }) + // Generate a dummy mnemonic + var ( + mnemonic = generateTestMnemonic(t) + accountIndex uint64 = 0 + numAccounts uint64 = 10 + ) + + // Set up the IO + mockOut := bytes.NewBufferString("") + + io := commands.NewTestIO() + io.SetOut(commands.WriteNopCloser(mockOut)) + + // Create the root command + cmd := newDeriveCmd(io) + + // Prepare the args + args := []string{ + "derive", + "--mnemonic", + mnemonic, + "--num-accounts", + fmt.Sprintf("%d", numAccounts), + "--account-index", + fmt.Sprintf("%d", accountIndex), + } + + // Run the command + require.NoError(t, cmd.ParseAndRun(context.Background(), args)) + + // Verify the addresses are derived correctly + expectedAccounts := generateAccounts( + mnemonic, + accountIndex, + numAccounts, + ) + + // Grab the output + deriveOutput := mockOut.String() + + for _, expectedAccount := range expectedAccounts { + assert.Contains(t, deriveOutput, expectedAccount.String()) + } } From 6b2a39e493102f601bd9abe8ca148aaa3a8fb186 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Fri, 1 Dec 2023 12:43:00 +0100 Subject: [PATCH 05/16] Transfer the logic to a flag --- tm2/pkg/crypto/keys/client/add.go | 82 ++++++++++++- tm2/pkg/crypto/keys/client/add_test.go | 66 +++++++++++ tm2/pkg/crypto/keys/client/derive.go | 134 ---------------------- tm2/pkg/crypto/keys/client/derive_test.go | 122 -------------------- tm2/pkg/crypto/keys/client/root.go | 1 - 5 files changed, 143 insertions(+), 262 deletions(-) delete mode 100644 tm2/pkg/crypto/keys/client/derive.go delete mode 100644 tm2/pkg/crypto/keys/client/derive_test.go diff --git a/tm2/pkg/crypto/keys/client/add.go b/tm2/pkg/crypto/keys/client/add.go index 71dc6f03090..0d4bcbbcece 100644 --- a/tm2/pkg/crypto/keys/client/add.go +++ b/tm2/pkg/crypto/keys/client/add.go @@ -10,8 +10,10 @@ import ( "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/crypto/bip39" + "github.com/gnolang/gno/tm2/pkg/crypto/hd" "github.com/gnolang/gno/tm2/pkg/crypto/keys" "github.com/gnolang/gno/tm2/pkg/crypto/multisig" + "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" ) type addCfg struct { @@ -27,6 +29,7 @@ type addCfg struct { dryRun bool account uint64 index uint64 + deriveAccounts uint64 } func newAddCmd(rootCfg *baseCfg, io commands.IO) *commands.Command { @@ -116,6 +119,13 @@ func (c *addCfg) RegisterFlags(fs *flag.FlagSet) { 0, "Address index number for HD derivation", ) + + fs.Uint64Var( + &c.deriveAccounts, + "derive-accounts", + 0, + "the number of accounts to derive from the mnemonic", + ) } // DryRunKeyPass contains the default key password for genesis transactions @@ -238,7 +248,9 @@ func execAdd(cfg *addCfg, args []string, io commands.IO) error { return err } - return printCreate(info, false, "", io) + printCreate(info, false, "", io) + + return nil } // Get bip39 mnemonic @@ -269,6 +281,9 @@ func execAdd(cfg *addCfg, args []string, io commands.IO) error { return err } + // Print the derived address info + printDerive(mnemonic, cfg.index, cfg.deriveAccounts, io) + // Recover key from seed passphrase if cfg.recover { // Hide mnemonic from output @@ -276,10 +291,13 @@ func execAdd(cfg *addCfg, args []string, io commands.IO) error { mnemonic = "" } - return printCreate(info, showMnemonic, mnemonic, io) + // Print the key create info + printCreate(info, showMnemonic, mnemonic, io) + + return nil } -func printCreate(info keys.Info, showMnemonic bool, mnemonic string, io commands.IO) error { +func printCreate(info keys.Info, showMnemonic bool, mnemonic string, io commands.IO) { io.Println("") printNewInfo(info, io) @@ -291,8 +309,6 @@ It is the only way to recover your account if you ever forget your password. %v `, mnemonic) } - - return nil } func printNewInfo(info keys.Info, io commands.IO) { @@ -305,3 +321,59 @@ func printNewInfo(info keys.Info, io commands.IO) { io.Printfln("* %s (%s) - addr: %v pub: %v, path: %v", keyname, keytype, keyaddr, keypub, keypath) } + +// printDerive prints the derived accounts, if any +func printDerive( + mnemonic string, + accountIndex, + numAccounts uint64, + io commands.IO, +) { + if numAccounts == 0 { + // No accounts to print + return + } + + // Generate the accounts + accounts := generateAccounts( + mnemonic, + accountIndex, + numAccounts, + ) + + io.Printf("[Derived Accounts]\n\n") + io.Printf("Account Index: %d\n\n", accountIndex) + + // Print them out + for index, account := range accounts { + io.Printfln("%d. %s", index, account.String()) + } +} + +// generateAccounts the accounts using the provided mnemonics +func generateAccounts(mnemonic string, accountIndex, numAccounts uint64) []crypto.Address { + addresses := make([]crypto.Address, numAccounts) + + // Generate the seed + seed := bip39.NewSeed(mnemonic, "") + + for i := uint64(0); i < numAccounts; i++ { + key := generateKeyFromSeed(seed, uint32(accountIndex), uint32(i)) + address := key.PubKey().Address() + + addresses[i] = address + } + + return addresses +} + +// generateKeyFromSeed generates a private key from +// the provided seed and index +func generateKeyFromSeed(seed []byte, account, index uint32) crypto.PrivKey { + pathParams := hd.NewFundraiserParams(account, crypto.CoinType, index) + + masterPriv, ch := hd.ComputeMastersFromSeed(seed) + derivedPriv, _ := hd.DerivePrivateKeyForPath(masterPriv, ch, pathParams.String()) + + return secp256k1.PrivKeySecp256k1(derivedPriv) +} diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index 94cb945f113..ca973be7eda 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -1,17 +1,33 @@ package client import ( + "bytes" "fmt" "strings" "testing" "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto/bip39" "github.com/gnolang/gno/tm2/pkg/crypto/keys" "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" "github.com/gnolang/gno/tm2/pkg/testutils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// generateTestMnemonic generates a random mnemonic +func generateTestMnemonic(t *testing.T) string { + t.Helper() + + entropy, entropyErr := bip39.NewEntropy(256) + require.NoError(t, entropyErr) + + mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) + require.NoError(t, mnemonicErr) + + return mnemonic +} + func Test_execAddBasic(t *testing.T) { t.Parallel() @@ -114,3 +130,53 @@ func Test_execAddRecover(t *testing.T) { s := fmt.Sprintf("%s", keypub) assert.Equal(t, s, test2PubkeyBech32) } + +func Test_execAddDerive(t *testing.T) { + t.Parallel() + + var ( + mnemonic = generateTestMnemonic(t) + accountIndex uint64 = 0 + numAccounts uint64 = 10 + + dummyPass = "dummy-pass" + ) + + kbHome, kbCleanUp := testutils.NewTestCaseDir(t) + require.NotNil(t, kbHome) + defer kbCleanUp() + + cfg := &addCfg{ + rootCfg: &baseCfg{ + BaseOptions: BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + }, + }, + recover: true, + deriveAccounts: numAccounts, + account: accountIndex, + } + + mockOut := bytes.NewBufferString("") + + io := commands.NewTestIO() + io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) + io.SetOut(commands.WriteNopCloser(mockOut)) + + require.NoError(t, execAdd(cfg, []string{"example-key"}, io)) + + // Verify the addresses are derived correctly + expectedAccounts := generateAccounts( + mnemonic, + accountIndex, + numAccounts, + ) + + // Grab the output + deriveOutput := mockOut.String() + + for _, expectedAccount := range expectedAccounts { + assert.Contains(t, deriveOutput, expectedAccount.String()) + } +} diff --git a/tm2/pkg/crypto/keys/client/derive.go b/tm2/pkg/crypto/keys/client/derive.go deleted file mode 100644 index 5ffe2cae55b..00000000000 --- a/tm2/pkg/crypto/keys/client/derive.go +++ /dev/null @@ -1,134 +0,0 @@ -package client - -import ( - "context" - "errors" - "flag" - "math" - - "github.com/gnolang/gno/tm2/pkg/commands" - "github.com/gnolang/gno/tm2/pkg/crypto" - "github.com/gnolang/gno/tm2/pkg/crypto/bip39" - "github.com/gnolang/gno/tm2/pkg/crypto/hd" - "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" -) - -var ( - errInvalidMnemonic = errors.New("invalid bip39 mnemonic") - errInvalidNumAccounts = errors.New("invalid number of accounts") - errInvalidAccountIndex = errors.New("invalid account index") -) - -type deriveCfg struct { - mnemonic string - numAccounts uint64 - accountIndex uint64 -} - -// newDeriveCmd creates a new gnokey derive subcommand -func newDeriveCmd(io *commands.IO) *commands.Command { - cfg := &deriveCfg{} - - return commands.NewCommand( - commands.Metadata{ - Name: "derive", - ShortUsage: "derive [flags]", - ShortHelp: "Derives the account addresses from the specified mnemonic", - }, - cfg, - func(_ context.Context, _ []string) error { - return execDerive(cfg, io) - }, - ) -} - -func (c *deriveCfg) RegisterFlags(fs *flag.FlagSet) { - fs.StringVar( - &c.mnemonic, - "mnemonic", - "", - "the bip39 mnemonic", - ) - - fs.Uint64Var( - &c.numAccounts, - "num-accounts", - 10, - "the number of accounts to derive from the mnemonic", - ) - - fs.Uint64Var( - &c.accountIndex, - "account-index", - 0, - "the account index in the mnemonic", - ) -} - -func execDerive(cfg *deriveCfg, io *commands.IO) error { - // Make sure the number of accounts is valid - if cfg.numAccounts == 0 || !isUint32(cfg.numAccounts) { - return errInvalidNumAccounts - } - - // Make sure the account index is valid - if !isUint32(cfg.accountIndex) { - return errInvalidAccountIndex - } - - // Make sure the mnemonic is valid - if !bip39.IsMnemonicValid(cfg.mnemonic) { - return errInvalidMnemonic - } - - // Generate the accounts - accounts := generateAccounts( - cfg.mnemonic, - cfg.accountIndex, - cfg.numAccounts, - ) - - io.Printf("[Derived Accounts]\n\n") - io.Printf("Account Index: %d\n\n", cfg.accountIndex) - - // Print them out - for index, account := range accounts { - io.Printfln("%d. %s", index, account.String()) - } - - return nil -} - -// isUint32 verifies a uint64 value can be represented -// as a uint32 -func isUint32(value uint64) bool { - return value <= math.MaxUint32 -} - -// generateAccounts the accounts using the provided mnemonics -func generateAccounts(mnemonic string, accountIndex, numAccounts uint64) []crypto.Address { - addresses := make([]crypto.Address, numAccounts) - - // Generate the seed - seed := bip39.NewSeed(mnemonic, "") - - for i := uint64(0); i < numAccounts; i++ { - key := generateKeyFromSeed(seed, uint32(accountIndex), uint32(i)) - address := key.PubKey().Address() - - addresses[i] = address - } - - return addresses -} - -// generateKeyFromSeed generates a private key from -// the provided seed and index -func generateKeyFromSeed(seed []byte, account, index uint32) crypto.PrivKey { - pathParams := hd.NewFundraiserParams(account, crypto.CoinType, index) - - masterPriv, ch := hd.ComputeMastersFromSeed(seed) - derivedPriv, _ := hd.DerivePrivateKeyForPath(masterPriv, ch, pathParams.String()) - - return secp256k1.PrivKeySecp256k1(derivedPriv) -} diff --git a/tm2/pkg/crypto/keys/client/derive_test.go b/tm2/pkg/crypto/keys/client/derive_test.go deleted file mode 100644 index e8b8914c5ce..00000000000 --- a/tm2/pkg/crypto/keys/client/derive_test.go +++ /dev/null @@ -1,122 +0,0 @@ -package client - -import ( - "bytes" - "context" - "fmt" - "math" - "testing" - - "github.com/gnolang/gno/tm2/pkg/commands" - "github.com/gnolang/gno/tm2/pkg/crypto/bip39" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// generateTestMnemonic generates a random mnemonic -func generateTestMnemonic(t *testing.T) string { - t.Helper() - - entropy, entropyErr := bip39.NewEntropy(256) - require.NoError(t, entropyErr) - - mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) - require.NoError(t, mnemonicErr) - - return mnemonic -} - -func TestDerive_InvalidDerive(t *testing.T) { - t.Parallel() - - t.Run("invalid number of accounts, no accounts requested", func(t *testing.T) { - t.Parallel() - - cfg := &deriveCfg{ - numAccounts: 0, - } - - assert.ErrorIs(t, execDerive(cfg, nil), errInvalidNumAccounts) - }) - - t.Run("invalid number of accounts, > uint32", func(t *testing.T) { - t.Parallel() - - cfg := &deriveCfg{ - numAccounts: math.MaxUint32 + 1, // > uint32 - } - - assert.ErrorIs(t, execDerive(cfg, nil), errInvalidNumAccounts) - }) - - t.Run("invalid account index", func(t *testing.T) { - t.Parallel() - - cfg := &deriveCfg{ - numAccounts: 1, - accountIndex: math.MaxUint32 + 1, // > uint32 - } - - assert.ErrorIs(t, execDerive(cfg, nil), errInvalidAccountIndex) - }) - - t.Run("invalid mnemonic", func(t *testing.T) { - t.Parallel() - - cfg := &deriveCfg{ - numAccounts: 1, - accountIndex: 0, - mnemonic: "one two", - } - - assert.ErrorIs(t, execDerive(cfg, nil), errInvalidMnemonic) - }) -} - -func TestDerive_ValidDerive(t *testing.T) { - t.Parallel() - - // Generate a dummy mnemonic - var ( - mnemonic = generateTestMnemonic(t) - accountIndex uint64 = 0 - numAccounts uint64 = 10 - ) - - // Set up the IO - mockOut := bytes.NewBufferString("") - - io := commands.NewTestIO() - io.SetOut(commands.WriteNopCloser(mockOut)) - - // Create the root command - cmd := newDeriveCmd(io) - - // Prepare the args - args := []string{ - "derive", - "--mnemonic", - mnemonic, - "--num-accounts", - fmt.Sprintf("%d", numAccounts), - "--account-index", - fmt.Sprintf("%d", accountIndex), - } - - // Run the command - require.NoError(t, cmd.ParseAndRun(context.Background(), args)) - - // Verify the addresses are derived correctly - expectedAccounts := generateAccounts( - mnemonic, - accountIndex, - numAccounts, - ) - - // Grab the output - deriveOutput := mockOut.String() - - for _, expectedAccount := range expectedAccounts { - assert.Contains(t, deriveOutput, expectedAccount.String()) - } -} diff --git a/tm2/pkg/crypto/keys/client/root.go b/tm2/pkg/crypto/keys/client/root.go index 804209b7fdf..e09b31d45dd 100644 --- a/tm2/pkg/crypto/keys/client/root.go +++ b/tm2/pkg/crypto/keys/client/root.go @@ -52,7 +52,6 @@ func NewRootCmdWithBaseConfig(io commands.IO, base BaseOptions) *commands.Comman newQueryCmd(cfg, io), newBroadcastCmd(cfg, io), newMakeTxCmd(cfg, io), - newDeriveCmd(io), ) return cmd From 2e39ea6a417df374cc45521771a3ec3ebea882b8 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Fri, 1 Dec 2023 13:41:44 +0100 Subject: [PATCH 06/16] Remove txtar test --- .../integration/testdata/gnokey_derive.txtar | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 gno.land/pkg/integration/testdata/gnokey_derive.txtar diff --git a/gno.land/pkg/integration/testdata/gnokey_derive.txtar b/gno.land/pkg/integration/testdata/gnokey_derive.txtar deleted file mode 100644 index 0c040581313..00000000000 --- a/gno.land/pkg/integration/testdata/gnokey_derive.txtar +++ /dev/null @@ -1,47 +0,0 @@ -# Verifies the gnokey derive subcommand outputs - -# Run the derivation subcommand on an example mnemonic -gnokey derive -mnemonic 'coyote brisk wagon cable quit opera mesh replace response scheme jelly adult income avoid box mass coil crash sort float salad dolphin blouse humor' -cmp stdout gnokey-derive-valid.stdout.golden -cmp stderr gnokey-derive-valid.stderr.golden - -# Run the derivation subcommand with an invalid mnemonic -! gnokey derive -mnemonic "one two three" -cmp stdout gnokey-derive-invalid-mnemonic.stdout.golden -cmp stderr gnokey-derive-invalid-mnemonic.stderr.golden - -# Run the derivation subcommand with an invalid account index (>uint32) -! gnokey derive --account-index 4294967296 -cmp stdout gnokey-derive-invalid-account-index.stdout.golden -cmp stderr gnokey-derive-invalid-account-index.stderr.golden - -# Run the derivation subcommand with an invalid number of accounts (>uint32) -! gnokey derive --num-accounts 4294967296 -cmp stdout gnokey-derive-invalid-num-accounts.stdout.golden -cmp stderr gnokey-derive-invalid-num-accounts.stderr.golden - --- gnokey-derive-valid.stdout.golden -- -[Derived Accounts] - -Account Index: 0 - -0. g1laeayqanrlqlrm3rfgfx9fa5zstct3mcxehe5s -1. g197zj3t30ag9p0szcqpq53stprh8vnvtu9xtw4w -2. g1zhrdmurv3yqwpct7s09rgk0l3yj6kgtyf7hd2f -3. g1ml8awz3pt0xamvawszsg0l4p2g7u93d356ypar -4. g1tq27aeelggyaa8cracgyq9pvzwleq82gtulx96 -5. g1u6ut7k2lz4tuhm73vt4cdfraj6uaja7xl9rz8s -6. g19fke4tu3nz04sxc342mqfgwhedjfllpmplva3s -7. g1g54regr9ce5yg8yzmhylpkwwg34fw0k9upk2vh -8. g1wmgd638xnnly7jagmnd8vvkvag74j0ykkkr0ut -9. g174t9ts3g5gx8a8u2wkx2xdppa8ymsfv7nm42j7 --- gnokey-derive-valid.stderr.golden -- --- gnokey-derive-invalid-account-index.stdout.golden -- --- gnokey-derive-invalid-account-index.stderr.golden -- -"gnokey" error: invalid account index --- gnokey-derive-invalid-num-accounts.stdout.golden -- --- gnokey-derive-invalid-num-accounts.stderr.golden -- -"gnokey" error: invalid number of accounts --- gnokey-derive-invalid-mnemonic.stdout.golden -- --- gnokey-derive-invalid-mnemonic.stderr.golden -- -"gnokey" error: invalid bip39 mnemonic \ No newline at end of file From 8fdc4e55e9a847dd33d34cd0532dabd090c7c73e Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Sat, 30 Mar 2024 16:01:20 +0900 Subject: [PATCH 07/16] Start breaking up gnokey add --- tm2/pkg/crypto/keys/client/add.go | 236 ++++++--------------- tm2/pkg/crypto/keys/client/add_bech32.go | 94 ++++++++ tm2/pkg/crypto/keys/client/add_ledger.go | 76 +++++++ tm2/pkg/crypto/keys/client/add_multisig.go | 135 ++++++++++++ tm2/pkg/crypto/keys/client/add_test.go | 12 +- tm2/pkg/crypto/keys/client/root.go | 4 - 6 files changed, 378 insertions(+), 179 deletions(-) create mode 100644 tm2/pkg/crypto/keys/client/add_bech32.go create mode 100644 tm2/pkg/crypto/keys/client/add_ledger.go create mode 100644 tm2/pkg/crypto/keys/client/add_multisig.go diff --git a/tm2/pkg/crypto/keys/client/add.go b/tm2/pkg/crypto/keys/client/add.go index 954cc4effa6..ff060b33ff5 100644 --- a/tm2/pkg/crypto/keys/client/add.go +++ b/tm2/pkg/crypto/keys/client/add.go @@ -5,31 +5,29 @@ import ( "errors" "flag" "fmt" - "sort" "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/crypto/bip39" "github.com/gnolang/gno/tm2/pkg/crypto/hd" "github.com/gnolang/gno/tm2/pkg/crypto/keys" - "github.com/gnolang/gno/tm2/pkg/crypto/multisig" "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" ) +var errInvalidMnemonic = errors.New("invalid bip39 mnemonic") + type AddCfg struct { RootCfg *BaseCfg - Multisig commands.StringArr - MultisigThreshold int - NoSort bool - PublicKey string - UseLedger bool - Recover bool - NoBackup bool - DryRun bool - Account uint64 - Index uint64 - DeriveAccounts uint64 + Recover bool + NoBackup bool + Account uint64 + Index uint64 + DeriveAccounts uint64 +} + +type AddBaseCfg struct { + RootCfg *BaseCfg } func NewAddCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { @@ -37,7 +35,7 @@ func NewAddCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { RootCfg: rootCfg, } - return commands.NewCommand( + cmd := commands.NewCommand( commands.Metadata{ Name: "add", ShortUsage: "add [flags] ", @@ -48,43 +46,17 @@ func NewAddCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { return execAdd(cfg, args, io) }, ) -} - -func (c *AddCfg) RegisterFlags(fs *flag.FlagSet) { - fs.Var( - &c.Multisig, - "multisig", - "construct and store a multisig public key (implies --pubkey)", - ) - fs.IntVar( - &c.MultisigThreshold, - "threshold", - 1, - "K out of N required signatures. For use in conjunction with --multisig", + cmd.AddSubCommands( + NewAddMultisigCmd(cfg, io), + NewAddLedgerCmd(cfg, io), + NewAddBech32Cmd(cfg, io), ) - fs.BoolVar( - &c.NoSort, - "nosort", - false, - "keys passed to --multisig are taken in the order they're supplied", - ) - - fs.StringVar( - &c.PublicKey, - "pubkey", - "", - "parse a public key in bech32 format and save it to disk", - ) - - fs.BoolVar( - &c.UseLedger, - "ledger", - false, - "store a local reference to a private key on a Ledger device", - ) + return cmd +} +func (c *AddCfg) RegisterFlags(fs *flag.FlagSet) { fs.BoolVar( &c.Recover, "recover", @@ -99,13 +71,6 @@ func (c *AddCfg) RegisterFlags(fs *flag.FlagSet) { "don't print out seed phrase (if others are watching the terminal)", ) - fs.BoolVar( - &c.DryRun, - "dryrun", - false, - "perform action, but don't add key to local keystore", - ) - fs.Uint64Var( &c.Account, "account", @@ -128,9 +93,6 @@ func (c *AddCfg) RegisterFlags(fs *flag.FlagSet) { ) } -// DryRunKeyPass contains the default key password for genesis transactions -const DryRunKeyPass = "12345678" - /* input - bip39 mnemonic @@ -142,143 +104,79 @@ output - armor encrypted private key (saved to file) */ func execAdd(cfg *AddCfg, args []string, io commands.IO) error { - var ( - kb keys.Keybase - err error - encryptPassword string - ) - + // Check if the key name is provided if len(args) != 1 { return flag.ErrHelp } name := args[0] - showMnemonic := !cfg.NoBackup - - if cfg.DryRun { - // we throw this away, so don't enforce args, - // we want to get a new random seed phrase quickly - kb = keys.NewInMemory() - encryptPassword = DryRunKeyPass - } else { - kb, err = keys.NewKeyBaseFromDir(cfg.RootCfg.Home) - if err != nil { - return err - } - - 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 { - return err2 - } - if !response { - return errors.New("aborted") - } - } - multisigKeys := cfg.Multisig - if len(multisigKeys) != 0 { - var pks []crypto.PubKey - - multisigThreshold := cfg.MultisigThreshold - if err := keys.ValidateMultisigThreshold(multisigThreshold, len(multisigKeys)); err != nil { - return err - } - - for _, keyname := range multisigKeys { - k, err := kb.GetByName(keyname) - if err != nil { - return err - } - pks = append(pks, k.GetPubKey()) - } - - // Handle --nosort - if !cfg.NoSort { - sort.Slice(pks, func(i, j int) bool { - return pks[i].Address().Compare(pks[j].Address()) < 0 - }) - } - - pk := multisig.NewPubKeyMultisigThreshold(multisigThreshold, pks) - if _, err := kb.CreateMulti(name, pk); err != nil { - return err - } - - io.Printfln("Key %q saved to disk.\n", name) - return nil - } - - // ask for a password when generating a local key - if cfg.PublicKey == "" && !cfg.UseLedger { - encryptPassword, err = io.GetCheckPassword( - [2]string{ - "Enter a passphrase to encrypt your key to disk:", - "Repeat the passphrase:", - }, - cfg.RootCfg.InsecurePasswordStdin, - ) - if err != nil { - return err - } - } + // Read the keybase from the home directory + kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.Home) + if err != nil { + return fmt.Errorf("unable to read keybase, %w", err) } - if cfg.PublicKey != "" { - pk, err := crypto.PubKeyFromBech32(cfg.PublicKey) - if err != nil { - return err - } - _, err = kb.CreateOffline(name, pk) - if err != nil { - return err - } - return nil + // Check if the key exists + exists, err := kb.HasByName(name) + if err != nil { + return fmt.Errorf("unable to fetch key, %w", err) } - account := cfg.Account - index := cfg.Index - - // If we're using ledger, only thing we need is the path and the bech32 prefix. - if cfg.UseLedger { - bech32PrefixAddr := crypto.Bech32AddrPrefix - info, err := kb.CreateLedger(name, keys.Secp256k1, bech32PrefixAddr, uint32(account), uint32(index)) + // Get overwrite confirmation, if any + if exists { + overwrite, err := io.GetConfirmation(fmt.Sprintf("Override the existing name %s", name)) if err != nil { - return err + return fmt.Errorf("unable to get confirmation, %w", err) } - printCreate(info, false, "", io) + if !overwrite { + return errOverwriteAborted + } + } - return nil + // Ask for a password when generating a local key + encryptPassword, err := io.GetCheckPassword( + [2]string{ + "Enter a passphrase to encrypt your key to disk:", + "Repeat the passphrase:", + }, + cfg.RootCfg.InsecurePasswordStdin, + ) + if err != nil { + return fmt.Errorf("unable to parse provided password, %w", err) } // Get bip39 mnemonic - var mnemonic string - const bip39Passphrase string = "" // XXX research. + mnemonic, err := GenerateMnemonic(mnemonicEntropySize) + if err != nil { + return fmt.Errorf("unable to generate mnemonic, %w", err) + } if cfg.Recover { bip39Message := "Enter your bip39 mnemonic" mnemonic, err = io.GetString(bip39Message) if err != nil { - return err + return fmt.Errorf("unable to parse mnemonic, %w", err) } + // Make sure it's valid if !bip39.IsMnemonicValid(mnemonic) { - return errors.New("invalid mnemonic") + return errInvalidMnemonic } } - if len(mnemonic) == 0 { - mnemonic, err = GenerateMnemonic(mnemonicEntropySize) - if err != nil { - return err - } - } - - info, err := kb.CreateAccount(name, mnemonic, bip39Passphrase, encryptPassword, uint32(account), uint32(index)) + // Save the account + info, err := kb.CreateAccount( + name, + mnemonic, + "", + encryptPassword, + uint32(cfg.Account), + uint32(cfg.Index), + ) if err != nil { - return err + return fmt.Errorf("unable to save account to keybase, %w", err) } // Print the derived address info @@ -286,13 +184,13 @@ func execAdd(cfg *AddCfg, args []string, io commands.IO) error { // Recover key from seed passphrase if cfg.Recover { - // Hide mnemonic from output - showMnemonic = false - mnemonic = "" + printCreate(info, false, "", io) + + return nil } // Print the key create info - printCreate(info, showMnemonic, mnemonic, io) + printCreate(info, !cfg.NoBackup, mnemonic, io) return nil } diff --git a/tm2/pkg/crypto/keys/client/add_bech32.go b/tm2/pkg/crypto/keys/client/add_bech32.go new file mode 100644 index 00000000000..7b7cb8aca2c --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_bech32.go @@ -0,0 +1,94 @@ +package client + +import ( + "context" + "flag" + "fmt" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" +) + +type AddBech32Cfg struct { + RootCfg *AddCfg + + PublicKey string +} + +// NewAddBech32Cmd creates a gnokey add bech32 command +func NewAddBech32Cmd(rootCfg *AddCfg, io commands.IO) *commands.Command { + cfg := &AddBech32Cfg{ + RootCfg: rootCfg, + } + + return commands.NewCommand( + commands.Metadata{ + Name: "bech32", + ShortUsage: "add bech32 [flags] ", + ShortHelp: "adds a public key to the keybase, using the bech32 representation", + }, + cfg, + func(_ context.Context, args []string) error { + return execAddBech32(cfg, args, io) + }, + ) +} + +func (c *AddBech32Cfg) RegisterFlags(fs *flag.FlagSet) { + fs.StringVar( + &c.PublicKey, + "pubkey", + "", + "parse a public key in bech32 format and save it to disk", + ) +} + +func execAddBech32(cfg *AddBech32Cfg, args []string, io commands.IO) error { + // Validate a key name was provided + if len(args) != 1 { + return flag.ErrHelp + } + + name := args[0] + + // Read the keybase from the home directory + kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.RootCfg.Home) + if err != nil { + return fmt.Errorf("unable to read keybase, %w", err) + } + + // Check if the key exists + exists, err := kb.HasByName(name) + if err != nil { + return fmt.Errorf("unable to fetch key, %w", err) + } + + // Get overwrite confirmation, if any + if exists { + overwrite, err := io.GetConfirmation(fmt.Sprintf("Override the existing name %s", name)) + if err != nil { + return fmt.Errorf("unable to get confirmation, %w", err) + } + + if !overwrite { + return errOverwriteAborted + } + } + + // Parse the public key + publicKey, err := crypto.PubKeyFromBech32(cfg.PublicKey) + if err != nil { + return fmt.Errorf("unable to parse public key from bech32, %w", err) + } + + // Save it offline in the keybase + _, err = kb.CreateOffline(name, publicKey) + if err != nil { + return fmt.Errorf("unable to save public key, %w", err) + } + + io.Printfln("Key %q saved to disk.\n", name) + + return nil +} diff --git a/tm2/pkg/crypto/keys/client/add_ledger.go b/tm2/pkg/crypto/keys/client/add_ledger.go new file mode 100644 index 00000000000..8ccf19eb264 --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_ledger.go @@ -0,0 +1,76 @@ +package client + +import ( + "context" + "flag" + "fmt" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" +) + +// NewAddLedgerCmd creates a gnokey add ledger command +func NewAddLedgerCmd(cfg *AddCfg, io commands.IO) *commands.Command { + return commands.NewCommand( + commands.Metadata{ + Name: "ledger", + ShortUsage: "add ledger [flags] ", + ShortHelp: "adds a Ledger key reference to the keybase", + }, + cfg, + func(_ context.Context, args []string) error { + return execAddLedger(cfg, args, io) + }, + ) +} + +func execAddLedger(cfg *AddCfg, args []string, io commands.IO) error { + // Validate a key name was provided + if len(args) != 1 { + return flag.ErrHelp + } + + name := args[0] + + // Read the keybase from the home directory + kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.Home) + if err != nil { + return fmt.Errorf("unable to read keybase, %w", err) + } + + // Check if the key exists + exists, err := kb.HasByName(name) + if err != nil { + return fmt.Errorf("unable to fetch key, %w", err) + } + + // Get overwrite confirmation, if any + if exists { + overwrite, err := io.GetConfirmation(fmt.Sprintf("Override the existing name %s", name)) + if err != nil { + return fmt.Errorf("unable to get confirmation, %w", err) + } + + if !overwrite { + return errOverwriteAborted + } + } + + // Create the ledger reference + info, err := kb.CreateLedger( + name, + keys.Secp256k1, + crypto.Bech32AddrPrefix, + uint32(cfg.Account), + uint32(cfg.Index), + ) + if err != nil { + return fmt.Errorf("unable to create Ledger reference in keybase, %w", err) + } + + // Print the information + printCreate(info, false, "", io) + + return nil +} diff --git a/tm2/pkg/crypto/keys/client/add_multisig.go b/tm2/pkg/crypto/keys/client/add_multisig.go new file mode 100644 index 00000000000..c3c2a798f0e --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_multisig.go @@ -0,0 +1,135 @@ +package client + +import ( + "context" + "errors" + "flag" + "fmt" + "sort" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" + "github.com/gnolang/gno/tm2/pkg/crypto/multisig" +) + +var errOverwriteAborted = errors.New("overwrite aborted") + +type AddMultisigCfg struct { + RootCfg *AddCfg + + NoSort bool + Multisig commands.StringArr + MultisigThreshold int +} + +// NewAddMultisigCmd creates a gnokey add multisig command +func NewAddMultisigCmd(rootCfg *AddCfg, io commands.IO) *commands.Command { + cfg := &AddMultisigCfg{ + RootCfg: rootCfg, + } + + return commands.NewCommand( + commands.Metadata{ + Name: "multisig", + ShortUsage: "add multisig [flags] ", + ShortHelp: "adds a multisig key reference to the keybase", + }, + cfg, + func(_ context.Context, args []string) error { + return execAddMultisig(cfg, args, io) + }, + ) +} + +func (c *AddMultisigCfg) RegisterFlags(fs *flag.FlagSet) { + fs.BoolVar( + &c.NoSort, + "nosort", + false, + "keys passed to --multisig are taken in the order they're supplied", + ) + + fs.Var( + &c.Multisig, + "multisig", + "construct and store a multisig public key", + ) + + fs.IntVar( + &c.MultisigThreshold, + "threshold", + 1, + "K out of N required signatures", + ) +} + +func execAddMultisig(cfg *AddMultisigCfg, args []string, io commands.IO) error { + // Validate a key name was provided + if len(args) != 1 { + return flag.ErrHelp + } + + // Validate the multisig threshold + if err := keys.ValidateMultisigThreshold( + cfg.MultisigThreshold, + len(cfg.Multisig), + ); err != nil { + return fmt.Errorf("unable to verify multisig threshold") + } + + name := args[0] + + // Read the keybase from the home directory + kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.RootCfg.Home) + if err != nil { + return fmt.Errorf("unable to read keybase, %w", err) + } + + // Check if the key exists + exists, err := kb.HasByName(name) + if err != nil { + return fmt.Errorf("unable to fetch key, %w", err) + } + + // Get overwrite confirmation, if any + if exists { + overwrite, err := io.GetConfirmation(fmt.Sprintf("Override the existing name %s", name)) + if err != nil { + return fmt.Errorf("unable to get confirmation, %w", err) + } + + if !overwrite { + return errOverwriteAborted + } + } + + publicKeys := make([]crypto.PubKey, 0) + for _, keyName := range cfg.Multisig { + k, err := kb.GetByName(keyName) + if err != nil { + return fmt.Errorf("unable to fetch key, %w", err) + } + + publicKeys = append(publicKeys, k.GetPubKey()) + } + + // Check if the keys should be sorted + if !cfg.NoSort { + sort.Slice(publicKeys, func(i, j int) bool { + return publicKeys[i].Address().Compare(publicKeys[j].Address()) < 0 + }) + } + + // Create a new public key with the multisig threshold + if _, err := kb.CreateMulti( + name, + multisig.NewPubKeyMultisigThreshold(cfg.MultisigThreshold, publicKeys), + ); err != nil { + return fmt.Errorf("unable to create multisig key reference, %w", err) + } + + io.Printfln("Key %q saved to disk.\n", name) + + return nil +} diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index 49f03808feb..578cbbe91cc 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -81,7 +81,7 @@ func Test_execAddPublicKey(t *testing.T) { Home: kbHome, }, }, - PublicKey: test2PubkeyBech32, // test2 account + // PublicKey: test2PubkeyBech32, // test2 account } if err := execAdd(cfg, []string{"test2"}, nil); err != nil { @@ -146,16 +146,16 @@ func Test_execAddDerive(t *testing.T) { require.NotNil(t, kbHome) defer kbCleanUp() - cfg := &addCfg{ - rootCfg: &baseCfg{ + cfg := &AddCfg{ + RootCfg: &BaseCfg{ BaseOptions: BaseOptions{ InsecurePasswordStdin: true, Home: kbHome, }, }, - recover: true, - deriveAccounts: numAccounts, - account: accountIndex, + Recover: true, + DeriveAccounts: numAccounts, + Account: accountIndex, } mockOut := bytes.NewBufferString("") diff --git a/tm2/pkg/crypto/keys/client/root.go b/tm2/pkg/crypto/keys/client/root.go index bfccdc26bab..f69155ace85 100644 --- a/tm2/pkg/crypto/keys/client/root.go +++ b/tm2/pkg/crypto/keys/client/root.go @@ -18,10 +18,6 @@ type BaseCfg struct { BaseOptions } -func NewRootCmd(io commands.IO) *commands.Command { - return NewRootCmdWithBaseConfig(io, DefaultBaseOptions) -} - func NewRootCmdWithBaseConfig(io commands.IO, base BaseOptions) *commands.Command { cfg := &BaseCfg{ BaseOptions: base, From 29ac1ce24a8a188c2a952b2e4ec85cf9490324c0 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Sat, 30 Mar 2024 17:11:30 +0900 Subject: [PATCH 08/16] Add a derivation path check --- tm2/pkg/crypto/keys/client/add.go | 91 +++++++++++++----------- tm2/pkg/crypto/keys/client/add_ledger.go | 2 +- tm2/pkg/crypto/keys/client/add_test.go | 20 +++--- 3 files changed, 59 insertions(+), 54 deletions(-) diff --git a/tm2/pkg/crypto/keys/client/add.go b/tm2/pkg/crypto/keys/client/add.go index ff060b33ff5..6b7cf82a94e 100644 --- a/tm2/pkg/crypto/keys/client/add.go +++ b/tm2/pkg/crypto/keys/client/add.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "regexp" "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto" @@ -14,20 +15,22 @@ import ( "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" ) -var errInvalidMnemonic = errors.New("invalid bip39 mnemonic") +var ( + errInvalidMnemonic = errors.New("invalid bip39 mnemonic") + errInvalidDerivationPath = errors.New("invalid derivation path") +) + +var reDerivationPath = regexp.MustCompile(`^44'\/118'\/\d+'\/0\/\d+$`) type AddCfg struct { RootCfg *BaseCfg - Recover bool - NoBackup bool - Account uint64 - Index uint64 - DeriveAccounts uint64 -} + Recover bool + NoBackup bool + Account uint64 + Index uint64 -type AddBaseCfg struct { - RootCfg *BaseCfg + DerivationPath commands.StringArr } func NewAddCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { @@ -85,30 +88,32 @@ func (c *AddCfg) RegisterFlags(fs *flag.FlagSet) { "address index number for HD derivation", ) - fs.Uint64Var( - &c.DeriveAccounts, - "derive-accounts", - 0, - "the number of accounts to derive from the mnemonic", + fs.Var( + &c.DerivationPath, + "derivation-path", + "derivation path for deriving the address", ) } -/* -input - - bip39 mnemonic - - bip39 passphrase - - bip44 path - - local encryption password - -output - - armor encrypted private key (saved to file) -*/ func execAdd(cfg *AddCfg, args []string, io commands.IO) error { // Check if the key name is provided if len(args) != 1 { return flag.ErrHelp } + // Validate the derivation paths are correct + for _, path := range cfg.DerivationPath { + // Make sure the path is valid + if _, err := hd.NewParamsFromPath(path); err != nil { + return fmt.Errorf("invalid derivation path, %w", err) + } + + // Make sure the path conforms to the Gno derivation path + if !reDerivationPath.MatchString(path) { + return errInvalidDerivationPath + } + } + name := args[0] // Read the keybase from the home directory @@ -180,7 +185,7 @@ func execAdd(cfg *AddCfg, args []string, io commands.IO) error { } // Print the derived address info - printDerive(mnemonic, cfg.Index, cfg.DeriveAccounts, io) + printDerive(mnemonic, cfg.DerivationPath, io) // Recover key from seed passphrase if cfg.Recover { @@ -223,11 +228,10 @@ func printNewInfo(info keys.Info, io commands.IO) { // printDerive prints the derived accounts, if any func printDerive( mnemonic string, - accountIndex, - numAccounts uint64, + paths []string, io commands.IO, ) { - if numAccounts == 0 { + if len(paths) == 0 { // No accounts to print return } @@ -235,43 +239,44 @@ func printDerive( // Generate the accounts accounts := generateAccounts( mnemonic, - accountIndex, - numAccounts, + paths, ) io.Printf("[Derived Accounts]\n\n") - io.Printf("Account Index: %d\n\n", accountIndex) // Print them out - for index, account := range accounts { - io.Printfln("%d. %s", index, account.String()) + for index, path := range paths { + io.Printfln( + "%d. %s: %s", + index, + path, + accounts[index].String(), + ) } } // generateAccounts the accounts using the provided mnemonics -func generateAccounts(mnemonic string, accountIndex, numAccounts uint64) []crypto.Address { - addresses := make([]crypto.Address, numAccounts) +func generateAccounts(mnemonic string, paths []string) []crypto.Address { + addresses := make([]crypto.Address, len(paths)) // Generate the seed seed := bip39.NewSeed(mnemonic, "") - for i := uint64(0); i < numAccounts; i++ { - key := generateKeyFromSeed(seed, uint32(accountIndex), uint32(i)) + for index, path := range paths { + key := generateKeyFromSeed(seed, path) address := key.PubKey().Address() - addresses[i] = address + addresses[index] = address } return addresses } // generateKeyFromSeed generates a private key from -// the provided seed and index -func generateKeyFromSeed(seed []byte, account, index uint32) crypto.PrivKey { - pathParams := hd.NewFundraiserParams(account, crypto.CoinType, index) - +// the provided seed and path +func generateKeyFromSeed(seed []byte, path string) crypto.PrivKey { masterPriv, ch := hd.ComputeMastersFromSeed(seed) - derivedPriv, _ := hd.DerivePrivateKeyForPath(masterPriv, ch, pathParams.String()) + derivedPriv, _ := hd.DerivePrivateKeyForPath(masterPriv, ch, path) return secp256k1.PrivKeySecp256k1(derivedPriv) } diff --git a/tm2/pkg/crypto/keys/client/add_ledger.go b/tm2/pkg/crypto/keys/client/add_ledger.go index 8ccf19eb264..97bd4a3bee5 100644 --- a/tm2/pkg/crypto/keys/client/add_ledger.go +++ b/tm2/pkg/crypto/keys/client/add_ledger.go @@ -18,7 +18,7 @@ func NewAddLedgerCmd(cfg *AddCfg, io commands.IO) *commands.Command { ShortUsage: "add ledger [flags] ", ShortHelp: "adds a Ledger key reference to the keybase", }, - cfg, + commands.NewEmptyConfig(), func(_ context.Context, args []string) error { return execAddLedger(cfg, args, io) }, diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index 578cbbe91cc..b40da2f96fb 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -75,16 +75,18 @@ func Test_execAddPublicKey(t *testing.T) { assert.NotNil(t, kbHome) defer kbCleanUp() - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - Home: kbHome, + cfg := &AddBech32Cfg{ + RootCfg: &AddCfg{ + RootCfg: &BaseCfg{ + BaseOptions: BaseOptions{ + Home: kbHome, + }, }, }, - // PublicKey: test2PubkeyBech32, // test2 account + PublicKey: test2PubkeyBech32, // test2 account } - if err := execAdd(cfg, []string{"test2"}, nil); err != nil { + if err := execAddBech32(cfg, []string{"test2"}, nil); err != nil { t.Fatalf("unable to execute add cmd, %v", err) } } @@ -137,7 +139,6 @@ func Test_execAddDerive(t *testing.T) { var ( mnemonic = generateTestMnemonic(t) accountIndex uint64 = 0 - numAccounts uint64 = 10 dummyPass = "dummy-pass" ) @@ -154,7 +155,7 @@ func Test_execAddDerive(t *testing.T) { }, }, Recover: true, - DeriveAccounts: numAccounts, + DerivationPath: nil, // TODO fix Account: accountIndex, } @@ -169,8 +170,7 @@ func Test_execAddDerive(t *testing.T) { // Verify the addresses are derived correctly expectedAccounts := generateAccounts( mnemonic, - accountIndex, - numAccounts, + nil, // TODO fix ) // Grab the output From 34d8bb4d948c6f50825048cd521d251799ba5489 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Sat, 30 Mar 2024 17:55:00 +0900 Subject: [PATCH 09/16] Add unit tests for path derivation --- tm2/pkg/crypto/keys/client/add.go | 6 +- tm2/pkg/crypto/keys/client/add_test.go | 225 +++++++++++++++++++------ 2 files changed, 182 insertions(+), 49 deletions(-) diff --git a/tm2/pkg/crypto/keys/client/add.go b/tm2/pkg/crypto/keys/client/add.go index 6b7cf82a94e..3c0c6aaf343 100644 --- a/tm2/pkg/crypto/keys/client/add.go +++ b/tm2/pkg/crypto/keys/client/add.go @@ -105,7 +105,11 @@ func execAdd(cfg *AddCfg, args []string, io commands.IO) error { for _, path := range cfg.DerivationPath { // Make sure the path is valid if _, err := hd.NewParamsFromPath(path); err != nil { - return fmt.Errorf("invalid derivation path, %w", err) + return fmt.Errorf( + "%w, %w", + errInvalidDerivationPath, + err, + ) } // Make sure the path conforms to the Gno derivation path diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index b40da2f96fb..05fc479248b 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -28,6 +28,183 @@ func generateTestMnemonic(t *testing.T) string { return mnemonic } +func TestAdd_Base_Add(t *testing.T) { + t.Parallel() + + t.Run("valid key addition, generated mnemonic", func(t *testing.T) { + t.Parallel() + + // TODO + }) + + t.Run("valid key addition, provided mnemonic", func(t *testing.T) { + t.Parallel() + + // TODO + }) + + t.Run("no overwrite permission", func(t *testing.T) { + t.Parallel() + + // TODO + }) +} + +func generateDerivationPaths(count int) []string { + paths := make([]string, count) + + for i := 0; i < count; i++ { + paths[i] = fmt.Sprintf("44'/118'/0'/0/%d", i) + } + + return paths +} + +func TestAdd_Derive(t *testing.T) { + t.Parallel() + + t.Run("valid address derivation", func(t *testing.T) { + t.Parallel() + + var ( + mnemonic = generateTestMnemonic(t) + paths = generateDerivationPaths(10) + + dummyPass = "dummy-pass" + ) + + kbHome, kbCleanUp := testutils.NewTestCaseDir(t) + require.NotNil(t, kbHome) + t.Cleanup(kbCleanUp) + + cfg := &AddCfg{ + RootCfg: &BaseCfg{ + BaseOptions: BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + }, + }, + Recover: true, + DerivationPath: paths, + } + + mockOut := bytes.NewBufferString("") + + io := commands.NewTestIO() + io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) + io.SetOut(commands.WriteNopCloser(mockOut)) + + require.NoError(t, + execAdd( + cfg, + []string{ + "example-key", + }, + io, + ), + ) + + // Verify the addresses are derived correctly + expectedAccounts := generateAccounts( + mnemonic, + paths, + ) + + // Grab the output + deriveOutput := mockOut.String() + + for _, expectedAccount := range expectedAccounts { + assert.Contains(t, deriveOutput, expectedAccount.String()) + } + }) + + t.Run("malformed derivation path", func(t *testing.T) { + t.Parallel() + + var ( + mnemonic = generateTestMnemonic(t) + dummyPass = "dummy-pass" + ) + + kbHome, kbCleanUp := testutils.NewTestCaseDir(t) + require.NotNil(t, kbHome) + t.Cleanup(kbCleanUp) + + cfg := &AddCfg{ + RootCfg: &BaseCfg{ + BaseOptions: BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + }, + }, + Recover: true, + DerivationPath: []string{ + "malformed path", + }, + } + + mockOut := bytes.NewBufferString("") + + io := commands.NewTestIO() + io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) + io.SetOut(commands.WriteNopCloser(mockOut)) + + require.ErrorIs( + t, execAdd( + cfg, + []string{ + "example-key", + }, + io, + ), + errInvalidDerivationPath, + ) + }) + + t.Run("invalid derivation path", func(t *testing.T) { + t.Parallel() + + var ( + mnemonic = generateTestMnemonic(t) + dummyPass = "dummy-pass" + ) + + kbHome, kbCleanUp := testutils.NewTestCaseDir(t) + require.NotNil(t, kbHome) + t.Cleanup(kbCleanUp) + + cfg := &AddCfg{ + RootCfg: &BaseCfg{ + BaseOptions: BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + }, + }, + Recover: true, + DerivationPath: []string{ + "44'/500'/0'/0/0", // invalid coin type + }, + } + + mockOut := bytes.NewBufferString("") + + io := commands.NewTestIO() + io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) + io.SetOut(commands.WriteNopCloser(mockOut)) + + require.ErrorIs( + t, execAdd( + cfg, + []string{ + "example-key", + }, + io, + ), + errInvalidDerivationPath, + ) + }) +} + func Test_execAddBasic(t *testing.T) { t.Parallel() @@ -132,51 +309,3 @@ func Test_execAddRecover(t *testing.T) { s := fmt.Sprintf("%s", keypub) assert.Equal(t, s, test2PubkeyBech32) } - -func Test_execAddDerive(t *testing.T) { - t.Parallel() - - var ( - mnemonic = generateTestMnemonic(t) - accountIndex uint64 = 0 - - dummyPass = "dummy-pass" - ) - - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - require.NotNil(t, kbHome) - defer kbCleanUp() - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - Recover: true, - DerivationPath: nil, // TODO fix - Account: accountIndex, - } - - mockOut := bytes.NewBufferString("") - - io := commands.NewTestIO() - io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) - io.SetOut(commands.WriteNopCloser(mockOut)) - - require.NoError(t, execAdd(cfg, []string{"example-key"}, io)) - - // Verify the addresses are derived correctly - expectedAccounts := generateAccounts( - mnemonic, - nil, // TODO fix - ) - - // Grab the output - deriveOutput := mockOut.String() - - for _, expectedAccount := range expectedAccounts { - assert.Contains(t, deriveOutput, expectedAccount.String()) - } -} From aadd296827541d3c45d44c4eaf6c8adf706ec5df Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Mon, 1 Apr 2024 10:44:53 +0200 Subject: [PATCH 10/16] Add unit tests for bech32 addition --- tm2/pkg/crypto/keys/client/add_bech32_test.go | 202 ++++++++ tm2/pkg/crypto/keys/client/add_test.go | 445 ++++++++++-------- 2 files changed, 458 insertions(+), 189 deletions(-) create mode 100644 tm2/pkg/crypto/keys/client/add_bech32_test.go diff --git a/tm2/pkg/crypto/keys/client/add_bech32_test.go b/tm2/pkg/crypto/keys/client/add_bech32_test.go new file mode 100644 index 00000000000..f7697c0184d --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_bech32_test.go @@ -0,0 +1,202 @@ +package client + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto/bip39" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAdd_Bech32(t *testing.T) { + t.Parallel() + + t.Run("valid bech32 addition", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + seed = bip39.NewSeed(generateTestMnemonic(t), "") + account = generateKeyFromSeed(seed, "44'/118'/0'/0/0") + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "bech32", + "--insecure-password-stdin", + "--home", + kbHome, + "--pubkey", + account.PubKey().String(), + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, original) + + assert.Equal(t, account.PubKey().Address().String(), original.GetAddress().String()) + }) + + t.Run("valid bech32 addition, overwrite", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + seed = bip39.NewSeed(generateTestMnemonic(t), "") + originalAccount = generateKeyFromSeed(seed, "44'/118'/0'/0/0") + copyAccount = generateKeyFromSeed(seed, "44'/118'/0'/0/1") + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + baseArgs := []string{ + "add", + "bech32", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + initialArgs := append(baseArgs, []string{ + "--pubkey", + originalAccount.PubKey().String(), + }...) + + require.NoError(t, cmd.ParseAndRun(ctx, initialArgs)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + + require.Equal(t, originalAccount.PubKey().Address().String(), original.GetAddress().String()) + + // Overwrite the key + io.SetIn(strings.NewReader("y\ntest1234\ntest1234\n")) + + secondaryArgs := append(baseArgs, []string{ + "--pubkey", + copyAccount.PubKey().String(), + }...) + + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.NoError(t, cmd.ParseAndRun(ctx, secondaryArgs)) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + require.Equal(t, copyAccount.PubKey().Address().String(), newKey.GetAddress().String()) + }) + + t.Run("no overwrite permission", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + seed = bip39.NewSeed(generateTestMnemonic(t), "") + originalAccount = generateKeyFromSeed(seed, "44'/118'/0'/0/0") + copyAccount = generateKeyFromSeed(seed, "44'/118'/0'/0/1") + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + baseArgs := []string{ + "add", + "bech32", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + initialArgs := append(baseArgs, []string{ + "--pubkey", + originalAccount.PubKey().String(), + }...) + + require.NoError(t, cmd.ParseAndRun(ctx, initialArgs)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + + io.SetIn(strings.NewReader("n\ntest1234\ntest1234\n")) + + // Confirm overwrite + secondaryArgs := append(baseArgs, []string{ + "--pubkey", + copyAccount.PubKey().String(), + }...) + + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.ErrorIs(t, cmd.ParseAndRun(ctx, secondaryArgs), errOverwriteAborted) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + // Make sure the key is not overwritten + assert.Equal(t, original.GetAddress(), newKey.GetAddress()) + }) +} diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index 05fc479248b..69313fc00d1 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -2,15 +2,15 @@ package client import ( "bytes" + "context" "fmt" "strings" "testing" + "time" "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto/bip39" "github.com/gnolang/gno/tm2/pkg/crypto/keys" - "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" - "github.com/gnolang/gno/tm2/pkg/testutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -34,19 +34,192 @@ func TestAdd_Base_Add(t *testing.T) { t.Run("valid key addition, generated mnemonic", func(t *testing.T) { t.Parallel() - // TODO + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, original) + }) + + t.Run("valid key addition, overwrite", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + + io.SetIn(strings.NewReader("y\ntest1234\ntest1234\n")) + + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + // Make sure the different key is generated and overwritten + assert.NotEqual(t, original.GetAddress(), newKey.GetAddress()) }) t.Run("valid key addition, provided mnemonic", func(t *testing.T) { t.Parallel() - // TODO + var ( + kbHome = t.TempDir() + mnemonic = generateTestMnemonic(t) + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234" + "\n" + "test1234" + "\n" + mnemonic + "\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + "--recover", + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + key, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, key) + + // Get the account + accounts := generateAccounts(mnemonic, []string{"44'/118'/0'/0/0"}) + + assert.Equal(t, accounts[0].String(), key.GetAddress().String()) }) t.Run("no overwrite permission", func(t *testing.T) { t.Parallel() - // TODO + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + + io.SetIn(strings.NewReader("n\ntest1234\ntest1234\n")) + + // Confirm overwrite + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.ErrorIs(t, cmd.ParseAndRun(ctx, args), errOverwriteAborted) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + // Make sure the key is not overwritten + assert.Equal(t, original.GetAddress(), newKey.GetAddress()) }) } @@ -67,26 +240,20 @@ func TestAdd_Derive(t *testing.T) { t.Parallel() var ( + kbHome = t.TempDir() mnemonic = generateTestMnemonic(t) paths = generateDerivationPaths(10) + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + dummyPass = "dummy-pass" ) - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - require.NotNil(t, kbHome) - t.Cleanup(kbCleanUp) - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - Recover: true, - DerivationPath: paths, - } + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() mockOut := bytes.NewBufferString("") @@ -94,15 +261,28 @@ func TestAdd_Derive(t *testing.T) { io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) io.SetOut(commands.WriteNopCloser(mockOut)) - require.NoError(t, - execAdd( - cfg, - []string{ - "example-key", - }, - io, - ), - ) + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + "--recover", + "example-key", + } + + for _, path := range paths { + args = append( + args, []string{ + "--derivation-path", + path, + }..., + ) + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) // Verify the addresses are derived correctly expectedAccounts := generateAccounts( @@ -122,26 +302,17 @@ func TestAdd_Derive(t *testing.T) { t.Parallel() var ( - mnemonic = generateTestMnemonic(t) - dummyPass = "dummy-pass" + kbHome = t.TempDir() + mnemonic = generateTestMnemonic(t) + dummyPass = "dummy-pass" + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } ) - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - require.NotNil(t, kbHome) - t.Cleanup(kbCleanUp) - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - Recover: true, - DerivationPath: []string{ - "malformed path", - }, - } + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() mockOut := bytes.NewBufferString("") @@ -149,42 +320,38 @@ func TestAdd_Derive(t *testing.T) { io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) io.SetOut(commands.WriteNopCloser(mockOut)) - require.ErrorIs( - t, execAdd( - cfg, - []string{ - "example-key", - }, - io, - ), - errInvalidDerivationPath, - ) + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + "--recover", + "example-key", + "--derivation-path", + "malformed path", + } + + require.ErrorIs(t, cmd.ParseAndRun(ctx, args), errInvalidDerivationPath) }) t.Run("invalid derivation path", func(t *testing.T) { t.Parallel() var ( - mnemonic = generateTestMnemonic(t) - dummyPass = "dummy-pass" + kbHome = t.TempDir() + mnemonic = generateTestMnemonic(t) + dummyPass = "dummy-pass" + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } ) - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - require.NotNil(t, kbHome) - t.Cleanup(kbCleanUp) - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - Recover: true, - DerivationPath: []string{ - "44'/500'/0'/0/0", // invalid coin type - }, - } + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() mockOut := bytes.NewBufferString("") @@ -192,120 +359,20 @@ func TestAdd_Derive(t *testing.T) { io.SetIn(strings.NewReader(dummyPass + "\n" + dummyPass + "\n" + mnemonic + "\n")) io.SetOut(commands.WriteNopCloser(mockOut)) - require.ErrorIs( - t, execAdd( - cfg, - []string{ - "example-key", - }, - io, - ), - errInvalidDerivationPath, - ) - }) -} - -func Test_execAddBasic(t *testing.T) { - t.Parallel() - - // make new test dir - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - assert.NotNil(t, kbHome) - defer kbCleanUp() - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - } - - keyName := "keyname1" - - io := commands.NewTestIO() - io.SetIn(strings.NewReader("test1234\ntest1234\n")) - - // Create a new key - if err := execAdd(cfg, []string{keyName}, io); err != nil { - t.Fatalf("unable to execute add cmd, %v", err) - } - - io.SetIn(strings.NewReader("y\ntest1234\ntest1234\n")) - - // Confirm overwrite - if err := execAdd(cfg, []string{keyName}, io); err != nil { - t.Fatalf("unable to execute add cmd, %v", err) - } -} - -var ( - test2Mnemonic = "hair stove window more scrap patient endorse left early pear lawn school loud divide vibrant family still bulk lyrics firm plate media critic dove" - test2PubkeyBech32 = "gpub1pgfj7ard9eg82cjtv4u4xetrwqer2dntxyfzxz3pqg5y7u93gpzug38k2p8s8322zpdm96t0ch87ax88sre4vnclz2jcy8uyhst" -) - -func Test_execAddPublicKey(t *testing.T) { - t.Parallel() - - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - assert.NotNil(t, kbHome) - defer kbCleanUp() - - cfg := &AddBech32Cfg{ - RootCfg: &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - Home: kbHome, - }, - }, - }, - PublicKey: test2PubkeyBech32, // test2 account - } - - if err := execAddBech32(cfg, []string{"test2"}, nil); err != nil { - t.Fatalf("unable to execute add cmd, %v", err) - } -} - -func Test_execAddRecover(t *testing.T) { - t.Parallel() - - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - assert.NotNil(t, kbHome) - defer kbCleanUp() - - cfg := &AddCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - InsecurePasswordStdin: true, - Home: kbHome, - }, - }, - Recover: true, // init test2 account - } - - test2Name := "test2" - test2Passphrase := "gn0rocks!" - - io := commands.NewTestIO() - io.SetIn(strings.NewReader(test2Passphrase + "\n" + test2Passphrase + "\n" + test2Mnemonic + "\n")) - - if err := execAdd(cfg, []string{test2Name}, io); err != nil { - t.Fatalf("unable to execute add cmd, %v", err) - } - - kb, err2 := keys.NewKeyBaseFromDir(kbHome) - assert.NoError(t, err2) - - infos, err3 := kb.List() - assert.NoError(t, err3) - - info := infos[0] - - keypub := info.GetPubKey() - keypub = keypub.(secp256k1.PubKeySecp256k1) + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "--insecure-password-stdin", + "--home", + kbHome, + "--recover", + "example-key", + "--derivation-path", + "44'/500'/0'/0/0", // invalid coin type + } - s := fmt.Sprintf("%s", keypub) - assert.Equal(t, s, test2PubkeyBech32) + require.ErrorIs(t, cmd.ParseAndRun(ctx, args), errInvalidDerivationPath) + }) } From ac7d5e23598777e43422eaf61b8500066db39ed2 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Mon, 1 Apr 2024 11:08:49 +0200 Subject: [PATCH 11/16] Add unit tests for multisig --- tm2/pkg/crypto/keys/client/add_multisig.go | 7 +- .../crypto/keys/client/add_multisig_test.go | 121 ++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 tm2/pkg/crypto/keys/client/add_multisig_test.go diff --git a/tm2/pkg/crypto/keys/client/add_multisig.go b/tm2/pkg/crypto/keys/client/add_multisig.go index c3c2a798f0e..39b90571143 100644 --- a/tm2/pkg/crypto/keys/client/add_multisig.go +++ b/tm2/pkg/crypto/keys/client/add_multisig.go @@ -13,7 +13,10 @@ import ( "github.com/gnolang/gno/tm2/pkg/crypto/multisig" ) -var errOverwriteAborted = errors.New("overwrite aborted") +var ( + errOverwriteAborted = errors.New("overwrite aborted") + errUnableToVerifyMultisig = errors.New("unable to verify multisig threshold") +) type AddMultisigCfg struct { RootCfg *AddCfg @@ -75,7 +78,7 @@ func execAddMultisig(cfg *AddMultisigCfg, args []string, io commands.IO) error { cfg.MultisigThreshold, len(cfg.Multisig), ); err != nil { - return fmt.Errorf("unable to verify multisig threshold") + return errUnableToVerifyMultisig } name := args[0] diff --git a/tm2/pkg/crypto/keys/client/add_multisig_test.go b/tm2/pkg/crypto/keys/client/add_multisig_test.go new file mode 100644 index 00000000000..4a350d5faa9 --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_multisig_test.go @@ -0,0 +1,121 @@ +package client + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAdd_Multisig(t *testing.T) { + t.Parallel() + + t.Run("invalid multisig threshold", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "multisig", + "--insecure-password-stdin", + "--home", + kbHome, + "--multisig", + "example", + "--threshold", + "2", + keyName, + } + + require.ErrorIs(t, cmd.ParseAndRun(ctx, args), errUnableToVerifyMultisig) + }) + + t.Run("valid multisig reference added", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + mnemonic = generateTestMnemonic(t) + + keyNames = []string{ + "key-1", + "key-2", + } + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("y\ntest1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "multisig", + "--insecure-password-stdin", + "--home", + kbHome, + "--multisig", + keyNames[0], + "--multisig", + keyNames[1], + keyNames[0], + } + + // Prepare the multisig keys + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + for index, keyName := range keyNames { + _, err = kb.CreateAccount( + keyName, + mnemonic, + "", + "123", + 0, + uint32(index), + ) + + require.NoError(t, err) + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Verify the key is multisig + original, err := kb.GetByName(keyNames[0]) + require.NoError(t, err) + require.NotNil(t, original) + + assert.Equal(t, original.GetType(), keys.TypeMulti) + }) +} From a68843ba4524f1fdfc1e207c76bd298c43f81d3f Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Tue, 2 Apr 2024 15:30:55 +0200 Subject: [PATCH 12/16] Add Ledger unit tests --- .github/workflows/tm2.yml | 2 +- tm2/Makefile | 2 +- .../keys/client/add_ledger_skipped_test.go | 10 ++ tm2/pkg/crypto/keys/client/add_ledger_test.go | 170 ++++++++++++++++++ tm2/pkg/crypto/ledger/discover.go | 19 ++ tm2/pkg/crypto/ledger/discover_mock.go | 69 +++++++ tm2/pkg/crypto/ledger/ledger_secp256k1.go | 12 -- 7 files changed, 270 insertions(+), 14 deletions(-) create mode 100644 tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go create mode 100644 tm2/pkg/crypto/keys/client/add_ledger_test.go create mode 100644 tm2/pkg/crypto/ledger/discover.go create mode 100644 tm2/pkg/crypto/ledger/discover_mock.go diff --git a/.github/workflows/tm2.yml b/.github/workflows/tm2.yml index ced6054f9e9..9ea54e27512 100644 --- a/.github/workflows/tm2.yml +++ b/.github/workflows/tm2.yml @@ -79,7 +79,7 @@ jobs: working-directory: tm2 run: | export GOPATH=$HOME/go - export GOTEST_FLAGS="-v -p 1 -timeout=20m -coverprofile=coverage.out -covermode=atomic" + export GOTEST_FLAGS="-v -p 1 -timeout=20m -coverprofile=coverage.out -covermode=atomic -tags='ledger_mock'" make ${{ matrix.args }} touch coverage.out - uses: actions/upload-artifact@v4 diff --git a/tm2/Makefile b/tm2/Makefile index 3103ef220b2..f48b6e47a95 100644 --- a/tm2/Makefile +++ b/tm2/Makefile @@ -20,7 +20,7 @@ GOFMT_FLAGS ?= -w # flags for `make imports`. GOIMPORTS_FLAGS ?= $(GOFMT_FLAGS) # test suite flags. -GOTEST_FLAGS ?= -v -p 1 -timeout=30m +GOTEST_FLAGS ?= -v -p 1 -timeout=30m -tags='ledger_mock' ######################################## # Dev tools diff --git a/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go b/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go new file mode 100644 index 00000000000..770593e337b --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go @@ -0,0 +1,10 @@ +//go:build !ledger_mock +// +build !ledger_mock + +package client + +import "testing" + +func TestAdd_Ledger(t *testing.T) { + t.Skip("Please enable the 'ledger_mock' build tags") +} diff --git a/tm2/pkg/crypto/keys/client/add_ledger_test.go b/tm2/pkg/crypto/keys/client/add_ledger_test.go new file mode 100644 index 00000000000..5a4e0cd6f92 --- /dev/null +++ b/tm2/pkg/crypto/keys/client/add_ledger_test.go @@ -0,0 +1,170 @@ +//go:build ledger_mock +// +build ledger_mock + +package client + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/gnolang/gno/tm2/pkg/commands" + "github.com/gnolang/gno/tm2/pkg/crypto/keys" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Make sure to run these tests with the following tag enabled: +// -tags='ledger_mock' +func TestAdd_Ledger(t *testing.T) { + t.Parallel() + + t.Run("valid ledger reference added", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "ledger", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, original) + }) + + t.Run("valid ledger reference added, overwrite", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "ledger", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, original) + + io.SetIn(strings.NewReader("y\ntest1234\ntest1234\n")) + + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + // Make sure the different key is generated and overwritten + assert.NotEqual(t, original.GetAddress(), newKey.GetAddress()) + }) + + t.Run("valid ledger reference added, no overwrite permission", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + keyName = "key-name" + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + io := commands.NewTestIO() + io.SetIn(strings.NewReader("test1234\ntest1234\n")) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "add", + "ledger", + "--insecure-password-stdin", + "--home", + kbHome, + keyName, + } + + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Check the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + original, err := kb.GetByName(keyName) + require.NoError(t, err) + require.NotNil(t, original) + + io.SetIn(strings.NewReader("n\ntest1234\ntest1234\n")) + + cmd = NewRootCmdWithBaseConfig(io, baseOptions) + require.ErrorIs(t, cmd.ParseAndRun(ctx, args), errOverwriteAborted) + + newKey, err := kb.GetByName(keyName) + require.NoError(t, err) + + // Make sure the key is not overwritten + assert.Equal(t, original.GetAddress(), newKey.GetAddress()) + }) +} diff --git a/tm2/pkg/crypto/ledger/discover.go b/tm2/pkg/crypto/ledger/discover.go new file mode 100644 index 00000000000..9375378f1ec --- /dev/null +++ b/tm2/pkg/crypto/ledger/discover.go @@ -0,0 +1,19 @@ +//go:build !ledger_mock +// +build !ledger_mock + +package ledger + +import ( + ledger_go "github.com/cosmos/ledger-cosmos-go" +) + +// discoverLedger defines a function to be invoked at runtime for discovering +// a connected Ledger device. +var discoverLedger discoverLedgerFn = func() (LedgerSECP256K1, error) { + device, err := ledger_go.FindLedgerCosmosUserApp() + if err != nil { + return nil, err + } + + return device, nil +} diff --git a/tm2/pkg/crypto/ledger/discover_mock.go b/tm2/pkg/crypto/ledger/discover_mock.go new file mode 100644 index 00000000000..9693ee1f2e8 --- /dev/null +++ b/tm2/pkg/crypto/ledger/discover_mock.go @@ -0,0 +1,69 @@ +//go:build ledger_mock +// +build ledger_mock + +package ledger + +import ( + btcec "github.com/btcsuite/btcd/btcec/v2" + "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" +) + +// discoverLedger defines a function to be invoked at runtime for discovering +// a connected Ledger device. +var discoverLedger discoverLedgerFn = func() (LedgerSECP256K1, error) { + privateKey := secp256k1.GenPrivKey() + + _, pubKeyObject := btcec.PrivKeyFromBytes(privateKey[:]) + + return &MockLedger{ + GetAddressPubKeySECP256K1Fn: func(data []uint32, str string) ([]byte, string, error) { + return pubKeyObject.SerializeCompressed(), privateKey.PubKey().Address().String(), nil + }, + }, nil +} + +type ( + closeDelegate func() error + getPublicKeySECP256K1Delegate func([]uint32) ([]byte, error) + getAddressPubKeySECP256K1Delegate func([]uint32, string) ([]byte, string, error) + signSECP256K1Delegate func([]uint32, []byte, byte) ([]byte, error) +) + +type MockLedger struct { + CloseFn closeDelegate + GetPublicKeySECP256K1Fn getPublicKeySECP256K1Delegate + GetAddressPubKeySECP256K1Fn getAddressPubKeySECP256K1Delegate + SignSECP256K1Fn signSECP256K1Delegate +} + +func (m *MockLedger) Close() error { + if m.CloseFn != nil { + return m.CloseFn() + } + + return nil +} + +func (m *MockLedger) GetPublicKeySECP256K1(data []uint32) ([]byte, error) { + if m.GetPublicKeySECP256K1Fn != nil { + return m.GetPublicKeySECP256K1Fn(data) + } + + return nil, nil +} + +func (m *MockLedger) GetAddressPubKeySECP256K1(data []uint32, str string) ([]byte, string, error) { + if m.GetAddressPubKeySECP256K1Fn != nil { + return m.GetAddressPubKeySECP256K1Fn(data, str) + } + + return nil, "", nil +} + +func (m *MockLedger) SignSECP256K1(d1 []uint32, d2 []byte, d3 byte) ([]byte, error) { + if m.SignSECP256K1Fn != nil { + return m.SignSECP256K1Fn(d1, d2, d3) + } + + return nil, nil +} diff --git a/tm2/pkg/crypto/ledger/ledger_secp256k1.go b/tm2/pkg/crypto/ledger/ledger_secp256k1.go index f154dbf376c..56877b813a5 100644 --- a/tm2/pkg/crypto/ledger/ledger_secp256k1.go +++ b/tm2/pkg/crypto/ledger/ledger_secp256k1.go @@ -9,7 +9,6 @@ import ( "github.com/btcsuite/btcd/btcec/v2/ecdsa" secp "github.com/decred/dcrd/dcrec/secp256k1/v4" - ledger "github.com/cosmos/ledger-cosmos-go" "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/crypto/hd" @@ -45,17 +44,6 @@ type ( } ) -// discoverLedger defines a function to be invoked at runtime for discovering -// a connected Ledger device. -var discoverLedger discoverLedgerFn = func() (LedgerSECP256K1, error) { - device, err := ledger.FindLedgerCosmosUserApp() - if err != nil { - return nil, err - } - - return device, nil -} - // NewPrivKeyLedgerSecp256k1Unsafe will generate a new key and store the public key for later use. // // This function is marked as unsafe as it will retrieve a pubkey without user verification. From ca415d27b940115006ddfd8a199cb2b0d94f2b2c Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Tue, 2 Apr 2024 16:54:47 +0200 Subject: [PATCH 13/16] Fix flaky test --- tm2/Makefile | 2 +- .../keys/client/add_ledger_skipped_test.go | 6 +++--- tm2/pkg/crypto/keys/client/add_ledger_test.go | 6 +++--- tm2/pkg/crypto/keys/keybase_test.go | 19 ++----------------- tm2/pkg/crypto/ledger/discover.go | 4 ++-- tm2/pkg/crypto/ledger/discover_mock.go | 4 ++-- 6 files changed, 13 insertions(+), 28 deletions(-) diff --git a/tm2/Makefile b/tm2/Makefile index f48b6e47a95..f841b989b77 100644 --- a/tm2/Makefile +++ b/tm2/Makefile @@ -20,7 +20,7 @@ GOFMT_FLAGS ?= -w # flags for `make imports`. GOIMPORTS_FLAGS ?= $(GOFMT_FLAGS) # test suite flags. -GOTEST_FLAGS ?= -v -p 1 -timeout=30m -tags='ledger_mock' +GOTEST_FLAGS ?= -v -p 1 -timeout=30m -tags='ledger_suite' ######################################## # Dev tools diff --git a/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go b/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go index 770593e337b..8a09d060b16 100644 --- a/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go +++ b/tm2/pkg/crypto/keys/client/add_ledger_skipped_test.go @@ -1,10 +1,10 @@ -//go:build !ledger_mock -// +build !ledger_mock +//go:build !ledger_suite +// +build !ledger_suite package client import "testing" func TestAdd_Ledger(t *testing.T) { - t.Skip("Please enable the 'ledger_mock' build tags") + t.Skip("Please enable the 'ledger_suite' build tags") } diff --git a/tm2/pkg/crypto/keys/client/add_ledger_test.go b/tm2/pkg/crypto/keys/client/add_ledger_test.go index 5a4e0cd6f92..c1384efcb79 100644 --- a/tm2/pkg/crypto/keys/client/add_ledger_test.go +++ b/tm2/pkg/crypto/keys/client/add_ledger_test.go @@ -1,5 +1,5 @@ -//go:build ledger_mock -// +build ledger_mock +//go:build ledger_suite +// +build ledger_suite package client @@ -16,7 +16,7 @@ import ( ) // Make sure to run these tests with the following tag enabled: -// -tags='ledger_mock' +// -tags='ledger_suite' func TestAdd_Ledger(t *testing.T) { t.Parallel() diff --git a/tm2/pkg/crypto/keys/keybase_test.go b/tm2/pkg/crypto/keys/keybase_test.go index 0c43dbd8dc5..2b727c6e244 100644 --- a/tm2/pkg/crypto/keys/keybase_test.go +++ b/tm2/pkg/crypto/keys/keybase_test.go @@ -42,29 +42,14 @@ func TestCreateLedger(t *testing.T) { // test_cover does not compile some dependencies so ledger is disabled // test_unit may add a ledger mock // both cases are acceptable - ledger, err := kb.CreateLedger("some_account", Secp256k1, "cosmos", 3, 1) - if err != nil { - assert.Error(t, err) - assert.Contains(t, err.Error(), "LedgerHID device (idx 0) not found.") - - assert.Nil(t, ledger) - t.Skip("ledger nano S: support for ledger devices is not available in this executable") - return - } - - // The mock is available, check that the address is correct - pubKey := ledger.GetPubKey() - pubs := crypto.PubKeyToBech32(pubKey) - assert.Equal(t, "cosmospub1addwnpepqdszcr95mrqqs8lw099aa9h8h906zmet22pmwe9vquzcgvnm93eqygufdlv", pubs) + _, err := kb.CreateLedger("some_account", Secp256k1, "cosmos", 3, 1) + require.NoError(t, err) // Check that restoring the key gets the same results restoredKey, err := kb.GetByName("some_account") assert.NotNil(t, restoredKey) assert.Equal(t, "some_account", restoredKey.GetName()) assert.Equal(t, TypeLedger, restoredKey.GetType()) - pubKey = restoredKey.GetPubKey() - pubs = crypto.PubKeyToBech32(pubKey) - assert.Equal(t, "cosmospub1addwnpepqdszcr95mrqqs8lw099aa9h8h906zmet22pmwe9vquzcgvnm93eqygufdlv", pubs) path, err := restoredKey.GetPath() assert.NoError(t, err) diff --git a/tm2/pkg/crypto/ledger/discover.go b/tm2/pkg/crypto/ledger/discover.go index 9375378f1ec..d610b56635e 100644 --- a/tm2/pkg/crypto/ledger/discover.go +++ b/tm2/pkg/crypto/ledger/discover.go @@ -1,5 +1,5 @@ -//go:build !ledger_mock -// +build !ledger_mock +//go:build !ledger_suite +// +build !ledger_suite package ledger diff --git a/tm2/pkg/crypto/ledger/discover_mock.go b/tm2/pkg/crypto/ledger/discover_mock.go index 9693ee1f2e8..1f5bdbafdf3 100644 --- a/tm2/pkg/crypto/ledger/discover_mock.go +++ b/tm2/pkg/crypto/ledger/discover_mock.go @@ -1,5 +1,5 @@ -//go:build ledger_mock -// +build ledger_mock +//go:build ledger_suite +// +build ledger_suite package ledger From fd07f58923f0c71542dca72819a3364521ab793b Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Tue, 2 Apr 2024 16:57:50 +0200 Subject: [PATCH 14/16] Add tag in workflow --- .github/workflows/tm2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tm2.yml b/.github/workflows/tm2.yml index 9ea54e27512..5c7c24e98e1 100644 --- a/.github/workflows/tm2.yml +++ b/.github/workflows/tm2.yml @@ -79,7 +79,7 @@ jobs: working-directory: tm2 run: | export GOPATH=$HOME/go - export GOTEST_FLAGS="-v -p 1 -timeout=20m -coverprofile=coverage.out -covermode=atomic -tags='ledger_mock'" + export GOTEST_FLAGS="-v -p 1 -timeout=20m -coverprofile=coverage.out -covermode=atomic -tags='ledger_suite'" make ${{ matrix.args }} touch coverage.out - uses: actions/upload-artifact@v4 From 9a4c9f86cffe39b0b563385138c1023378c9145c Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Tue, 16 Apr 2024 16:55:42 +0200 Subject: [PATCH 15/16] Wrap up skipped tests --- .../keys/keybase_ledger_skipped_test.go | 18 ++++++++ tm2/pkg/crypto/keys/keybase_ledger_test.go | 43 +++++++++++++++++++ tm2/pkg/crypto/keys/keybase_test.go | 32 -------------- 3 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 tm2/pkg/crypto/keys/keybase_ledger_skipped_test.go create mode 100644 tm2/pkg/crypto/keys/keybase_ledger_test.go diff --git a/tm2/pkg/crypto/keys/keybase_ledger_skipped_test.go b/tm2/pkg/crypto/keys/keybase_ledger_skipped_test.go new file mode 100644 index 00000000000..d406f10f2ed --- /dev/null +++ b/tm2/pkg/crypto/keys/keybase_ledger_skipped_test.go @@ -0,0 +1,18 @@ +//go:build !ledger_suite +// +build !ledger_suite + +package keys + +import "testing" + +func TestCreateLedgerUnsupportedAlgo(t *testing.T) { + t.Parallel() + + t.Skip("this test needs to be run with the `ledger_suite` tag enabled") +} + +func TestCreateLedger(t *testing.T) { + t.Parallel() + + t.Skip("this test needs to be run with the `ledger_suite` tag enabled") +} diff --git a/tm2/pkg/crypto/keys/keybase_ledger_test.go b/tm2/pkg/crypto/keys/keybase_ledger_test.go new file mode 100644 index 00000000000..0f2fca79f90 --- /dev/null +++ b/tm2/pkg/crypto/keys/keybase_ledger_test.go @@ -0,0 +1,43 @@ +//go:build ledger_suite +// +build ledger_suite + +package keys + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCreateLedgerUnsupportedAlgo(t *testing.T) { + t.Parallel() + + kb := NewInMemory() + _, err := kb.CreateLedger("some_account", Ed25519, "cosmos", 0, 1) + assert.Error(t, err) + assert.Equal(t, "unsupported signing algo: only secp256k1 is supported", err.Error()) +} + +func TestCreateLedger(t *testing.T) { + t.Parallel() + + kb := NewInMemory() + + // test_cover and test_unit will result in different answers + // test_cover does not compile some dependencies so ledger is disabled + // test_unit may add a ledger mock + // both cases are acceptable + _, err := kb.CreateLedger("some_account", Secp256k1, "cosmos", 3, 1) + require.NoError(t, err) + + // Check that restoring the key gets the same results + restoredKey, err := kb.GetByName("some_account") + assert.NotNil(t, restoredKey) + assert.Equal(t, "some_account", restoredKey.GetName()) + assert.Equal(t, TypeLedger, restoredKey.GetType()) + + path, err := restoredKey.GetPath() + assert.NoError(t, err) + assert.Equal(t, "44'/118'/3'/0/1", path.String()) +} diff --git a/tm2/pkg/crypto/keys/keybase_test.go b/tm2/pkg/crypto/keys/keybase_test.go index 2b727c6e244..32cc8788b52 100644 --- a/tm2/pkg/crypto/keys/keybase_test.go +++ b/tm2/pkg/crypto/keys/keybase_test.go @@ -24,38 +24,6 @@ func TestCreateAccountInvalidMnemonic(t *testing.T) { assert.Equal(t, "invalid mnemonic", err.Error()) } -func TestCreateLedgerUnsupportedAlgo(t *testing.T) { - t.Parallel() - - kb := NewInMemory() - _, err := kb.CreateLedger("some_account", Ed25519, "cosmos", 0, 1) - assert.Error(t, err) - assert.Equal(t, "unsupported signing algo: only secp256k1 is supported", err.Error()) -} - -func TestCreateLedger(t *testing.T) { - t.Parallel() - - kb := NewInMemory() - - // test_cover and test_unit will result in different answers - // test_cover does not compile some dependencies so ledger is disabled - // test_unit may add a ledger mock - // both cases are acceptable - _, err := kb.CreateLedger("some_account", Secp256k1, "cosmos", 3, 1) - require.NoError(t, err) - - // Check that restoring the key gets the same results - restoredKey, err := kb.GetByName("some_account") - assert.NotNil(t, restoredKey) - assert.Equal(t, "some_account", restoredKey.GetName()) - assert.Equal(t, TypeLedger, restoredKey.GetType()) - - path, err := restoredKey.GetPath() - assert.NoError(t, err) - assert.Equal(t, "44'/118'/3'/0/1", path.String()) -} - // TestKeyManagement makes sure we can manipulate these keys well func TestKeyManagement(t *testing.T) { t.Parallel() From 9efe4b01024260944519a6ff9c3887c2c89da02b Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Fri, 26 Apr 2024 14:10:47 +0200 Subject: [PATCH 16/16] Remove duplicate helper --- tm2/pkg/crypto/keys/client/add_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tm2/pkg/crypto/keys/client/add_test.go b/tm2/pkg/crypto/keys/client/add_test.go index 69313fc00d1..37638f995bd 100644 --- a/tm2/pkg/crypto/keys/client/add_test.go +++ b/tm2/pkg/crypto/keys/client/add_test.go @@ -9,25 +9,11 @@ import ( "time" "github.com/gnolang/gno/tm2/pkg/commands" - "github.com/gnolang/gno/tm2/pkg/crypto/bip39" "github.com/gnolang/gno/tm2/pkg/crypto/keys" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// generateTestMnemonic generates a random mnemonic -func generateTestMnemonic(t *testing.T) string { - t.Helper() - - entropy, entropyErr := bip39.NewEntropy(256) - require.NoError(t, entropyErr) - - mnemonic, mnemonicErr := bip39.NewMnemonic(entropy) - require.NoError(t, mnemonicErr) - - return mnemonic -} - func TestAdd_Base_Add(t *testing.T) { t.Parallel()