diff --git a/.github/workflows/release-sims.yml b/.github/workflows/release-sims.yml index f0037a1ad1..2009c15c8e 100644 --- a/.github/workflows/release-sims.yml +++ b/.github/workflows/release-sims.yml @@ -43,7 +43,7 @@ jobs: - name: install runsim run: | export GO111MODULE="on" && go install github.com/cosmos/tools/cmd/runsim@v1.0.0 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -56,7 +56,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version: '1.20' - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -76,7 +76,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version: '1.20' - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -96,7 +96,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version: '1.20' - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index 78e72c430c..f03031f1a0 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -42,7 +42,7 @@ jobs: run: go version - name: Install runsim run: export GO111MODULE="on" && go install github.com/cosmos/tools/cmd/runsim@v1.0.0 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -64,7 +64,7 @@ jobs: !**/**_test.go go.mod go.sum - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -93,7 +93,7 @@ jobs: go.sum SET_ENV_NAME_INSERTIONS: 1 SET_ENV_NAME_LINES: 1 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -122,7 +122,7 @@ jobs: go.sum SET_ENV_NAME_INSERTIONS: 1 SET_ENV_NAME_LINES: 1 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -151,7 +151,7 @@ jobs: go.sum SET_ENV_NAME_INSERTIONS: 1 SET_ENV_NAME_LINES: 1 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary diff --git a/.github/workflows/sims_normal.yml b/.github/workflows/sims_normal.yml index f6883460a0..8a0f92e925 100644 --- a/.github/workflows/sims_normal.yml +++ b/.github/workflows/sims_normal.yml @@ -29,7 +29,7 @@ jobs: run: go version - name: Install runsim run: export GO111MODULE="on" && go install github.com/cosmos/tools/cmd/runsim@v1.0.0 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -44,7 +44,7 @@ jobs: go-version: '1.20' - name: Display go version run: go version - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -62,7 +62,7 @@ jobs: go-version: '1.20' - name: Display go version run: go version - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary @@ -80,7 +80,7 @@ jobs: go-version: '1.20' - name: Display go version run: go version - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-runsim-binary diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5adbc8afa6..ce48eb8bc1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,7 +29,7 @@ jobs: - name: install tparse run: | go install github.com/mfridman/tparse@v0.8.3 - - uses: actions/cache@v4.0.0 + - uses: actions/cache@v4.0.2 with: path: ~/go/bin key: ${{ runner.os }}-go-tparse-binary diff --git a/CHANGELOG.md b/CHANGELOG.md index 709b043bdd..3b889797b0 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 * (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis ### 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()) }) } }