Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(x/circuit)!: Use collections, KVStoreService, and context.Context #16415

Merged
merged 17 commits into from
Jun 7, 2023
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (baseapp) [#16415](https://github.com/cosmos/cosmos-sdk/issues/16415) `IsAllowed` from the `CircuitBreaker` interface now takes a `context.Context` instead of a `sdk.Context` and returns `bool, error`.
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
* (x/slashing) [#16246](https://github.com/cosmos/cosmos-sdk/issues/16246) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`. `GetValidatorSigningInfo` now returns an error instead of a `found bool`, the error can be `nil` (found), `ErrNoSigningInfoFound` (not found) and any other error.
* (x/mint) [#16179](https://github.com/cosmos/cosmos-sdk/issues/16179) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`.
* (x/crisis) [#16216](https://github.com/cosmos/cosmos-sdk/issues/16216) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error` instead of panicking.
Expand Down
2 changes: 2 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ The following modules `NewKeeper` function now take a `KVStoreService` instead o
* `x/auth`
* `x/authz`
* `x/bank`
* `x/circuit`
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
* `x/consensus`
* `x/crisis`
* `x/distribution`
Expand All @@ -105,6 +106,7 @@ The following modules' `Keeper` methods now take in a `context.Context` instead

* `x/authz`
* `x/bank`
* `x/circuit`
* `x/mint`
* `x/crisis`
* `x/distribution`
Expand Down
6 changes: 2 additions & 4 deletions baseapp/circuit.go
Original file line number Diff line number Diff line change
@@ -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)
}
7 changes: 6 additions & 1 deletion baseapp/msg_service_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
3 changes: 2 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1"
"cosmossdk.io/client/v2/autocli"
"cosmossdk.io/core/appmodule"

"github.com/cosmos/cosmos-sdk/codec/address"

authcodec "github.com/cosmos/cosmos-sdk/x/auth/codec"
Expand Down Expand Up @@ -339,7 +340,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)
Expand Down
15 changes: 15 additions & 0 deletions x/auth/ante/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions x/circuit/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
# Changelog

## [Unreleased]

### API Breaking
facundomedica marked this conversation as resolved.
Show resolved Hide resolved

* (x/circuit) [#16415](https://github.com/cosmos/cosmos-sdk/issues/16415) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey` and requires a codec. Methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`. Migrated to collections. `IsAllowed` from the `CircuitBreaker` interface now takes a `context.Context` instead of a `sdk.Context` and returns `bool, error`.
12 changes: 10 additions & 2 deletions x/circuit/ante/circuit.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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")
}
}
Expand Down
14 changes: 9 additions & 5 deletions x/circuit/ante/circuit_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
})

Expand Down
29 changes: 19 additions & 10 deletions x/circuit/keeper/genesis.go
Original file line number Diff line number Diff line change
@@ -1,34 +1,43 @@
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
permissions = append(permissions, &types.GenesisAccountPermissions{
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,
Expand All @@ -37,15 +46,15 @@ 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 {
panic(err)
}

// 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)
}
}
Expand Down
Loading