Skip to content

Commit

Permalink
feat(x/circuit): Allow msg Reset with empty msgURL (#22459)
Browse files Browse the repository at this point in the history
(cherry picked from commit 42c616c)
  • Loading branch information
GNaD13 authored and mergify[bot] committed Nov 8, 2024
1 parent cb32699 commit 34bdf96
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 2 deletions.
1 change: 1 addition & 0 deletions x/circuit/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (deps) [#19810](https://github.com/cosmos/cosmos-sdk/pull/19810) Upgrade SDK version due to Prometheus breaking change.
* (deps) [#19810](https://github.com/cosmos/cosmos-sdk/pull/19810) Bump `cosmossdk.io/store` to v1.1.0.
* (feat) [#22459](https://github.com/cosmos/cosmos-sdk/pull/22459) Allow msg Reset with empty msgURL
* (feat) [#22460](https://github.com/cosmos/cosmos-sdk/pull/22460) Add validation for permission when an account is assigned and validation for msgURL

## [v0.1.0](https://github.com/cosmos/cosmos-sdk/releases/tag/x/circuit/v0.1.0) - 2023-11-07
34 changes: 32 additions & 2 deletions x/circuit/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"slices"
"strings"

"cosmossdk.io/collections"
Expand Down Expand Up @@ -143,6 +144,7 @@ func (srv msgServer) TripCircuitBreaker(ctx context.Context, msg *types.MsgTripC
// have been paused using TripCircuitBreaker.
func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgResetCircuitBreaker) (*types.MsgResetCircuitBreakerResponse, error) {
keeper := srv.Keeper
msgTypeUrls := types.MsgTypeURLValidation(msg.MsgTypeUrls)
address, err := srv.addressCodec.StringToBytes(msg.Authority)
if err != nil {
return nil, err
Expand All @@ -154,7 +156,35 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
return nil, err
}

msgTypeUrls := types.MsgTypeURLValidation(msg.MsgTypeUrls)
// check if msgURL is empty
if len(msgTypeUrls) == 0 {
switch {
case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()):
// if the sender is a super admin or the module authority, will remove all disabled msgs
err := srv.DisableList.Walk(ctx, nil, func(msgUrl string) (stop bool, err error) {
msgTypeUrls = append(msgTypeUrls, msgUrl)
return false, nil
})
if err != nil {
return nil, err
}

case perms.Level == types.Permissions_LEVEL_SOME_MSGS:
// if the sender has permission for some messages, will remove all disabled msgs that in the perms.LimitTypeUrls
err := srv.DisableList.Walk(ctx, nil, func(msgUrl string) (stop bool, err error) {
if slices.Contains(perms.LimitTypeUrls, msgUrl) {
msgTypeUrls = append(msgTypeUrls, msgUrl)
}
return false, nil
})
if err != nil {
return nil, err
}
default:
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker")
}
}

for _, msgTypeURL := range msgTypeUrls {
// check if the message is in the list of allowed messages
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
Expand Down Expand Up @@ -183,7 +213,7 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
}
}

urls := strings.Join(msg.GetMsgTypeUrls(), ",")
urls := strings.Join(msgTypeUrls, ",")

if err = srv.Keeper.EventService.EventManager(ctx).EmitKV(
"reset_circuit_breaker",
Expand Down
71 changes: 71 additions & 0 deletions x/circuit/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,3 +450,74 @@ func TestResetCircuitBreakerSomeMsgs(t *testing.T) {
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")
}

func TestResetCircuitBreakerEmptyMsgs(t *testing.T) {
ft := initFixture(t)
authority, err := ft.ac.BytesToString(ft.mockAddr)
require.NoError(t, err)

srv := keeper.NewMsgServerImpl(ft.keeper)

// admin resets circuit breaker
url := msgSend
url2 := "/the_only_message_acc2_can_trip_and_reset"

// add acc2 as an authorized account for only url2
authmsg := &types.MsgAuthorizeCircuitBreaker{
Granter: authority,
Grantee: addresses[2],
Permissions: &types.Permissions{
Level: types.Permissions_LEVEL_SOME_MSGS,
LimitTypeUrls: []string{url2},
},
}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, authmsg)
require.NoError(t, err)

// admin trips circuit breaker
admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url, url2}}
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
require.NoError(t, err)

// sanity check, both messages should be tripped
allowed, err := ft.keeper.IsAllowed(ft.ctx, url)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")

allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")

// now let's try to reset url using acc2 (should fail)
acc2Reset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}}
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
require.Error(t, err)

// now let's try to reset url2 with an empty url using acc2 (should pass)
acc2Reset = &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{}}
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
require.NoError(t, err)

// Only url2 should be reset, url should still be tripped
allowed, err = ft.keeper.IsAllowed(ft.ctx, url)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")

allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")

// now let's try to reset url with empty url using an authorized account (should pass)
authAccReset := &types.MsgResetCircuitBreaker{Authority: authority, MsgTypeUrls: []string{}}
_, err = srv.ResetCircuitBreaker(ft.ctx, authAccReset)
require.NoError(t, err)

// Both 2 url should be reset
allowed, err = ft.keeper.IsAllowed(ft.ctx, url)
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")

allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")
}

0 comments on commit 34bdf96

Please sign in to comment.