diff --git a/api/cosmos/circuit/v1/tx.pulsar.go b/api/cosmos/circuit/v1/tx.pulsar.go index ca2f3e5012e4..a6a43bb1e716 100644 --- a/api/cosmos/circuit/v1/tx.pulsar.go +++ b/api/cosmos/circuit/v1/tx.pulsar.go @@ -2965,7 +2965,7 @@ func (x *MsgAuthorizeCircuitBreaker) GetPermissions() *Permissions { return nil } -// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type. +// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type. type MsgAuthorizeCircuitBreakerResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -3051,7 +3051,7 @@ func (x *MsgTripCircuitBreaker) GetMsgTypeUrls() []string { return nil } -// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type. +// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type. type MsgTripCircuitBreakerResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache diff --git a/proto/cosmos/circuit/v1/query.proto b/proto/cosmos/circuit/v1/query.proto index 1b952020d1f1..0115d335f439 100644 --- a/proto/cosmos/circuit/v1/query.proto +++ b/proto/cosmos/circuit/v1/query.proto @@ -8,7 +8,7 @@ import "cosmos/circuit/v1/types.proto"; import "google/api/annotations.proto"; import "cosmos/query/v1/query.proto"; -// Query defines the circuit Msg service. +// Query defines the circuit gRPC querier service. service Query { // Account returns account permissions. rpc Account(QueryAccountRequest) returns (AccountResponse) { diff --git a/proto/cosmos/circuit/v1/tx.proto b/proto/cosmos/circuit/v1/tx.proto index c0684316d04a..71f708bb2ad9 100644 --- a/proto/cosmos/circuit/v1/tx.proto +++ b/proto/cosmos/circuit/v1/tx.proto @@ -39,7 +39,7 @@ message MsgAuthorizeCircuitBreaker { Permissions permissions = 3; } -// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type. +// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type. message MsgAuthorizeCircuitBreakerResponse { bool success = 1; } @@ -59,7 +59,7 @@ message MsgTripCircuitBreaker { repeated string msg_type_urls = 2; } -// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type. +// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type. message MsgTripCircuitBreakerResponse { bool success = 1; } diff --git a/x/circuit/README.md b/x/circuit/README.md index 348efb2cc46b..7386680e3ef3 100644 --- a/x/circuit/README.md +++ b/x/circuit/README.md @@ -59,7 +59,7 @@ Authorize, is called by the module authority (default governance module account) ### Trip -Trip, is called by an account to disable message execution for a specific msgURL. +Trip, is called by an authorized account to disable message execution for a specific msgURL. If empty, all the msgs will be disabled. ```protobuf // TripCircuitBreaker pauses processing of Msg's in the state machine. @@ -68,7 +68,7 @@ Trip, is called by an account to disable message execution for a specific msgURL ### Reset -Reset is called to enable execution of a previously disabled message. +Reset is called by an authorized account to enable execution for a specific msgURL of previously disabled message. If empty, all the disabled messages will be enabled. ```protobuf // ResetCircuitBreaker resumes processing of Msg's in the state machine that @@ -118,8 +118,8 @@ The circuit module emits the following events: | Type | Attribute Key | Attribute Value | |---------|---------------|---------------------------| -| string | granter | {granteeAddress} | -| string | grantee | {granterAddress} | +| string | granter | {granterAddress} | +| string | grantee | {granteeAddress} | | string | permission | {granteePermissions} | | message | module | circuit | | message | action | authorize_circuit_breaker | diff --git a/x/circuit/client/cli/tx.go b/x/circuit/client/cli/tx.go index 71d4c9f0045d..a3c44f8101ef 100644 --- a/x/circuit/client/cli/tx.go +++ b/x/circuit/client/cli/tx.go @@ -44,7 +44,7 @@ func AuthorizeCircuitBreakerCmd() *cobra.Command { "ALL_MSGS" = 2, "SUPER_ADMIN" = 3,`, Example: fmt.Sprintf(`%s circuit authorize [address] 0 "cosmos.bank.v1beta1.MsgSend,cosmos.bank.v1beta1.MsgMultiSend"`, version.AppName), - Args: cobra.RangeArgs(3, 4), + Args: cobra.RangeArgs(2, 3), RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientTxContext(cmd) if err != nil { @@ -62,7 +62,7 @@ func AuthorizeCircuitBreakerCmd() *cobra.Command { } var typeUrls []string - if len(args) == 4 { + if len(args) == 3 { typeUrls = strings.Split(args[2], ",") } diff --git a/x/circuit/keeper/genesis_test.go b/x/circuit/keeper/genesis_test.go new file mode 100644 index 000000000000..e85f3f286859 --- /dev/null +++ b/x/circuit/keeper/genesis_test.go @@ -0,0 +1,88 @@ +package keeper_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + + storetypes "cosmossdk.io/store/types" + "cosmossdk.io/x/circuit" + "cosmossdk.io/x/circuit/keeper" + "cosmossdk.io/x/circuit/types" + + "github.com/cosmos/cosmos-sdk/codec" + addresscodec "github.com/cosmos/cosmos-sdk/codec/address" + "github.com/cosmos/cosmos-sdk/runtime" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +type GenesisTestSuite struct { + suite.Suite + + ctx context.Context + keeper keeper.Keeper + cdc *codec.ProtoCodec + addrBytes []byte +} + +func TestGenesisTestSuite(t *testing.T) { + suite.Run(t, new(GenesisTestSuite)) +} + +func (s *GenesisTestSuite) SetupTest() { + key := storetypes.NewKVStoreKey(types.StoreKey) + testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) + encCfg := moduletestutil.MakeTestEncodingConfig(circuit.AppModuleBasic{}) + + sdkCtx := testCtx.Ctx + s.ctx = sdkCtx + s.cdc = codec.NewProtoCodec(encCfg.InterfaceRegistry) + authority := authtypes.NewModuleAddress("gov") + ac := addresscodec.NewBech32Codec("cosmos") + + bz, err := ac.StringToBytes(authority.String()) + s.Require().NoError(err) + s.addrBytes = bz + + s.keeper = keeper.NewKeeper(s.cdc, runtime.NewKVStoreService(key), authority.String(), ac) +} + +func (s *GenesisTestSuite) TestInitExportGenesis() { + perms := types.Permissions{ + Level: 3, + LimitTypeUrls: []string{"test"}, + } + err := s.keeper.Permissions.Set(s.ctx, s.addrBytes, perms) + s.Require().NoError(err) + + var accounts []*types.GenesisAccountPermissions + genAccsPerms := types.GenesisAccountPermissions{ + Address: sdk.AccAddress(s.addrBytes).String(), + Permissions: &perms, + } + accounts = append(accounts, &genAccsPerms) + + url := "test_url" + + genesisState := &types.GenesisState{ + AccountPermissions: accounts, + DisabledTypeUrls: []string{url}, + } + + s.keeper.InitGenesis(s.ctx, genesisState) + + exported := s.keeper.ExportGenesis(s.ctx) + bz, err := s.cdc.MarshalJSON(exported) + s.Require().NoError(err) + + var exportedGenesisState types.GenesisState + err = s.cdc.UnmarshalJSON(bz, &exportedGenesisState) + s.Require().NoError(err) + + s.Require().Equal(genesisState.AccountPermissions, exportedGenesisState.AccountPermissions) + s.Require().Equal(genesisState.DisabledTypeUrls, exportedGenesisState.DisabledTypeUrls) +} diff --git a/x/circuit/keeper/keeper.go b/x/circuit/keeper/keeper.go index 9741c514e7c7..25513991f8c1 100644 --- a/x/circuit/keeper/keeper.go +++ b/x/circuit/keeper/keeper.go @@ -69,6 +69,7 @@ func (k *Keeper) GetAuthority() []byte { return k.authority } +// IsAllowed returns true when msg URL is not found in the DisableList for given context, else false. func (k *Keeper) IsAllowed(ctx context.Context, msgURL string) (bool, error) { has, err := k.DisableList.Has(ctx, msgURL) return !has, err diff --git a/x/circuit/keeper/msg_server_test.go b/x/circuit/keeper/msg_server_test.go index 69629c36d48b..4c567cdf0569 100644 --- a/x/circuit/keeper/msg_server_test.go +++ b/x/circuit/keeper/msg_server_test.go @@ -12,7 +12,7 @@ import ( const msgSend = "cosmos.bank.v1beta1.MsgSend" -func Test_AuthorizeCircuitBreaker(t *testing.T) { +func TestAuthorizeCircuitBreaker(t *testing.T) { ft := initFixture(t) srv := keeper.NewMsgServerImpl(ft.keeper) @@ -31,7 +31,7 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) { perms, err := ft.keeper.Permissions.Get(ft.ctx, add1) require.NoError(t, err) - require.Equal(t, adminPerms, perms, "admin perms are not the same") + require.Equal(t, adminPerms, perms, "admin perms are the same") // add a super user allmsgs := types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}} @@ -45,22 +45,21 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) { perms, err = ft.keeper.Permissions.Get(ft.ctx, add2) require.NoError(t, err) - require.Equal(t, allmsgs, perms, "admin perms are not the same") + require.Equal(t, allmsgs, perms) // unauthorized user who does not have perms trying to authorize superPerms := &types.Permissions{Level: types.Permissions_LEVEL_SUPER_ADMIN, LimitTypeUrls: []string{}} msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[3], Grantee: addresses[2], Permissions: superPerms} _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) - require.Error(t, err, "user with no permission should fail in authorizing others") + require.Error(t, err, "user with no permission fails in authorizing others") // user with permission level all_msgs tries to grant another user perms somePerms := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{}} msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[2], Grantee: addresses[3], Permissions: somePerms} _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) - require.Error(t, err, "user[2] does not have permission to grant others permission") - - // user successfully grants another user perms to a specific permission + require.Error(t, err, "super user[2] does not have permission to grant others permission") + // admin successfully grants another user perms to a specific permission somemsgs := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}} msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[3], Permissions: &somemsgs} _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) @@ -72,24 +71,24 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) { perms, err = ft.keeper.Permissions.Get(ft.ctx, add3) require.NoError(t, err) - require.Equal(t, somemsgs, perms, "admin perms are not the same") + require.Equal(t, somemsgs, perms) add4, err := ft.ac.StringToBytes(addresses[4]) require.NoError(t, err) perms, err = ft.keeper.Permissions.Get(ft.ctx, add4) - require.ErrorIs(t, err, collections.ErrNotFound, "user should have no perms by default") + require.ErrorIs(t, err, collections.ErrNotFound, "users have no perms by default") - require.Equal(t, types.Permissions{Level: types.Permissions_LEVEL_NONE_UNSPECIFIED, LimitTypeUrls: nil}, perms, "user should have no perms by default") + require.Equal(t, types.Permissions{Level: types.Permissions_LEVEL_NONE_UNSPECIFIED, LimitTypeUrls: nil}, perms, "users have no perms by default") - // admin tries grants another user permission ALL_MSGS with limited urls populated - invalidmsgs := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}} - msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[4], Permissions: &invalidmsgs} + // admin tries grants another user permission SOME_MSGS with limited urls populated + permis := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}} + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[4], Permissions: &permis} _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) } -func Test_TripCircuitBreaker(t *testing.T) { +func TestTripCircuitBreaker(t *testing.T) { ft := initFixture(t) srv := keeper.NewMsgServerImpl(ft.keeper) @@ -124,7 +123,7 @@ func Test_TripCircuitBreaker(t *testing.T) { require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") - // user with no permission attempts to trips circuit breaker + // user with no permission attempts to trip circuit breaker unknownTrip := &types.MsgTripCircuitBreaker{Authority: addresses[4], MsgTypeUrls: []string{url}} _, err = srv.TripCircuitBreaker(ft.ctx, unknownTrip) require.Error(t, err) @@ -139,16 +138,16 @@ func Test_TripCircuitBreaker(t *testing.T) { // try to trip two messages but user only has permission for one someTrip := &types.MsgTripCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url, url2}} _, err = srv.TripCircuitBreaker(ft.ctx, someTrip) - require.ErrorContains(t, err, "MsgEditValidator") + require.ErrorContains(t, err, "MsgEditValidator: unauthorized") // user tries to trip an already tripped circuit breaker alreadyTripped := "cosmos.bank.v1beta1.MsgSend" twoTrip := &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{alreadyTripped}} _, err = srv.TripCircuitBreaker(ft.ctx, twoTrip) - require.Error(t, err) + require.ErrorContains(t, err, "already disabled") } -func Test_ResetCircuitBreaker(t *testing.T) { +func TestResetCircuitBreaker(t *testing.T) { ft := initFixture(t) authority, err := ft.ac.BytesToString(ft.mockAddr) require.NoError(t, err) @@ -174,7 +173,6 @@ func Test_ResetCircuitBreaker(t *testing.T) { require.NoError(t, err) require.True(t, allowed, "circuit breaker should be reset") - // user has no permission to reset circuit breaker // admin trips circuit breaker _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) @@ -183,6 +181,7 @@ func Test_ResetCircuitBreaker(t *testing.T) { require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") + // user has no permission to reset circuit breaker unknownUserReset := &types.MsgResetCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url}} _, err = srv.ResetCircuitBreaker(ft.ctx, unknownUserReset) require.Error(t, err) @@ -208,8 +207,7 @@ func Test_ResetCircuitBreaker(t *testing.T) { _, err = srv.ResetCircuitBreaker(ft.ctx, allMsgsReset) require.NoError(t, err) - // user tries to reset an message they dont have permission to reset - + // user tries to reset a message they dont have permission to reset url = "cosmos.staking.v1beta1.MsgCreateValidator" // give restricted perms to a user someMsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{url2}} @@ -221,7 +219,7 @@ func Test_ResetCircuitBreaker(t *testing.T) { _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) - // user with all messages resets circuit breaker + // user with some messages resets circuit breaker someMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}} _, err = srv.ResetCircuitBreaker(ft.ctx, someMsgsReset) require.NoError(t, err) diff --git a/x/circuit/keeper/query.go b/x/circuit/keeper/query.go index 3c889206a561..681a04c210ae 100644 --- a/x/circuit/keeper/query.go +++ b/x/circuit/keeper/query.go @@ -17,7 +17,7 @@ type QueryServer struct { keeper Keeper } -// NewMsgServerImpl returns an implementation of the circuit MsgServer interface +// NewQueryServer returns an implementation of the circuit QueryServer interface // for the provided Keeper. func NewQueryServer(keeper Keeper) types.QueryServer { return &QueryServer{keeper: keeper} diff --git a/x/circuit/keeper/query_test.go b/x/circuit/keeper/query_test.go index d4eaa360882f..9fd6845aea58 100644 --- a/x/circuit/keeper/query_test.go +++ b/x/circuit/keeper/query_test.go @@ -31,6 +31,18 @@ func TestQueryAccount(t *testing.T) { require.Equal(t, res.Permission.LimitTypeUrls, []string{ "test", }) + + // test invalid address + res, err = qs.Account(f.ctx, &types.QueryAccountRequest{Address: "invalid"}) + require.Error(t, err) + require.ErrorContains(t, err, "invalid bech32 string") + require.Nil(t, res) + + // test account not found + res, err = qs.Account(f.ctx, &types.QueryAccountRequest{Address: addresses[1]}) + require.Error(t, err) + require.ErrorContains(t, err, "not found") + require.Nil(t, res) } func TestQueryAccounts(t *testing.T) { @@ -40,19 +52,26 @@ func TestQueryAccounts(t *testing.T) { add, err := f.ac.StringToBytes(addresses[0]) require.NoError(t, err) - err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms) - require.NoError(t, err) - // create a new query server qs := keeper.NewQueryServer(f.keeper) - // test the Accounts method + // test the Accounts method with no accounts res1, err := qs.Accounts(f.ctx, &types.QueryAccountsRequest{ Pagination: &query.PageRequest{Limit: 10}, }) require.NoError(t, err) + require.Len(t, res1.Accounts, 0) + + err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms) + require.NoError(t, err) + + // test the Accounts method + res2, err := qs.Accounts(f.ctx, &types.QueryAccountsRequest{ + Pagination: &query.PageRequest{Limit: 10}, + }) + require.NoError(t, err) - for _, a := range res1.Accounts { + for _, a := range res2.Accounts { require.Equal(t, addresses[0], a.Address) require.Equal(t, f.mockPerms, *a.Permissions) } @@ -64,13 +83,18 @@ func TestQueryDisabledList(t *testing.T) { t.Parallel() f := initFixture(t) - require.NoError(t, f.keeper.DisableList.Set(f.ctx, f.mockMsgURL)) - // create a new query server qs := keeper.NewQueryServer(f.keeper) // test the DisabledList method disabledList, err := qs.DisabledList(f.ctx, &types.QueryDisabledListRequest{}) require.NoError(t, err) + require.Len(t, disabledList.DisabledList, 0) + + require.NoError(t, f.keeper.DisableList.Set(f.ctx, f.mockMsgURL)) + + // test the DisabledList method + disabledList, err = qs.DisabledList(f.ctx, &types.QueryDisabledListRequest{}) + require.NoError(t, err) require.Equal(t, []string{f.mockMsgURL}, disabledList.DisabledList) } diff --git a/x/circuit/types/msgs.go b/x/circuit/types/msgs.go index 216a26514b1c..0f23cf14cb6a 100644 --- a/x/circuit/types/msgs.go +++ b/x/circuit/types/msgs.go @@ -25,13 +25,6 @@ func (m MsgAuthorizeCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) } // Type Implements Msg. func (m MsgAuthorizeCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) } -// GetSigners returns the expected signers for a MsgAuthorizeCircuitBreaker. -func (m MsgAuthorizeCircuitBreaker) GetSigners() []sdk.AccAddress { - granter := sdk.MustAccAddressFromBech32(m.Granter) - - return []sdk.AccAddress{granter} -} - // NewMsgTripCircuitBreaker creates a new MsgTripCircuitBreaker instance. func NewMsgTripCircuitBreaker(authority string, urls []string) *MsgTripCircuitBreaker { return &MsgTripCircuitBreaker{ @@ -46,13 +39,6 @@ func (m MsgTripCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) } // Type Implements Msg. func (m MsgTripCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) } -// GetSigners returns the expected signers for a MsgTripCircuitBreaker. -func (m MsgTripCircuitBreaker) GetSigners() []sdk.AccAddress { - granter := sdk.MustAccAddressFromBech32(m.Authority) - - return []sdk.AccAddress{granter} -} - // NewMsgResetCircuitBreaker creates a new MsgResetCircuitBreaker instance. func NewMsgResetCircuitBreaker(authority string, urls []string) *MsgResetCircuitBreaker { return &MsgResetCircuitBreaker{ @@ -66,10 +52,3 @@ func (m MsgResetCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) } // Type Implements Msg. func (m MsgResetCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) } - -// GetSigners returns the expected signers for a MsgResetCircuitBreaker. -func (m MsgResetCircuitBreaker) GetSigners() []sdk.AccAddress { - granter := sdk.MustAccAddressFromBech32(m.Authority) - - return []sdk.AccAddress{granter} -} diff --git a/x/circuit/types/tx.pb.go b/x/circuit/types/tx.pb.go index dcd824939bb5..df1f62291b2d 100644 --- a/x/circuit/types/tx.pb.go +++ b/x/circuit/types/tx.pb.go @@ -95,7 +95,7 @@ func (m *MsgAuthorizeCircuitBreaker) GetPermissions() *Permissions { return nil } -// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type. +// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type. type MsgAuthorizeCircuitBreakerResponse struct { Success bool `protobuf:"varint,1,opt,name=success,proto3" json:"success,omitempty"` } @@ -199,7 +199,7 @@ func (m *MsgTripCircuitBreaker) GetMsgTypeUrls() []string { return nil } -// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type. +// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type. type MsgTripCircuitBreakerResponse struct { Success bool `protobuf:"varint,1,opt,name=success,proto3" json:"success,omitempty"` }