Skip to content

Commit

Permalink
fix: use accAddress to compare in validatebasic function in collectio…
Browse files Browse the repository at this point in the history 
…n & token modules (#1288)

* fix: use accAddress to compare in validatebasic function in collection & token modules

Signed-off-by: 170210 <j170210@icloud.com>

* chore: add test cases

Signed-off-by: 170210 <j170210@icloud.com>

* chore: update CHANGELOG.md

Signed-off-by: 170210 <j170210@icloud.com>

* fixup: fix for comment

Signed-off-by: 170210 <j170210@icloud.com>

* fixup: add test cases

Signed-off-by: 170210 <j170210@icloud.com>

---------

Signed-off-by: 170210 <j170210@icloud.com>
  • Loading branch information
@170210
170210 authored Mar 21, 2024
1 parent 0930944 commit 819a938
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 12 additions & 6 deletions x/collection/msgs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
52 changes: 38 additions & 14 deletions x/collection/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -407,36 +407,48 @@ 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)
if tc.err != nil {
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": {
Expand All @@ -459,22 +471,34 @@ 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)
if tc.err != nil {
return
}

require.Equal(t, []sdk.AccAddress{tc.holder}, msg.GetSigners())
require.Equal(t, []sdk.AccAddress{sdk.MustAccAddressFromBech32(tc.holder)}, msg.GetSigners())
})
}
}
Expand Down
18 changes: 12 additions & 6 deletions x/token/msgs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
40 changes: 26 additions & 14 deletions x/token/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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()
Expand All @@ -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": {
Expand Down Expand Up @@ -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()
Expand All @@ -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())
})
}
}
Expand Down

0 comments on commit 819a938

Please sign in to comment.