Skip to content

Commit 34bdf96

Browse files
GNaD13mergify[bot]
authored andcommitted
feat(x/circuit): Allow msg Reset with empty msgURL (#22459)
(cherry picked from commit 42c616c)
1 parent cb32699 commit 34bdf96

File tree

3 files changed

+104
-2
lines changed

3 files changed

+104
-2
lines changed

x/circuit/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
4141

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

4647
## [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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"fmt"
7+
"slices"
78
"strings"
89

910
"cosmossdk.io/collections"
@@ -143,6 +144,7 @@ func (srv msgServer) TripCircuitBreaker(ctx context.Context, msg *types.MsgTripC
143144
// have been paused using TripCircuitBreaker.
144145
func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgResetCircuitBreaker) (*types.MsgResetCircuitBreakerResponse, error) {
145146
keeper := srv.Keeper
147+
msgTypeUrls := types.MsgTypeURLValidation(msg.MsgTypeUrls)
146148
address, err := srv.addressCodec.StringToBytes(msg.Authority)
147149
if err != nil {
148150
return nil, err
@@ -154,7 +156,35 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
154156
return nil, err
155157
}
156158

157-
msgTypeUrls := types.MsgTypeURLValidation(msg.MsgTypeUrls)
159+
// check if msgURL is empty
160+
if len(msgTypeUrls) == 0 {
161+
switch {
162+
case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()):
163+
// if the sender is a super admin or the module authority, will remove all disabled msgs
164+
err := srv.DisableList.Walk(ctx, nil, func(msgUrl string) (stop bool, err error) {
165+
msgTypeUrls = append(msgTypeUrls, msgUrl)
166+
return false, nil
167+
})
168+
if err != nil {
169+
return nil, err
170+
}
171+
172+
case perms.Level == types.Permissions_LEVEL_SOME_MSGS:
173+
// if the sender has permission for some messages, will remove all disabled msgs that in the perms.LimitTypeUrls
174+
err := srv.DisableList.Walk(ctx, nil, func(msgUrl string) (stop bool, err error) {
175+
if slices.Contains(perms.LimitTypeUrls, msgUrl) {
176+
msgTypeUrls = append(msgTypeUrls, msgUrl)
177+
}
178+
return false, nil
179+
})
180+
if err != nil {
181+
return nil, err
182+
}
183+
default:
184+
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker")
185+
}
186+
}
187+
158188
for _, msgTypeURL := range msgTypeUrls {
159189
// check if the message is in the list of allowed messages
160190
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
@@ -183,7 +213,7 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
183213
}
184214
}
185215

186-
urls := strings.Join(msg.GetMsgTypeUrls(), ",")
216+
urls := strings.Join(msgTypeUrls, ",")
187217

188218
if err = srv.Keeper.EventService.EventManager(ctx).EmitKV(
189219
"reset_circuit_breaker",

x/circuit/keeper/msg_server_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,74 @@ func TestResetCircuitBreakerSomeMsgs(t *testing.T) {
450450
require.NoError(t, err)
451451
require.True(t, allowed, "circuit breaker should be reset")
452452
}
453+
454+
func TestResetCircuitBreakerEmptyMsgs(t *testing.T) {
455+
ft := initFixture(t)
456+
authority, err := ft.ac.BytesToString(ft.mockAddr)
457+
require.NoError(t, err)
458+
459+
srv := keeper.NewMsgServerImpl(ft.keeper)
460+
461+
// admin resets circuit breaker
462+
url := msgSend
463+
url2 := "/the_only_message_acc2_can_trip_and_reset"
464+
465+
// add acc2 as an authorized account for only url2
466+
authmsg := &types.MsgAuthorizeCircuitBreaker{
467+
Granter: authority,
468+
Grantee: addresses[2],
469+
Permissions: &types.Permissions{
470+
Level: types.Permissions_LEVEL_SOME_MSGS,
471+
LimitTypeUrls: []string{url2},
472+
},
473+
}
474+
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, authmsg)
475+
require.NoError(t, err)
476+
477+
// admin trips circuit breaker
478+
admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url, url2}}
479+
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
480+
require.NoError(t, err)
481+
482+
// sanity check, both messages should be tripped
483+
allowed, err := ft.keeper.IsAllowed(ft.ctx, url)
484+
require.NoError(t, err)
485+
require.False(t, allowed, "circuit breaker should be tripped")
486+
487+
allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
488+
require.NoError(t, err)
489+
require.False(t, allowed, "circuit breaker should be tripped")
490+
491+
// now let's try to reset url using acc2 (should fail)
492+
acc2Reset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}}
493+
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
494+
require.Error(t, err)
495+
496+
// now let's try to reset url2 with an empty url using acc2 (should pass)
497+
acc2Reset = &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{}}
498+
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
499+
require.NoError(t, err)
500+
501+
// Only url2 should be reset, url should still be tripped
502+
allowed, err = ft.keeper.IsAllowed(ft.ctx, url)
503+
require.NoError(t, err)
504+
require.False(t, allowed, "circuit breaker should be tripped")
505+
506+
allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
507+
require.NoError(t, err)
508+
require.True(t, allowed, "circuit breaker should be reset")
509+
510+
// now let's try to reset url with empty url using an authorized account (should pass)
511+
authAccReset := &types.MsgResetCircuitBreaker{Authority: authority, MsgTypeUrls: []string{}}
512+
_, err = srv.ResetCircuitBreaker(ft.ctx, authAccReset)
513+
require.NoError(t, err)
514+
515+
// Both 2 url should be reset
516+
allowed, err = ft.keeper.IsAllowed(ft.ctx, url)
517+
require.NoError(t, err)
518+
require.True(t, allowed, "circuit breaker should be reset")
519+
520+
allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
521+
require.NoError(t, err)
522+
require.True(t, allowed, "circuit breaker should be reset")
523+
}

0 commit comments

Comments
 (0)