From 87e2abb38471c552d23de00dd7094d4e176e17dc Mon Sep 17 00:00:00 2001 From: Marko Date: Thu, 27 Jul 2023 21:59:52 +0200 Subject: [PATCH] fix(circuit): partial perms bug fix (#17165) (cherry picked from commit 31eb54c27563b57c0f0a44ea30ce544b10018766) --- x/circuit/keeper/msg_server.go | 14 +++++++++----- x/circuit/keeper/msg_server_test.go | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/x/circuit/keeper/msg_server.go b/x/circuit/keeper/msg_server.go index b4976ec4318c..91bbca507a46 100644 --- a/x/circuit/keeper/msg_server.go +++ b/x/circuit/keeper/msg_server.go @@ -115,15 +115,19 @@ func (srv msgServer) TripCircuitBreaker(ctx context.Context, msg *types.MsgTripC if !isAllowed { return nil, fmt.Errorf("message %s is already disabled", msgTypeURL) } + permExists := false for _, msgurl := range perms.LimitTypeUrls { if msgTypeURL == msgurl { - if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil { - return nil, err - } - } else { - return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker for message %s", msgTypeURL) + permExists = true } } + + if !permExists { + return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker for message %s", msgTypeURL) + } + if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil { + return nil, err + } } default: return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker") diff --git a/x/circuit/keeper/msg_server_test.go b/x/circuit/keeper/msg_server_test.go index a3a91b370823..fff87ebf4bd5 100644 --- a/x/circuit/keeper/msg_server_test.go +++ b/x/circuit/keeper/msg_server_test.go @@ -138,15 +138,27 @@ func TestTripCircuitBreaker(t *testing.T) { require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") + // user with enough permissions tries to trip circuit breaker for two messages + url, url2 := "cosmos.gov.v1beta1.MsgDeposit", "cosmos.gov.v1beta1.MsgVote" + twomsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{url, url2}} + msg := &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[3], Permissions: twomsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) + require.NoError(t, err) + + // try to trip two messages with enough permissions + twoMsgTrip := &types.MsgTripCircuitBreaker{Authority: addresses[3], MsgTypeUrls: []string{url, url2}} + _, err = srv.TripCircuitBreaker(ft.ctx, twoMsgTrip) + require.NoError(t, err) + // user with all messages trips circuit breaker // add a super user allmsgs := &types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}} - msg := &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[1], Permissions: allmsgs} + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[1], Permissions: allmsgs} _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) // try to trip the circuit breaker - url2 := "cosmos.staking.v1beta1.MsgDelegate" + url2 = "cosmos.staking.v1beta1.MsgDelegate" superTrip := &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url2}} _, err = srv.TripCircuitBreaker(ft.ctx, superTrip) require.NoError(t, err)