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

…22491)

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

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",

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")
+}