From 93edb12f142ffad5c0cb9bd53b4a733de92419a5 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Tue, 23 Apr 2024 19:27:09 +0200 Subject: [PATCH 1/3] Update gnokey sign --- gno.land/tx-signed.jsonl | 0 gno.land/tx.jsonl | 1 + tm2/pkg/crypto/keys/client/maketx.go | 25 +-- tm2/pkg/crypto/keys/client/root.go | 4 - tm2/pkg/crypto/keys/client/sign.go | 213 +++++++++++++----------- tm2/pkg/crypto/keys/client/sign_test.go | 81 +-------- tm2/pkg/std/tx.go | 5 +- 7 files changed, 133 insertions(+), 196 deletions(-) create mode 100644 gno.land/tx-signed.jsonl create mode 100644 gno.land/tx.jsonl diff --git a/gno.land/tx-signed.jsonl b/gno.land/tx-signed.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/gno.land/tx.jsonl b/gno.land/tx.jsonl new file mode 100644 index 00000000000..d7f95f75e75 --- /dev/null +++ b/gno.land/tx.jsonl @@ -0,0 +1 @@ +{"msg":[{"@type":"/bank.MsgSend","from_address":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","to_address":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","amount":"1ugnot"}],"fee":{"gas_wanted":"100000","gas_fee":"1000000ugnot"},"signatures":[{"pub_key":{"@type":"/tm.PubKeySecp256k1","value":"A+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2y"},"signature":"7xGZV+8qYNgiBifk0kwP2k72nndbZalzMJAQDf258aE5nDVkIFKHtNOaC/MH/i/2QYwec/yiPt6vFoXpcLAg8Q=="}],"memo":""} \ No newline at end of file diff --git a/tm2/pkg/crypto/keys/client/maketx.go b/tm2/pkg/crypto/keys/client/maketx.go index acaee970ac7..f5cd596f2c0 100644 --- a/tm2/pkg/crypto/keys/client/maketx.go +++ b/tm2/pkg/crypto/keys/client/maketx.go @@ -119,25 +119,26 @@ func SignAndBroadcastHandler( // sign tx accountNumber := qret.BaseAccount.AccountNumber sequence := qret.BaseAccount.Sequence - sopts := &SignCfg{ - Pass: pass, - RootCfg: baseopts, - Sequence: sequence, - AccountNumber: accountNumber, - ChainID: txopts.ChainID, - NameOrBech32: nameOrBech32, - TxJSON: amino.MustMarshalJSON(tx), + + sOpts := signOpts{ + chainID: txopts.ChainID, + accountSequence: sequence, + accountNumber: accountNumber, } - signedTx, err := SignHandler(sopts) - if err != nil { - return nil, errors.Wrap(err, "sign tx") + kOpts := keyOpts{ + keyName: nameOrBech32, + decryptPass: pass, + } + + if err := signTx(&tx, kb, sOpts, kOpts); err != nil { + return nil, fmt.Errorf("unable to sign transaction, %w", err) } // broadcast signed tx bopts := &BroadcastCfg{ RootCfg: baseopts, - tx: signedTx, + tx: &tx, } return BroadcastHandler(bopts) 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, diff --git a/tm2/pkg/crypto/keys/client/sign.go b/tm2/pkg/crypto/keys/client/sign.go index 1a7b07d2c5f..8de11c1d3b3 100644 --- a/tm2/pkg/crypto/keys/client/sign.go +++ b/tm2/pkg/crypto/keys/client/sign.go @@ -13,6 +13,19 @@ import ( "github.com/gnolang/gno/tm2/pkg/std" ) +var errInvalidTxFile = errors.New("invalid transaction file") + +type signOpts struct { + chainID string + accountSequence uint64 + accountNumber uint64 +} + +type keyOpts struct { + keyName string + decryptPass string +} + type SignCfg struct { RootCfg *BaseCfg @@ -35,7 +48,7 @@ func NewSignCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { commands.Metadata{ Name: "sign", ShortUsage: "sign [flags] ", - ShortHelp: "signs the document", + ShortHelp: "signs the given tx document and saves it to disk", }, cfg, func(_ context.Context, args []string) error { @@ -47,161 +60,167 @@ func NewSignCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { func (c *SignCfg) RegisterFlags(fs *flag.FlagSet) { fs.StringVar( &c.TxPath, - "txpath", - "-", - "path to file of tx to sign", + "tx-path", + "", + "path to the Amino JSON-encoded tx (file) to sign", ) fs.StringVar( &c.ChainID, "chainid", "dev", - "chainid to sign for", + "the ID of the chain", ) fs.Uint64Var( &c.AccountNumber, - "number", + "account-number", 0, - "account number to sign with (required)", + "account number to sign with", ) fs.Uint64Var( &c.Sequence, - "sequence", + "account-sequence", 0, - "sequence to sign with (required)", + "account sequence to sign with", ) fs.BoolVar( &c.ShowSignBytes, - "show-signbytes", + "show", false, - "show sign bytes and quit", + "display the sign bytes", ) } func execSign(cfg *SignCfg, args []string, io commands.IO) error { - var err error - + // Make sure the key name is provided if len(args) != 1 { return flag.ErrHelp } - cfg.NameOrBech32 = args[0] - - // read tx to sign - txpath := cfg.TxPath - if txpath == "-" { // from stdin. - txjsonstr, err := io.GetString( - "Enter tx to sign, terminated by a newline.", - ) + // saveTx saves the given transaction to the given path (Amino-encoded JSON) + saveTx := func(tx *std.Tx, path string) error { + // Encode the transaction + encodedTx, err := amino.MarshalJSON(tx) if err != nil { - return err + return fmt.Errorf("unable ot marshal tx to JSON, %w", err) } - cfg.TxJSON = []byte(txjsonstr) - } else { // from file - cfg.TxJSON, err = os.ReadFile(txpath) - if err != nil { - return err + + // Save the transaction + if err := os.WriteFile(path, encodedTx, 0o644); err != nil { + return fmt.Errorf("unable to write tx to %s, %w", path, err) } - } - if cfg.RootCfg.Quiet { - cfg.Pass, err = io.GetPassword( - "", - cfg.RootCfg.InsecurePasswordStdin, - ) - } else { - cfg.Pass, err = io.GetPassword( - "Enter password.", - cfg.RootCfg.InsecurePasswordStdin, - ) - } - if err != nil { - return err + io.Printf("\nTx successfully signed and saved to %s\n", path) + + return nil } - signedTx, err := SignHandler(cfg) + // Load the keybase + kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.Home) if err != nil { - return err + return fmt.Errorf("unable to load keybase, %w", err) } - signedJSON, err := amino.MarshalJSON(signedTx) + // Get the transaction bytes + txRaw, err := os.ReadFile(cfg.TxPath) if err != nil { - return err + return fmt.Errorf("unable to read transaction file") } - io.Println(string(signedJSON)) - return nil -} + // Make sure there is something to actually sign + if len(txRaw) == 0 { + return errInvalidTxFile + } -func SignHandler(cfg *SignCfg) (*std.Tx, error) { - var err error + // Make sure the tx is valid Amino JSON var tx std.Tx - - if cfg.TxJSON == nil { - return nil, errors.New("invalid tx content") + if err := amino.UnmarshalJSON(txRaw, &tx); err != nil { + return fmt.Errorf("unable to unmarshal tx, %w", err) } - kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.Home) - if err != nil { - return nil, err + // Get the keybase decryption password + prompt := "Enter password to decrypt key" + if cfg.RootCfg.Quiet { + prompt = "" // No prompt } - err = amino.UnmarshalJSON(cfg.TxJSON, &tx) + password, err := io.GetPassword( + prompt, + cfg.RootCfg.InsecurePasswordStdin, + ) if err != nil { - return nil, err + return fmt.Errorf("unable to get decryption key, %w", err) } - // fill tx signatures. - signers := tx.GetSigners() - if tx.Signatures == nil { - for range signers { - tx.Signatures = append(tx.Signatures, std.Signature{ - PubKey: nil, // zero signature - Signature: nil, // zero signature - }) - } + sOpts := signOpts{ + chainID: cfg.ChainID, + accountSequence: cfg.Sequence, + accountNumber: cfg.AccountNumber, } - // validate document to sign. - err = tx.ValidateBasic() - if err != nil { - return nil, err + kOpts := keyOpts{ + keyName: args[0], + decryptPass: password, } - // derive sign doc bytes. - chainID := cfg.ChainID - accountNumber := cfg.AccountNumber - sequence := cfg.Sequence - signbz := tx.GetSignBytes(chainID, accountNumber, sequence) - if cfg.ShowSignBytes { - fmt.Printf("sign bytes: %X\n", signbz) - return nil, nil + // Sign the transaction + if err := signTx(&tx, kb, sOpts, kOpts); err != nil { + return fmt.Errorf("unable to sign transaction, %w", err) } - sig, pub, err := kb.Sign(cfg.NameOrBech32, cfg.Pass, signbz) + return saveTx(&tx, cfg.TxPath) +} + +func signTx( + tx *std.Tx, + kb keys.Keybase, + signOpts signOpts, + keyOpts keyOpts, +) error { + // Sign the transaction data + sig, pub, err := kb.Sign( + keyOpts.keyName, + keyOpts.decryptPass, + tx.GetSignBytes( + signOpts.chainID, + signOpts.accountNumber, + signOpts.accountSequence, + )) if err != nil { - return nil, err + return fmt.Errorf("unable to sign transaction bytes, %w", err) } - addr := pub.Address() - found := false - for i := range tx.Signatures { - // override signature for matching slot. - if signers[i] == addr { - found = true - tx.Signatures[i] = std.Signature{ - PubKey: pub, - Signature: sig, - } - } + + // Save the signature + if tx.Signatures == nil { + tx.Signatures = make([]std.Signature, 0, 1) } - if !found { - return nil, errors.New( - fmt.Sprintf("addr %v (%s) not in signer set", addr, cfg.NameOrBech32), - ) + + // Check if the signature needs to be overwritten + for index, signature := range tx.Signatures { + if !signature.PubKey.Equals(pub) { + continue + } + + // Save the signature + tx.Signatures[index] = std.Signature{ + PubKey: pub, + Signature: sig, + } + + return nil } - return &tx, nil + // Append the signature, since it wasn't + // present before + tx.Signatures = append( + tx.Signatures, std.Signature{ + PubKey: pub, + Signature: sig, + }, + ) + + return nil } diff --git a/tm2/pkg/crypto/keys/client/sign_test.go b/tm2/pkg/crypto/keys/client/sign_test.go index 0d73d247637..bb3cd3a119e 100644 --- a/tm2/pkg/crypto/keys/client/sign_test.go +++ b/tm2/pkg/crypto/keys/client/sign_test.go @@ -1,87 +1,10 @@ package client import ( - "fmt" - "strings" "testing" - - "github.com/gnolang/gno/tm2/pkg/amino" - "github.com/gnolang/gno/tm2/pkg/commands" - "github.com/gnolang/gno/tm2/pkg/crypto/keys" - sdkutils "github.com/gnolang/gno/tm2/pkg/sdk/testutils" - "github.com/gnolang/gno/tm2/pkg/std" - "github.com/gnolang/gno/tm2/pkg/testutils" - "github.com/stretchr/testify/assert" ) -func Test_execSign(t *testing.T) { +func TestSign_SignTx(t *testing.T) { t.Parallel() - - // make new test dir - kbHome, kbCleanUp := testutils.NewTestCaseDir(t) - assert.NotNil(t, kbHome) - defer kbCleanUp() - - // initialize test options - cfg := &SignCfg{ - RootCfg: &BaseCfg{ - BaseOptions: BaseOptions{ - Home: kbHome, - InsecurePasswordStdin: true, - }, - }, - TxPath: "-", // stdin - ChainID: "dev", - AccountNumber: 0, - Sequence: 0, - } - - fakeKeyName1 := "signApp_Key1" - fakeKeyName2 := "signApp_Key2" - encPassword := "12345678" - - io := commands.NewTestIO() - - // add test account to keybase. - kb, err := keys.NewKeyBaseFromDir(cfg.RootCfg.Home) - assert.NoError(t, err) - acc, err := kb.CreateAccount(fakeKeyName1, testMnemonic, "", encPassword, 0, 0) - addr := acc.GetAddress() - assert.NoError(t, err) - - // create a tx to sign. - msg := sdkutils.NewTestMsg(addr) - fee := std.NewFee(1, std.Coin{"ugnot", 1000000}) - tx := std.NewTx([]std.Msg{msg}, fee, nil, "") - txjson := string(amino.MustMarshalJSON(tx)) - - args := []string{fakeKeyName1} - io.SetIn(strings.NewReader(txjson)) - err = execSign(cfg, args, io) - assert.Error(t, err) - - args = []string{fakeKeyName1} - io.SetIn(strings.NewReader(txjson + "\n")) - err = execSign(cfg, args, io) - assert.Error(t, err) - - args = []string{fakeKeyName2} - io.SetIn(strings.NewReader( - fmt.Sprintf("%s\n%s\n", - txjson, - encPassword, - ), - )) - err = execSign(cfg, args, io) - assert.Error(t, err) - - args = []string{fakeKeyName1} - io.SetIn(strings.NewReader( - fmt.Sprintf("%s\n%s\n", - txjson, - encPassword, - ), - )) - err = execSign(cfg, args, io) - assert.NoError(t, err) + t.Skip("TODO implement") } diff --git a/tm2/pkg/std/tx.go b/tm2/pkg/std/tx.go index d6fa4fe8fe3..c10a8e85a0a 100644 --- a/tm2/pkg/std/tx.go +++ b/tm2/pkg/std/tx.go @@ -45,9 +45,6 @@ func (tx Tx) ValidateBasic() error { if len(stdSigs) == 0 { return ErrNoSignatures("no signers") } - if len(stdSigs) != len(tx.GetSigners()) { - return ErrUnauthorized("wrong number of signers") - } return nil } @@ -103,7 +100,7 @@ func (tx Tx) GetSignBytes(chainID string, accountNumber uint64, sequence uint64) return SignBytes(chainID, accountNumber, sequence, tx.Fee, tx.Msgs, tx.Memo) } -//__________________________________________________________ +// __________________________________________________________ // Fee includes the amount of coins paid in fees and the maximum // gas to be used by the transaction. The ratio yields an effective "gasprice", From 4260cea9417bf02c97379cf9bb864930f783026c Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Wed, 24 Apr 2024 11:44:48 +0200 Subject: [PATCH 2/3] Update signing logic, add unit tests --- gno.land/tx-signed.jsonl | 0 gno.land/tx.jsonl | 1 - tm2/pkg/crypto/keys/client/sign.go | 54 ++- tm2/pkg/crypto/keys/client/sign_test.go | 591 +++++++++++++++++++++++- tm2/pkg/crypto/keys/utils.go | 3 - 5 files changed, 622 insertions(+), 27 deletions(-) delete mode 100644 gno.land/tx-signed.jsonl delete mode 100644 gno.land/tx.jsonl diff --git a/gno.land/tx-signed.jsonl b/gno.land/tx-signed.jsonl deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/gno.land/tx.jsonl b/gno.land/tx.jsonl deleted file mode 100644 index d7f95f75e75..00000000000 --- a/gno.land/tx.jsonl +++ /dev/null @@ -1 +0,0 @@ -{"msg":[{"@type":"/bank.MsgSend","from_address":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","to_address":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","amount":"1ugnot"}],"fee":{"gas_wanted":"100000","gas_fee":"1000000ugnot"},"signatures":[{"pub_key":{"@type":"/tm.PubKeySecp256k1","value":"A+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2y"},"signature":"7xGZV+8qYNgiBifk0kwP2k72nndbZalzMJAQDf258aE5nDVkIFKHtNOaC/MH/i/2QYwec/yiPt6vFoXpcLAg8Q=="}],"memo":""} \ No newline at end of file diff --git a/tm2/pkg/crypto/keys/client/sign.go b/tm2/pkg/crypto/keys/client/sign.go index 8de11c1d3b3..160b7a3447f 100644 --- a/tm2/pkg/crypto/keys/client/sign.go +++ b/tm2/pkg/crypto/keys/client/sign.go @@ -33,10 +33,7 @@ type SignCfg struct { ChainID string AccountNumber uint64 Sequence uint64 - ShowSignBytes bool NameOrBech32 string - TxJSON []byte - Pass string } func NewSignCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { @@ -85,13 +82,6 @@ func (c *SignCfg) RegisterFlags(fs *flag.FlagSet) { 0, "account sequence to sign with", ) - - fs.BoolVar( - &c.ShowSignBytes, - "show", - false, - "display the sign bytes", - ) } func execSign(cfg *SignCfg, args []string, io commands.IO) error { @@ -124,6 +114,12 @@ func execSign(cfg *SignCfg, args []string, io commands.IO) error { return fmt.Errorf("unable to load keybase, %w", err) } + // Fetch the key info from the keybase + info, err := kb.GetByNameOrAddress(args[0]) + if err != nil { + return fmt.Errorf("unable to get key from keybase, %w", err) + } + // Get the transaction bytes txRaw, err := os.ReadFile(cfg.TxPath) if err != nil { @@ -138,23 +134,30 @@ func execSign(cfg *SignCfg, args []string, io commands.IO) error { // Make sure the tx is valid Amino JSON var tx std.Tx if err := amino.UnmarshalJSON(txRaw, &tx); err != nil { - return fmt.Errorf("unable to unmarshal tx, %w", err) + return fmt.Errorf("unable to unmarshal transaction, %w", err) } - // Get the keybase decryption password - prompt := "Enter password to decrypt key" - if cfg.RootCfg.Quiet { - prompt = "" // No prompt - } + var password string - password, err := io.GetPassword( - prompt, - cfg.RootCfg.InsecurePasswordStdin, - ) - if err != nil { - return fmt.Errorf("unable to get decryption key, %w", err) + // Check if we need to get a decryption password. + // This is only required for local keys + if info.GetType() != keys.TypeLedger { + // Get the keybase decryption password + prompt := "Enter password to decrypt key" + if cfg.RootCfg.Quiet { + prompt = "" // No prompt + } + + password, err = io.GetPassword( + prompt, + cfg.RootCfg.InsecurePasswordStdin, + ) + if err != nil { + return fmt.Errorf("unable to get decryption key, %w", err) + } } + // Prepare the signature ops sOpts := signOpts{ chainID: cfg.ChainID, accountSequence: cfg.Sequence, @@ -174,6 +177,8 @@ func execSign(cfg *SignCfg, args []string, io commands.IO) error { return saveTx(&tx, cfg.TxPath) } +// signTx generates the transaction signature, +// and saves it to the given transaction func signTx( tx *std.Tx, kb keys.Keybase, @@ -222,5 +227,10 @@ func signTx( }, ) + // Validate the tx after signing + if err := tx.ValidateBasic(); err != nil { + return fmt.Errorf("unable to validate transaction, %w", err) + } + return nil } diff --git a/tm2/pkg/crypto/keys/client/sign_test.go b/tm2/pkg/crypto/keys/client/sign_test.go index bb3cd3a119e..f85cf767a62 100644 --- a/tm2/pkg/crypto/keys/client/sign_test.go +++ b/tm2/pkg/crypto/keys/client/sign_test.go @@ -1,10 +1,599 @@ package client import ( + "context" + "flag" + "fmt" + "os" + "strings" "testing" + "time" + + "github.com/gnolang/gno/tm2/pkg/amino" + "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/keys/keyerror" + "github.com/gnolang/gno/tm2/pkg/std" + "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 TestSign_SignTx(t *testing.T) { t.Parallel() - t.Skip("TODO implement") + + t.Run("no key provided", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command + cmd := NewRootCmdWithBaseConfig(commands.NewTestIO(), baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + } + + assert.ErrorIs(t, cmd.ParseAndRun(ctx, args), flag.ErrHelp) + }) + + t.Run("non-existing key", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + ) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command + cmd := NewRootCmdWithBaseConfig(commands.NewTestIO(), baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "TotallyExistingKey", + } + + assert.True(t, keyerror.IsErrKeyNotFound(cmd.ParseAndRun(ctx, args))) + }) + + t.Run("non-existing tx file", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + _, err = kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command + cmd := NewRootCmdWithBaseConfig(commands.NewTestIO(), baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + "./TotallyExistingTxFile.json", + keyName, + } + + assert.ErrorContains(t, cmd.ParseAndRun(ctx, args), "unable to read transaction file") + }) + + t.Run("empty tx file", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + _, err = kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command + cmd := NewRootCmdWithBaseConfig(commands.NewTestIO(), baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + assert.ErrorIs(t, cmd.ParseAndRun(ctx, args), errInvalidTxFile) + }) + + t.Run("corrupted tx amino JSON", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + _, err = kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + // Write invalid JSON + _, err = txFile.WriteString("{this is absolutely valid JSON]") + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command + cmd := NewRootCmdWithBaseConfig(commands.NewTestIO(), baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + assert.ErrorContains( + t, + cmd.ParseAndRun(ctx, args), + "unable to unmarshal transaction", + ) + }) + + t.Run("invalid tx params", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + Quiet: true, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + + tx = std.Tx{ + Fee: std.Fee{ + GasFee: std.Coin{ // invalid gas fee + Amount: 0, + Denom: "ugnot", + }, + }, + } + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + _, err = kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + // Marshal the tx and write it to the file + encodedTx, err := amino.MarshalJSON(tx) + require.NoError(t, err) + + _, err = txFile.Write(encodedTx) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command IO + io := commands.NewTestIO() + io.SetIn( + strings.NewReader( + fmt.Sprintf( + "%s\n%s\n", + encryptPassword, + encryptPassword, + ), + ), + ) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + assert.ErrorContains( + t, + cmd.ParseAndRun(ctx, args), + "unable to validate transaction", + ) + }) + + t.Run("empty signature list", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + Quiet: true, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + + tx = std.Tx{ + Fee: std.Fee{ + GasWanted: 10, + GasFee: std.Coin{ + Amount: 10, + Denom: "ugnot", + }, + }, + Signatures: nil, // no signatures + } + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + info, err := kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + // Marshal the tx and write it to the file + encodedTx, err := amino.MarshalJSON(tx) + require.NoError(t, err) + + _, err = txFile.Write(encodedTx) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command IO + io := commands.NewTestIO() + io.SetIn( + strings.NewReader( + fmt.Sprintf( + "%s\n%s\n", + encryptPassword, + encryptPassword, + ), + ), + ) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + // Run the command + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Make sure the tx file was updated with the signature + savedTxRaw, err := os.ReadFile(txFile.Name()) + require.NoError(t, err) + + var savedTx std.Tx + require.NoError(t, amino.UnmarshalJSON(savedTxRaw, &savedTx)) + + require.Len(t, savedTx.Signatures, 1) + assert.True(t, savedTx.Signatures[0].PubKey.Equals(info.GetPubKey())) + }) + + t.Run("existing signature list", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + Quiet: true, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + + anotherKey = "another-key" + + tx = std.Tx{ + Fee: std.Fee{ + GasWanted: 10, + GasFee: std.Coin{ + Amount: 10, + Denom: "ugnot", + }, + }, + } + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + // Create an initial account + info, err := kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Create a new account + anotherKeyInfo, err := kb.CreateAccount(anotherKey, mnemonic, "", encryptPassword, 0, 1) + require.NoError(t, err) + + // Generate the signature + signature, pubKey, err := kb.Sign(anotherKey, encryptPassword, tx.GetSignBytes("id", 1, 0)) + require.NoError(t, err) + + tx.Signatures = []std.Signature{ + { + PubKey: pubKey, + Signature: signature, + }, + } + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + // Marshal the tx and write it to the file + encodedTx, err := amino.MarshalJSON(tx) + require.NoError(t, err) + + _, err = txFile.Write(encodedTx) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command IO + io := commands.NewTestIO() + io.SetIn( + strings.NewReader( + fmt.Sprintf( + "%s\n%s\n", + encryptPassword, + encryptPassword, + ), + ), + ) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + // Run the command + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Make sure the tx file was updated with the signature + savedTxRaw, err := os.ReadFile(txFile.Name()) + require.NoError(t, err) + + var savedTx std.Tx + require.NoError(t, amino.UnmarshalJSON(savedTxRaw, &savedTx)) + + require.Len(t, savedTx.Signatures, 2) + assert.True(t, savedTx.Signatures[0].PubKey.Equals(anotherKeyInfo.GetPubKey())) + assert.True(t, savedTx.Signatures[1].PubKey.Equals(info.GetPubKey())) + assert.NotEqual(t, savedTx.Signatures[0].Signature, savedTx.Signatures[1].Signature) + }) + + t.Run("overwrite existing signature", func(t *testing.T) { + t.Parallel() + + var ( + kbHome = t.TempDir() + baseOptions = BaseOptions{ + InsecurePasswordStdin: true, + Home: kbHome, + Quiet: true, + } + + mnemonic = generateTestMnemonic(t) + keyName = "generated-key" + encryptPassword = "encrypt" + + tx = std.Tx{ + Fee: std.Fee{ + GasWanted: 10, + GasFee: std.Coin{ + Amount: 10, + Denom: "ugnot", + }, + }, + } + ) + + // Generate a key in the keybase + kb, err := keys.NewKeyBaseFromDir(kbHome) + require.NoError(t, err) + + info, err := kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) + require.NoError(t, err) + + // Generate the signature + signature, pubKey, err := kb.Sign(keyName, encryptPassword, tx.GetSignBytes("id", 0, 0)) + require.NoError(t, err) + + tx.Signatures = []std.Signature{ + { + PubKey: pubKey, + Signature: signature, + }, + } + + // Create an empty tx file + txFile, err := os.CreateTemp("", "") + require.NoError(t, err) + + // Marshal the tx and write it to the file + encodedTx, err := amino.MarshalJSON(tx) + require.NoError(t, err) + + _, err = txFile.Write(encodedTx) + require.NoError(t, err) + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + // Create the command IO + io := commands.NewTestIO() + io.SetIn( + strings.NewReader( + fmt.Sprintf( + "%s\n%s\n", + encryptPassword, + encryptPassword, + ), + ), + ) + + // Create the command + cmd := NewRootCmdWithBaseConfig(io, baseOptions) + + args := []string{ + "sign", + "--insecure-password-stdin", + "--home", + kbHome, + "--tx-path", + txFile.Name(), + keyName, + } + + // Run the command + require.NoError(t, cmd.ParseAndRun(ctx, args)) + + // Make sure the tx file was updated with the signature + savedTxRaw, err := os.ReadFile(txFile.Name()) + require.NoError(t, err) + + var savedTx std.Tx + require.NoError(t, amino.UnmarshalJSON(savedTxRaw, &savedTx)) + + require.Len(t, savedTx.Signatures, 1) + assert.True(t, savedTx.Signatures[0].PubKey.Equals(info.GetPubKey())) + }) } diff --git a/tm2/pkg/crypto/keys/utils.go b/tm2/pkg/crypto/keys/utils.go index 623d3094005..90e4e6c446d 100644 --- a/tm2/pkg/crypto/keys/utils.go +++ b/tm2/pkg/crypto/keys/utils.go @@ -12,9 +12,6 @@ func NewKeyBaseFromDir(rootDir string) (Keybase, error) { return NewLazyDBKeybase(defaultKeyDBName, filepath.Join(rootDir, "data")), nil } -// NewInMemoryKeyBase returns a storage-less keybase. -func NewInMemoryKeyBase() Keybase { return NewInMemory() } - func ValidateMultisigThreshold(k, nKeys int) error { if k <= 0 { return fmt.Errorf("threshold must be a positive integer") From 529a8329516e35bc129ce03355008b884dce3401 Mon Sep 17 00:00:00 2001 From: Milos Zivkovic Date: Wed, 24 Apr 2024 12:06:51 +0200 Subject: [PATCH 3/3] Restore check --- tm2/pkg/crypto/keys/client/sign_test.go | 28 +++++++++++++++++++++++++ tm2/pkg/std/tx.go | 3 +++ 2 files changed, 31 insertions(+) diff --git a/tm2/pkg/crypto/keys/client/sign_test.go b/tm2/pkg/crypto/keys/client/sign_test.go index f85cf767a62..d1a2dafaa61 100644 --- a/tm2/pkg/crypto/keys/client/sign_test.go +++ b/tm2/pkg/crypto/keys/client/sign_test.go @@ -14,6 +14,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/crypto/bip39" "github.com/gnolang/gno/tm2/pkg/crypto/keys" "github.com/gnolang/gno/tm2/pkg/crypto/keys/keyerror" + "github.com/gnolang/gno/tm2/pkg/sdk/bank" "github.com/gnolang/gno/tm2/pkg/std" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -341,6 +342,14 @@ func TestSign_SignTx(t *testing.T) { info, err := kb.CreateAccount(keyName, mnemonic, "", encryptPassword, 0, 0) require.NoError(t, err) + // We need to prepare the message signer as well + // for validation to complete + tx.Msgs = []std.Msg{ + bank.MsgSend{ + FromAddress: info.GetAddress(), + }, + } + // Create an empty tx file txFile, err := os.CreateTemp("", "") require.NoError(t, err) @@ -445,6 +454,17 @@ func TestSign_SignTx(t *testing.T) { }, } + // We need to prepare the message signers as well + // for validation to complete + tx.Msgs = []std.Msg{ + bank.MsgSend{ + FromAddress: info.GetAddress(), + }, + bank.MsgSend{ + FromAddress: anotherKeyInfo.GetAddress(), + }, + } + // Create an empty tx file txFile, err := os.CreateTemp("", "") require.NoError(t, err) @@ -544,6 +564,14 @@ func TestSign_SignTx(t *testing.T) { }, } + // We need to prepare the message signer as well + // for validation to complete + tx.Msgs = []std.Msg{ + bank.MsgSend{ + FromAddress: info.GetAddress(), + }, + } + // Create an empty tx file txFile, err := os.CreateTemp("", "") require.NoError(t, err) diff --git a/tm2/pkg/std/tx.go b/tm2/pkg/std/tx.go index c10a8e85a0a..ae548f11d08 100644 --- a/tm2/pkg/std/tx.go +++ b/tm2/pkg/std/tx.go @@ -45,6 +45,9 @@ func (tx Tx) ValidateBasic() error { if len(stdSigs) == 0 { return ErrNoSignatures("no signers") } + if len(stdSigs) != len(tx.GetSigners()) { + return ErrUnauthorized("wrong number of signers") + } return nil }