From 8a36e926e821817c2703be1b24fab58208a5f17f Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Wed, 4 Mar 2020 16:41:13 +0000 Subject: [PATCH] tidy up circuit-breaker --- x/circuit-breaker/ante/ante.go | 28 ++++++++------ x/circuit-breaker/gov-handler.go | 12 +++++- x/circuit-breaker/keeper/keeper.go | 18 +++++---- x/circuit-breaker/spec/README.md | 14 +++++-- x/circuit-breaker/types/types.go | 61 ++++++++++++++++++++++++++---- x/groupgov/spec/README.md | 19 +++++++++- 6 files changed, 120 insertions(+), 32 deletions(-) diff --git a/x/circuit-breaker/ante/ante.go b/x/circuit-breaker/ante/ante.go index df8ed9dd..0ad9e78c 100644 --- a/x/circuit-breaker/ante/ante.go +++ b/x/circuit-breaker/ante/ante.go @@ -1,28 +1,34 @@ -package circuit-breaker +package ante -// CircuiteBreakerDecorator needs to be combined with other standard decorators (from auth) to create the app's AnteHandler. +import ( + "fmt" + + "github.com/kava-labs/kava/x/circuit-breaker/keeper" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// CircuitBreakerDecorator needs to be combined with other standard decorators (from auth) to create the app's AnteHandler. // CircuitBreakerDecorator errors if a tx contains a disallowed msg type // Call next AnteHandler if all msgs are allowed type CircuitBreakerDecorator struct { - cbk Keeper + cbk keeper.Keeper } -func NewCircuitBreakerDecorator(cbk Keeper) CircuitBreakerDecorator { +func NewCircuitBreakerDecorator(cbk keeper.Keeper) CircuitBreakerDecorator { return CircuitBreakerDecorator{ - cbk: cbk, + cbk: cbk, } } func (cbd CircuitBreakerDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { -// TODO need to tidy up the types used to store broken routes // get msg route, error if not allowed - disallowedRoutes := cbd.cbk.GetRoutes(ctx) - requestedRoutes := tx.Msgs - for m, _ := range tx.Msgs { - for r, _ := range disallowedRoutes { - if r == m.Route() { + disallowedRoutes := cbd.cbk.GetMsgRoutes(ctx) + for _, m := range tx.GetMsgs() { + for _, r := range disallowedRoutes { + if r.Route == m.Route() && r.Msg == m.Type() { return ctx, fmt.Errorf("route %s has been circuit broken, tx rejected", r) } } diff --git a/x/circuit-breaker/gov-handler.go b/x/circuit-breaker/gov-handler.go index c0397b10..0d5dd79c 100644 --- a/x/circuit-breaker/gov-handler.go +++ b/x/circuit-breaker/gov-handler.go @@ -1,7 +1,15 @@ package circuit-breaker -func NewCircuitBreakerProposalHandler(k Keeper) govtypes.Handler { - return func(ctx sdk.Context, content govtypes.Content) sdk.Error { +import ( + "github.com/cosmos/cosmos-sdk/x/gov" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/kava-labs/kava/x/circuit-breaker/types" + "github.com/kava-labs/kava/x/circuit-breaker/keeper" +) + +func NewCircuitBreakerProposalHandler(k Keeper) gov.Handler { + return func(ctx sdk.Context, content gov.Content) sdk.Error { switch c := content.(type) { case types.CircuitBreakerProposal: return keeper.HandleCircuitBreakerProposal(ctx, k, c) diff --git a/x/circuit-breaker/keeper/keeper.go b/x/circuit-breaker/keeper/keeper.go index d8efcd16..0a1b650a 100644 --- a/x/circuit-breaker/keeper/keeper.go +++ b/x/circuit-breaker/keeper/keeper.go @@ -1,18 +1,22 @@ -package circuit-breaker +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/kava-labs/kava/x/circuit-breaker/types" +) // Keeper stores routes that have been "broken" type Keeper struct { } - -func (k Keeper) GetRoutes(ctx sdk.Context) []string { +func (k Keeper) GetMsgRoutes(ctx sdk.Context) []types.MsgRoute { // TODO } -func (k Keeper) SetRoutes(ctx sdk.Context, routes []string) { +func (k Keeper) SetMsgRoutes(ctx sdk.Context, routes []types.MsgRoute) { // TODO } -func (k Keeper) HandleCircuitBreakerProposal(ctx sdk.Context, c Content) { - k.SetRoutes(ctx, c.Routes) -} \ No newline at end of file +func HandleCircuitBreakerProposal(ctx sdk.Context, k Keeper, c types.CircuitBreakProposal) { + k.SetMsgRoutes(ctx, c.MsgRoutes) +} diff --git a/x/circuit-breaker/spec/README.md b/x/circuit-breaker/spec/README.md index 6f9f0eb7..5c53f051 100644 --- a/x/circuit-breaker/spec/README.md +++ b/x/circuit-breaker/spec/README.md @@ -1,10 +1,18 @@ -# `groupgov` +# `circuit-breaker` ## Table of Contents ## Overview -The `x/groupgov` module is an additional governance module to `cosmos-sdk/x/gov`. It allows groups of accounts to vote on and enact proposals, mainly to allow certain proposal types to be decided on quickly in emergency situations, or to delegate low risk parameter updates to a smaller group of individuals. +The `x/circuit-breaker` module allows certain message types to be disabled based on governance votes. -Groups have permissions. +Msgs and routes are disabled via an antehandler decorator. The decorator checks incoming all txs and rejects them if they contain a disallowed msg type. +Disallowed msg types are stored in a circuit breaker keeper. + +The list of disallowed msg types is updated via a custom governance proposal and handler. + +Design Alternatives: + +- store list of disallowed msg types in params, then don't need custom gov proposal +- replace the app Router with a custom one to avoid using the antehandler - can't be done with current baseapp, but v0.38.x enables this. (https://github.com/cosmos/cosmos-sdk/issues/5455) \ No newline at end of file diff --git a/x/circuit-breaker/types/types.go b/x/circuit-breaker/types/types.go index 44db626a..8f700025 100644 --- a/x/circuit-breaker/types/types.go +++ b/x/circuit-breaker/types/types.go @@ -1,14 +1,61 @@ package types -// TODO implement a gov proposal for adding a route to the circuit breaker keeper. - -type CircuitBreakProposal struct { - MsgRoutes []MsgRoute -} +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" +) type MsgRoute struct { Route string - Msg sdk.Msg // how best to store a Msg type? as a string? + Msg string // how best to store a Msg type? } -// TODO gov.Proposal methods... +const ( + ProposalTypeCircuitBreak = "CircuitBreak" +) + +// Assert CircuitBreakProposal implements govtypes.Content at compile-time +var _ govtypes.Content = CircuitBreakProposal{} + +type CircuitBreakProposal struct { + Title string + Description string + MsgRoutes []MsgRoute +} + +// GetTitle returns the title of a community pool spend proposal. +func (cbp CircuitBreakProposal) GetTitle() string { return cbp.Title } + +// GetDescription returns the description of a community pool spend proposal. +func (cbp CircuitBreakProposal) GetDescription() string { return cbp.Description } + +// GetDescription returns the routing key of a community pool spend proposal. +func (cbp CircuitBreakProposal) ProposalRoute() string { return RouterKey } + +// ProposalType returns the type of a community pool spend proposal. +func (cbp CircuitBreakProposal) ProposalType() string { return ProposalTypeCircuitBreak } + +// ValidateBasic runs basic stateless validity checks +func (cbp CircuitBreakProposal) ValidateBasic() sdk.Error { + err := govtypes.ValidateAbstract(DefaultCodespace, cbp) + if err != nil { + return err + } + // TODO + return nil +} + +// String implements the Stringer interface. +func (cbp CircuitBreakProposal) String() string { + // TODO +} + +const ( + DefaultCodespace sdk.CodespaceType = ModuleName + + // ModuleName is the module name constant used in many places + ModuleName = "circuit-breaker" + + // RouterKey is the message route for distribution + RouterKey = ModuleName +) diff --git a/x/groupgov/spec/README.md b/x/groupgov/spec/README.md index 0b3294f0..db9f0c5b 100644 --- a/x/groupgov/spec/README.md +++ b/x/groupgov/spec/README.md @@ -1,8 +1,23 @@ -# `circuit-breaker` +# `groupgov` ## Table of Contents ## Overview -The `x/circuit-breaker` module allows certain message types to be disabled based on governance votes. +The `x/groupgov` module is an additional governance module to `cosmos-sdk/x/gov`. + +It allows groups of accounts to vote on and enact proposals, mainly to allow certain proposal types to be decided on quickly in emergency situations, or to delegate low risk parameter updates to a smaller group of individuals. + +Groups have members and permissions. + +Members vote on proposals, with just simple one vote per member, no deposits or slashing. More sophisticated voting could be added. + +A permission acts as a filter for incoming gov proposals, rejecting them if they do not pass. A permission can be anything with a method `Allows(p Proposal) bool`. They reject all proposals that they don't explicitly allow. + +This allows permissions to be parameterized to allow fine grained control specified at runtime. For example a generic parameter permission type can exist, but then on a live chain a permission can be added to a group to allow them to only change a particular param, even restricting the range of allowed change. + +Design Alternatives + +- Should this define its own gov types, or reuse those from gov module? +- Should we push changes to sdk gov to make it more general purpose? \ No newline at end of file