Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(x/circuit): Allow msg Reset with empty msgURL #22459

Merged
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")
}
Loading