LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Add ledger/multisig detection in SignTx functions (#9026)

Browse Source 
* Add ledger/multisig detection in SignTx functions

* Fix tests

* Update CHANGELOG.md

* update cl

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
This commit is contained in:
Amaury 2021-03-31 11:25:23 +02:00 committed by GitHub
parent feed37dc56
commit 413938c5ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* updated the keyring display structure (it uses protobuf JSON serialization) - the output is more verbose.
* Renamed `MarshalAny` and `UnmarshalAny` to `MarshalInterface` and `UnmarshalInterface` respectively. These functions must take an interface as parameter (not a concrete type nor `Any` object). Underneath they use `Any` wrapping for correct protobuf serialization.
* CLI: removed `--text` flag from `show-node-id` command; the text format for public keys is not used any more - instead we use ProtoJSON.
* [\#9026](https://github.com/cosmos/cosmos-sdk/pull/9026) The `tx sign` and `tx sign-batch` CLI commands use SIGN_MODE_DIRECT by default for local pubkeys. For multisigs and ledger keys, the default LEGACY_AMINO_JSON is kept.
### API Breaking Changes
@ -106,6 +107,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/bank) [\#8434](https://github.com/cosmos/cosmos-sdk/pull/8434) Fix legacy REST API `GET /bank/total` and `GET /bank/total/{denom}` in swagger
* (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command
* (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value
* [\#9026](https://github.com/cosmos/cosmos-sdk/pull/9026) Fix bug of `gentx` command not working with ledger keys.
## [v0.42.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.42.2) - 2021-03-19

22
x/auth/client/cli/cli_test.go
View File

@ -132,7 +132,7 @@ func (s *IntegrationTestSuite) TestCLISignBatch() {
s.Require().Error(err)
}
func (s *IntegrationTestSuite) TestCLISign() {
func (s *IntegrationTestSuite) TestCLISign_AminoJSON() {
require := s.Require()
val1 := s.network.Validators[0]
txCfg := val1.ClientCtx.TxConfig
@ -140,6 +140,7 @@ func (s *IntegrationTestSuite) TestCLISign() {
fileUnsigned := testutil.WriteToNewTempFile(s.T(), txBz.String())
chainFlag := fmt.Sprintf("--%s=%s", flags.FlagChainID, val1.ClientCtx.ChainID)
sigOnlyFlag := "--signature-only"
signModeAminoFlag := "--sign-mode=amino-json"
// SIC! validators have same key names and same adddresses as those registered in the keyring,
// BUT the keys are different!
@ -154,7 +155,7 @@ func (s *IntegrationTestSuite) TestCLISign() {
/**** test signature-only ****/
res, err := authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag,
sigOnlyFlag)
sigOnlyFlag, signModeAminoFlag)
require.NoError(err)
checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey())
sigs, err := txCfg.UnmarshalSignatureJSON(res.Bytes())
@ -163,7 +164,7 @@ func (s *IntegrationTestSuite) TestCLISign() {
require.Equal(account.GetSequence(), sigs[0].Sequence)
/**** test full output ****/
res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag)
res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag, signModeAminoFlag)
require.NoError(err)
// txCfg.UnmarshalSignatureJSON can't unmarshal a fragment of the signature, so we create this structure.
@ -178,7 +179,7 @@ func (s *IntegrationTestSuite) TestCLISign() {
/**** test file output ****/
filenameSigned := filepath.Join(s.T().TempDir(), "test_sign_out.json")
fileFlag := fmt.Sprintf("--%s=%s", flags.FlagOutputDocument, filenameSigned)
_, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag, fileFlag)
_, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag, fileFlag, signModeAminoFlag)
require.NoError(err)
fContent, err := ioutil.ReadFile(filenameSigned)
require.NoError(err)
@ -186,7 +187,7 @@ func (s *IntegrationTestSuite) TestCLISign() {
/**** try to append to the previously signed transaction ****/
res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, filenameSigned, chainFlag,
sigOnlyFlag)
sigOnlyFlag, signModeAminoFlag)
require.NoError(err)
checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey(), valInfo.GetPubKey())
@ -197,12 +198,12 @@ func (s *IntegrationTestSuite) TestCLISign() {
// provide functionality to check / manage `auth_info`.
// Cases with different keys are are covered in unit tests of `tx.Sign`.
res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, filenameSigned, chainFlag,
sigOnlyFlag, "--overwrite")
sigOnlyFlag, "--overwrite", signModeAminoFlag)
checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey())
/**** test flagAmino ****/
res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, filenameSigned, chainFlag,
"--amino=true")
"--amino=true", signModeAminoFlag)
require.NoError(err)
var txAmino authrest.BroadcastReq
@ -1096,7 +1097,8 @@ func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() {
_, _, addr1 := testdata.KeyTestPubAddr()
// Creating a tx with 2 msgs from 2 signers: val0 and val1.
// The validators sign with SIGN_MODE_LEGACY_AMINO_JSON by default.
// The validators need to sign with SIGN_MODE_LEGACY_AMINO_JSON,
// because DIRECT doesn't support multi signers via the CLI.
// Since we we amino, we don't need to pre-populate signer_infos.
txBuilder := val0.ClientCtx.TxConfig.NewTxBuilder()
txBuilder.SetMsgs(
@ -1113,7 +1115,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() {
unsignedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))
// Let val0 sign first the file with the unsignedTx.
signedByVal0, err := authtest.TxSignExec(val0.ClientCtx, val0.Address, unsignedTxFile.Name(), "--overwrite")
signedByVal0, err := authtest.TxSignExec(val0.ClientCtx, val0.Address, unsignedTxFile.Name(), "--overwrite", "--sign-mode=amino-json")
require.NoError(err)
signedByVal0File := testutil.WriteToNewTempFile(s.T(), signedByVal0.String())
@ -1122,7 +1124,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() {
require.NoError(err)
signedTx, err := authtest.TxSignExec(
val1.ClientCtx, val1.Address, signedByVal0File.Name(),
"--offline", fmt.Sprintf("--account-number=%d", val1AccNum), fmt.Sprintf("--sequence=%d", val1Seq),
"--offline", fmt.Sprintf("--account-number=%d", val1AccNum), fmt.Sprintf("--sequence=%d", val1Seq), "--sign-mode=amino-json",
)
require.NoError(err)
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())

11
x/auth/client/cli/tx_sign.go
View File

@ -10,7 +10,6 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
"github.com/cosmos/cosmos-sdk/x/auth/client/rest"
)
@ -114,9 +113,6 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error {
return err
}
} else {
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}
err = authclient.SignTxWithSignerAddress(
txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, true)
}
@ -212,15 +208,12 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
return err
}
f := cmd.Flags()
txFactory := tx.NewFactoryCLI(clientCtx, f)
clientCtx, txF, newTx, err := readTxAndInitContexts(clientCtx, cmd, args[0])
if err != nil {
return err
}
if txF.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txF = txF.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}
txCfg := clientCtx.TxConfig
txBuilder, err := txCfg.WrapTxBuilder(newTx)
if err != nil {
@ -230,7 +223,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly)
multisigAddrStr, _ := cmd.Flags().GetString(flagMultisig)
from, _ := cmd.Flags().GetString(flags.FlagFrom)
_, fromName, _, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly)
_, fromName, _, err := client.GetFromFields(txF.Keybase(), from, clientCtx.GenerateOnly)
if err != nil {
return fmt.Errorf("error getting account from keybase: %w", err)
}

