From d7d37e95a8f0f6bb21fbd893c10a82a3ec9c366e Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 28 May 2021 16:13:18 +0530 Subject: [PATCH 1/9] update `GetSigners` to return []string --- server/rosetta/converter.go | 2 +- testutil/testdata/tx.go | 18 +++---------- types/tx/types.go | 11 +++++--- types/tx_msg.go | 2 +- x/auth/legacy/legacytx/stdtx.go | 10 ++++--- x/auth/vesting/types/msgs.go | 8 ++---- x/authz/keeper/keeper.go | 6 +++-- x/authz/msgs.go | 24 +++++------------ x/bank/types/msgs.go | 15 ++++------- x/crisis/types/msgs.go | 5 ++-- x/distribution/types/msg.go | 28 ++++++-------------- x/evidence/types/msgs.go | 9 ++----- x/feegrant/msgs.go | 16 +++--------- x/feegrant/msgs_test.go | 4 +-- x/gov/types/msgs.go | 20 ++++++-------- x/slashing/types/msg.go | 4 +-- x/staking/types/msg.go | 46 +++++++++++---------------------- 17 files changed, 81 insertions(+), 147 deletions(-) diff --git a/server/rosetta/converter.go b/server/rosetta/converter.go index 4fdd87c75b67..54808821e247 100644 --- a/server/rosetta/converter.go +++ b/server/rosetta/converter.go @@ -254,7 +254,7 @@ func (c converter) Ops(status string, msg sdk.Msg) ([]*rosettatypes.Operation, e op := &rosettatypes.Operation{ Type: opName, Status: &status, - Account: &rosettatypes.AccountIdentifier{Address: signer.String()}, + Account: &rosettatypes.AccountIdentifier{Address: signer}, Metadata: meta, } diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index 653400ffb67c..d4d2a758ebfb 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -62,22 +62,12 @@ func (msg *TestMsg) GetSignBytes() []byte { } return sdk.MustSortJSON(bz) } -func (msg *TestMsg) GetSigners() []sdk.AccAddress { - addrs := make([]sdk.AccAddress, len(msg.Signers)) - for i, in := range msg.Signers { - addr, err := sdk.AccAddressFromBech32(in) - if err != nil { - panic(err) - } - - addrs[i] = addr - } - - return addrs +func (msg *TestMsg) GetSigners() []string { + return msg.Signers } func (msg *TestMsg) ValidateBasic() error { return nil } var _ sdk.Msg = &MsgCreateDog{} -func (msg *MsgCreateDog) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{} } -func (msg *MsgCreateDog) ValidateBasic() error { return nil } +func (msg *MsgCreateDog) GetSigners() []string { return []string{} } +func (msg *MsgCreateDog) ValidateBasic() error { return nil } diff --git a/types/tx/types.go b/types/tx/types.go index 84ce81edcbf8..a35e9f27ddea 100644 --- a/types/tx/types.go +++ b/types/tx/types.go @@ -101,9 +101,14 @@ func (t *Tx) GetSigners() []sdk.AccAddress { for _, msg := range t.GetMsgs() { for _, addr := range msg.GetSigners() { - if !seen[addr.String()] { - signers = append(signers, addr) - seen[addr.String()] = true + if !seen[addr] { + signer, err := sdk.AccAddressFromBech32(addr) + if err != nil { + panic(err) + } + + signers = append(signers, signer) + seen[addr] = true } } } diff --git a/types/tx_msg.go b/types/tx_msg.go index d7f15e83f078..80ba0a22c2ed 100644 --- a/types/tx_msg.go +++ b/types/tx_msg.go @@ -20,7 +20,7 @@ type ( // Signers returns the addrs of signers that must sign. // CONTRACT: All signatures must be present to be valid. // CONTRACT: Returns addrs in some deterministic order. - GetSigners() []AccAddress + GetSigners() []string } // Fee defines an interface for an application application-defined concrete diff --git a/x/auth/legacy/legacytx/stdtx.go b/x/auth/legacy/legacytx/stdtx.go index 55ec38e603cc..d213fcac9c6b 100644 --- a/x/auth/legacy/legacytx/stdtx.go +++ b/x/auth/legacy/legacytx/stdtx.go @@ -143,9 +143,13 @@ func (tx StdTx) GetSigners() []sdk.AccAddress { for _, msg := range tx.GetMsgs() { for _, addr := range msg.GetSigners() { - if !seen[addr.String()] { - signers = append(signers, addr) - seen[addr.String()] = true + if !seen[addr] { + signer, err := sdk.AccAddressFromBech32(addr) + if err != nil { + panic(err) + } + signers = append(signers, signer) + seen[addr] = true } } } diff --git a/x/auth/vesting/types/msgs.go b/x/auth/vesting/types/msgs.go index 3308e0304881..51c85f0695e1 100644 --- a/x/auth/vesting/types/msgs.go +++ b/x/auth/vesting/types/msgs.go @@ -68,10 +68,6 @@ func (msg MsgCreateVestingAccount) GetSignBytes() []byte { } // GetSigners returns the expected signers for a MsgCreateVestingAccount. -func (msg MsgCreateVestingAccount) GetSigners() []sdk.AccAddress { - from, err := sdk.AccAddressFromBech32(msg.FromAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{from} +func (msg MsgCreateVestingAccount) GetSigners() []string { + return []string{msg.FromAddress} } diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go index 10d516040029..67985887a9df 100644 --- a/x/authz/keeper/keeper.go +++ b/x/authz/keeper/keeper.go @@ -74,13 +74,15 @@ func (k Keeper) update(ctx sdk.Context, grantee sdk.AccAddress, granter sdk.AccA // grants from the message signer to the grantee. func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []sdk.Msg) (*sdk.Result, error) { var msgResult *sdk.Result - var err error for _, msg := range msgs { signers := msg.GetSigners() if len(signers) != 1 { return nil, sdkerrors.ErrInvalidRequest.Wrap("authorization can be given to msg with only one signer") } - granter := signers[0] + granter, err := sdk.AccAddressFromBech32(signers[0]) + if err != nil { + return nil, err + } // if granter != grantee then check authorization.Accept, otherwise we implicitly accept. if !granter.Equals(grantee) { authorization, _ := k.GetCleanAuthorization(ctx, grantee, granter, sdk.MsgTypeURL(msg)) diff --git a/x/authz/msgs.go b/x/authz/msgs.go index dd6bc8b19deb..c66ca6963f72 100644 --- a/x/authz/msgs.go +++ b/x/authz/msgs.go @@ -35,12 +35,8 @@ func NewMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a Authorization } // GetSigners implements Msg -func (msg MsgGrant) GetSigners() []sdk.AccAddress { - granter, err := sdk.AccAddressFromBech32(msg.Granter) - if err != nil { - panic(err) - } - return []sdk.AccAddress{granter} +func (msg MsgGrant) GetSigners() []string { + return []string{msg.Granter} } // ValidateBasic implements Msg @@ -108,12 +104,8 @@ func NewMsgRevoke(granter sdk.AccAddress, grantee sdk.AccAddress, msgTypeURL str } // GetSigners implements Msg -func (msg MsgRevoke) GetSigners() []sdk.AccAddress { - granter, err := sdk.AccAddressFromBech32(msg.Granter) - if err != nil { - panic(err) - } - return []sdk.AccAddress{granter} +func (msg MsgRevoke) GetSigners() []string { + return []string{msg.Granter} } // ValidateBasic implements MsgRequest.ValidateBasic @@ -172,12 +164,8 @@ func (msg MsgExec) GetMessages() ([]sdk.Msg, error) { } // GetSigners implements Msg -func (msg MsgExec) GetSigners() []sdk.AccAddress { - grantee, err := sdk.AccAddressFromBech32(msg.Grantee) - if err != nil { - panic(err) - } - return []sdk.AccAddress{grantee} +func (msg MsgExec) GetSigners() []string { + return []string{msg.Grantee} } // ValidateBasic implements Msg diff --git a/x/bank/types/msgs.go b/x/bank/types/msgs.go index 22fdc30b3886..9a7d6fccedd8 100644 --- a/x/bank/types/msgs.go +++ b/x/bank/types/msgs.go @@ -54,12 +54,8 @@ func (msg MsgSend) GetSignBytes() []byte { } // GetSigners Implements Msg. -func (msg MsgSend) GetSigners() []sdk.AccAddress { - from, err := sdk.AccAddressFromBech32(msg.FromAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{from} +func (msg MsgSend) GetSigners() []string { + return []string{msg.FromAddress} } var _ sdk.Msg = &MsgMultiSend{} @@ -96,11 +92,10 @@ func (msg MsgMultiSend) GetSignBytes() []byte { } // GetSigners Implements Msg. -func (msg MsgMultiSend) GetSigners() []sdk.AccAddress { - addrs := make([]sdk.AccAddress, len(msg.Inputs)) +func (msg MsgMultiSend) GetSigners() []string { + addrs := make([]string, len(msg.Inputs)) for i, in := range msg.Inputs { - addr, _ := sdk.AccAddressFromBech32(in.Address) - addrs[i] = addr + addrs[i] = in.Address } return addrs diff --git a/x/crisis/types/msgs.go b/x/crisis/types/msgs.go index 6161a1d4b73e..f21fd5e500a0 100644 --- a/x/crisis/types/msgs.go +++ b/x/crisis/types/msgs.go @@ -21,9 +21,8 @@ func (msg MsgVerifyInvariant) Route() string { return ModuleName } func (msg MsgVerifyInvariant) Type() string { return "verify_invariant" } // get the bytes for the message signer to sign on -func (msg MsgVerifyInvariant) GetSigners() []sdk.AccAddress { - sender, _ := sdk.AccAddressFromBech32(msg.Sender) - return []sdk.AccAddress{sender} +func (msg MsgVerifyInvariant) GetSigners() []string { + return []string{msg.Sender} } // GetSignBytes gets the sign bytes for the msg MsgVerifyInvariant diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index 09e9994c16a9..ad9c6afe8feb 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -27,12 +27,8 @@ func (msg MsgSetWithdrawAddress) Route() string { return ModuleName } func (msg MsgSetWithdrawAddress) Type() string { return TypeMsgSetWithdrawAddress } // Return address that must sign over msg.GetSignBytes() -func (msg MsgSetWithdrawAddress) GetSigners() []sdk.AccAddress { - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{delAddr} +func (msg MsgSetWithdrawAddress) GetSigners() []string { + return []string{msg.DelegatorAddress} } // get the bytes for the message signer to sign on @@ -64,12 +60,8 @@ func (msg MsgWithdrawDelegatorReward) Route() string { return ModuleName } func (msg MsgWithdrawDelegatorReward) Type() string { return TypeMsgWithdrawDelegatorReward } // Return address that must sign over msg.GetSignBytes() -func (msg MsgWithdrawDelegatorReward) GetSigners() []sdk.AccAddress { - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{delAddr} +func (msg MsgWithdrawDelegatorReward) GetSigners() []string { + return []string{msg.DelegatorAddress} } // get the bytes for the message signer to sign on @@ -99,12 +91,12 @@ func (msg MsgWithdrawValidatorCommission) Route() string { return ModuleName } func (msg MsgWithdrawValidatorCommission) Type() string { return TypeMsgWithdrawValidatorCommission } // Return address that must sign over msg.GetSignBytes() -func (msg MsgWithdrawValidatorCommission) GetSigners() []sdk.AccAddress { +func (msg MsgWithdrawValidatorCommission) GetSigners() []string { valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { panic(err) } - return []sdk.AccAddress{valAddr.Bytes()} + return []string{sdk.AccAddress(valAddr).String()} } // get the bytes for the message signer to sign on @@ -138,12 +130,8 @@ func (msg MsgFundCommunityPool) Type() string { return TypeMsgFundCommunityPool // GetSigners returns the signer addresses that are expected to sign the result // of GetSignBytes. -func (msg MsgFundCommunityPool) GetSigners() []sdk.AccAddress { - depoAddr, err := sdk.AccAddressFromBech32(msg.Depositor) - if err != nil { - panic(err) - } - return []sdk.AccAddress{depoAddr} +func (msg MsgFundCommunityPool) GetSigners() []string { + return []string{msg.Depositor} } // GetSignBytes returns the raw bytes for a MsgFundCommunityPool message that diff --git a/x/evidence/types/msgs.go b/x/evidence/types/msgs.go index cd2e2ba0321d..7581f6e09f76 100644 --- a/x/evidence/types/msgs.go +++ b/x/evidence/types/msgs.go @@ -66,13 +66,8 @@ func (m MsgSubmitEvidence) GetSignBytes() []byte { } // GetSigners returns the single expected signer for a MsgSubmitEvidence. -func (m MsgSubmitEvidence) GetSigners() []sdk.AccAddress { - accAddr, err := sdk.AccAddressFromBech32(m.Submitter) - if err != nil { - return nil - } - - return []sdk.AccAddress{accAddr} +func (m MsgSubmitEvidence) GetSigners() []string { + return []string{m.Submitter} } func (m MsgSubmitEvidence) GetEvidence() exported.Evidence { diff --git a/x/feegrant/msgs.go b/x/feegrant/msgs.go index 3731668a91a5..5ac5d6735b15 100644 --- a/x/feegrant/msgs.go +++ b/x/feegrant/msgs.go @@ -53,12 +53,8 @@ func (msg MsgGrantAllowance) ValidateBasic() error { } // GetSigners gets the granter account associated with an allowance -func (msg MsgGrantAllowance) GetSigners() []sdk.AccAddress { - granter, err := sdk.AccAddressFromBech32(msg.Granter) - if err != nil { - panic(err) - } - return []sdk.AccAddress{granter} +func (msg MsgGrantAllowance) GetSigners() []string { + return []string{msg.Granter} } // GetFeeAllowanceI returns unpacked FeeAllowance @@ -101,10 +97,6 @@ func (msg MsgRevokeAllowance) ValidateBasic() error { // GetSigners gets the granter address associated with an Allowance // to revoke. -func (msg MsgRevokeAllowance) GetSigners() []sdk.AccAddress { - granter, err := sdk.AccAddressFromBech32(msg.Granter) - if err != nil { - panic(err) - } - return []sdk.AccAddress{granter} +func (msg MsgRevokeAllowance) GetSigners() []string { + return []string{msg.Granter} } diff --git a/x/feegrant/msgs_test.go b/x/feegrant/msgs_test.go index c5eceb3b6bd3..2e7d8e654dd7 100644 --- a/x/feegrant/msgs_test.go +++ b/x/feegrant/msgs_test.go @@ -64,7 +64,7 @@ func TestMsgGrantAllowance(t *testing.T) { require.NoError(t, err) addrSlice := msg.GetSigners() - require.Equal(t, tc.granter.String(), addrSlice[0].String()) + require.Equal(t, tc.granter.String(), addrSlice[0]) allowance, err := msg.GetFeeAllowanceI() require.NoError(t, err) @@ -126,7 +126,7 @@ func TestMsgRevokeAllowance(t *testing.T) { if tc.valid { require.NoError(t, err) addrSlice := msg.GetSigners() - require.Equal(t, tc.granter.String(), addrSlice[0].String()) + require.Equal(t, tc.granter.String(), addrSlice[0]) } else { require.Error(t, err) } diff --git a/x/gov/types/msgs.go b/x/gov/types/msgs.go index f1c351e6ddca..ba86026637c2 100644 --- a/x/gov/types/msgs.go +++ b/x/gov/types/msgs.go @@ -114,9 +114,8 @@ func (m MsgSubmitProposal) GetSignBytes() []byte { } // GetSigners implements Msg -func (m MsgSubmitProposal) GetSigners() []sdk.AccAddress { - proposer, _ := sdk.AccAddressFromBech32(m.Proposer) - return []sdk.AccAddress{proposer} +func (m MsgSubmitProposal) GetSigners() []string { + return []string{m.Proposer} } // String implements the Stringer interface @@ -171,9 +170,8 @@ func (msg MsgDeposit) GetSignBytes() []byte { } // GetSigners implements Msg -func (msg MsgDeposit) GetSigners() []sdk.AccAddress { - depositor, _ := sdk.AccAddressFromBech32(msg.Depositor) - return []sdk.AccAddress{depositor} +func (msg MsgDeposit) GetSigners() []string { + return []string{msg.Depositor} } // NewMsgVote creates a message to cast a vote on an active proposal @@ -214,9 +212,8 @@ func (msg MsgVote) GetSignBytes() []byte { } // GetSigners implements Msg -func (msg MsgVote) GetSigners() []sdk.AccAddress { - voter, _ := sdk.AccAddressFromBech32(msg.Voter) - return []sdk.AccAddress{voter} +func (msg MsgVote) GetSigners() []string { + return []string{msg.Voter} } // NewMsgVoteWeighted creates a message to cast a vote on an active proposal @@ -278,7 +275,6 @@ func (msg MsgVoteWeighted) GetSignBytes() []byte { } // GetSigners implements Msg -func (msg MsgVoteWeighted) GetSigners() []sdk.AccAddress { - voter, _ := sdk.AccAddressFromBech32(msg.Voter) - return []sdk.AccAddress{voter} +func (msg MsgVoteWeighted) GetSigners() []string { + return []string{msg.Voter} } diff --git a/x/slashing/types/msg.go b/x/slashing/types/msg.go index d86ff5eb7249..b24be89b97e5 100644 --- a/x/slashing/types/msg.go +++ b/x/slashing/types/msg.go @@ -22,12 +22,12 @@ func NewMsgUnjail(validatorAddr sdk.ValAddress) *MsgUnjail { func (msg MsgUnjail) Route() string { return RouterKey } func (msg MsgUnjail) Type() string { return TypeMsgUnjail } -func (msg MsgUnjail) GetSigners() []sdk.AccAddress { +func (msg MsgUnjail) GetSigners() []string { valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddr) if err != nil { panic(err) } - return []sdk.AccAddress{valAddr.Bytes()} + return []string{sdk.AccAddress(valAddr).String()} } // GetSignBytes gets the bytes for the message signer to sign on diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index af60ac370d24..5dc76ae2e7c5 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -1,8 +1,6 @@ package types import ( - "bytes" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -62,19 +60,17 @@ func (msg MsgCreateValidator) Type() string { return TypeMsgCreateValidator } // must sign over msg.GetSignBytes(). // If the validator address is not same as delegator's, then the validator must // sign the msg as well. -func (msg MsgCreateValidator) GetSigners() []sdk.AccAddress { +func (msg MsgCreateValidator) GetSigners() []string { // delegator is first signer so delegator pays fees - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - addrs := []sdk.AccAddress{delAddr} - addr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) + addrs := []string{msg.DelegatorAddress} + valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { panic(err) } - if !bytes.Equal(delAddr.Bytes(), addr.Bytes()) { - addrs = append(addrs, sdk.AccAddress(addr)) + + valAccAddr := sdk.AccAddress(valAddr).String() + if msg.DelegatorAddress != valAccAddr { + addrs = append(addrs, valAccAddr) } return addrs @@ -167,12 +163,12 @@ func (msg MsgEditValidator) Route() string { return RouterKey } func (msg MsgEditValidator) Type() string { return TypeMsgEditValidator } // GetSigners implements the sdk.Msg interface. -func (msg MsgEditValidator) GetSigners() []sdk.AccAddress { +func (msg MsgEditValidator) GetSigners() []string { valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { panic(err) } - return []sdk.AccAddress{valAddr.Bytes()} + return []string{sdk.AccAddress(valAddr).String()} } // GetSignBytes implements the sdk.Msg interface. @@ -224,12 +220,8 @@ func (msg MsgDelegate) Route() string { return RouterKey } func (msg MsgDelegate) Type() string { return TypeMsgDelegate } // GetSigners implements the sdk.Msg interface. -func (msg MsgDelegate) GetSigners() []sdk.AccAddress { - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{delAddr} +func (msg MsgDelegate) GetSigners() []string { + return []string{msg.DelegatorAddress} } // GetSignBytes implements the sdk.Msg interface. @@ -278,12 +270,8 @@ func (msg MsgBeginRedelegate) Route() string { return RouterKey } func (msg MsgBeginRedelegate) Type() string { return TypeMsgBeginRedelegate } // GetSigners implements the sdk.Msg interface -func (msg MsgBeginRedelegate) GetSigners() []sdk.AccAddress { - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{delAddr} +func (msg MsgBeginRedelegate) GetSigners() []string { + return []string{msg.DelegatorAddress} } // GetSignBytes implements the sdk.Msg interface. @@ -333,12 +321,8 @@ func (msg MsgUndelegate) Route() string { return RouterKey } func (msg MsgUndelegate) Type() string { return TypeMsgUndelegate } // GetSigners implements the sdk.Msg interface. -func (msg MsgUndelegate) GetSigners() []sdk.AccAddress { - delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddress) - if err != nil { - panic(err) - } - return []sdk.AccAddress{delAddr} +func (msg MsgUndelegate) GetSigners() []string { + return []string{msg.DelegatorAddress} } // GetSignBytes implements the sdk.Msg interface. From f7f56d70e89ddb2f43c3237bb096cf4b38b7a196 Mon Sep 17 00:00:00 2001 From: atheesh Date: Sat, 29 May 2021 13:05:24 +0530 Subject: [PATCH 2/9] fix failing tests --- server/mock/tx.go | 2 +- types/tx_msg_test.go | 2 +- x/bank/types/msgs_test.go | 43 +++++++++++------------------------ x/evidence/types/msgs_test.go | 2 +- 4 files changed, 16 insertions(+), 33 deletions(-) diff --git a/server/mock/tx.go b/server/mock/tx.go index 0cb79c28986f..9bdadf46d0e9 100644 --- a/server/mock/tx.go +++ b/server/mock/tx.go @@ -58,7 +58,7 @@ func (tx kvstoreTx) ValidateBasic() error { return nil } -func (tx kvstoreTx) GetSigners() []sdk.AccAddress { +func (tx kvstoreTx) GetSigners() []string { return nil } diff --git a/types/tx_msg_test.go b/types/tx_msg_test.go index 7a55c7d63e64..2d7f98adf87a 100644 --- a/types/tx_msg_test.go +++ b/types/tx_msg_test.go @@ -23,7 +23,7 @@ func (s *testMsgSuite) TestMsg() { msg := testdata.NewTestMsg(accAddr) s.Require().NotNil(msg) - s.Require().Equal([]sdk.AccAddress{accAddr}, msg.GetSigners()) + s.Require().Equal([]string{accAddr.String()}, msg.GetSigners()) s.Require().Equal("TestMsg", msg.Route()) s.Require().Equal("Test message", msg.Type()) s.Require().Nil(msg.ValidateBasic()) diff --git a/x/bank/types/msgs_test.go b/x/bank/types/msgs_test.go index 334d72c13243..0deaf09b2932 100644 --- a/x/bank/types/msgs_test.go +++ b/x/bank/types/msgs_test.go @@ -65,13 +65,6 @@ func TestMsgSendGetSignBytes(t *testing.T) { require.Equal(t, expected, string(res)) } -func TestMsgSendGetSigners(t *testing.T) { - var msg = NewMsgSend(sdk.AccAddress([]byte("input111111111111111")), sdk.AccAddress{}, sdk.NewCoins()) - res := msg.GetSigners() - // TODO: fix this ! - require.Equal(t, fmt.Sprintf("%v", res), "[696E707574313131313131313131313131313131]") -} - func TestMsgMultiSendRoute(t *testing.T) { // Construct a MsgSend addr1 := sdk.AccAddress([]byte("input")) @@ -238,32 +231,22 @@ func TestMsgMultiSendGetSignBytes(t *testing.T) { } func TestMsgMultiSendGetSigners(t *testing.T) { - var msg = MsgMultiSend{ - Inputs: []Input{ - NewInput(sdk.AccAddress([]byte("input111111111111111")), nil), - NewInput(sdk.AccAddress([]byte("input222222222222222")), nil), - NewInput(sdk.AccAddress([]byte("input333333333333333")), nil), - }, + addrs := make([]string, 3) + inputs := make([]Input, 3) + for i, v := range []string{"input111111111111111", "input222222222222222", "input333333333333333"} { + addr := sdk.AccAddress([]byte(v)) + inputs[i] = NewInput(addr, nil) + addrs[i] = addr.String() } + var msg = NewMsgMultiSend(inputs, nil) res := msg.GetSigners() - // TODO: fix this ! - require.Equal(t, "[696E707574313131313131313131313131313131 696E707574323232323232323232323232323232 696E707574333333333333333333333333333333]", fmt.Sprintf("%v", res)) + require.Equal(t, fmt.Sprintf("%v", addrs), fmt.Sprintf("%v", res)) } -func TestMsgSendSigners(t *testing.T) { - signers := []sdk.AccAddress{ - {1, 2, 3}, - {4, 5, 6}, - {7, 8, 9}, - } - - someCoins := sdk.NewCoins(sdk.NewInt64Coin("atom", 123)) - inputs := make([]Input, len(signers)) - for i, signer := range signers { - inputs[i] = NewInput(signer, someCoins) - } - tx := NewMsgMultiSend(inputs, nil) - - require.Equal(t, signers, tx.GetSigners()) +func TestMsgSendGetSigners(t *testing.T) { + from := sdk.AccAddress([]byte("input111111111111111")) + msg := NewMsgSend(from, sdk.AccAddress{}, sdk.NewCoins()) + res := msg.GetSigners() + require.Equal(t, fmt.Sprintf("%v", res), fmt.Sprintf("%v", []string{from.String()})) } diff --git a/x/evidence/types/msgs_test.go b/x/evidence/types/msgs_test.go index b591e7fc86ab..c0dcd224aebf 100644 --- a/x/evidence/types/msgs_test.go +++ b/x/evidence/types/msgs_test.go @@ -54,7 +54,7 @@ func TestMsgSubmitEvidence(t *testing.T) { require.Equal(t, tc.expectErr, tc.msg.ValidateBasic() != nil, "unexpected result for tc #%d", i) if !tc.expectErr { - require.Equal(t, tc.msg.GetSigners(), []sdk.AccAddress{tc.submitter}, "unexpected result for tc #%d", i) + require.Equal(t, tc.msg.GetSigners(), []string{tc.submitter.String()}, "unexpected result for tc #%d", i) } } } From f5fa9a219e8934647bf5fcb3826f82595069e158 Mon Sep 17 00:00:00 2001 From: atheesh Date: Sat, 29 May 2021 20:28:11 +0530 Subject: [PATCH 3/9] fix tests --- baseapp/baseapp_test.go | 30 +++++++++++++++--------------- x/genutil/client/testutil/suite.go | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 80d0f179efb3..d5da2b28bf26 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -717,10 +717,10 @@ func (msg msgCounter) String() string { return "TODO" } func (msg msgCounter) ProtoMessage() {} // Implements Msg -func (msg msgCounter) Route() string { return routeMsgCounter } -func (msg msgCounter) Type() string { return "counter1" } -func (msg msgCounter) GetSignBytes() []byte { return nil } -func (msg msgCounter) GetSigners() []sdk.AccAddress { return nil } +func (msg msgCounter) Route() string { return routeMsgCounter } +func (msg msgCounter) Type() string { return "counter1" } +func (msg msgCounter) GetSignBytes() []byte { return nil } +func (msg msgCounter) GetSigners() []string { return nil } func (msg msgCounter) ValidateBasic() error { if msg.Counter >= 0 { return nil @@ -762,10 +762,10 @@ func (msg msgCounter2) String() string { return "TODO" } func (msg msgCounter2) ProtoMessage() {} // Implements Msg -func (msg msgCounter2) Route() string { return routeMsgCounter2 } -func (msg msgCounter2) Type() string { return "counter2" } -func (msg msgCounter2) GetSignBytes() []byte { return nil } -func (msg msgCounter2) GetSigners() []sdk.AccAddress { return nil } +func (msg msgCounter2) Route() string { return routeMsgCounter2 } +func (msg msgCounter2) Type() string { return "counter2" } +func (msg msgCounter2) GetSignBytes() []byte { return nil } +func (msg msgCounter2) GetSigners() []string { return nil } func (msg msgCounter2) ValidateBasic() error { if msg.Counter >= 0 { return nil @@ -779,13 +779,13 @@ type msgKeyValue struct { Value []byte } -func (msg msgKeyValue) Reset() {} -func (msg msgKeyValue) String() string { return "TODO" } -func (msg msgKeyValue) ProtoMessage() {} -func (msg msgKeyValue) Route() string { return routeMsgKeyValue } -func (msg msgKeyValue) Type() string { return "keyValue" } -func (msg msgKeyValue) GetSignBytes() []byte { return nil } -func (msg msgKeyValue) GetSigners() []sdk.AccAddress { return nil } +func (msg msgKeyValue) Reset() {} +func (msg msgKeyValue) String() string { return "TODO" } +func (msg msgKeyValue) ProtoMessage() {} +func (msg msgKeyValue) Route() string { return routeMsgKeyValue } +func (msg msgKeyValue) Type() string { return "keyValue" } +func (msg msgKeyValue) GetSignBytes() []byte { return nil } +func (msg msgKeyValue) GetSigners() []string { return nil } func (msg msgKeyValue) ValidateBasic() error { if msg.Key == nil { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "key cannot be nil") diff --git a/x/genutil/client/testutil/suite.go b/x/genutil/client/testutil/suite.go index 4c5c38918506..119ab646eda9 100644 --- a/x/genutil/client/testutil/suite.go +++ b/x/genutil/client/testutil/suite.go @@ -85,7 +85,7 @@ func (s *IntegrationTestSuite) TestGenTxCmd() { s.Require().Len(msgs, 1) s.Require().Equal(sdk.MsgTypeURL(&types.MsgCreateValidator{}), sdk.MsgTypeURL(msgs[0])) - s.Require().Equal([]sdk.AccAddress{val.Address}, msgs[0].GetSigners()) + s.Require().Equal([]string{val.Address.String()}, msgs[0].GetSigners()) s.Require().Equal(amount, msgs[0].(*types.MsgCreateValidator).Value) err = tx.ValidateBasic() s.Require().NoError(err) From cd28bd443351fc04904105d44b5f92e907fe1538 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 1 Jun 2021 12:45:39 +0530 Subject: [PATCH 4/9] replace `ValidateBasic` with `ValidateMsg` --- client/tx/tx.go | 19 ++++++++++++++++++- x/distribution/client/cli/tx.go | 9 --------- x/gov/client/cli/tx.go | 5 ----- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/client/tx/tx.go b/client/tx/tx.go index 8e20be369bfd..1183a581122e 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -24,6 +24,23 @@ import ( authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" ) +func ValidateMsg(msg sdk.Msg) error { + err := msg.ValidateBasic() + if err != nil { + return err + } + + signers := msg.GetSigners() + for _, signer := range signers { + _, err = sdk.AccAddressFromBech32(signer) + if err != nil { + return err + } + } + + return nil +} + // GenerateOrBroadcastTxCLI will either generate and print and unsigned transaction // or sign it and broadcast it returning an error upon failure. func GenerateOrBroadcastTxCLI(clientCtx client.Context, flagSet *pflag.FlagSet, msgs ...sdk.Msg) error { @@ -39,7 +56,7 @@ func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msg // Right now, we're factorizing that call inside this function. // ref: https://github.com/cosmos/cosmos-sdk/pull/9236#discussion_r623803504 for _, msg := range msgs { - if err := msg.ValidateBasic(); err != nil { + if err := ValidateMsg(msg); err != nil { return err } } diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index ab81fc44e4a9..03f2163e33a4 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -110,12 +110,6 @@ $ %s tx distribution withdraw-rewards %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj msgs = append(msgs, types.NewMsgWithdrawValidatorCommission(valAddr)) } - for _, msg := range msgs { - if err := msg.ValidateBasic(); err != nil { - return err - } - } - return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msgs...) }, } @@ -169,9 +163,6 @@ $ %s tx distribution withdraw-all-rewards --from mykey } msg := types.NewMsgWithdrawDelegatorReward(delAddr, val) - if err := msg.ValidateBasic(); err != nil { - return err - } msgs = append(msgs, msg) } diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 7e19b4838891..c20df19f6927 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -275,11 +275,6 @@ $ %s tx gov weighted-vote 1 yes=0.6,no=0.3,abstain=0.05,no_with_veto=0.05 --from // Build vote message and run basic validation msg := types.NewMsgVoteWeighted(from, proposalID, options) - err = msg.ValidateBasic() - if err != nil { - return err - } - return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) }, } From 964acf4a5818f6c2f3963305aae27145762ed966 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 28 Jun 2021 17:08:52 +0530 Subject: [PATCH 5/9] add change log --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e5a759a8204..977ea6d32258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### API Breaking Changes + +* [\#9418](https://github.com/cosmos/cosmos-sdk/pull/9418) `sdk.Msg`'s `GetSigners()` method updated to return `[]string`. + ## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25 ### Features From b572a65e793ac2b649c4b870d4d309fa4f384424 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 13:46:31 +0530 Subject: [PATCH 6/9] add `ValidateMsg` in ante --- x/auth/ante/basic.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 52c219f79e4d..05d4da012dec 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -1,6 +1,7 @@ package ante import ( + txvalidate "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -11,7 +12,8 @@ import ( authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" ) -// ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error. +// ValidateBasicDecorator will call tx.ValidateBasic, ValidateMsg(for each msg inside tx) +// and return any non-nil error. // If ValidateBasic passes, decorator calls next AnteHandler in chain. Note, // ValidateBasicDecorator decorator will not get executed on ReCheckTx since it // is not dependent on application state. @@ -27,6 +29,12 @@ func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulat return next(ctx, tx, simulate) } + for _, msg := range tx.GetMsgs() { + if err := txvalidate.ValidateMsg(msg); err != nil { + return ctx, err + } + } + if err := tx.ValidateBasic(); err != nil { return ctx, err } From 72e5bfcbb5746958c95a3c19ac3c0912aa232a65 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 17:46:44 +0530 Subject: [PATCH 7/9] review changes --- baseapp/baseapp.go | 3 ++- types/tx_msg.go | 2 +- x/auth/ante/basic.go | 7 ------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 062a2bc8bf0a..b2218aaa43ef 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -13,6 +13,7 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" + txvalidate "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/snapshots" "github.com/cosmos/cosmos-sdk/store" @@ -508,7 +509,7 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error { } for _, msg := range msgs { - err := msg.ValidateBasic() + err := txvalidate.ValidateMsg(msg) if err != nil { return err } diff --git a/types/tx_msg.go b/types/tx_msg.go index 90745043ef50..ae84b9e4fed6 100644 --- a/types/tx_msg.go +++ b/types/tx_msg.go @@ -15,7 +15,7 @@ type ( // doesn't require access to any other information. ValidateBasic() error - // Signers returns the addrs of signers that must sign. + // Signers returns the bech32-encoded addrs of signers that must sign. // CONTRACT: All signatures must be present to be valid. // CONTRACT: Returns addrs in some deterministic order. GetSigners() []string diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 05d4da012dec..d456b8b1b230 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -1,7 +1,6 @@ package ante import ( - txvalidate "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -29,12 +28,6 @@ func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulat return next(ctx, tx, simulate) } - for _, msg := range tx.GetMsgs() { - if err := txvalidate.ValidateMsg(msg); err != nil { - return ctx, err - } - } - if err := tx.ValidateBasic(); err != nil { return ctx, err } From f9260182af43dc25222ce8454aa177a742ade414 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 18:22:56 +0530 Subject: [PATCH 8/9] move `ValidateMsg` to `types/tx` --- baseapp/baseapp.go | 4 ++-- client/tx/tx.go | 19 +------------------ types/tx/types.go | 19 +++++++++++++++++++ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b2218aaa43ef..ceb553dd7d62 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -13,13 +13,13 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" - txvalidate "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/snapshots" "github.com/cosmos/cosmos-sdk/store" "github.com/cosmos/cosmos-sdk/store/rootmulti" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" ) @@ -509,7 +509,7 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error { } for _, msg := range msgs { - err := txvalidate.ValidateMsg(msg) + err := sdktx.ValidateMsg(msg) if err != nil { return err } diff --git a/client/tx/tx.go b/client/tx/tx.go index fff4b242e515..b6dffd727a0b 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -20,23 +20,6 @@ import ( authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" ) -func ValidateMsg(msg sdk.Msg) error { - err := msg.ValidateBasic() - if err != nil { - return err - } - - signers := msg.GetSigners() - for _, signer := range signers { - _, err = sdk.AccAddressFromBech32(signer) - if err != nil { - return err - } - } - - return nil -} - // GenerateOrBroadcastTxCLI will either generate and print and unsigned transaction // or sign it and broadcast it returning an error upon failure. func GenerateOrBroadcastTxCLI(clientCtx client.Context, flagSet *pflag.FlagSet, msgs ...sdk.Msg) error { @@ -52,7 +35,7 @@ func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msg // Right now, we're factorizing that call inside this function. // ref: https://github.com/cosmos/cosmos-sdk/pull/9236#discussion_r623803504 for _, msg := range msgs { - if err := ValidateMsg(msg); err != nil { + if err := tx.ValidateMsg(msg); err != nil { return err } } diff --git a/types/tx/types.go b/types/tx/types.go index a35e9f27ddea..2982a60b2bcf 100644 --- a/types/tx/types.go +++ b/types/tx/types.go @@ -207,3 +207,22 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { registry.RegisterInterface("cosmos.tx.v1beta1.Tx", (*sdk.Tx)(nil)) registry.RegisterImplementations((*sdk.Tx)(nil), &Tx{}) } + +// ValidateMsg calls the `sdk.Msg.ValidateBasic()` +// also validates all the signers are valid bech32 addresses. +func ValidateMsg(msg sdk.Msg) error { + err := msg.ValidateBasic() + if err != nil { + return err + } + + signers := msg.GetSigners() + for _, signer := range signers { + _, err = sdk.AccAddressFromBech32(signer) + if err != nil { + return err + } + } + + return nil +} From 52ec9b091745a6b9e9bd04845d8ff7c9c8d63729 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 7 Jul 2021 15:36:06 +0530 Subject: [PATCH 9/9] add test case --- types/tx/types_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 types/tx/types_test.go diff --git a/types/tx/types_test.go b/types/tx/types_test.go new file mode 100644 index 000000000000..c5f2660ce2ce --- /dev/null +++ b/types/tx/types_test.go @@ -0,0 +1,43 @@ +package tx_test + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/testutil/testdata" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/stretchr/testify/suite" +) + +type testMsgSuite struct { + suite.Suite +} + +func TestValidateMsg(t *testing.T) { + suite.Run(t, new(testMsgSuite)) +} + +func (s *testMsgSuite) TestMsg() { + cases := []struct { + signer []byte + expErr bool + }{ + { + []byte(""), + true, + }, + { + []byte("validAddress"), + false, + }, + } + + for _, c := range cases { + msg := testdata.NewTestMsg(c.signer) + err := tx.ValidateMsg(msg) + if c.expErr { + s.Require().Error(err) + } else { + s.Require().NoError(err) + } + } +}