diff --git a/x/circuit/CHANGELOG.md b/x/circuit/CHANGELOG.md index 66d6a9275f..c21915e98a 100644 --- a/x/circuit/CHANGELOG.md +++ b/x/circuit/CHANGELOG.md @@ -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 diff --git a/x/circuit/keeper/msg_server.go b/x/circuit/keeper/msg_server.go index 356c34620d..ef1eda853d 100644 --- a/x/circuit/keeper/msg_server.go +++ b/x/circuit/keeper/msg_server.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "slices" "strings" "cosmossdk.io/collections" @@ -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 @@ -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) @@ -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", diff --git a/x/circuit/keeper/msg_server_test.go b/x/circuit/keeper/msg_server_test.go index e5fda2d19c..fdd51553b9 100644 --- a/x/circuit/keeper/msg_server_test.go +++ b/x/circuit/keeper/msg_server_test.go @@ -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") +}