Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow REST endpoint to query txs with multisig #8730

Merged
merged 8 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

## [v0.41.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.41.3) - 2021-02-18

Expand Down
105 changes: 105 additions & 0 deletions x/auth/client/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rest_test

import (
"fmt"
"strings"
"testing"

"github.com/spf13/cobra"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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))
}
4 changes: 0 additions & 4 deletions x/auth/legacy/legacytx/stdsign.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@ 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"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"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)
Expand Down
22 changes: 19 additions & 3 deletions x/auth/legacy/legacytx/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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
}