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

Implement ADR 031: Protobuf Msg Services #7500

Closed
11 of 16 tasks
aaronc opened this issue Oct 9, 2020 · 2 comments
Closed
11 of 16 tasks

Implement ADR 031: Protobuf Msg Services #7500

aaronc opened this issue Oct 9, 2020 · 2 comments
Assignees
Labels
C:Encoding T: Client UX T: Dev UX UX for SDK developers (i.e. how to call our code)
Milestone

Comments

@aaronc
Copy link
Member

aaronc commented Oct 9, 2020

Summary

Implement ADR 031: Protobuf Msg Services as documented in #7458 and #7122 .

Roadmap

Implementation Notes

Modules

  1. Add a service definition called Msg in each module's tx.proto file. Re-use the existing Msg types for the request type and add an unique Response type for each rpc method.
  2. Refactor all handler functions into an implementation of the generated MsgServer interface
// x/bank/handler.go

// Handle MsgSend.
func handleMsgSend(ctx sdk.Context, k keeper.Keeper, msg *types.MsgSend) (*sdk.Result, error) {
    ...
    return &sdk.Result{Events: ctx.EventManager().ABCIEvents()}, nil
}

becomes:

// x/bank/keeper/msg_server.go

type msgServer struct {
    Keeper
}

func (k msgServer) Send(goCtx context.Context, msg *types.MsgSend) (*types.MsgSendResponse, error) {
    ctx := sdk.UnwrapSDKContext(goCtx)
    ...
    return &types.MsgSendResponse{}, nil
}
  1. Refactor the root handler to just call those MsgService methods. Ex:
func NewHandler(k keeper.Keeper) sdk.Handler {
	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
		ctx = ctx.WithEventManager(sdk.NewEventManager())
                
                msgServer := keeper.NewMsgServerImpl(k)

		switch msg := msg.(type) {
		case *types.MsgSend:
			res, err := msgServer.Send(sdk.WrapSDKContext(ctx), msg)
                        return sdk.WrapServiceResult(ctx, res, err)

		case *types.MsgMultiSend:
			res, err := msgServer.MultiSend(sdk.WrapSDKContext(ctx), msg)
                        return sdk.WrapServiceResult(ctx, res, err)

		default:
			return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized bank message type: %T", msg)
		}
	}
}
  1. Wire up MsgServer in RegisterServices (if possible - that functionality is currently not ready, so skip this step for now)
@aaronc aaronc added this to the v0.40.1 milestone Oct 9, 2020
@aaronc aaronc added T: Client UX T: Dev UX UX for SDK developers (i.e. how to call our code) labels Oct 9, 2020
@aaronc
Copy link
Member Author

aaronc commented Oct 10, 2020

I’m inclined to start on this ASAP. If we get this done pretty quickly clients won’t get too used to the existing API so that we have backwards compatibility concerns. Maybe this could even be part of a quick v0.40.1 in a couple of weeks.

@amaury1093 amaury1093 self-assigned this Oct 12, 2020
@clevinson
Copy link
Contributor

Closing in favor of #7540 (Codetree Epic)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Encoding T: Client UX T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

No branches or pull requests

3 participants