diff --git a/baseapp/circuit.go b/baseapp/circuit.go index 022ee6632c2e..3db0bc1bdcda 100644 --- a/baseapp/circuit.go +++ b/baseapp/circuit.go @@ -1,10 +1,8 @@ package baseapp -import ( - sdk "github.com/cosmos/cosmos-sdk/types" -) +import "context" // CircuitBreaker is an interface that defines the methods for a circuit breaker. type CircuitBreaker interface { - IsAllowed(ctx sdk.Context, typeURL string) bool + IsAllowed(ctx context.Context, typeURL string) (bool, error) } diff --git a/baseapp/msg_service_router.go b/baseapp/msg_service_router.go index d0031180f00b..b3876075af98 100644 --- a/baseapp/msg_service_router.go +++ b/baseapp/msg_service_router.go @@ -135,7 +135,12 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter if msr.circuitBreaker != nil { msgURL := sdk.MsgTypeURL(msg) - if !msr.circuitBreaker.IsAllowed(ctx, msgURL) { + isAllowed, err := msr.circuitBreaker.IsAllowed(ctx, msgURL) + if err != nil { + return nil, err + } + + if !isAllowed { return nil, fmt.Errorf("circuit breaker disables execution of this message: %s", msgURL) } } diff --git a/simapp/app.go b/simapp/app.go index fb451bf50a4e..7e4681399232 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -343,7 +343,7 @@ func NewSimApp( stakingtypes.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()), ) - app.CircuitKeeper = circuitkeeper.NewKeeper(keys[circuittypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AccountKeeper.AddressCodec()) + app.CircuitKeeper = circuitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[circuittypes.StoreKey]), authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AccountKeeper.AddressCodec()) app.BaseApp.SetCircuitBreaker(&app.CircuitKeeper) app.AuthzKeeper = authzkeeper.NewKeeper(runtime.NewKVStoreService(keys[authzkeeper.StoreKey]), appCodec, app.MsgServiceRouter(), app.AccountKeeper) diff --git a/x/auth/ante/testutil/expected_keepers_mocks.go b/x/auth/ante/testutil/expected_keepers_mocks.go index 1302bd240b83..f6c54d54179d 100644 --- a/x/auth/ante/testutil/expected_keepers_mocks.go +++ b/x/auth/ante/testutil/expected_keepers_mocks.go @@ -8,6 +8,7 @@ import ( context "context" reflect "reflect" + address "cosmossdk.io/core/address" types "github.com/cosmos/cosmos-sdk/types" types0 "github.com/cosmos/cosmos-sdk/x/auth/types" gomock "github.com/golang/mock/gomock" @@ -36,6 +37,20 @@ func (m *MockAccountKeeper) EXPECT() *MockAccountKeeperMockRecorder { return m.recorder } +// AddressCodec mocks base method. +func (m *MockAccountKeeper) AddressCodec() address.Codec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddressCodec") + ret0, _ := ret[0].(address.Codec) + return ret0 +} + +// AddressCodec indicates an expected call of AddressCodec. +func (mr *MockAccountKeeperMockRecorder) AddressCodec() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddressCodec", reflect.TypeOf((*MockAccountKeeper)(nil).AddressCodec)) +} + // GetAccount mocks base method. func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types.AccAddress) types.AccountI { m.ctrl.T.Helper() diff --git a/x/circuit/CHANGELOG.md b/x/circuit/CHANGELOG.md index 16de809f2717..63534e456362 100644 --- a/x/circuit/CHANGELOG.md +++ b/x/circuit/CHANGELOG.md @@ -29,4 +29,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog -## [Unreleased] +## [Unreleased] \ No newline at end of file diff --git a/x/circuit/ante/circuit.go b/x/circuit/ante/circuit.go index e33c5d2d6f55..80ed8ce5f231 100644 --- a/x/circuit/ante/circuit.go +++ b/x/circuit/ante/circuit.go @@ -1,13 +1,16 @@ package ante import ( + "context" + "github.com/cockroachdb/errors" + sdk "github.com/cosmos/cosmos-sdk/types" ) // CircuitBreaker is an interface that defines the methods for a circuit breaker. type CircuitBreaker interface { - IsAllowed(ctx sdk.Context, typeURL string) bool + IsAllowed(ctx context.Context, typeURL string) (bool, error) } // CircuitBreakerDecorator is an AnteDecorator that checks if the transaction type is allowed to enter the mempool or be executed @@ -24,7 +27,12 @@ func NewCircuitBreakerDecorator(ck CircuitBreaker) CircuitBreakerDecorator { func (cbd CircuitBreakerDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { // loop through all the messages and check if the message type is allowed for _, msg := range tx.GetMsgs() { - if !cbd.circuitKeeper.IsAllowed(ctx, sdk.MsgTypeURL(msg)) { + isAllowed, err := cbd.circuitKeeper.IsAllowed(ctx, sdk.MsgTypeURL(msg)) + if err != nil { + return ctx, err + } + + if !isAllowed { return ctx, errors.New("tx type not allowed") } } diff --git a/x/circuit/ante/circuit_test.go b/x/circuit/ante/circuit_test.go index 659060a6b088..4f4610520c63 100644 --- a/x/circuit/ante/circuit_test.go +++ b/x/circuit/ante/circuit_test.go @@ -1,12 +1,14 @@ package ante_test import ( + "context" "testing" storetypes "cosmossdk.io/store/types" cbtypes "cosmossdk.io/x/circuit/types" abci "github.com/cometbft/cometbft/abci/types" cmproto "github.com/cometbft/cometbft/proto/tendermint/types" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" @@ -16,12 +18,13 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank" "cosmossdk.io/x/circuit/ante" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" ) type fixture struct { - ctx sdk.Context + ctx context.Context mockStoreKey storetypes.StoreKey mockMsgURL string mockclientCtx client.Context @@ -32,8 +35,8 @@ type MockCircuitBreaker struct { isAllowed bool } -func (m MockCircuitBreaker) IsAllowed(ctx sdk.Context, typeURL string) bool { - return typeURL == "/cosmos.circuit.v1.MsgAuthorizeCircuitBreaker" +func (m MockCircuitBreaker) IsAllowed(ctx context.Context, typeURL string) (bool, error) { + return typeURL == "/cosmos.circuit.v1.MsgAuthorizeCircuitBreaker", nil } func initFixture(t *testing.T) *fixture { @@ -78,7 +81,8 @@ func TestCircuitBreakerDecorator(t *testing.T) { f.txBuilder.SetMsgs(tc.msg) tx := f.txBuilder.GetTx() - _, err := decorator.AnteHandle(f.ctx, tx, false, func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { + sdkCtx := sdk.UnwrapSDKContext(f.ctx) + _, err := decorator.AnteHandle(sdkCtx, tx, false, func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { return ctx, nil }) diff --git a/x/circuit/keeper/genesis.go b/x/circuit/keeper/genesis.go index bb8b2b2930b3..50fd6af67c5f 100644 --- a/x/circuit/keeper/genesis.go +++ b/x/circuit/keeper/genesis.go @@ -1,20 +1,23 @@ package keeper import ( + context "context" + + "cosmossdk.io/collections" + errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/circuit/types" - sdk "github.com/cosmos/cosmos-sdk/types" ) -func (k *Keeper) ExportGenesis(ctx sdk.Context) (data *types.GenesisState) { +func (k *Keeper) ExportGenesis(ctx context.Context) (data *types.GenesisState) { var ( permissions []*types.GenesisAccountPermissions disabledMsgs []string ) - k.IteratePermissions(ctx, func(address []byte, perm types.Permissions) (stop bool) { + err := k.Permissions.Walk(ctx, nil, func(address []byte, perm types.Permissions) (stop bool, err error) { add, err := k.addressCodec.BytesToString(address) if err != nil { - panic(err) + return true, err } // Convert the Permissions struct to a GenesisAccountPermissions struct // and add it to the permissions slice @@ -22,13 +25,19 @@ func (k *Keeper) ExportGenesis(ctx sdk.Context) (data *types.GenesisState) { Address: add, Permissions: &perm, }) - return false + return false, nil }) + if err != nil && !errorsmod.IsOf(err, collections.ErrInvalidIterator) { + panic(err) + } - k.IterateDisableLists(ctx, func(address []byte, perm types.Permissions) (stop bool) { - disabledMsgs = append(disabledMsgs, perm.LimitTypeUrls...) - return false + err = k.DisableList.Walk(ctx, nil, func(msgUrl string) (stop bool, err error) { + disabledMsgs = append(disabledMsgs, msgUrl) + return false, nil }) + if err != nil && !errorsmod.IsOf(err, collections.ErrInvalidIterator) { + panic(err) + } return &types.GenesisState{ AccountPermissions: permissions, @@ -37,7 +46,7 @@ func (k *Keeper) ExportGenesis(ctx sdk.Context) (data *types.GenesisState) { } // InitGenesis initializes the circuit module's state from a given genesis state. -func (k *Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { +func (k *Keeper) InitGenesis(ctx context.Context, genState *types.GenesisState) { for _, accounts := range genState.AccountPermissions { add, err := k.addressCodec.StringToBytes(accounts.Address) if err != nil { @@ -45,12 +54,14 @@ func (k *Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { } // Set the permissions for the account - if err := k.SetPermissions(ctx, add, accounts.Permissions); err != nil { + if err := k.Permissions.Set(ctx, add, *accounts.Permissions); err != nil { panic(err) } } for _, url := range genState.DisabledTypeUrls { // Set the disabled type urls - k.DisableMsg(ctx, url) + if err := k.DisableList.Set(ctx, url); err != nil { + panic(err) + } } } diff --git a/x/circuit/keeper/keeper.go b/x/circuit/keeper/keeper.go index 115c71f0774b..e095a89c8b3f 100644 --- a/x/circuit/keeper/keeper.go +++ b/x/circuit/keeper/keeper.go @@ -1,125 +1,75 @@ package keeper import ( - proto "github.com/cosmos/gogoproto/proto" + context "context" + "github.com/cosmos/cosmos-sdk/codec" + + "cosmossdk.io/collections" "cosmossdk.io/core/address" - storetypes "cosmossdk.io/store/types" + "cosmossdk.io/core/store" "cosmossdk.io/x/circuit/types" - sdk "github.com/cosmos/cosmos-sdk/types" ) // Keeper defines the circuit module's keeper. type Keeper struct { - storekey storetypes.StoreKey + cdc codec.BinaryCodec + storeService store.KVStoreService authority []byte addressCodec address.Codec + + Schema collections.Schema + // Permissions contains the permissions for each account + Permissions collections.Map[[]byte, types.Permissions] + // DisableList contains the message URLs that are disabled + DisableList collections.KeySet[string] } // NewKeeper constructs a new Circuit Keeper instance -func NewKeeper(storeKey storetypes.StoreKey, authority string, addressCodec address.Codec) Keeper { +func NewKeeper(cdc codec.BinaryCodec, storeService store.KVStoreService, authority string, addressCodec address.Codec) Keeper { auth, err := addressCodec.StringToBytes(authority) if err != nil { panic(err) } - return Keeper{ - storekey: storeKey, + sb := collections.NewSchemaBuilder(storeService) + + k := Keeper{ + cdc: cdc, + storeService: storeService, authority: auth, addressCodec: addressCodec, - } -} - -func (k *Keeper) GetAuthority() []byte { - return k.authority -} - -func (k *Keeper) GetPermissions(ctx sdk.Context, address []byte) (*types.Permissions, error) { - store := ctx.KVStore(k.storekey) - - key := types.CreateAddressPrefix(address) - bz := store.Get(key) - - perms := &types.Permissions{} - if err := proto.Unmarshal(bz, perms); err != nil { - return &types.Permissions{}, err + Permissions: collections.NewMap( + sb, + types.AccountPermissionPrefix, + "permissions", + collections.BytesKey, + codec.CollValue[types.Permissions](cdc), + ), + DisableList: collections.NewKeySet( + sb, + types.DisableListPrefix, + "disable_list", + collections.StringKey, + ), } - return perms, nil -} - -func (k *Keeper) SetPermissions(ctx sdk.Context, address []byte, perms *types.Permissions) error { - store := ctx.KVStore(k.storekey) - - bz, err := proto.Marshal(perms) + schema, err := sb.Build() if err != nil { - return err + panic(err) } + k.Schema = schema - key := types.CreateAddressPrefix(address) - store.Set(key, bz) - - return nil -} - -func (k *Keeper) IsAllowed(ctx sdk.Context, msgURL string) bool { - store := ctx.KVStore(k.storekey) - return !store.Has(types.CreateDisableMsgPrefix(msgURL)) -} - -func (k *Keeper) DisableMsg(ctx sdk.Context, msgURL string) { - ctx.KVStore(k.storekey).Set(types.CreateDisableMsgPrefix(msgURL), []byte{}) -} - -func (k *Keeper) EnableMsg(ctx sdk.Context, msgURL string) { - ctx.KVStore(k.storekey).Delete(types.CreateDisableMsgPrefix(msgURL)) + return k } -func (k *Keeper) IteratePermissions(ctx sdk.Context, cb func(address []byte, perms types.Permissions) (stop bool)) { - store := ctx.KVStore(k.storekey) - iter := storetypes.KVStorePrefixIterator(store, types.AccountPermissionPrefix) - defer func(iter storetypes.Iterator) { - err := iter.Close() - if err != nil { - return - } - }(iter) - - for ; iter.Valid(); iter.Next() { - var perms types.Permissions - err := proto.Unmarshal(iter.Value(), &perms) - if err != nil { - panic(err) - } - - if cb(iter.Key()[len(types.AccountPermissionPrefix):], perms) { - break - } - } +func (k *Keeper) GetAuthority() []byte { + return k.authority } -func (k *Keeper) IterateDisableLists(ctx sdk.Context, cb func(url []byte, perms types.Permissions) (stop bool)) { - store := ctx.KVStore(k.storekey) - - iter := storetypes.KVStorePrefixIterator(store, types.AccountPermissionPrefix) - defer func(iter storetypes.Iterator) { - err := iter.Close() - if err != nil { - return - } - }(iter) - - for ; iter.Valid(); iter.Next() { - var perms types.Permissions - err := proto.Unmarshal(iter.Value(), &perms) - if err != nil { - panic(err) - } - - if cb(iter.Key()[len(types.DisableListPrefix):], perms) { - break - } - } +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/keeper_test.go b/x/circuit/keeper/keeper_test.go index 3b22ff7f9fbd..f9f42b9b35c0 100644 --- a/x/circuit/keeper/keeper_test.go +++ b/x/circuit/keeper/keeper_test.go @@ -2,32 +2,48 @@ package keeper_test import ( "bytes" + context "context" "testing" cmproto "github.com/cometbft/cometbft/proto/tendermint/types" "github.com/stretchr/testify/require" + "cosmossdk.io/core/address" storetypes "cosmossdk.io/store/types" + "cosmossdk.io/x/circuit" "cosmossdk.io/x/circuit/keeper" "cosmossdk.io/x/circuit/types" + 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" ) +var addresses = []string{ + "cosmos1zglwfu6xjzvzagqcmvzewyzjp9xwqw5qwrr8n9", + "cosmos1p8s0p6gqc6c9gt77lgr2qqujz49huhu6a80smx", + "cosmos1qasf9ehx8m7cnat39ndc74rx3fg7z66u8lw0fd", + "cosmos1uxrdj5zfuudhypsmmjxnj4gpu432ycht06a05a", + "cosmos1wn7k8a7fwpmrwnm94ndj0germfnxnhl6hs8spj", +} + type fixture struct { - ctx sdk.Context + ctx context.Context keeper keeper.Keeper mockAddr []byte mockPerms types.Permissions mockMsgURL string + ac address.Codec } func initFixture(t *testing.T) *fixture { + encCfg := moduletestutil.MakeTestEncodingConfig(circuit.AppModuleBasic{}) ac := addresscodec.NewBech32Codec("cosmos") mockStoreKey := storetypes.NewKVStoreKey("test") - k := keeper.NewKeeper(mockStoreKey, authtypes.NewModuleAddress("gov").String(), ac) + storeService := runtime.NewKVStoreService(mockStoreKey) + k := keeper.NewKeeper(encCfg.Codec, storeService, authtypes.NewModuleAddress("gov").String(), ac) bz, err := ac.StringToBytes(authtypes.NewModuleAddress("gov").String()) require.NoError(t, err) @@ -37,9 +53,11 @@ func initFixture(t *testing.T) *fixture { keeper: k, mockAddr: bz, mockPerms: types.Permissions{ - Level: 3, + Level: 3, + LimitTypeUrls: []string{"test"}, }, mockMsgURL: "mock_url", + ac: ac, } } @@ -55,15 +73,15 @@ func TestGetAndSetPermissions(t *testing.T) { f := initFixture(t) // Set the permissions for the mock address. - err := f.keeper.SetPermissions(f.ctx, f.mockAddr, &f.mockPerms) + err := f.keeper.Permissions.Set(f.ctx, f.mockAddr, f.mockPerms) require.NoError(t, err) // Retrieve the permissions for the mock address. - perms, err := f.keeper.GetPermissions(f.ctx, f.mockAddr) + perms, err := f.keeper.Permissions.Get(f.ctx, f.mockAddr) require.NoError(t, err) //// Assert that the retrieved permissions match the expected value. - require.Equal(t, &f.mockPerms, perms) + require.Equal(t, f.mockPerms, perms) } func TestIteratePermissions(t *testing.T) { @@ -83,17 +101,18 @@ func TestIteratePermissions(t *testing.T) { []byte("mock_address_3"), } for i, addr := range mockAddrs { - f.keeper.SetPermissions(f.ctx, addr, &mockPerms[i]) + f.keeper.Permissions.Set(f.ctx, addr, mockPerms[i]) } // Define a variable to store the returned permissions var returnedPerms []types.Permissions // Iterate through the permissions and append them to the returnedPerms slice - f.keeper.IteratePermissions(f.ctx, func(address []byte, perms types.Permissions) (stop bool) { + err := f.keeper.Permissions.Walk(f.ctx, nil, func(address []byte, perms types.Permissions) (stop bool, err error) { returnedPerms = append(returnedPerms, perms) - return false + return false, nil }) + require.NoError(t, err) // Assert that the returned permissions match the set mock permissions require.Equal(t, mockPerms, returnedPerms) @@ -103,33 +122,41 @@ func TestIterateDisabledList(t *testing.T) { t.Parallel() f := initFixture(t) - mockPerms := []types.Permissions{ - {Level: types.Permissions_LEVEL_SUPER_ADMIN, LimitTypeUrls: []string{"url1", "url2"}}, - {Level: types.Permissions_LEVEL_ALL_MSGS}, - {Level: types.Permissions_LEVEL_NONE_UNSPECIFIED}, + mockMsgs := []string{ + "mockUrl1", + "mockUrl2", + "mockUrl3", } - // Set the permissions for a set of mock addresses - mockAddrs := [][]byte{ - []byte("mock_address_1"), - []byte("mock_address_2"), - []byte("mock_address_3"), - } - - for i, addr := range mockAddrs { - f.keeper.SetPermissions(f.ctx, addr, &mockPerms[i]) + for _, url := range mockMsgs { + require.NoError(t, f.keeper.DisableList.Set(f.ctx, url)) } // Define a variable to store the returned disabled URLs - var returnedDisabled []types.Permissions + var returnedDisabled []string - f.keeper.IterateDisableLists(f.ctx, func(address []byte, perms types.Permissions) bool { - returnedDisabled = append(returnedDisabled, perms) - return false + err := f.keeper.DisableList.Walk(f.ctx, nil, func(msgUrl string) (bool, error) { + returnedDisabled = append(returnedDisabled, msgUrl) + return false, nil }) + require.NoError(t, err) // Assert that the returned disabled URLs match the set mock disabled URLs - require.Equal(t, mockPerms[0].LimitTypeUrls, returnedDisabled[0].LimitTypeUrls) - require.Equal(t, mockPerms[1].LimitTypeUrls, returnedDisabled[1].LimitTypeUrls) - require.Equal(t, mockPerms[2].LimitTypeUrls, returnedDisabled[2].LimitTypeUrls) + require.Equal(t, mockMsgs[0], returnedDisabled[0]) + require.Equal(t, mockMsgs[1], returnedDisabled[1]) + require.Equal(t, mockMsgs[2], returnedDisabled[2]) + + // re-enable mockMsgs[0] + require.NoError(t, f.keeper.DisableList.Remove(f.ctx, mockMsgs[0])) + returnedDisabled = []string{} + + err = f.keeper.DisableList.Walk(f.ctx, nil, func(msgUrl string) (bool, error) { + returnedDisabled = append(returnedDisabled, msgUrl) + return false, nil + }) + require.NoError(t, err) + + require.Len(t, returnedDisabled, 2) + require.Equal(t, mockMsgs[1], returnedDisabled[0]) + require.Equal(t, mockMsgs[2], returnedDisabled[1]) } diff --git a/x/circuit/keeper/msg_server.go b/x/circuit/keeper/msg_server.go index d2aeafaddfbf..ec5ccf6a8312 100644 --- a/x/circuit/keeper/msg_server.go +++ b/x/circuit/keeper/msg_server.go @@ -6,9 +6,11 @@ import ( fmt "fmt" "strings" + "cosmossdk.io/collections" errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/circuit/types" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -25,9 +27,7 @@ func NewMsgServerImpl(keeper Keeper) types.MsgServer { return &msgServer{Keeper: keeper} } -func (srv msgServer) AuthorizeCircuitBreaker(goCtx context.Context, msg *types.MsgAuthorizeCircuitBreaker) (*types.MsgAuthorizeCircuitBreakerResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - +func (srv msgServer) AuthorizeCircuitBreaker(ctx context.Context, msg *types.MsgAuthorizeCircuitBreaker) (*types.MsgAuthorizeCircuitBreakerResponse, error) { address, err := srv.addressCodec.StringToBytes(msg.Granter) if err != nil { return nil, err @@ -36,9 +36,9 @@ func (srv msgServer) AuthorizeCircuitBreaker(goCtx context.Context, msg *types.M // if the granter is the module authority no need to check perms if !bytes.Equal(address, srv.GetAuthority()) { // Check that the authorizer has the permission level of "super admin" - perms, err := srv.GetPermissions(ctx, address) + perms, err := srv.Permissions.Get(ctx, address) if err != nil { - return nil, fmt.Errorf("user permission does not exist %w", err) + return nil, err } if perms.Level != types.Permissions_LEVEL_SUPER_ADMIN { @@ -51,12 +51,17 @@ func (srv msgServer) AuthorizeCircuitBreaker(goCtx context.Context, msg *types.M return nil, err } + if msg.Permissions == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "permissions cannot be nil") + } + // Append the account in the msg to the store's set of authorized super admins - if err = srv.SetPermissions(ctx, grantee, msg.Permissions); err != nil { + if err = srv.Permissions.Set(ctx, grantee, *msg.Permissions); err != nil { return nil, err } - ctx.EventManager().EmitEvents(sdk.Events{ + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( "authorize_circuit_breaker", sdk.NewAttribute("granter", msg.Granter), @@ -70,40 +75,52 @@ func (srv msgServer) AuthorizeCircuitBreaker(goCtx context.Context, msg *types.M }, nil } -func (srv msgServer) TripCircuitBreaker(goCtx context.Context, msg *types.MsgTripCircuitBreaker) (*types.MsgTripCircuitBreakerResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - +func (srv msgServer) TripCircuitBreaker(ctx context.Context, msg *types.MsgTripCircuitBreaker) (*types.MsgTripCircuitBreakerResponse, error) { address, err := srv.addressCodec.StringToBytes(msg.Authority) if err != nil { return nil, err } // Check that the account has the permissions - perms, err := srv.GetPermissions(ctx, address) - if err != nil { - return nil, fmt.Errorf("user permission does not exist %w", err) + perms, err := srv.Permissions.Get(ctx, address) + if err != nil && !errorsmod.IsOf(err, collections.ErrNotFound) { + return nil, err } - store := ctx.KVStore(srv.storekey) - switch { case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()): for _, msgTypeURL := range msg.MsgTypeUrls { // check if the message is in the list of allowed messages - if !srv.IsAllowed(ctx, msgTypeURL) { + isAllowed, err := srv.IsAllowed(ctx, msgTypeURL) + if err != nil { + return nil, err + } + + if !isAllowed { return nil, fmt.Errorf("message %s is already disabled", msgTypeURL) } - store.Set(types.CreateDisableMsgPrefix(msgTypeURL), []byte{0x01}) + + if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil { + return nil, err + } + } case perms.Level == types.Permissions_LEVEL_SOME_MSGS: for _, msgTypeURL := range msg.MsgTypeUrls { // check if the message is in the list of allowed messages - if !srv.IsAllowed(ctx, msgTypeURL) { + isAllowed, err := srv.IsAllowed(ctx, msgTypeURL) + if err != nil { + return nil, err + } + + if !isAllowed { return nil, fmt.Errorf("message %s is already disabled", msgTypeURL) } for _, msgurl := range perms.LimitTypeUrls { if msgTypeURL == msgurl { - store.Set(types.CreateDisableMsgPrefix(msgTypeURL), []byte{0x01}) + 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) } @@ -118,7 +135,8 @@ func (srv msgServer) TripCircuitBreaker(goCtx context.Context, msg *types.MsgTri urls = strings.Join(msg.GetMsgTypeUrls(), ",") } - ctx.EventManager().EmitEvents(sdk.Events{ + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( "trip_circuit_breaker", sdk.NewAttribute("authority", msg.Authority), @@ -133,30 +151,34 @@ func (srv msgServer) TripCircuitBreaker(goCtx context.Context, msg *types.MsgTri // ResetCircuitBreaker resumes processing of Msg's in the state machine that // have been been paused using TripCircuitBreaker. -func (srv msgServer) ResetCircuitBreaker(goCtx context.Context, msg *types.MsgResetCircuitBreaker) (*types.MsgResetCircuitBreakerResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) +func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgResetCircuitBreaker) (*types.MsgResetCircuitBreakerResponse, error) { keeper := srv.Keeper - address, err := srv.addressCodec.StringToBytes(msg.Authority) if err != nil { return nil, err } // Get the permissions for the account specified in the msg.Authority field - perms, err := keeper.GetPermissions(ctx, address) - if err != nil { - return nil, fmt.Errorf("user permission does not exist %w", err) + perms, err := keeper.Permissions.Get(ctx, address) + if err != nil && !errorsmod.IsOf(err, collections.ErrNotFound) { + return nil, err } - store := ctx.KVStore(srv.storekey) - if perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || perms.Level == types.Permissions_LEVEL_SOME_MSGS || bytes.Equal(address, srv.GetAuthority()) { // add all msg type urls to the disable list for _, msgTypeURL := range msg.MsgTypeUrls { - if srv.IsAllowed(ctx, msgTypeURL) { + isAllowed, err := srv.IsAllowed(ctx, msgTypeURL) + if err != nil { + return nil, err + } + + if isAllowed { return nil, fmt.Errorf("message %s is not disabled", msgTypeURL) } - store.Delete(types.CreateDisableMsgPrefix(msgTypeURL)) + + if err = srv.DisableList.Remove(ctx, msgTypeURL); err != nil { + return nil, err + } } } else { return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker") @@ -167,7 +189,8 @@ func (srv msgServer) ResetCircuitBreaker(goCtx context.Context, msg *types.MsgRe urls = strings.Join(msg.GetMsgTypeUrls(), ",") } - ctx.EventManager().EmitEvents(sdk.Events{ + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( "reset_circuit_breaker", sdk.NewAttribute("authority", msg.Authority), diff --git a/x/circuit/keeper/msg_server_test.go b/x/circuit/keeper/msg_server_test.go index e6f96e8b89e3..b5fcfc284553 100644 --- a/x/circuit/keeper/msg_server_test.go +++ b/x/circuit/keeper/msg_server_test.go @@ -1,8 +1,10 @@ -package keeper +package keeper_test import ( "testing" + "cosmossdk.io/collections" + "cosmossdk.io/x/circuit/keeper" "cosmossdk.io/x/circuit/types" "github.com/stretchr/testify/require" ) @@ -10,36 +12,36 @@ import ( const msgSend = "cosmos.bank.v1beta1.MsgSend" func Test_AuthorizeCircuitBreaker(t *testing.T) { - ft := setupFixture(t) + ft := initFixture(t) - srv := msgServer{ - Keeper: ft.Keeper, - } + srv := keeper.NewMsgServerImpl(ft.keeper) + authority, err := ft.ac.BytesToString(ft.mockAddr) + require.NoError(t, err) // add a new super admin - adminPerms := &types.Permissions{Level: types.Permissions_LEVEL_SUPER_ADMIN, LimitTypeUrls: []string{""}} - msg := &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[1], Permissions: adminPerms} - _, err := srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + adminPerms := types.Permissions{Level: types.Permissions_LEVEL_SUPER_ADMIN, LimitTypeUrls: []string{""}} + msg := &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[1], Permissions: &adminPerms} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) - add1, err := ft.Keeper.addressCodec.StringToBytes(addresses[1]) + add1, err := ft.ac.StringToBytes(addresses[1]) require.NoError(t, err) - perms, err := ft.Keeper.GetPermissions(ft.Ctx, add1) + perms, err := ft.keeper.Permissions.Get(ft.ctx, add1) require.NoError(t, err) require.Equal(t, adminPerms, perms, "admin perms are not the same") // add a super user - allmsgs := &types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}} - msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[2], Permissions: allmsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + allmsgs := types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}} + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[2], Permissions: &allmsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) - add2, err := ft.Keeper.addressCodec.StringToBytes(addresses[2]) + add2, err := ft.ac.StringToBytes(addresses[2]) require.NoError(t, err) - perms, err = ft.Keeper.GetPermissions(ft.Ctx, add2) + perms, err = ft.keeper.Permissions.Get(ft.ctx, add2) require.NoError(t, err) require.Equal(t, allmsgs, perms, "admin perms are not the same") @@ -47,155 +49,162 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) { // 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) + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.Error(t, err, "user with no permission should fail 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) + _, 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 - somemsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}} - msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[3], Permissions: somemsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + 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) require.NoError(t, err) - add3, err := ft.Keeper.addressCodec.StringToBytes(addresses[3]) + add3, err := ft.ac.StringToBytes(addresses[3]) require.NoError(t, err) - perms, err = ft.Keeper.GetPermissions(ft.Ctx, add3) + perms, err = ft.keeper.Permissions.Get(ft.ctx, add3) require.NoError(t, err) require.Equal(t, somemsgs, perms, "admin perms are not the same") - add4, err := ft.Keeper.addressCodec.StringToBytes(addresses[4]) + add4, err := ft.ac.StringToBytes(addresses[4]) require.NoError(t, err) - perms, err = ft.Keeper.GetPermissions(ft.Ctx, add4) - 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.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, "user should 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: addresses[0], Grantee: addresses[4], Permissions: invalidmsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + invalidmsgs := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}} + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[4], Permissions: &invalidmsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) } func Test_TripCircuitBreaker(t *testing.T) { - ft := setupFixture(t) + ft := initFixture(t) - srv := msgServer{ - Keeper: ft.Keeper, - } + srv := keeper.NewMsgServerImpl(ft.keeper) url := msgSend + authority, err := ft.ac.BytesToString(ft.mockAddr) + require.NoError(t, err) + // admin trips circuit breaker - admintrip := &types.MsgTripCircuitBreaker{Authority: addresses[0], MsgTypeUrls: []string{url}} - _, err := srv.TripCircuitBreaker(ft.Ctx, admintrip) + admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url}} + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) - allowed := ft.Keeper.IsAllowed(ft.Ctx, url) + allowed, err := ft.keeper.IsAllowed(ft.ctx, url) + require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") // 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: addresses[0], Grantee: addresses[1], Permissions: allmsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + 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" superTrip := &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url2}} - _, err = srv.TripCircuitBreaker(ft.Ctx, superTrip) + _, err = srv.TripCircuitBreaker(ft.ctx, superTrip) require.NoError(t, err) - allowed = ft.Keeper.IsAllowed(ft.Ctx, url2) + allowed, err = ft.keeper.IsAllowed(ft.ctx, url2) + require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") // user with no permission attempts to trips circuit breaker unknownTrip := &types.MsgTripCircuitBreaker{Authority: addresses[4], MsgTypeUrls: []string{url}} - _, err = srv.TripCircuitBreaker(ft.Ctx, unknownTrip) + _, err = srv.TripCircuitBreaker(ft.ctx, unknownTrip) require.Error(t, err) // user has permission to trip circuit breaker for two messages but only has permission for one url, url2 = "cosmos.staking.v1beta1.MsgCreateValidator", "cosmos.staking.v1beta1.MsgEditValidator" somemsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{url}} - msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[2], Permissions: somemsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[2], Permissions: somemsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) // 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) + _, err = srv.TripCircuitBreaker(ft.ctx, someTrip) require.ErrorContains(t, err, "MsgEditValidator") // 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) + _, err = srv.TripCircuitBreaker(ft.ctx, twoTrip) require.Error(t, err) } func Test_ResetCircuitBreaker(t *testing.T) { - ft := setupFixture(t) + ft := initFixture(t) + authority, err := ft.ac.BytesToString(ft.mockAddr) + require.NoError(t, err) - srv := msgServer{ - Keeper: ft.Keeper, - } + srv := keeper.NewMsgServerImpl(ft.keeper) // admin resets circuit breaker url := "cosmos.bank.v1beta1.MsgSend" // admin trips circuit breaker - admintrip := &types.MsgTripCircuitBreaker{Authority: addresses[0], MsgTypeUrls: []string{url}} - _, err := srv.TripCircuitBreaker(ft.Ctx, admintrip) + admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url}} + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) - allowed := ft.Keeper.IsAllowed(ft.Ctx, url) + allowed, err := ft.keeper.IsAllowed(ft.ctx, url) + require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") - adminReset := &types.MsgResetCircuitBreaker{Authority: addresses[0], MsgTypeUrls: []string{url}} - _, err = srv.ResetCircuitBreaker(ft.Ctx, adminReset) + adminReset := &types.MsgResetCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url}} + _, err = srv.ResetCircuitBreaker(ft.ctx, adminReset) require.NoError(t, err) - allowed = ft.Keeper.IsAllowed(ft.Ctx, url) + allowed, err = ft.keeper.IsAllowed(ft.ctx, url) + 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) + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) - allowed = ft.Keeper.IsAllowed(ft.Ctx, url) + allowed, err = ft.keeper.IsAllowed(ft.ctx, url) + require.NoError(t, err) require.False(t, allowed, "circuit breaker should be tripped") unknownUserReset := &types.MsgResetCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url}} - _, err = srv.ResetCircuitBreaker(ft.Ctx, unknownUserReset) + _, err = srv.ResetCircuitBreaker(ft.ctx, unknownUserReset) require.Error(t, err) - allowed = ft.Keeper.IsAllowed(ft.Ctx, url) + allowed, err = ft.keeper.IsAllowed(ft.ctx, url) + require.NoError(t, err) require.False(t, allowed, "circuit breaker should be reset") // user with all messages resets circuit breaker allmsgs := &types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}} - msg := &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[1], Permissions: allmsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + msg := &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[1], Permissions: allmsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) // trip the circuit breaker url2 := "cosmos.staking.v1beta1.MsgDelegate" - admintrip = &types.MsgTripCircuitBreaker{Authority: addresses[0], MsgTypeUrls: []string{url2}} - _, err = srv.TripCircuitBreaker(ft.Ctx, admintrip) + admintrip = &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url2}} + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) // user with all messages resets circuit breaker allMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url}} - _, err = srv.ResetCircuitBreaker(ft.Ctx, allMsgsReset) + _, err = srv.ResetCircuitBreaker(ft.ctx, allMsgsReset) require.NoError(t, err) // user tries to reset an message they dont have permission to reset @@ -203,21 +212,21 @@ func Test_ResetCircuitBreaker(t *testing.T) { url = "cosmos.staking.v1beta1.MsgCreateValidator" // give restricted perms to a user someMsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{url2}} - msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[0], Grantee: addresses[2], Permissions: someMsgs} - _, err = srv.AuthorizeCircuitBreaker(ft.Ctx, msg) + msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[2], Permissions: someMsgs} + _, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg) require.NoError(t, err) - admintrip = &types.MsgTripCircuitBreaker{Authority: addresses[0], MsgTypeUrls: []string{url}} - _, err = srv.TripCircuitBreaker(ft.Ctx, admintrip) + admintrip = &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url}} + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.NoError(t, err) // user with all messages resets circuit breaker someMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}} - _, err = srv.ResetCircuitBreaker(ft.Ctx, someMsgsReset) + _, err = srv.ResetCircuitBreaker(ft.ctx, someMsgsReset) require.NoError(t, err) // user tries to reset an already reset circuit breaker admintrip = &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url2}} - _, err = srv.TripCircuitBreaker(ft.Ctx, admintrip) + _, err = srv.TripCircuitBreaker(ft.ctx, admintrip) require.Error(t, err) } diff --git a/x/circuit/keeper/query.go b/x/circuit/keeper/query.go index 4b8f505f6769..3c889206a561 100644 --- a/x/circuit/keeper/query.go +++ b/x/circuit/keeper/query.go @@ -1,14 +1,14 @@ package keeper import ( - "bytes" "context" - "cosmossdk.io/store/prefix" + "cosmossdk.io/collections" + errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/circuit/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" - "github.com/cosmos/gogoproto/proto" ) var _ types.QueryServer = QueryServer{} @@ -32,59 +32,49 @@ func (qs QueryServer) Account(c context.Context, req *types.QueryAccountRequest) return nil, err } - perms, err := qs.keeper.GetPermissions(sdkCtx, add) + perms, err := qs.keeper.Permissions.Get(sdkCtx, add) if err != nil { return nil, err } - return &types.AccountResponse{Permission: perms}, nil + return &types.AccountResponse{Permission: &perms}, nil } // Account returns account permissions. -func (qs QueryServer) Accounts(c context.Context, req *types.QueryAccountsRequest) (*types.AccountsResponse, error) { - sdkCtx := sdk.UnwrapSDKContext(c) - // Iterate over accounts and perform the callback - +func (qs QueryServer) Accounts(ctx context.Context, req *types.QueryAccountsRequest) (*types.AccountsResponse, error) { var accounts []*types.GenesisAccountPermissions - store := sdkCtx.KVStore(qs.keeper.storekey) - accountsStore := prefix.NewStore(store, types.AccountPermissionPrefix) - - pageRes, err := query.Paginate(accountsStore, req.Pagination, func(key, value []byte) error { - perm := &types.Permissions{} - if err := proto.Unmarshal(value, perm); err != nil { - return err - } + results, pageRes, err := query.CollectionPaginate[[]byte, types.Permissions](ctx, qs.keeper.Permissions, req.Pagination) + if err != nil { + return nil, err + } - // remove key suffix - address, err := qs.keeper.addressCodec.BytesToString(bytes.TrimRight(key, "\x00")) + for _, result := range results { + result := result + address, err := qs.keeper.addressCodec.BytesToString(result.Key) if err != nil { - return err + return nil, err } accounts = append(accounts, &types.GenesisAccountPermissions{ Address: address, - Permissions: perm, + Permissions: &result.Value, }) - - return nil - }) - if err != nil { - return nil, err } return &types.AccountsResponse{Accounts: accounts, Pagination: pageRes}, nil } // DisabledList returns a list of disabled message urls -func (qs QueryServer) DisabledList(c context.Context, req *types.QueryDisabledListRequest) (*types.DisabledListResponse, error) { - sdkCtx := sdk.UnwrapSDKContext(c) +func (qs QueryServer) DisabledList(ctx context.Context, req *types.QueryDisabledListRequest) (*types.DisabledListResponse, error) { // Iterate over disabled list and perform the callback - var msgs []string - qs.keeper.IterateDisableLists(sdkCtx, func(address []byte, perm types.Permissions) (stop bool) { - msgs = append(msgs, perm.LimitTypeUrls...) - return false + err := qs.keeper.DisableList.Walk(ctx, nil, func(msgUrl string) (bool, error) { + msgs = append(msgs, msgUrl) + return false, nil }) + if err != nil && !errorsmod.IsOf(err, collections.ErrInvalidIterator) { + return nil, err + } return &types.DisabledListResponse{DisabledList: msgs}, nil } diff --git a/x/circuit/keeper/query_test.go b/x/circuit/keeper/query_test.go index c8749696336b..c6b22c32fb8d 100644 --- a/x/circuit/keeper/query_test.go +++ b/x/circuit/keeper/query_test.go @@ -1,28 +1,30 @@ -package keeper +package keeper_test import ( "testing" + "cosmossdk.io/x/circuit/keeper" "cosmossdk.io/x/circuit/types" - "github.com/cosmos/cosmos-sdk/types/query" "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/types/query" ) func TestQueryAccount(t *testing.T) { t.Parallel() - f := setupFixture(t) + f := initFixture(t) - add, err := f.Keeper.addressCodec.StringToBytes(addresses[0]) + add, err := f.ac.StringToBytes(addresses[0]) require.NoError(t, err) - err = f.Keeper.SetPermissions(f.Ctx, add, &f.MockPerms) + err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms) require.NoError(t, err) // create a new query server - qs := QueryServer{keeper: f.Keeper} + qs := keeper.NewQueryServer(f.keeper) // test the Account method - res, err := qs.Account(f.Ctx, &types.QueryAccountRequest{Address: addresses[0]}) + res, err := qs.Account(f.ctx, &types.QueryAccountRequest{Address: addresses[0]}) require.NoError(t, err) require.Equal(t, res.Permission.Level, types.Permissions_Level(3)) require.Equal(t, res.Permission.LimitTypeUrls, []string{ @@ -32,26 +34,26 @@ func TestQueryAccount(t *testing.T) { func TestQueryAccounts(t *testing.T) { t.Parallel() - f := setupFixture(t) + f := initFixture(t) - add, err := f.Keeper.addressCodec.StringToBytes(addresses[0]) + add, err := f.ac.StringToBytes(addresses[0]) require.NoError(t, err) - err = f.Keeper.SetPermissions(f.Ctx, add, &f.MockPerms) + err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms) require.NoError(t, err) // create a new query server - qs := QueryServer{keeper: f.Keeper} + qs := keeper.NewQueryServer(f.keeper) // test the Accounts method - res1, err := qs.Accounts(f.Ctx, &types.QueryAccountsRequest{ + res1, err := qs.Accounts(f.ctx, &types.QueryAccountsRequest{ Pagination: &query.PageRequest{Limit: 10}, }) require.NoError(t, err) for _, a := range res1.Accounts { require.Equal(t, addresses[0], a.Address) - require.Equal(t, f.MockPerms, *a.Permissions) + require.Equal(t, f.mockPerms, *a.Permissions) } require.NotNil(t, res1) @@ -59,19 +61,15 @@ func TestQueryAccounts(t *testing.T) { func TestQueryDisabledList(t *testing.T) { t.Parallel() - f := setupFixture(t) + f := initFixture(t) - add, err := f.Keeper.addressCodec.StringToBytes(addresses[0]) - require.NoError(t, err) - - err = f.Keeper.SetPermissions(f.Ctx, add, &f.MockPerms) - require.NoError(t, err) + require.NoError(t, f.keeper.DisableList.Set(f.ctx, f.mockMsgURL)) // create a new query server - qs := QueryServer{keeper: f.Keeper} + qs := keeper.NewQueryServer(f.keeper) // test the DisabledList method - disabledList, err := qs.DisabledList(f.Ctx, &types.QueryDisabledListRequest{}) + disabledList, err := qs.DisabledList(f.ctx, &types.QueryDisabledListRequest{}) require.NoError(t, err) - require.Equal(t, []string{"test"}, disabledList.DisabledList) + require.Equal(t, []string{f.mockMsgURL}, disabledList.DisabledList) } diff --git a/x/circuit/keeper/test_utils.go b/x/circuit/keeper/test_utils.go deleted file mode 100644 index 7c9f2658c577..000000000000 --- a/x/circuit/keeper/test_utils.go +++ /dev/null @@ -1,47 +0,0 @@ -package keeper - -import ( - "testing" - - storetypes "cosmossdk.io/store/types" - cmproto "github.com/cometbft/cometbft/proto/tendermint/types" - addresscodec "github.com/cosmos/cosmos-sdk/codec/address" - "github.com/cosmos/cosmos-sdk/testutil" - sdk "github.com/cosmos/cosmos-sdk/types" - - "cosmossdk.io/x/circuit/types" -) - -var addresses = []string{ - "cosmos1zglwfu6xjzvzagqcmvzewyzjp9xwqw5qwrr8n9", - "cosmos1p8s0p6gqc6c9gt77lgr2qqujz49huhu6a80smx", - "cosmos1qasf9ehx8m7cnat39ndc74rx3fg7z66u8lw0fd", - "cosmos1uxrdj5zfuudhypsmmjxnj4gpu432ycht06a05a", - "cosmos1wn7k8a7fwpmrwnm94ndj0germfnxnhl6hs8spj", -} - -type fixture struct { - Ctx sdk.Context - Keeper Keeper - MockPerms types.Permissions - MockMsgURL string -} - -func setupFixture(t *testing.T) *fixture { - mockStoreKey := storetypes.NewKVStoreKey("circuit") - keeperX := NewKeeper(mockStoreKey, addresses[0], addresscodec.NewBech32Codec("cosmos")) - mockMsgURL := "mock_url" - mockCtx := testutil.DefaultContextWithDB(t, mockStoreKey, storetypes.NewTransientStoreKey("transient_test")) - ctx := mockCtx.Ctx.WithBlockHeader(cmproto.Header{}) - mockPerms := types.Permissions{ - Level: 3, - LimitTypeUrls: []string{"test"}, - } - - return &fixture{ - Ctx: ctx, - Keeper: keeperX, - MockPerms: mockPerms, - MockMsgURL: mockMsgURL, - } -} diff --git a/x/circuit/module.go b/x/circuit/module.go index 3b95da6045dc..4a4f73557be8 100644 --- a/x/circuit/module.go +++ b/x/circuit/module.go @@ -9,10 +9,13 @@ import ( modulev1 "cosmossdk.io/api/cosmos/circuit/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" + "cosmossdk.io/core/store" "cosmossdk.io/depinject" - store "cosmossdk.io/store/types" "cosmossdk.io/x/circuit/client/cli" abci "github.com/cometbft/cometbft/abci/types" + gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/spf13/cobra" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" @@ -22,8 +25,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime" - "github.com/spf13/cobra" "cosmossdk.io/x/circuit/keeper" "cosmossdk.io/x/circuit/types" @@ -149,9 +150,9 @@ func init() { type ModuleInputs struct { depinject.In - Config *modulev1.Module - Cdc codec.Codec - Key *store.KVStoreKey + Config *modulev1.Module + Cdc codec.Codec + StoreService store.KVStoreService AddressCodec address.Codec } @@ -172,7 +173,8 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } circuitkeeper := keeper.NewKeeper( - in.Key, + in.Cdc, + in.StoreService, authority.String(), in.AddressCodec, ) diff --git a/x/circuit/types/keys.go b/x/circuit/types/keys.go index e315590e6c6f..db4e1dc6203e 100644 --- a/x/circuit/types/keys.go +++ b/x/circuit/types/keys.go @@ -1,5 +1,7 @@ package types +import "cosmossdk.io/collections" + const ( // ModuleName defines the module name ModuleName = "circuit" @@ -13,20 +15,6 @@ const ( // KVStore keys var ( - AccountPermissionPrefix = []byte{0x01} - DisableListPrefix = []byte{0x02} + AccountPermissionPrefix = collections.NewPrefix(1) + DisableListPrefix = collections.NewPrefix(2) ) - -func CreateAddressPrefix(account []byte) []byte { - key := make([]byte, len(AccountPermissionPrefix)+len(account)+1) - copy(key, AccountPermissionPrefix) - copy(key[len(AccountPermissionPrefix):], account) - return key -} - -func CreateDisableMsgPrefix(msgURL string) []byte { - key := make([]byte, len(DisableListPrefix)+len(msgURL)+1) - copy(key, DisableListPrefix) - copy(key[len(DisableListPrefix):], msgURL) - return key -}