Refactor x/{gov, crisis} according to ADR 031 (#7533)

* Refactor x/gov according to ADR 31

* fix tests

* Refactor x/crisis according to ADR 031

* fix lint

* lint

* lint

* review changes

* lint

* review change

* fic doc

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

proto/cosmos/crisis/v1beta1/tx.proto

@@ -5,6 +5,12 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/crisis/types";
 
 import "gogoproto/gogo.proto";
 
+// Msg defines the bank Msg service.
+service Msg {
+  // VerifyInvariant defines a method to verify a particular invariance.
+  rpc VerifyInvariant(MsgVerifyInvariant) returns (MsgVerifyInvariantResponse);
+}
+
 // MsgVerifyInvariant represents a message to verify a particular invariance.
 message MsgVerifyInvariant {
   option (gogoproto.equal)           = false;
@@ -14,3 +20,6 @@ message MsgVerifyInvariant {
   string invariant_module_name = 2 [(gogoproto.moretags) = "yaml:\"invariant_module_name\""];
   string invariant_route       = 3 [(gogoproto.moretags) = "yaml:\"invariant_route\""];
 }
+
+// MsgVerifyInvariantResponse defines the Msg/VerifyInvariant response type.
+message MsgVerifyInvariantResponse {  }

proto/cosmos/gov/v1beta1/tx.proto

@@ -8,14 +8,26 @@ import "gogoproto/gogo.proto";
 import "google/protobuf/any.proto";
 
 option go_package                       = "github.com/cosmos/cosmos-sdk/x/gov/types";
-option (gogoproto.goproto_stringer_all) = false;
-option (gogoproto.stringer_all)         = false;
-option (gogoproto.goproto_getters_all)  = false;
+
+// Msg defines the bank Msg service.
+service Msg {
+  // SubmitProposal defines a method to create new proposal given a content.
+  rpc SubmitProposal(MsgSubmitProposal) returns (MsgSubmitProposalResponse);
+
+  // Vote defines a method to add a vote on a specific proposal.
+  rpc Vote(MsgVote) returns (MsgVoteResponse);
+
+  // Deposit defines a method to add deposit on a specific proposal.
+  rpc Deposit(MsgDeposit) returns (MsgDepositResponse);
+}
 
 // MsgSubmitProposal defines an sdk.Msg type that supports submitting arbitrary
 // proposal Content.
 message MsgSubmitProposal {
   option (gogoproto.equal) = false;
+  option (gogoproto.goproto_stringer) = false;
+  option (gogoproto.stringer)         = false;
+  option (gogoproto.goproto_getters)  = false;
 
   google.protobuf.Any content                       = 1 [(cosmos_proto.accepts_interface) = "Content"];
   repeated cosmos.base.v1beta1.Coin initial_deposit = 2 [
@@ -26,21 +38,38 @@ message MsgSubmitProposal {
   string proposer = 3;
 }
 
+// MsgSubmitProposalResponse defines the Msg/SubmitProposal response type.
+message MsgSubmitProposalResponse {
+  uint64     proposal_id = 1 [(gogoproto.jsontag) = "proposal_id", (gogoproto.moretags) = "yaml:\"proposal_id\""];
+}
+
 // MsgVote defines a message to cast a vote.
 message MsgVote {
   option (gogoproto.equal) = false;
+  option (gogoproto.goproto_stringer) = false;
+  option (gogoproto.stringer)         = false;
+  option (gogoproto.goproto_getters)  = false;
 
   uint64     proposal_id = 1 [(gogoproto.jsontag) = "proposal_id", (gogoproto.moretags) = "yaml:\"proposal_id\""];
   string     voter       = 2;
   VoteOption option      = 3;
 }
 
+// MsgVoteResponse defines the Msg/Vote response type.
+message MsgVoteResponse {}
+
 // MsgDeposit defines a message to submit a deposit to an existing proposal.
 message MsgDeposit {
   option (gogoproto.equal) = false;
+  option (gogoproto.goproto_stringer) = false;
+  option (gogoproto.stringer)         = false;
+  option (gogoproto.goproto_getters)  = false;
 
   uint64   proposal_id = 1 [(gogoproto.jsontag) = "proposal_id", (gogoproto.moretags) = "yaml:\"proposal_id\""];
   string   depositor   = 2;
   repeated cosmos.base.v1beta1.Coin amount = 3
       [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
-}
+}
+
+// MsgDepositResponse defines the Msg/Deposit response type.
+message MsgDepositResponse {}

x/crisis/handler.go

@@ -3,90 +3,23 @@ package crisis
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	"github.com/cosmos/cosmos-sdk/x/crisis/keeper"
 	"github.com/cosmos/cosmos-sdk/x/crisis/types"
 )
 
 // RouterKey
 const RouterKey = types.ModuleName
 
-func NewHandler(k keeper.Keeper) sdk.Handler {
+func NewHandler(k types.MsgServer) sdk.Handler {
 	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
 		ctx = ctx.WithEventManager(sdk.NewEventManager())
 
 		switch msg := msg.(type) {
 		case *types.MsgVerifyInvariant:
-			return handleMsgVerifyInvariant(ctx, msg, k)
+			res, err := k.VerifyInvariant(sdk.WrapSDKContext(ctx), msg)
+			return sdk.WrapServiceResult(ctx, res, err)
 
 		default:
 			return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized crisis message type: %T", msg)
 		}
 	}
 }
-
-func handleMsgVerifyInvariant(ctx sdk.Context, msg *types.MsgVerifyInvariant, k keeper.Keeper) (*sdk.Result, error) {
-	constantFee := sdk.NewCoins(k.GetConstantFee(ctx))
-
-	sender, err := sdk.AccAddressFromBech32(msg.Sender)
-	if err != nil {
-		return nil, err
-	}
-	if err := k.SendCoinsFromAccountToFeeCollector(ctx, sender, constantFee); err != nil {
-		return nil, err
-	}
-
-	// use a cached context to avoid gas costs during invariants
-	cacheCtx, _ := ctx.CacheContext()
-
-	found := false
-	msgFullRoute := msg.FullInvariantRoute()
-
-	var res string
-	var stop bool
-	for _, invarRoute := range k.Routes() {
-		if invarRoute.FullRoute() == msgFullRoute {
-			res, stop = invarRoute.Invar(cacheCtx)
-			found = true
-
-			break
-		}
-	}
-
-	if !found {
-		return nil, types.ErrUnknownInvariant
-	}
-
-	if stop {
-		// NOTE currently, because the chain halts here, this transaction will never be included
-		// in the blockchain thus the constant fee will have never been deducted. Thus no
-		// refund is required.
-
-		// TODO uncomment the following code block with implementation of the circuit breaker
-		//// refund constant fee
-		//err := k.distrKeeper.DistributeFromFeePool(ctx, constantFee, msg.Sender)
-		//if err != nil {
-		//// if there are insufficient coins to refund, log the error,
-		//// but still halt the chain.
-		//logger := ctx.Logger().With("module", "x/crisis")
-		//logger.Error(fmt.Sprintf(
-		//"WARNING: insufficient funds to allocate to sender from fee pool, err: %s", err))
-		//}
-
-		// TODO replace with circuit breaker
-		panic(res)
-	}
-
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.EventTypeInvariant,
-			sdk.NewAttribute(types.AttributeKeyRoute, msg.InvariantRoute),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCrisis),
-			sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
-		),
-	})
-
-	return &sdk.Result{Events: ctx.EventManager().ABCIEvents()}, nil
-}

