Revert "fix: Prevent signing from wrong key in multisig (#12446)"

This reverts commit 28f4fb9.

client/keys/show_test.go

@@ -31,7 +31,7 @@ func Test_multiSigKey_Properties(t *testing.T) {
 	require.Equal(t, "myMultisig", k.Name)
 	require.Equal(t, keyring.TypeMulti, k.GetType())
 
-	pub, err := k.GetMultisigPubKey()
+	pub, err := k.GetPubKey()
 	require.NoError(t, err)
 	require.Equal(t, "D3923267FA8A3DD367BB768FA8BDC8FF7F89DA3F", pub.Address().String())
 

crypto/keyring/output_test.go

@@ -20,7 +20,7 @@ func TestBech32KeysOutput(t *testing.T) {
 	k, err := NewMultiRecord("multisig", multisigPk)
 	require.NotNil(t, k)
 	require.NoError(t, err)
-	pubKey, err := k.GetMultisigPubKey()
+	pubKey, err := k.GetPubKey()
 	require.NoError(t, err)
 	accAddr := sdk.AccAddress(pubKey.Address())
 	expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk)

crypto/keyring/record.go

@@ -5,7 +5,6 @@ import (
 
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/crypto/hd"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/types"
 )
@@ -60,16 +59,6 @@ func NewMultiRecord(name string, pk cryptotypes.PubKey) (*Record, error) {
 	return newRecord(name, pk, recordMultiItem)
 }
 
-// GetMultisigPubKey fetches a public key of the multi type record
-func (k *Record) GetMultisigPubKey() (*multisig.LegacyAminoPubKey, error) {
-	pk, ok := k.PubKey.GetCachedValue().(*multisig.LegacyAminoPubKey)
-	if !ok {
-		return nil, errors.New("unable to cast any to multisig.LegacyAminoPubKey")
-	}
-
-	return pk, nil
-}
-
 // GetPubKey fetches a public key of the record
 func (k *Record) GetPubKey() (cryptotypes.PubKey, error) {
 	pk, ok := k.PubKey.GetCachedValue().(cryptotypes.PubKey)

crypto/keyring/record_test.go

@@ -10,35 +10,24 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/crypto/hd"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 )
 
 type RecordTestSuite struct {
 	suite.Suite
 
-	appName  string
-	cdc      codec.Codec
-	priv     cryptotypes.PrivKey
-	pub      cryptotypes.PubKey
-	multiPub *multisig.LegacyAminoPubKey
+	appName string
+	cdc     codec.Codec
+	priv    cryptotypes.PrivKey
+	pub     cryptotypes.PubKey
 }
 
 func (s *RecordTestSuite) SetupSuite() {
 	s.appName = "cosmos"
 	s.cdc = getCodec()
 	s.priv = cryptotypes.PrivKey(ed25519.GenPrivKey())
 	s.pub = s.priv.PubKey()
-
-	multiKey := secp256k1.GenPrivKeyFromSecret([]byte("myMultiKey"))
-	pk := multisig.NewLegacyAminoPubKey(
-		1,
-		[]cryptotypes.PubKey{multiKey.PubKey()},
-	)
-	s.multiPub = pk
-
 }
 
 func (s *RecordTestSuite) TestOfflineRecordMarshaling() {
@@ -58,32 +47,6 @@ func (s *RecordTestSuite) TestOfflineRecordMarshaling() {
 	s.Require().True(s.pub.Equals(pk2))
 }
 
-func (s *RecordTestSuite) TestMultiRecordMarshaling() {
-	dir := s.T().TempDir()
-	mockIn := strings.NewReader("")
-
-	kb, err := New(s.appName, BackendTest, dir, mockIn, s.cdc)
-	s.Require().NoError(err)
-
-	k, err := NewMultiRecord("testrecord", s.multiPub)
-	s.Require().NoError(err)
-
-	ks, ok := kb.(keystore)
-	s.Require().True(ok)
-
-	bz, err := ks.cdc.Marshal(k)
-	s.Require().NoError(err)
-
-	k2, err := ks.protoUnmarshalRecord(bz)
-	s.Require().NoError(err)
-	s.Require().Equal(k.Name, k2.Name)
-	s.Require().True(k.PubKey.Equal(k2.PubKey))
-
-	pub2, err := k2.GetMultisigPubKey()
-	s.Require().NoError(err)
-	s.Require().True(s.multiPub.Equals(pub2))
-}
-
 func (s *RecordTestSuite) TestLocalRecordMarshaling() {
 	dir := s.T().TempDir()
 	mockIn := strings.NewReader("")

x/auth/client/cli/tx_sign.go

@@ -224,14 +224,15 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
 			return err
 		}
 
+		txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
 		txCfg := clientCtx.TxConfig
 		txBuilder, err := txCfg.WrapTxBuilder(newTx)
 		if err != nil {
 			return err
 		}
 
 		printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly)
-		multisigkey, _ := cmd.Flags().GetString(flagMultisig)
+		multisig, _ := cmd.Flags().GetString(flagMultisig)
 		if err != nil {
 			return err
 		}
@@ -242,44 +243,15 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
 		}
 
 		overwrite, _ := f.GetBool(flagOverwrite)
-		if multisigkey != "" {
-			var (
-				multisigName string
-				multisigAddr sdk.AccAddress
-			)
-			// Bech32 decode error, maybe it's a name, we try to fetch from keyring
-			multisigAddr, multisigName, _, err = client.GetFromFields(clientCtx, txF.Keybase(), multisigkey)
-			if err != nil {
-				return fmt.Errorf("error getting account from keybase: %w", err)
-			}
-			multisigRecord, err := clientCtx.Keyring.Key(multisigName)
-			if err != nil {
-				return err
-			}
-			multisigPub, err := multisigRecord.GetMultisigPubKey()
-			if err != nil {
-				return err
-			}
-
-			fromRecord, err := clientCtx.Keyring.Key(fromName)
+		if multisig != "" {
+			multisigAddr, err := sdk.AccAddressFromBech32(multisig)
 			if err != nil {
-				return err
-			}
-			fromPubKey, err := fromRecord.GetPubKey()
-			if err != nil {
-				return err
-			}
-
-			var found bool
-			for _, pubkey := range multisigPub.GetPubKeys() {
-				if pubkey.Equals(fromPubKey) {
-					found = true
+				// Bech32 decode error, maybe it's a name, we try to fetch from keyring
+				multisigAddr, _, _, err = client.GetFromFields(clientCtx, txFactory.Keybase(), multisig)
+				if err != nil {
+					return fmt.Errorf("error getting account from keybase: %w", err)
 				}
 			}
-			if !found {
-				return fmt.Errorf("from key is not a part of multisig key")
-			}
-
 			err = authclient.SignTxWithSignerAddress(
 				txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite)
 			if err != nil {

x/auth/client/testutil/suite.go

@@ -74,10 +74,6 @@ func (s *IntegrationTestSuite) SetupSuite() {
 	pub2, err := account2.GetPubKey()
 	s.Require().NoError(err)
 
-	// Create a dummy account for testing purpose
-	_, _, err = kb.NewMnemonic("dummyAccount", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
-	s.Require().NoError(err)
-
 	multi := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pub1, pub2})
 	_, err = kb.SaveMultisig("multi", multi)
 	s.Require().NoError(err)
@@ -942,10 +938,6 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
 	multisigRecord, err := val1.ClientCtx.Keyring.Key("multi")
 	s.Require().NoError(err)
 
-	// Generate dummy account which is not a part of multisig.
-	dummyAcc, err := val1.ClientCtx.Keyring.Key("dummyAccount")
-	s.Require().NoError(err)
-
 	addr, err := multisigRecord.GetAddress()
 	s.Require().NoError(err)
 	resp, err := bankcli.QueryBalancesExec(val1.ClientCtx, addr)
@@ -1003,21 +995,14 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
 
 	sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String())
 
-	// Sign with account2
+	// Sign with account1
 	addr2, err := account2.GetAddress()
 	s.Require().NoError(err)
 	account2Signature, err := TxSignExec(val1.ClientCtx, addr2, multiGeneratedTxFile.Name(), "--multisig", addr.String())
 	s.Require().NoError(err)
 
 	sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())
 
-	// Sign with dummy account
-	dummyAddr, err := dummyAcc.GetAddress()
-	s.Require().NoError(err)
-	_, err = TxSignExec(val1.ClientCtx, dummyAddr, multiGeneratedTxFile.Name(), "--multisig", addr.String())
-	s.Require().Error(err)
-	s.Require().Contains(err.Error(), "from key is not a part of multisig key")
-
 	multiSigWith2Signatures, err := TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name())
 	s.Require().NoError(err)
 
@@ -1072,7 +1057,7 @@ func (s *IntegrationTestSuite) TestSignWithMultisig() {
 	// as the main point of this test is to test the `--multisig` flag with an address
 	// that is not in the keyring.
 	_, err = TxSignExec(val1.ClientCtx, addr1, multiGeneratedTx2File.Name(), "--multisig", multisigAddr.String())
-	s.Require().Contains(err.Error(), "error getting account from keybase")
+	s.Require().Contains(err.Error(), "tx intended signer does not match the given signer")
 }
 
 func (s *IntegrationTestSuite) TestCLIMultisign() {
@@ -1139,7 +1124,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {
 
 	addr2, err := account2.GetAddress()
 	s.Require().NoError(err)
-	// Sign with account2
+	// Sign with account1
 	account2Signature, err := TxSignExec(val1.ClientCtx, addr2, multiGeneratedTxFile.Name(), "--multisig", addr.String())
 	s.Require().NoError(err)