diff --git a/CHANGELOG.md b/CHANGELOG.md index d14bde6448..38defd73bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. * (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis * (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic +* (x/collection, x/token) [\#1288](https://github.com/Finschia/finschia-sdk/pull/1288) use accAddress to compare in validatebasic function in collection & token modules ### Removed diff --git a/x/collection/msgs.go b/x/collection/msgs.go index 4c416335f0..3da74a2c0e 100644 --- a/x/collection/msgs.go +++ b/x/collection/msgs.go @@ -428,14 +428,17 @@ func (m MsgAuthorizeOperator) ValidateBasic() error { return err } - if _, err := sdk.AccAddressFromBech32(m.Holder); err != nil { + holderAcc, err := sdk.AccAddressFromBech32(m.Holder) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid holder address: %s", m.Holder) } - if _, err := sdk.AccAddressFromBech32(m.Operator); err != nil { + + operatorAcc, err := sdk.AccAddressFromBech32(m.Operator) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid operator address: %s", m.Operator) } - if m.Operator == m.Holder { + if holderAcc.Equals(operatorAcc) { return ErrApproverProxySame } @@ -471,14 +474,17 @@ func (m MsgRevokeOperator) ValidateBasic() error { return err } - if _, err := sdk.AccAddressFromBech32(m.Holder); err != nil { + holderAcc, err := sdk.AccAddressFromBech32(m.Holder) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid holder address: %s", m.Holder) } - if _, err := sdk.AccAddressFromBech32(m.Operator); err != nil { + + operatorAcc, err := sdk.AccAddressFromBech32(m.Operator) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid operator address: %s", m.Operator) } - if m.Operator == m.Holder { + if holderAcc.Equals(operatorAcc) { return ErrApproverProxySame } diff --git a/x/collection/msgs_test.go b/x/collection/msgs_test.go index 236629d1ab..aff826fbf4 100644 --- a/x/collection/msgs_test.go +++ b/x/collection/msgs_test.go @@ -376,15 +376,15 @@ func TestMsgOperatorSendNFT(t *testing.T) { } func TestMsgAuthorizeOperator(t *testing.T) { - addrs := make([]sdk.AccAddress, 2) + addrs := make([]string, 2) for i := range addrs { - addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() } testCases := map[string]struct { contractID string - holder sdk.AccAddress - operator sdk.AccAddress + holder string + operator string err error }{ "valid msg": { @@ -407,14 +407,26 @@ func TestMsgAuthorizeOperator(t *testing.T) { holder: addrs[0], err: sdkerrors.ErrInvalidAddress, }, + "proxy and approver should be different": { + contractID: "deadbeef", + holder: addrs[0], + operator: addrs[0], + err: collection.ErrApproverProxySame, + }, + "proxy and approver should be different (uppercase)": { + contractID: "deadbeef", + holder: addrs[0], + operator: strings.ToUpper(addrs[0]), + err: collection.ErrApproverProxySame, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { msg := collection.MsgAuthorizeOperator{ ContractId: tc.contractID, - Holder: tc.holder.String(), - Operator: tc.operator.String(), + Holder: tc.holder, + Operator: tc.operator, } require.ErrorIs(t, msg.ValidateBasic(), tc.err) @@ -422,21 +434,21 @@ func TestMsgAuthorizeOperator(t *testing.T) { return } - require.Equal(t, []sdk.AccAddress{tc.holder}, msg.GetSigners()) + require.Equal(t, []sdk.AccAddress{sdk.MustAccAddressFromBech32(tc.holder)}, msg.GetSigners()) }) } } func TestMsgRevokeOperator(t *testing.T) { - addrs := make([]sdk.AccAddress, 2) + addrs := make([]string, 2) for i := range addrs { - addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() } testCases := map[string]struct { contractID string - holder sdk.AccAddress - operator sdk.AccAddress + holder string + operator string err error }{ "valid msg": { @@ -459,14 +471,26 @@ func TestMsgRevokeOperator(t *testing.T) { holder: addrs[0], err: sdkerrors.ErrInvalidAddress, }, + "proxy and approver should be different": { + contractID: "deadbeef", + holder: addrs[0], + operator: addrs[0], + err: collection.ErrApproverProxySame, + }, + "proxy and approver should be different (uppercase)": { + contractID: "deadbeef", + holder: addrs[0], + operator: strings.ToUpper(addrs[0]), + err: collection.ErrApproverProxySame, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { msg := collection.MsgRevokeOperator{ ContractId: tc.contractID, - Holder: tc.holder.String(), - Operator: tc.operator.String(), + Holder: tc.holder, + Operator: tc.operator, } require.ErrorIs(t, msg.ValidateBasic(), tc.err) @@ -474,7 +498,7 @@ func TestMsgRevokeOperator(t *testing.T) { return } - require.Equal(t, []sdk.AccAddress{tc.holder}, msg.GetSigners()) + require.Equal(t, []sdk.AccAddress{sdk.MustAccAddressFromBech32(tc.holder)}, msg.GetSigners()) }) } } diff --git a/x/token/msgs.go b/x/token/msgs.go index d3d315aa0a..7ba1b886e0 100644 --- a/x/token/msgs.go +++ b/x/token/msgs.go @@ -103,14 +103,17 @@ func (m MsgRevokeOperator) ValidateBasic() error { return err } - if _, err := sdk.AccAddressFromBech32(m.Holder); err != nil { + holderAcc, err := sdk.AccAddressFromBech32(m.Holder) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid holder address: %s", m.Holder) } - if _, err := sdk.AccAddressFromBech32(m.Operator); err != nil { + + operatorAcc, err := sdk.AccAddressFromBech32(m.Operator) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid operator address: %s", m.Operator) } - if m.Operator == m.Holder { + if holderAcc.Equals(operatorAcc) { return ErrApproverProxySame } @@ -146,14 +149,17 @@ func (m MsgAuthorizeOperator) ValidateBasic() error { return err } - if _, err := sdk.AccAddressFromBech32(m.Holder); err != nil { + holderAcc, err := sdk.AccAddressFromBech32(m.Holder) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid holder address: %s", m.Holder) } - if _, err := sdk.AccAddressFromBech32(m.Operator); err != nil { + + operatorAcc, err := sdk.AccAddressFromBech32(m.Operator) + if err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("invalid operator address: %s", m.Operator) } - if m.Operator == m.Holder { + if holderAcc.Equals(operatorAcc) { return ErrApproverProxySame } diff --git a/x/token/msgs_test.go b/x/token/msgs_test.go index 298dfb6c4b..9fe3dd4549 100644 --- a/x/token/msgs_test.go +++ b/x/token/msgs_test.go @@ -162,15 +162,15 @@ func TestMsgOperatorSend(t *testing.T) { } func TestMsgRevokeOperator(t *testing.T) { - addrs := make([]sdk.AccAddress, 2) + addrs := make([]string, 2) for i := range addrs { - addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() } testCases := map[string]struct { contractID string - holder sdk.AccAddress - operator sdk.AccAddress + holder string + operator string err error }{ "valid msg": { @@ -199,14 +199,20 @@ func TestMsgRevokeOperator(t *testing.T) { operator: addrs[0], err: token.ErrApproverProxySame, }, + "operator and holder should be different (uppercase)": { + contractID: "deadbeef", + holder: addrs[0], + operator: strings.ToUpper(addrs[0]), + err: token.ErrApproverProxySame, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { msg := token.MsgRevokeOperator{ ContractId: tc.contractID, - Holder: tc.holder.String(), - Operator: tc.operator.String(), + Holder: tc.holder, + Operator: tc.operator, } err := msg.ValidateBasic() @@ -215,21 +221,21 @@ func TestMsgRevokeOperator(t *testing.T) { return } - require.Equal(t, []sdk.AccAddress{tc.holder}, msg.GetSigners()) + require.Equal(t, []sdk.AccAddress{sdk.MustAccAddressFromBech32(tc.holder)}, msg.GetSigners()) }) } } func TestMsgAuthorizeOperator(t *testing.T) { - addrs := make([]sdk.AccAddress, 2) + addrs := make([]string, 2) for i := range addrs { - addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + addrs[i] = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() } testCases := map[string]struct { contractID string - holder sdk.AccAddress - operator sdk.AccAddress + holder string + operator string err error }{ "valid msg": { @@ -258,14 +264,20 @@ func TestMsgAuthorizeOperator(t *testing.T) { operator: addrs[0], err: token.ErrApproverProxySame, }, + "proxy and approver should be different (uppercase)": { + contractID: "deadbeef", + holder: addrs[0], + operator: strings.ToUpper(addrs[0]), + err: token.ErrApproverProxySame, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { msg := token.MsgAuthorizeOperator{ ContractId: tc.contractID, - Holder: tc.holder.String(), - Operator: tc.operator.String(), + Holder: tc.holder, + Operator: tc.operator, } err := msg.ValidateBasic() @@ -274,7 +286,7 @@ func TestMsgAuthorizeOperator(t *testing.T) { return } - require.Equal(t, []sdk.AccAddress{tc.holder}, msg.GetSigners()) + require.Equal(t, []sdk.AccAddress{sdk.MustAccAddressFromBech32(tc.holder)}, msg.GetSigners()) }) } }