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.

Sign up for GitHub

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

Jump to bottom

Deprecate sdk.Msg #8864

Closed
4 tasks
aaronc opened this issue Mar 12, 2021 · 0 comments · Fixed by #9139
Closed
4 tasks

Deprecate sdk.Msg #8864

aaronc opened this issue Mar 12, 2021 · 0 comments · Fixed by #9139
Labels
C:baseapp S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Dev UX UX for SDK developers (i.e. how to call our code) T: State Machine Breaking State machine breaking changes (impacts consensus).

Comments

@aaronc
Copy link
Member

aaronc commented Mar 12, 2021

Summary

It's confusing to have both sdk.Msg and ServiceMsg. ServiceMsg is intended to be used going forward.

Problem Definition

It's confusing when calling Tx.GetMsgs to have either sdk.Msg for legacy cases or ServiceMsg.

Proposal

In a future release, require that all transactions submit messages using service methods. Recently we added a conversion from ServiceMsg to legacy amino sdk.Msgs for SIGN_MODE_LEGACY_AMINO_JSON (#8346). With this change, there is really no reason to continue to allow legacy sdk.Msgs to be packed into Txs.

With this deprecation, Tx.GetMsgs would return []ServiceMsg and TxBuilder would work with ServiceMsgs primarily through better codegen (#8270).

We will need to make sure that clients are aware that this is part of the migration to protobuf and give them ample time before this support is turned off. Likely we will want to coordinate this with complete removal of legacy REST APIs.

/cc @AmauryM @fdymylja

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@aaronc aaronc added T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Dev UX UX for SDK developers (i.e. how to call our code) T: State Machine Breaking State machine breaking changes (impacts consensus). C:baseapp S:proposed labels Mar 12, 2021
@amaury1093 amaury1093 mentioned this issue Apr 7, 2021
Closed
@amaury1093 amaury1093 mentioned this issue Apr 19, 2021
Merged
9 tasks
@mergify mergify bot closed this as completed in #9139 Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C:baseapp S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Dev UX UX for SDK developers (i.e. how to call our code) T: State Machine Breaking State machine breaking changes (impacts consensus).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove ServiceMsgs from ADR-031 cosmos/cosmos-sdk
1 participant
@aaronc