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

fix: Add RegisterLegacyAminoCodec for authz/feegrant #11214

Closed
wants to merge 3 commits into from
Closed

fix: Add RegisterLegacyAminoCodec for authz/feegrant #11214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78c16d2
fix: Add RegisterLegacyAminoCodec for authz/feegrant
amaury1093 Feb 17, 2022
d88839f
add module name
amaury1093 Feb 17, 2022
9a42a24
Fix GetSignByes, add tests
amaury1093 Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#11011](https://github.com/cosmos/cosmos-sdk/pull/11011) Remove burning of deposits when qourum is not reached on a governance proposal and when the deposit is not fully met.
* [\#11019](https://github.com/cosmos/cosmos-sdk/pull/11019) Add `MsgCreatePermanentLockedAccount` and CLI method for creating permanent locked account
* (x/feegrant) [\#10830](https://github.com/cosmos/cosmos-sdk/pull/10830) Expired allowances will be pruned from state.
* (x/authz,x/feegrant) [\#11214](https://github.com/cosmos/cosmos-sdk/pull/11214) Fix Amino JSON encoding of authz and feegrant Msgs to be consistent with other modules.

### Deprecated

Expand Down
26 changes: 26 additions & 0 deletions x/authz/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package authz

import (
"github.com/cosmos/cosmos-sdk/codec"
types "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
)

// RegisterLegacyAminoCodec registers the necessary x/authz interfaces and concrete types
// on the provided LegacyAmino codec. These types are used for Amino JSON serialization.
func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
cdc.RegisterConcrete(&MsgGrant{}, "cosmos-sdk/authz/MsgGrant", nil)
cdc.RegisterConcrete(&MsgRevoke{}, "cosmos-sdk/authz/MsgRevoke", nil)
cdc.RegisterConcrete(&MsgExec{}, "cosmos-sdk/authz/MsgExec", nil)
}

// RegisterInterfaces registers the interfaces types with the interface registry
func RegisterInterfaces(registry types.InterfaceRegistry) {
registry.RegisterImplementations((*sdk.Msg)(nil),
Expand All @@ -22,3 +31,20 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {

msgservice.RegisterMsgServiceDesc(registry, MsgServiceDesc())
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/authz module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
// The actual codec used for serialization should be provided to x/authz and
// defined at the application level.
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
amino.Seal()
}
4 changes: 3 additions & 1 deletion x/authz/module/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
}

// RegisterLegacyAminoCodec registers the authz module's types for the given codec.
func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {}
func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
authz.RegisterLegacyAminoCodec(cdc)
}

// RegisterInterfaces registers the authz module's interface types
func (AppModuleBasic) RegisterInterfaces(registry cdctypes.InterfaceRegistry) {
Expand Down
7 changes: 3 additions & 4 deletions x/authz/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec/legacy"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -76,7 +75,7 @@ func (msg MsgGrant) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgGrant) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetAuthorization returns the cache value from the MsgGrant.Authorization if present.
Expand Down Expand Up @@ -166,7 +165,7 @@ func (msg MsgRevoke) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgRevoke) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// NewMsgExec creates a new MsgExecAuthorized
Expand Down Expand Up @@ -233,5 +232,5 @@ func (msg MsgExec) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgExec) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}
41 changes: 41 additions & 0 deletions x/authz/msgs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
"github.com/cosmos/cosmos-sdk/x/authz"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
)
Expand Down Expand Up @@ -120,3 +121,43 @@ func TestMsgGrantGetAuthorization(t *testing.T) {
require.NoError(err)
require.Equal(a, &g)
}

func TestAminoJSON(t *testing.T) {
tx := legacytx.StdTx{}
var msg legacytx.LegacyMsg
someDate := time.Date(1, 1, 1, 1, 1, 1, 1, time.UTC)
msgSend := banktypes.MsgSend{FromAddress: "cosmos1ghi", ToAddress: "cosmos1jkl"}
typeURL := sdk.MsgTypeURL(&msgSend)
msgSendAny, err := cdctypes.NewAnyWithValue(&msgSend)
require.NoError(t, err)
grant, err := authz.NewGrant(someDate, authz.NewGenericAuthorization(typeURL), someDate.Add(time.Hour))
require.NoError(t, err)

// Amino JSON encoding has changed in authz since v0.46.
// Before, it was outputting something like:
// `{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"grant":{"authorization":{"msg":"/cosmos.bank.v1beta1.MsgSend"},"expiration":"0001-01-01T02:01:01.000000001Z"},"grantee":"cosmos1def","granter":"cosmos1abc"}],"sequence":"1","timeout_height":"1"}`
//
// This was a bug. Now, it's as below, See how there's `type` & `value` fields.
// ref: https://github.com/cosmos/cosmos-sdk/issues/11190
// ref: https://github.com/cosmos/cosmjs/issues/1026
msg = &authz.MsgGrant{Granter: "cosmos1abc", Grantee: "cosmos1def", Grant: grant}
tx.Msgs = []sdk.Msg{msg}
require.Equal(t,
`{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/authz/MsgGrant","value":{"grant":{"authorization":{"msg":"/cosmos.bank.v1beta1.MsgSend"},"expiration":"0001-01-01T02:01:01.000000001Z"},"grantee":"cosmos1def","granter":"cosmos1abc"}}],"sequence":"1","timeout_height":"1"}`,
string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)),
)