x/crisis/keeper/msg_server.go

@@ -0,0 +1,78 @@
+package keeper
+
+import (
+	"context"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/crisis/types"
+)
+
+var _ types.MsgServer = Keeper{}
+
+func (k Keeper) VerifyInvariant(goCtx context.Context, msg *types.MsgVerifyInvariant) (*types.MsgVerifyInvariantResponse, error) {
+	ctx := sdk.UnwrapSDKContext(goCtx)
+	constantFee := sdk.NewCoins(k.GetConstantFee(ctx))
+
+	sender, err := sdk.AccAddressFromBech32(msg.Sender)
+	if err != nil {
+		return nil, err
+	}
+	if err := k.SendCoinsFromAccountToFeeCollector(ctx, sender, constantFee); err != nil {
+		return nil, err
+	}
+
+	// use a cached context to avoid gas costs during invariants
+	cacheCtx, _ := ctx.CacheContext()
+
+	found := false
+	msgFullRoute := msg.FullInvariantRoute()
+
+	var res string
+	var stop bool
+	for _, invarRoute := range k.Routes() {
+		if invarRoute.FullRoute() == msgFullRoute {
+			res, stop = invarRoute.Invar(cacheCtx)
+			found = true
+
+			break
+		}
+	}
+
+	if !found {
+		return nil, types.ErrUnknownInvariant
+	}
+
+	if stop {
+		// NOTE currently, because the chain halts here, this transaction will never be included
+		// in the blockchain thus the constant fee will have never been deducted. Thus no
+		// refund is required.
+
+		// TODO uncomment the following code block with implementation of the circuit breaker
+		//// refund constant fee
+		//err := k.distrKeeper.DistributeFromFeePool(ctx, constantFee, msg.Sender)
+		//if err != nil {
+		//// if there are insufficient coins to refund, log the error,
+		//// but still halt the chain.
+		//logger := ctx.Logger().With("module", "x/crisis")
+		//logger.Error(fmt.Sprintf(
+		//"WARNING: insufficient funds to allocate to sender from fee pool, err: %s", err))
+		//}
+
+		// TODO replace with circuit breaker
+		panic(res)
+	}
+
+	ctx.EventManager().EmitEvents(sdk.Events{
+		sdk.NewEvent(
+			types.EventTypeInvariant,
+			sdk.NewAttribute(types.AttributeKeyRoute, msg.InvariantRoute),
+		),
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCrisis),
+			sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
+		),
+	})
+
+	return &types.MsgVerifyInvariantResponse{}, nil
+}