21
x/auth/client/tx.go
View File

@ -14,8 +14,10 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
)
@ -37,13 +39,20 @@ func PrintUnsignedStdTx(txBldr tx.Factory, clientCtx client.Context, msgs []sdk.
// SignTx signs a transaction managed by the TxBuilder using a `name` key stored in Keybase.
// The new signature is appended to the TxBuilder when overwrite=false or overwritten otherwise.
// Don't perform online validation or lookups if offline is true.
func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx client.TxBuilder, offline, overwriteSig bool) error {
func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, txBuilder client.TxBuilder, offline, overwriteSig bool) error {
info, err := txFactory.Keybase().Key(name)
if err != nil {
return err
}
// Ledger and Multisigs only support LEGACY_AMINO_JSON signing.
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED &&
(info.GetType() == keyring.TypeLedger || info.GetType() == keyring.TypeMulti) {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}
addr := sdk.AccAddress(info.GetPubKey().Address())
if !isTxSigner(addr, stdTx.GetTx().GetSigners()) {
if !isTxSigner(addr, txBuilder.GetTx().GetSigners()) {
return fmt.Errorf("%s: %s", sdkerrors.ErrorInvalidSigner, name)
}
if !offline {
@ -53,14 +62,20 @@ func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx c
}
}
return tx.Sign(txFactory, name, stdTx, overwriteSig)
return tx.Sign(txFactory, name, txBuilder, overwriteSig)
}
// SignTxWithSignerAddress attaches a signature to a transaction.
// Don't perform online validation or lookups if offline is true, else
// populate account and sequence numbers from a foreign account.
// This function should only be used when signing with a multisig. For
// normal keys, please use SignTx directly.
func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, addr sdk.AccAddress,
name string, txBuilder client.TxBuilder, offline, overwrite bool) (err error) {
// Multisigs only support LEGACY_AMINO_JSON signing.
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}
// check whether the address is a signer
if !isTxSigner(addr, txBuilder.GetTx().GetSigners()) {