diff --git a/CHANGELOG.md b/CHANGELOG.md index 963a60161..847242542 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,15 +55,24 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/upgrade) [\#7979](https://github.com/cosmos/cosmos-sdk/pull/7979) keeper pubkey storage serialization migration from bech32 to protobuf. + +### Features +* (codec/types) [\#8106](https://github.com/cosmos/cosmos-sdk/pull/8106) Adding `NewAnyWithCustomTypeURL` to correctly marshal Messages in TxBuilder. + ### Bug Fixes * (crypto) [\#7966](https://github.com/cosmos/cosmos-sdk/issues/7966) `Bip44Params` `String()` function now correctly returns the absolute HD path by adding the `m/` prefix. +* (x/auth/client/cli) [\#8106](https://github.com/cosmos/cosmos-sdk/pull/8106) fixing regression bugs in transaction signing. + ### API Breaking * [\#8080](https://github.com/cosmos/cosmos-sdk/pull/8080) Updated the `codec.Marshaler` interface * Moved `MarshalAny` and `UnmarshalAny` helper functions to `codec.Marshaler` and renamed to `MarshalInterface` and `UnmarshalInterface` respectively. These functions must take interface as a parameter (not a concrete type nor `Any` object). Underneath they use `Any` wrapping for correct protobuf serialization. * (client) [\#8107](https://github.com/cosmos/cosmos-sdk/pull/8107) Renamed `PrintOutput` and `PrintOutputLegacy` methods of the `context.Client` object to `PrintProto` and `PrintObjectLegacy`. +* (x/auth/tx) [\#8106](https://github.com/cosmos/cosmos-sdk/pull/8106) change related to missing append functionality in client transaction signing + + added `overwriteSig` argument to `x/auth/client.SignTx` and `client/tx.Sign` functions. + + removed `x/auth/tx.go:wrapper.GetSignatures`. The `wrapper` provides `TxBuilder` functionality, and it's a private structure. That function was not used at all and it's not exposed through the `TxBuilder` interface. ## [v0.40.0-rc3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.0-rc3) - 2020-11-06 diff --git a/client/context.go b/client/context.go index c63d557bd..0e3d1181a 100644 --- a/client/context.go +++ b/client/context.go @@ -205,14 +205,20 @@ func (ctx Context) WithInterfaceRegistry(interfaceRegistry codectypes.InterfaceR return ctx } -// PrintString prints the raw string to ctx.Output or os.Stdout +// PrintString prints the raw string to ctx.Output if it's defined, otherwise to os.Stdout func (ctx Context) PrintString(str string) error { + return ctx.PrintBytes([]byte(str)) +} + +// PrintBytes prints the raw bytes to ctx.Output if it's defined, otherwise to os.Stdout. +// NOTE: for printing a complex state object, you should use ctx.PrintOutput +func (ctx Context) PrintBytes(o []byte) error { writer := ctx.Output if writer == nil { writer = os.Stdout } - _, err := writer.Write([]byte(str)) + _, err := writer.Write(o) return err } diff --git a/client/tx/tx.go b/client/tx/tx.go index 980c253a8..ff941df21 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -117,7 +117,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { } } - err = Sign(txf, clientCtx.GetFromName(), tx) + err = Sign(txf, clientCtx.GetFromName(), tx, true) if err != nil { return err } @@ -375,10 +375,21 @@ func SignWithPrivKey( return sigV2, nil } -// Sign signs a given tx with the provided name and passphrase. The bytes signed -// over are canconical. The resulting signature will be set on the transaction. +func checkMultipleSigners(mode signing.SignMode, tx authsigning.Tx) error { + if mode == signing.SignMode_SIGN_MODE_DIRECT && + len(tx.GetSigners()) > 1 { + return sdkerrors.Wrap(sdkerrors.ErrNotSupported, "Signing in DIRECT mode is only supported for transactions with one signer only") + } + return nil +} + +// Sign signs a given tx with a named key. The bytes signed over are canconical. +// The resulting signature will be added to the transaction builder overwriting the previous +// ones if overwrite=true (otherwise, the signature will be appended). +// Signing a transaction with mutltiple signers in the DIRECT mode is not supprted and will +// return an error. // An error is returned upon failure. -func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { +func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig bool) error { if txf.keybase == nil { return errors.New("keybase must be set prior to signing a transaction") } @@ -388,12 +399,14 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { // use the SignModeHandler's default mode if unspecified signMode = txf.txConfig.SignModeHandler().DefaultMode() } + if err := checkMultipleSigners(signMode, txBuilder.GetTx()); err != nil { + return err + } key, err := txf.keybase.Key(name) if err != nil { return err } - pubKey := key.GetPubKey() signerData := authsigning.SignerData{ ChainID: txf.chainID, @@ -418,18 +431,25 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { Data: &sigData, Sequence: txf.Sequence(), } + var prevSignatures []signing.SignatureV2 + if !overwriteSig { + prevSignatures, err = txBuilder.GetTx().GetSignaturesV2() + if err != nil { + return err + } + } if err := txBuilder.SetSignatures(sig); err != nil { return err } // Generate the bytes to be signed. - signBytes, err := txf.txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) + bytesToSign, err := txf.txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) if err != nil { return err } // Sign those bytes - sigBytes, _, err := txf.keybase.Sign(name, signBytes) + sigBytes, _, err := txf.keybase.Sign(name, bytesToSign) if err != nil { return err } @@ -445,8 +465,11 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { Sequence: txf.Sequence(), } - // And here the tx is populated with the signature - return txBuilder.SetSignatures(sig) + if overwriteSig { + return txBuilder.SetSignatures(sig) + } + prevSignatures = append(prevSignatures, sig) + return txBuilder.SetSignatures(prevSignatures...) } // GasEstimateResponse defines a response definition for tx gas estimation. diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 3325e738d..3c7a4bebe 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -10,9 +10,11 @@ import ( "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" txtypes "github.com/cosmos/cosmos-sdk/types/tx" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/auth/signing" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -121,49 +123,110 @@ func TestBuildUnsignedTx(t *testing.T) { } func TestSign(t *testing.T) { + requireT := require.New(t) path := hd.CreateHDPath(118, 0, 0).String() kr, err := keyring.New(t.Name(), "test", t.TempDir(), nil) - require.NoError(t, err) + requireT.NoError(err) - var from = "test_sign" + var from1 = "test_key1" + var from2 = "test_key2" - _, seed, err := kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) - require.NoError(t, err) - require.NoError(t, kr.Delete(from)) + // create a new key using a mnemonic generator and test if we can reuse seed to recreate that account + _, seed, err := kr.NewMnemonic(from1, keyring.English, path, hd.Secp256k1) + requireT.NoError(err) + requireT.NoError(kr.Delete(from1)) + info1, _, err := kr.NewMnemonic(from1, keyring.English, path, hd.Secp256k1) + requireT.NoError(err) - info, err := kr.NewAccount(from, seed, "", path, hd.Secp256k1) - require.NoError(t, err) + info2, err := kr.NewAccount(from2, seed, "", path, hd.Secp256k1) + requireT.NoError(err) - txf := tx.Factory{}. + pubKey1 := info1.GetPubKey() + pubKey2 := info2.GetPubKey() + requireT.NotEqual(pubKey1.Bytes(), pubKey2.Bytes()) + t.Log("Pub keys:", pubKey1, pubKey2) + + txfNoKeybase := tx.Factory{}. WithTxConfig(NewTestTxConfig()). WithAccountNumber(50). WithSequence(23). WithFees("50stake"). WithMemo("memo"). WithChainID("test-chain") - - msg := banktypes.NewMsgSend(info.GetAddress(), sdk.AccAddress("to"), nil) - txn, err := tx.BuildUnsignedTx(txf, msg) - require.NoError(t, err) - - t.Log("should failed if txf without keyring") - err = tx.Sign(txf, from, txn) - require.Error(t, err) - - txf = tx.Factory{}. + txfDirect := txfNoKeybase. WithKeybase(kr). - WithTxConfig(NewTestTxConfig()). - WithAccountNumber(50). - WithSequence(23). - WithFees("50stake"). - WithMemo("memo"). - WithChainID("test-chain") + WithSignMode(signingtypes.SignMode_SIGN_MODE_DIRECT) + txfAmino := txfDirect. + WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) + msg1 := banktypes.NewMsgSend(info1.GetAddress(), sdk.AccAddress("to"), nil) + msg2 := banktypes.NewMsgSend(info2.GetAddress(), sdk.AccAddress("to"), nil) + txb, err := tx.BuildUnsignedTx(txfNoKeybase, msg1, msg2) + requireT.NoError(err) + txb2, err := tx.BuildUnsignedTx(txfNoKeybase, msg1, msg2) + requireT.NoError(err) + txbSimple, err := tx.BuildUnsignedTx(txfNoKeybase, msg2) + requireT.NoError(err) - t.Log("should succeed if txf with keyring") - err = tx.Sign(txf, from, txn) - require.NoError(t, err) + testCases := []struct { + name string + txf tx.Factory + txb client.TxBuilder + from string + overwrite bool + expectedPKs []cryptotypes.PubKey + matchingSigs []int // if not nil, check matching signature against old ones. + }{ + {"should fail if txf without keyring", + txfNoKeybase, txb, from1, true, nil, nil}, + {"should fail for non existing key", + txfAmino, txb, "unknown", true, nil, nil}, + {"amino: should succeed with keyring", + txfAmino, txbSimple, from1, true, []cryptotypes.PubKey{pubKey1}, nil}, + {"direct: should succeed with keyring", + txfDirect, txbSimple, from1, true, []cryptotypes.PubKey{pubKey1}, nil}, - t.Log("should fail for non existing key") - err = tx.Sign(txf, "non_existing_key", txn) - require.Error(t, err) + /**** test double sign Amino mode ****/ + {"amino: should sign multi-signers tx", + txfAmino, txb, from1, true, []cryptotypes.PubKey{pubKey1}, nil}, + {"amino: should append a second signature and not overwrite", + txfAmino, txb, from2, false, []cryptotypes.PubKey{pubKey1, pubKey2}, []int{0, 0}}, + {"amino: should overwrite a signature", + txfAmino, txb, from2, true, []cryptotypes.PubKey{pubKey2}, []int{1, 0}}, + + /**** test double sign Direct mode + signing transaction with more than 2 signers should fail in DIRECT mode ****/ + {"direct: should fail to append a signature with different mode", + txfDirect, txb, from1, false, []cryptotypes.PubKey{}, nil}, + {"direct: should fail to sign multi-signers tx", + txfDirect, txb2, from1, false, []cryptotypes.PubKey{}, nil}, + {"direct: should fail to overwrite multi-signers tx", + txfDirect, txb2, from1, true, []cryptotypes.PubKey{}, nil}, + } + var prevSigs []signingtypes.SignatureV2 + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err = tx.Sign(tc.txf, tc.from, tc.txb, tc.overwrite) + if len(tc.expectedPKs) == 0 { + requireT.Error(err) + } else { + requireT.NoError(err) + sigs := testSigners(requireT, tc.txb.GetTx(), tc.expectedPKs...) + if tc.matchingSigs != nil { + requireT.Equal(prevSigs[tc.matchingSigs[0]], sigs[tc.matchingSigs[1]]) + } + prevSigs = sigs + } + }) + } +} + +func testSigners(require *require.Assertions, tr signing.Tx, pks ...cryptotypes.PubKey) []signingtypes.SignatureV2 { + sigs, err := tr.GetSignaturesV2() + require.Len(sigs, len(pks)) + require.NoError(err) + require.Len(sigs, len(pks)) + for i := range pks { + require.True(sigs[i].PubKey.Equals(pks[i]), "Signature is signed with a wrong pubkey. Got: %s, expected: %s", sigs[i].PubKey, pks[i]) + } + return sigs } diff --git a/codec/types/any.go b/codec/types/any.go index 4a30ddad4..d978199fe 100644 --- a/codec/types/any.go +++ b/codec/types/any.go @@ -69,6 +69,19 @@ func NewAnyWithValue(value proto.Message) (*Any, error) { return any, nil } +// NewAnyWithCustomTypeURL same as NewAnyWithValue, but sets a custom type url, instead +// using the one from proto.Message. +// NOTE: This functions should be only used for types with additional logic bundled +// into the protobuf Any serialization. For simple marshaling you should use NewAnyWithValue. +func NewAnyWithCustomTypeURL(v proto.Message, typeURL string) (*Any, error) { + bz, err := proto.Marshal(v) + return &Any{ + TypeUrl: typeURL, + Value: bz, + cachedValue: v, + }, err +} + // Pack packs the value x in the Any or returns an error. This also caches // the packed value so that it can be retrieved from GetCachedValue without // unmarshaling diff --git a/server/grpc/server_test.go b/server/grpc/server_test.go index 5648233f8..846ce31a5 100644 --- a/server/grpc/server_test.go +++ b/server/grpc/server_test.go @@ -154,7 +154,7 @@ func (s *IntegrationTestSuite) TestGRPCServer_BroadcastTx() { WithSignMode(signing.SignMode_SIGN_MODE_DIRECT) // Sign Tx. - err := authclient.SignTx(txFactory, val0.ClientCtx, val0.Moniker, txBuilder, false) + err := authclient.SignTx(txFactory, val0.ClientCtx, val0.Moniker, txBuilder, false, true) s.Require().NoError(err) txBytes, err := val0.ClientCtx.TxConfig.TxEncoder()(txBuilder.GetTx()) diff --git a/simapp/simd/cmd/testnet.go b/simapp/simd/cmd/testnet.go index 3355c04b4..5144373f9 100644 --- a/simapp/simd/cmd/testnet.go +++ b/simapp/simd/cmd/testnet.go @@ -230,7 +230,7 @@ func InitTestnet( WithKeybase(kb). WithTxConfig(clientCtx.TxConfig) - if err := tx.Sign(txFactory, nodeDirName, txBuilder); err != nil { + if err := tx.Sign(txFactory, nodeDirName, txBuilder, true); err != nil { return err } diff --git a/testutil/network/network.go b/testutil/network/network.go index a50c6fa93..23706c12c 100644 --- a/testutil/network/network.go +++ b/testutil/network/network.go @@ -323,7 +323,7 @@ func New(t *testing.T, cfg Config) *Network { WithKeybase(kb). WithTxConfig(cfg.TxConfig) - err = tx.Sign(txFactory, nodeDirName, txBuilder) + err = tx.Sign(txFactory, nodeDirName, txBuilder, true) require.NoError(t, err) txBz, err := cfg.TxConfig.TxJSONEncoder()(txBuilder.GetTx()) diff --git a/types/errors/errors.go b/types/errors/errors.go index 0c9492082..d08412524 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -131,6 +131,10 @@ var ( // the same resource and one of them fails. ErrConflict = Register(RootCodespace, 36, "conflict") + // ErrNotSupported is returned when we call a branch of a code which is currently not + // supported. + ErrNotSupported = Register(RootCodespace, 37, "feature not supported") + // ErrPanic is only set when we recover from a panic, so we know to // redact potentially sensitive system info ErrPanic = Register(UndefinedCodespace, 111222, "panic") diff --git a/types/tx/types.go b/types/tx/types.go index c572d448a..0392b2fcf 100644 --- a/types/tx/types.go +++ b/types/tx/types.go @@ -28,6 +28,10 @@ func (t *Tx) GetMsgs() []sdk.Msg { for i, any := range anys { var msg sdk.Msg if isServiceMsg(any.TypeUrl) { + req := any.GetCachedValue() + if req == nil { + panic("Any cached value is nil. Transaction messages must be correctly packed Any values.") + } msg = sdk.ServiceMsg{ MethodName: any.TypeUrl, Request: any.GetCachedValue().(sdk.MsgRequest), diff --git a/x/auth/client/cli/broadcast.go b/x/auth/client/cli/broadcast.go index d234b0252..0a728bb9b 100644 --- a/x/auth/client/cli/broadcast.go +++ b/x/auth/client/cli/broadcast.go @@ -26,6 +26,10 @@ $ tx broadcast ./mytxn.json Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { clientCtx := client.GetClientContextFromCmd(cmd) + clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags()) + if err != nil { + return err + } if offline, _ := cmd.Flags().GetBool(flags.FlagOffline); offline { return errors.New("cannot broadcast tx during offline mode") diff --git a/x/auth/client/cli/cli_test.go b/x/auth/client/cli/cli_test.go index b935d6641..6a6a9b1d5 100644 --- a/x/auth/client/cli/cli_test.go +++ b/x/auth/client/cli/cli_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "encoding/json" "fmt" "io/ioutil" "path/filepath" @@ -14,7 +15,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" - codec2 "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" @@ -29,6 +29,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" authcli "github.com/cosmos/cosmos-sdk/x/auth/client/cli" + authrest "github.com/cosmos/cosmos-sdk/x/auth/client/rest" authtest "github.com/cosmos/cosmos-sdk/x/auth/client/testutil" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" @@ -46,7 +47,7 @@ func (s *IntegrationTestSuite) SetupSuite() { s.T().Log("setting up integration test suite") cfg := network.DefaultConfig() - cfg.NumValidators = 1 + cfg.NumValidators = 2 s.cfg = cfg s.network = network.New(s.T(), cfg) @@ -76,24 +77,11 @@ func (s *IntegrationTestSuite) TearDownSuite() { func (s *IntegrationTestSuite) TestCLIValidateSignatures() { val := s.network.Validators[0] - res, err := bankcli.MsgSendExec( - val.ClientCtx, - val.Address, - val.Address, - sdk.NewCoins( - sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)), - sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)), - ), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), - ) - s.Require().NoError(err) + res := s.createBankMsg(val, val.Address) // write unsigned tx to file unsignedTx := testutil.WriteToNewTempFile(s.T(), res.String()) - res, err = authtest.TxSignExec(val.ClientCtx, val.Address, unsignedTx.Name()) + res, err := authtest.TxSignExec(val.ClientCtx, val.Address, unsignedTx.Name()) s.Require().NoError(err) signedTx, err := val.ClientCtx.TxConfig.TxJSONDecoder()(res.Bytes()) s.Require().NoError(err) @@ -115,26 +103,11 @@ func (s *IntegrationTestSuite) TestCLIValidateSignatures() { func (s *IntegrationTestSuite) TestCLISignBatch() { val := s.network.Validators[0] - generatedStd, err := bankcli.MsgSendExec( - val.ClientCtx, - val.Address, - val.Address, - sdk.NewCoins( - sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)), - sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)), - ), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), - ) - - s.Require().NoError(err) - - // Write the output to disk + generatedStd := s.createBankMsg(val, val.Address) outputFile := testutil.WriteToNewTempFile(s.T(), strings.Repeat(generatedStd.String(), 3)) - // sign-batch file - offline is set but account-number and sequence are not val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1) + + // sign-batch file - offline is set but account-number and sequence are not res, err := authtest.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--offline") s.Require().EqualError(err, "required flag(s) \"account-number\", \"sequence\" not set") @@ -158,6 +131,87 @@ func (s *IntegrationTestSuite) TestCLISignBatch() { s.Require().Error(err) } +func (s *IntegrationTestSuite) TestCLISign() { + require := s.Require() + val1 := s.network.Validators[0] + txCfg := val1.ClientCtx.TxConfig + txBz := s.createBankMsg(val1, val1.Address) + fileUnsigned := testutil.WriteToNewTempFile(s.T(), txBz.String()) + chainFlag := fmt.Sprintf("--%s=%s", flags.FlagChainID, val1.ClientCtx.ChainID) + sigOnlyFlag := "--signature-only" + + // SIC! validators have same key names and same adddresses as those registered in the keyring, + // BUT the keys are different! + valInfo, err := val1.ClientCtx.Keyring.Key(val1.Moniker) + require.NoError(err) + + /**** test signature-only ****/ + res, err := authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag, + sigOnlyFlag) + require.NoError(err) + checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey()) + + /**** test full output ****/ + res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, fileUnsigned.Name(), chainFlag) + require.NoError(err) + + // txCfg.UnmarshalSignatureJSON can't unmarshal a fragment of the signature, so we create this structure. + type txFragment struct { + Signatures []json.RawMessage + } + var txOut txFragment + err = json.Unmarshal(res.Bytes(), &txOut) + require.NoError(err) + require.Len(txOut.Signatures, 1) + + /**** 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) + require.NoError(err) + fContent, err := ioutil.ReadFile(filenameSigned) + require.NoError(err) + require.Equal(res.String(), string(fContent)) + + /**** try to append to the previously signed transaction ****/ + res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, filenameSigned, chainFlag, + sigOnlyFlag) + require.NoError(err) + checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey(), valInfo.GetPubKey()) + + /**** try to overwrite the previously signed transaction ****/ + + // We can't sign with other address, because the bank send message supports only one signer for a simple + // account. Changing the file is too much hacking, because TxDecoder returns sdk.Tx, which doesn't + // 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") + checkSignatures(require, txCfg, res.Bytes(), valInfo.GetPubKey()) + + /**** test flagAmino ****/ + res, err = authtest.TxSignExec(val1.ClientCtx, val1.Address, filenameSigned, chainFlag, + "--amino=true") + require.NoError(err) + + var txAmino authrest.BroadcastReq + err = val1.ClientCtx.LegacyAmino.UnmarshalJSON(res.Bytes(), &txAmino) + require.NoError(err) + require.Len(txAmino.Tx.Signatures, 2) + require.Equal(txAmino.Tx.Signatures[0].PubKey, valInfo.GetPubKey()) + require.Equal(txAmino.Tx.Signatures[1].PubKey, valInfo.GetPubKey()) +} + +func checkSignatures(require *require.Assertions, txCfg client.TxConfig, output []byte, pks ...cryptotypes.PubKey) { + sigs, err := txCfg.UnmarshalSignatureJSON(output) + require.NoError(err, string(output)) + require.Len(sigs, len(pks)) + for i := range pks { + require.True(sigs[i].PubKey.Equals(pks[i]), "Pub key doesn't match. Got: %s, expected: %s, idx: %d", sigs[i].PubKey, pks[i], i) + require.NotEmpty(sigs[i].Data) + } +} + func (s *IntegrationTestSuite) TestCLIQueryTxCmd() { val := s.network.Validators[0] @@ -417,12 +471,7 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() { } func (s *IntegrationTestSuite) TestCLIMultisignInsufficientCosigners() { - val1 := s.network.Validators[0] - - codec := codec2.NewLegacyAmino() - sdk.RegisterLegacyAminoCodec(codec) - banktypes.RegisterLegacyAminoCodec(codec) - val1.ClientCtx.LegacyAmino = codec + val1 := *s.network.Validators[0] // Generate 2 accounts and a multisig. account1, err := val1.ClientCtx.Keyring.Key("newAccount1") @@ -479,10 +528,8 @@ func (s *IntegrationTestSuite) TestCLIMultisignInsufficientCosigners() { // Save tx to file multiSigWith1SignatureFile := testutil.WriteToNewTempFile(s.T(), multiSigWith1Signature.String()) - exec, err := authtest.TxValidateSignaturesExec(val1.ClientCtx, multiSigWith1SignatureFile.Name()) + _, err = authtest.TxValidateSignaturesExec(val1.ClientCtx, multiSigWith1SignatureFile.Name()) s.Require().Error(err) - - fmt.Printf("%s", exec) } func (s *IntegrationTestSuite) TestCLIEncode() { @@ -501,17 +548,14 @@ func (s *IntegrationTestSuite) TestCLIEncode() { fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), "--memo", "deadbeef", ) s.Require().NoError(err) - - // Save tx to file savedTxFile := testutil.WriteToNewTempFile(s.T(), normalGeneratedTx.String()) // Encode encodeExec, err := authtest.TxEncodeExec(val1.ClientCtx, savedTxFile.Name()) s.Require().NoError(err) - trimmedBase64 := strings.Trim(encodeExec.String(), "\"\n") - // Check that the transaction decodes as expected + // Check that the transaction decodes as expected decodedTx, err := authtest.TxDecodeExec(val1.ClientCtx, trimmedBase64) s.Require().NoError(err) @@ -524,12 +568,7 @@ func (s *IntegrationTestSuite) TestCLIEncode() { } func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() { - val1 := s.network.Validators[0] - - codec := codec2.NewLegacyAmino() - sdk.RegisterLegacyAminoCodec(codec) - banktypes.RegisterLegacyAminoCodec(codec) - val1.ClientCtx.LegacyAmino = codec + val1 := *s.network.Validators[0] // Generate 2 accounts and a multisig. account1, err := val1.ClientCtx.Keyring.Key("newAccount1") @@ -621,12 +660,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() { } func (s *IntegrationTestSuite) TestCLIMultisign() { - val1 := s.network.Validators[0] - - codec := codec2.NewLegacyAmino() - sdk.RegisterLegacyAminoCodec(codec) - banktypes.RegisterLegacyAminoCodec(codec) - val1.ClientCtx.LegacyAmino = codec + val1 := *s.network.Validators[0] // Generate 2 accounts and a multisig. account1, err := val1.ClientCtx.Keyring.Key("newAccount1") @@ -839,7 +873,6 @@ func TestGetBroadcastCommand_WithoutOfflineFlag(t *testing.T) { cmd := authcli.GetBroadcastCommand() _, out := testutil.ApplyMockIO(cmd) - testDir := t.TempDir() // Create new file with tx builder := txCfg.NewTxBuilder() @@ -851,13 +884,14 @@ func TestGetBroadcastCommand_WithoutOfflineFlag(t *testing.T) { err = builder.SetMsgs(banktypes.NewMsgSend(from, to, sdk.Coins{sdk.NewInt64Coin("stake", 10000)})) require.NoError(t, err) txContents, err := txCfg.TxJSONEncoder()(builder.GetTx()) - txFileName := filepath.Join(testDir, "tx.json") - err = ioutil.WriteFile(txFileName, txContents, 0644) require.NoError(t, err) + txFile := testutil.WriteToNewTempFile(t, string(txContents)) - cmd.SetArgs([]string{txFileName}) - require.Error(t, cmd.ExecuteContext(ctx)) - require.Contains(t, out.String(), "unsupported return type ; supported types: sync, async, block") + cmd.SetArgs([]string{txFile.Name()}) + err = cmd.ExecuteContext(ctx) + require.Error(t, err) + require.Contains(t, err.Error(), "connect: connection refused") + require.Contains(t, out.String(), "connect: connection refused") } func (s *IntegrationTestSuite) TestQueryParamsCmd() { @@ -951,10 +985,88 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() { // Broadcast tx, test that it shouldn't panic. val1.ClientCtx.BroadcastMode = flags.BroadcastSync out, err := authtest.TxBroadcastExec(val1.ClientCtx, signedTxFile.Name()) + s.Require().NoError(err) var res sdk.TxResponse s.Require().NoError(val1.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &res)) s.Require().NotEqual(0, res.Code) +} + +func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() { + // test case: + // Create a transaction with 2 messages which has to be signed with 2 different keys + // Sign and append the signatures using the CLI with Amino signing mode. + // Finally send the transaction to the blockchain. It should work. + + require := s.Require() + val0, val1 := s.network.Validators[0], s.network.Validators[1] + val0Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val0.Moniker), sdk.NewInt(10)) + val1Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val1.Moniker), sdk.NewInt(10)) + _, _, 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. + // Since we we amino, we don't need to pre-populate signer_infos. + txBuilder := val0.ClientCtx.TxConfig.NewTxBuilder() + txBuilder.SetMsgs( + banktypes.NewMsgSend(val0.Address, addr1, sdk.NewCoins(val0Coin)), + banktypes.NewMsgSend(val1.Address, addr1, sdk.NewCoins(val1Coin)), + ) + txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)))) + txBuilder.SetGasLimit(testdata.NewTestGasLimit()) + require.Equal([]sdk.AccAddress{val0.Address, val1.Address}, txBuilder.GetTx().GetSigners()) + + // Write the unsigned tx into a file. + txJSON, err := val0.ClientCtx.TxConfig.TxJSONEncoder()(txBuilder.GetTx()) + require.NoError(err) + 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") + require.NoError(err) + signedByVal0File := testutil.WriteToNewTempFile(s.T(), signedByVal0.String()) + + // Then let val1 sign the file with signedByVal0. + val1AccNum, val1Seq, err := val0.ClientCtx.AccountRetriever.GetAccountNumberSequence(val0.ClientCtx, val1.Address) + 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), + ) + require.NoError(err) + signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String()) + + // Now let's try to send this tx. + res, err := authtest.TxBroadcastExec(val0.ClientCtx, signedTxFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock)) + require.NoError(err) + var txRes sdk.TxResponse + require.NoError(val0.ClientCtx.JSONMarshaler.UnmarshalJSON(res.Bytes(), &txRes)) + require.Equal(uint32(0), txRes.Code) + + // Make sure the addr1's balance got funded. + queryResJSON, err := bankcli.QueryBalancesExec(val0.ClientCtx, addr1) + require.NoError(err) + var queryRes banktypes.QueryAllBalancesResponse + err = val0.ClientCtx.JSONMarshaler.UnmarshalJSON(queryResJSON.Bytes(), &queryRes) + require.NoError(err) + require.Equal(sdk.NewCoins(val0Coin, val1Coin), queryRes.Balances) +} + +func (s *IntegrationTestSuite) createBankMsg(val *network.Validator, toAddr sdk.AccAddress) testutil.BufferWriter { + res, err := bankcli.MsgSendExec( + val.ClientCtx, + val.Address, + toAddr, + sdk.NewCoins( + sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)), + sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)), + ), + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), + ) s.Require().NoError(err) + return res } func TestIntegrationTestSuite(t *testing.T) { diff --git a/x/auth/client/cli/decode.go b/x/auth/client/cli/decode.go index b83e8e405..e1807efb0 100644 --- a/x/auth/client/cli/decode.go +++ b/x/auth/client/cli/decode.go @@ -3,7 +3,6 @@ package cli import ( "encoding/base64" "encoding/hex" - "fmt" "github.com/spf13/cobra" @@ -43,7 +42,7 @@ func GetDecodeCommand() *cobra.Command { return err } - return clientCtx.PrintString(fmt.Sprintf("%s\n", json)) + return clientCtx.PrintBytes(json) }, } diff --git a/x/auth/client/cli/encode.go b/x/auth/client/cli/encode.go index 92db2e0a8..c883980de 100644 --- a/x/auth/client/cli/encode.go +++ b/x/auth/client/cli/encode.go @@ -2,7 +2,6 @@ package cli import ( "encoding/base64" - "fmt" "github.com/spf13/cobra" @@ -38,7 +37,7 @@ If you supply a dash (-) argument in place of an input filename, the command rea // base64 encode the encoded tx bytes txBytesBase64 := base64.StdEncoding.EncodeToString(txBytes) - return clientCtx.PrintString(fmt.Sprintf("%s\n", txBytesBase64)) + return clientCtx.PrintString(txBytesBase64 + "\n") }, } diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index fc5e907de..56e1e1b5c 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -60,7 +60,7 @@ recommended to set such parameters manually. return cmd } -func makeMultiSignCmd() func(cmd *cobra.Command, args []string) error { +func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) { return func(cmd *cobra.Command, args []string) (err error) { clientCtx := client.GetClientContextFromCmd(cmd) clientCtx, err = client.ReadTxCommandFlags(clientCtx, cmd.Flags()) @@ -183,9 +183,17 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer fp.Close() - return clientCtx.PrintString(fmt.Sprintf("%s\n", json)) + defer func() { + err2 := fp.Close() + if err == nil { + err = err2 + } + }() + + err = clientCtx.PrintBytes(json) + + return } } diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 15fb06695..9f5f33501 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -16,10 +16,10 @@ import ( ) const ( - flagMultisig = "multisig" - flagAppend = "append" - flagSigOnly = "signature-only" - flagAmino = "amino" + flagMultisig = "multisig" + flagOverwrite = "overwrite" + flagSigOnly = "signature-only" + flagAmino = "amino" ) // GetSignBatchCommand returns the transaction sign-batch command. @@ -30,10 +30,9 @@ func GetSignBatchCommand() *cobra.Command { Long: `Sign batch files of transactions generated with --generate-only. The command processes list of transactions from file (one StdTx each line), generate signed transactions or signatures and print their JSON encoding, delimited by '\n'. -As the signatures are generated, the command updates the sequence number accordingly. +As the signatures are generated, the command updates the account sequence number accordingly. -If the flag --signature-only flag is set, it will output a JSON representation -of the generated signature only. +If the --signature-only flag is set, it will output the signature parts only. The --offline flag makes sure that the client will not reach out to full node. As a result, the account and the sequence number queries will not be performed and @@ -67,14 +66,10 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { return err } txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags()) - txCfg := clientCtx.TxConfig - generateSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) - - var ( - multisigAddr sdk.AccAddress - infile = os.Stdin - ) + printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) + infile := os.Stdin + var multisigAddr sdk.AccAddress // validate multisig address if there's any if ms, _ := cmd.Flags().GetString(flagMultisig); ms != "" { @@ -114,7 +109,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } - err = authclient.SignTx(txFactory, clientCtx, fromName, txBuilder, true) + err = authclient.SignTx(txFactory, clientCtx, fromName, txBuilder, true, true) if err != nil { return err } @@ -122,14 +117,15 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED { txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) } - err = authclient.SignTxWithSignerAddress(txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline) + err = authclient.SignTxWithSignerAddress( + txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, true) } if err != nil { return err } - json, err := marshalSignatureJSON(txCfg, txBuilder, generateSignatureOnly) + json, err := marshalSignatureJSON(txCfg, txBuilder, printSignatureOnly) if err != nil { return err } @@ -166,12 +162,11 @@ func setOutputFile(cmd *cobra.Command) (func(), error) { func GetSignCommand() *cobra.Command { cmd := &cobra.Command{ Use: "sign [file]", - Short: "Sign transactions generated offline", - Long: `Sign transactions created with the --generate-only flag. + Short: "Sign a transaction generated offline", + Long: `Sign a transaction created with the --generate-only flag. It will read a transaction from [file], sign it, and print its JSON encoding. -If the flag --signature-only flag is set, it will output a JSON representation -of the generated signature only. +If the --signature-only flag is set, it will output the signature parts only. The --offline flag makes sure that the client will not reach out to full node. As a result, the account and sequence number queries will not be performed and @@ -188,8 +183,8 @@ be generated via the 'multisign' command. } cmd.Flags().String(flagMultisig, "", "Address of the multisig account on behalf of which the transaction shall be signed") - cmd.Flags().Bool(flagAppend, true, "Append the signature to the existing ones. If disabled, old signatures would be overwritten. Ignored if --multisig is on") - cmd.Flags().Bool(flagSigOnly, false, "Print only the generated signature, then exit") + cmd.Flags().Bool(flagOverwrite, false, "Overwrite existing signatures with a new one. If disabled, new signature will be appended") + cmd.Flags().Bool(flagSigOnly, false, "Print only the signatures") cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") cmd.Flags().String(flags.FlagChainID, "", "The network chain ID") cmd.Flags().Bool(flagAmino, false, "Generate Amino encoded JSON suitable for submiting to the txs REST endpoint") @@ -209,13 +204,14 @@ func preSignCmd(cmd *cobra.Command, _ []string) { } func makeSignCmd() func(cmd *cobra.Command, args []string) error { - return func(cmd *cobra.Command, args []string) error { + return func(cmd *cobra.Command, args []string) (err error) { clientCtx := client.GetClientContextFromCmd(cmd) - clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags()) + f := cmd.Flags() + clientCtx, err = client.ReadTxCommandFlags(clientCtx, f) if err != nil { return err } - txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags()) + txFactory := tx.NewFactoryCLI(clientCtx, f) clientCtx, txF, newTx, err := readTxAndInitContexts(clientCtx, cmd, args[0]) if err != nil { @@ -230,64 +226,52 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { return err } - // if --signature-only is on, then override --append - generateSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) + 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) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } + overwrite, _ := f.GetBool(flagOverwrite) if multisigAddrStr != "" { var multisigAddr sdk.AccAddress - multisigAddr, err = sdk.AccAddressFromBech32(multisigAddrStr) if err != nil { return err } - err = authclient.SignTxWithSignerAddress( - txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, - ) - generateSignatureOnly = true + txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) + printSignatureOnly = true } else { - flagAppend, _ := cmd.Flags().GetBool(flagAppend) - appendSig := flagAppend && !generateSignatureOnly - if appendSig { - err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline) - } + err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) } if err != nil { return err } - aminoJSON, _ := cmd.Flags().GetBool(flagAmino) - + aminoJSON, err := f.GetBool(flagAmino) if err != nil { return err } var json []byte - if aminoJSON { stdTx, err := tx.ConvertTxToStdTx(clientCtx.LegacyAmino, txBuilder.GetTx()) if err != nil { return err } - req := rest.BroadcastReq{ Tx: stdTx, Mode: "block|sync|async", } - json, err = clientCtx.LegacyAmino.MarshalJSON(req) if err != nil { return err } } else { - json, err = marshalSignatureJSON(txCfg, txBuilder, generateSignatureOnly) + json, err = marshalSignatureJSON(txCfg, txBuilder, printSignatureOnly) if err != nil { return err } @@ -303,16 +287,21 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer fp.Close() + defer func() { + err2 := fp.Close() + if err == nil { + err = err2 + } + }() - return clientCtx.PrintString(fmt.Sprintf("%s\n", json)) + _, err = fp.Write(append(json, '\n')) + return } } -func marshalSignatureJSON(txConfig client.TxConfig, txBldr client.TxBuilder, generateSignatureOnly bool) ([]byte, error) { +func marshalSignatureJSON(txConfig client.TxConfig, txBldr client.TxBuilder, signatureOnly bool) ([]byte, error) { parsedTx := txBldr.GetTx() - - if generateSignatureOnly { + if signatureOnly { sigs, err := parsedTx.GetSignaturesV2() if err != nil { return nil, err diff --git a/x/auth/client/rest/rest_test.go b/x/auth/client/rest/rest_test.go index cc1a5d1cd..e20874f6e 100644 --- a/x/auth/client/rest/rest_test.go +++ b/x/auth/client/rest/rest_test.go @@ -379,7 +379,7 @@ func (s *IntegrationTestSuite) createTestStdTx(val *network.Validator, accNum, s WithSequence(sequence) // sign Tx (offline mode so we can manually set sequence number) - err := authclient.SignTx(txFactory, val.ClientCtx, val.Moniker, txBuilder, true) + err := authclient.SignTx(txFactory, val.ClientCtx, val.Moniker, txBuilder, true, true) s.Require().NoError(err) stdTx := txBuilder.GetTx().(legacytx.StdTx) diff --git a/x/auth/client/tx.go b/x/auth/client/tx.go index 79570d519..70229c39e 100644 --- a/x/auth/client/tx.go +++ b/x/auth/client/tx.go @@ -43,10 +43,10 @@ func PrintUnsignedStdTx(txBldr tx.Factory, clientCtx client.Context, msgs []sdk. return err } -// SignTx appends a signature to a transaction. If appendSig -// is false, it replaces the signatures already attached with the new signature. +// 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 bool) error { +func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx client.TxBuilder, offline, overwriteSig bool) error { info, err := txFactory.Keybase().Key(name) if err != nil { return err @@ -62,14 +62,14 @@ func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx c } } - return tx.Sign(txFactory, name, stdTx) + return tx.Sign(txFactory, name, stdTx, 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. func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, addr sdk.AccAddress, - name string, txBuilder client.TxBuilder, offline bool) (err error) { + name string, txBuilder client.TxBuilder, offline, overwrite bool) (err error) { // check whether the address is a signer if !isTxSigner(addr, txBuilder.GetTx().GetSigners()) { @@ -83,7 +83,7 @@ func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, add } } - return tx.Sign(txFactory, name, txBuilder) + return tx.Sign(txFactory, name, txBuilder, overwrite) } // Read and decode a StdTx from the given filename. Can pass "-" to read from stdin. diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index 4c76d529a..18a555f90 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -157,10 +157,6 @@ func (w *wrapper) GetMemo() string { return w.tx.Body.Memo } -func (w *wrapper) GetSignatures() [][]byte { - return w.tx.Signatures -} - // GetTimeoutHeight returns the transaction's timeout height (if set). func (w *wrapper) GetTimeoutHeight() uint64 { return w.tx.Body.TimeoutHeight @@ -204,25 +200,13 @@ func (w *wrapper) SetMsgs(msgs ...sdk.Msg) error { var err error switch msg := msg.(type) { case sdk.ServiceMsg: - { - bz, err := proto.Marshal(msg.Request) - if err != nil { - return err - } - anys[i] = &codectypes.Any{ - TypeUrl: msg.MethodName, - Value: bz, - } - } + anys[i], err = codectypes.NewAnyWithCustomTypeURL(msg.Request, msg.MethodName) default: - { - anys[i], err = codectypes.NewAnyWithValue(msg) - if err != nil { - return err - } - } + anys[i], err = codectypes.NewAnyWithValue(msg) + } + if err != nil { + return err } - } w.tx.Body.Messages = anys diff --git a/x/auth/tx/service_test.go b/x/auth/tx/service_test.go index 341684acb..2fffdb902 100644 --- a/x/auth/tx/service_test.go +++ b/x/auth/tx/service_test.go @@ -448,7 +448,7 @@ func (s IntegrationTestSuite) mkTxBuilder() client.TxBuilder { WithSignMode(signing.SignMode_SIGN_MODE_DIRECT) // Sign Tx. - err := authclient.SignTx(txFactory, val.ClientCtx, val.Moniker, txBuilder, false) + err := authclient.SignTx(txFactory, val.ClientCtx, val.Moniker, txBuilder, false, true) s.Require().NoError(err) return txBuilder diff --git a/x/bank/client/cli/cli_test.go b/x/bank/client/cli/cli_test.go index 9b8c5cf61..a8e94d245 100644 --- a/x/bank/client/cli/cli_test.go +++ b/x/bank/client/cli/cli_test.go @@ -335,7 +335,6 @@ func (s *IntegrationTestSuite) TestBankMsgService() { s.Run(tc.name, func() { clientCtx := val.ClientCtx - bz, err := banktestutil.ServiceMsgSendExec(clientCtx, tc.from, tc.to, tc.amount, tc.args...) if tc.expectErr { s.Require().Error(err) diff --git a/x/genutil/client/cli/gentx.go b/x/genutil/client/cli/gentx.go index 33a37760e..790cc9921 100644 --- a/x/genutil/client/cli/gentx.go +++ b/x/genutil/client/cli/gentx.go @@ -41,9 +41,9 @@ func GenTxCmd(mbm module.BasicManager, txEncCfg client.TxEncodingConfig, genBalI Long: fmt.Sprintf(`Generate a genesis transaction that creates a validator with a self-delegation, that is signed by the key in the Keyring referenced by a given name. A node ID and Bech32 consensus pubkey may optionally be provided. If they are omitted, they will be retrieved from the priv_validator.json -file. The following default parameters are included: +file. The following default parameters are included: %s - + Example: $ %s gentx my-key-name --home=/path/to/home/dir --keyring-backend=os --chain-id=test-chain-1 \ --amount=1000000stake \ @@ -164,7 +164,7 @@ $ %s gentx my-key-name --home=/path/to/home/dir --keyring-backend=os --chain-id= return fmt.Errorf("error creating tx builder: %w", err) } - err = authclient.SignTx(txFactory, clientCtx, name, txBuilder, true) + err = authclient.SignTx(txFactory, clientCtx, name, txBuilder, true, true) if err != nil { return errors.Wrap(err, "failed to sign std tx") }