msg = &authz.MsgRevoke{Granter: "cosmos1abc", Grantee: "cosmos1def", MsgTypeUrl: typeURL}
tx.Msgs = []sdk.Msg{msg}
require.Equal(t,
`{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/authz/MsgRevoke","value":{"grantee":"cosmos1def","granter":"cosmos1abc","msg_type_url":"/cosmos.bank.v1beta1.MsgSend"}}],"sequence":"1","timeout_height":"1"}`,
string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)),
)

msg = &authz.MsgExec{Grantee: "cosmos1def", Msgs: []*cdctypes.Any{msgSendAny}}
tx.Msgs = []sdk.Msg{msg}
require.Equal(t,
`{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/authz/MsgExec","value":{"grantee":"cosmos1def","msgs":[{"amount":[],"from_address":"cosmos1ghi","to_address":"cosmos1jkl"}]}}],"sequence":"1","timeout_height":"1"}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: for nested Msgs, Amino doesn't show their type/value pair. That's because we use a local ModuleCdc to do msg.GetSignBytes, and this local ModuleCdc isn't aware of other modules' amino registrations. I think it's fine, as long as the top-level Msg has a type/value.

string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)),
)
}
25 changes: 25 additions & 0 deletions x/feegrant/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
package feegrant

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
)

// RegisterLegacyAminoCodec registers the necessary x/feegrant interfaces and concrete types
// on the provided LegacyAmino codec. These types are used for Amino JSON serialization.
func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
cdc.RegisterConcrete(&MsgGrantAllowance{}, "cosmos-sdk/feegrant/MsgGrantAllowance", nil)
cdc.RegisterConcrete(&MsgRevokeAllowance{}, "cosmos-sdk/feegrant/MsgRevokeAllowance", nil)
}

// RegisterInterfaces registers the interfaces types with the interface registry
func RegisterInterfaces(registry types.InterfaceRegistry) {
registry.RegisterImplementations((*sdk.Msg)(nil),
Expand All @@ -23,3 +31,20 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {

msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/feegrant module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
// The actual codec used for serialization should be provided to x/feegrant and
// defined at the application level.
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
amino.Seal()
}
1 change: 1 addition & 0 deletions x/feegrant/module/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {

// RegisterLegacyAminoCodec registers the feegrant module's types for the given codec.
func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
feegrant.RegisterLegacyAminoCodec(cdc)
}

// RegisterInterfaces registers the feegrant module's interface types
Expand Down
5 changes: 2 additions & 3 deletions x/feegrant/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package feegrant
import (
"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -73,7 +72,7 @@ func (msg MsgGrantAllowance) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgGrantAllowance) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetFeeAllowanceI returns unpacked FeeAllowance
Expand Down Expand Up @@ -133,5 +132,5 @@ func (msg MsgRevokeAllowance) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgRevokeAllowance) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}
29 changes: 29 additions & 0 deletions x/feegrant/msgs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
"github.com/cosmos/cosmos-sdk/x/feegrant"
)

Expand Down Expand Up @@ -132,3 +133,31 @@ func TestMsgRevokeAllowance(t *testing.T) {
}
}
}

func TestAminoJSON(t *testing.T) {
tx := legacytx.StdTx{}
var msg legacytx.LegacyMsg
allowanceAny, err := codectypes.NewAnyWithValue(&feegrant.BasicAllowance{SpendLimit: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)))})
require.NoError(t, err)

// Amino JSON encoding has changed in feegrant since v0.46.
// Before, it was outputting something like:
// `{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"allowance":{"spend_limit":[{"amount":"100","denom":"foo"}]},"grantee":"cosmos1def","granter":"cosmos1abc"}],"sequence":"1","timeout_height":"1"}`
//
// This was a bug. Now, it's as below, See how there's `type` & `value` fields.
// ref: https://github.com/cosmos/cosmos-sdk/issues/11190
// ref: https://github.com/cosmos/cosmjs/issues/1026
msg = &feegrant.MsgGrantAllowance{Granter: "cosmos1abc", Grantee: "cosmos1def", Allowance: allowanceAny}
tx.Msgs = []sdk.Msg{msg}
require.Equal(t,
`{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/feegrant/MsgGrantAllowance","value":{"allowance":{"spend_limit":[{"amount":"100","denom":"foo"}]},"grantee":"cosmos1def","granter":"cosmos1abc"}}],"sequence":"1","timeout_height":"1"}`,
string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)),
)

msg = &feegrant.MsgRevokeAllowance{Granter: "cosmos1abc", Grantee: "cosmos1def"}
tx.Msgs = []sdk.Msg{msg}
require.Equal(t,
`{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/feegrant/MsgRevokeAllowance","value":{"grantee":"cosmos1def","granter":"cosmos1abc"}}],"sequence":"1","timeout_height":"1"}`,
string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)),
)
}