diff --git a/CHANGELOG.md b/CHANGELOG.md index cd4682f2f..9fdd0a915 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (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 * (server) [\#8641](https://github.com/cosmos/cosmos-sdk/pull/8641) Fix Tendermint and application configuration reading from file +* (rest) [\#8730](https://github.com/cosmos/cosmos-sdk/pull/8730) Fix querying txs with multisigs on legacy REST endpoints. * (client/keys) [\#8639] (https://github.com/cosmos/cosmos-sdk/pull/8639) Fix keys migrate for mulitisig, offline, and ledger keys. The migrate command now takes a positional old_home_dir argument. ## [v0.41.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.41.3) - 2021-02-18 diff --git a/x/auth/client/rest/rest_test.go b/x/auth/client/rest/rest_test.go index cfca52750..9ecb3e842 100644 --- a/x/auth/client/rest/rest_test.go +++ b/x/auth/client/rest/rest_test.go @@ -2,6 +2,7 @@ package rest_test import ( "fmt" + "strings" "testing" "github.com/spf13/cobra" @@ -11,6 +12,8 @@ import ( "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" @@ -22,6 +25,7 @@ import ( authclient "github.com/cosmos/cosmos-sdk/x/auth/client" 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" "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/cosmos/cosmos-sdk/x/bank/types" @@ -52,6 +56,16 @@ func (s *IntegrationTestSuite) SetupSuite() { _, _, err := kb.NewMnemonic("newAccount", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) s.Require().NoError(err) + account1, _, err := kb.NewMnemonic("newAccount1", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + s.Require().NoError(err) + + account2, _, err := kb.NewMnemonic("newAccount2", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + s.Require().NoError(err) + + multi := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{account1.GetPubKey(), account2.GetPubKey()}) + _, err = kb.SaveMultisig("multi", multi) + s.Require().NoError(err) + _, err = s.network.WaitForHeight(1) s.Require().NoError(err) @@ -560,6 +574,97 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { } } +// TestLegacyMultiSig creates a legacy multisig transaction, and makes sure +// we can query it via the legacy REST endpoint. +// ref: https://github.com/cosmos/cosmos-sdk/issues/8679 +func (s *IntegrationTestSuite) TestLegacyMultisig() { + val1 := *s.network.Validators[0] + + // Generate 2 accounts and a multisig. + account1, err := val1.ClientCtx.Keyring.Key("newAccount1") + s.Require().NoError(err) + + account2, err := val1.ClientCtx.Keyring.Key("newAccount2") + s.Require().NoError(err) + + multisigInfo, err := val1.ClientCtx.Keyring.Key("multi") + s.Require().NoError(err) + + // Send coins from validator to multisig. + sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 1000) + _, err = bankcli.MsgSendExec( + val1.ClientCtx, + val1.Address, + multisigInfo.GetAddress(), + sdk.NewCoins(sendTokens), + 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("--gas=%d", flags.DefaultGasLimit), + ) + + s.Require().NoError(s.network.WaitForNextBlock()) + + // Generate multisig transaction to a random address. + _, _, recipient := testdata.KeyTestPubAddr() + multiGeneratedTx, err := bankcli.MsgSendExec( + val1.ClientCtx, + multisigInfo.GetAddress(), + recipient, + sdk.NewCoins( + sdk.NewInt64Coin(s.cfg.BondDenom, 5), + ), + 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) + + // Save tx to file + multiGeneratedTxFile := testutil.WriteToNewTempFile(s.T(), multiGeneratedTx.String()) + + // Sign with account1 + val1.ClientCtx.HomeDir = strings.Replace(val1.ClientCtx.HomeDir, "simd", "simcli", 1) + account1Signature, err := authtest.TxSignExec(val1.ClientCtx, account1.GetAddress(), multiGeneratedTxFile.Name(), "--multisig", multisigInfo.GetAddress().String()) + s.Require().NoError(err) + + sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String()) + + // Sign with account1 + account2Signature, err := authtest.TxSignExec(val1.ClientCtx, account2.GetAddress(), multiGeneratedTxFile.Name(), "--multisig", multisigInfo.GetAddress().String()) + s.Require().NoError(err) + + sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String()) + + // Does not work in offline mode. + _, err = authtest.TxMultiSignExec(val1.ClientCtx, multisigInfo.GetName(), multiGeneratedTxFile.Name(), "--offline", sign1File.Name(), sign2File.Name()) + s.Require().EqualError(err, fmt.Sprintf("couldn't verify signature for address %s", account1.GetAddress())) + + val1.ClientCtx.Offline = false + multiSigWith2Signatures, err := authtest.TxMultiSignExec(val1.ClientCtx, multisigInfo.GetName(), multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name()) + s.Require().NoError(err) + + // Write the output to disk + signedTxFile := testutil.WriteToNewTempFile(s.T(), multiSigWith2Signatures.String()) + + _, err = authtest.TxValidateSignaturesExec(val1.ClientCtx, signedTxFile.Name()) + s.Require().NoError(err) + + val1.ClientCtx.BroadcastMode = flags.BroadcastBlock + out, err := authtest.TxBroadcastExec(val1.ClientCtx, signedTxFile.Name()) + s.Require().NoError(err) + + s.Require().NoError(s.network.WaitForNextBlock()) + + var txRes sdk.TxResponse + err = val1.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes) + s.Require().NoError(err) + s.Require().Equal(uint32(0), txRes.Code) + + s.testQueryTx(txRes.Height, txRes.TxHash, recipient.String()) +} + func TestIntegrationTestSuite(t *testing.T) { suite.Run(t, new(IntegrationTestSuite)) } diff --git a/x/auth/legacy/legacytx/stdsign.go b/x/auth/legacy/legacytx/stdsign.go index 466981369..f87900620 100644 --- a/x/auth/legacy/legacytx/stdsign.go +++ b/x/auth/legacy/legacytx/stdsign.go @@ -5,7 +5,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" @@ -13,9 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" ) -// Interface implementation checks -var _ codectypes.UnpackInterfacesMessage = StdTx{} - // StdSignDoc is replay-prevention structure. // It includes the result of msg.GetSignBytes(), // as well as the ChainID (prevent cross chain replay) diff --git a/x/auth/legacy/legacytx/stdtx.go b/x/auth/legacy/legacytx/stdtx.go index 8cb3fb699..2b8a6c0e0 100644 --- a/x/auth/legacy/legacytx/stdtx.go +++ b/x/auth/legacy/legacytx/stdtx.go @@ -16,9 +16,12 @@ import ( // Interface implementation checks var ( - _ sdk.Tx = (*StdTx)(nil) - _ sdk.TxWithMemo = (*StdTx)(nil) - _ sdk.FeeTx = (*StdTx)(nil) + _ sdk.Tx = (*StdTx)(nil) + _ sdk.TxWithMemo = (*StdTx)(nil) + _ sdk.FeeTx = (*StdTx)(nil) + _ codectypes.UnpackInterfacesMessage = (*StdTx)(nil) + + _ codectypes.UnpackInterfacesMessage = (*StdSignature)(nil) ) // StdFee includes the amount of coins paid in fees and the maximum @@ -116,6 +119,10 @@ func (ss StdSignature) MarshalYAML() (interface{}, error) { return string(bz), err } +func (ss StdSignature) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + return codectypes.UnpackInterfaces(ss.PubKey, unpacker) +} + // StdTx is the legacy transaction format for wrapping a Msg with Fee and Signatures. // It only works with Amino, please prefer the new protobuf Tx in types/tx. // NOTE: the first signature is the fee payer (Signatures must not be nil). @@ -277,5 +284,14 @@ func (tx StdTx) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { return err } } + + // Signatures contain PubKeys, which need to be unpacked. + for _, s := range tx.Signatures { + err := codectypes.UnpackInterfaces(s, unpacker) + if err != nil { + return err + } + } + return nil }