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

fix: implement Amino serialization for x/authz and x/feegrant #224

Merged
merged 5 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 1 addition & 2 deletions codec/legacy/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import (
// has all Tendermint crypto and evidence types registered.
//
// TODO: Deprecated - remove this global.
var Cdc *codec.LegacyAmino
var Cdc = codec.NewLegacyAmino()

func init() {
Cdc = codec.NewLegacyAmino()
cryptocodec.RegisterCrypto(Cdc)
codec.RegisterEvidences(Cdc)
}
Expand Down
18 changes: 18 additions & 0 deletions docs/core/encoding.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,24 @@ Note, there are length-prefixed variants of the above functionality and this is
typically used for when the data needs to be streamed or grouped together
(e.g. `ResponseDeliverTx.Data`)

#### Authz authorizations

Since the `MsgExec` message type can contain different messages instances, it is important that developers
add the following code inside the `init` method of their module's `codec.go` file:

```go
import authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"

init() {
// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
```

This will allow the `x/authz` module to properly serialize and de-serializes `MsgExec` instances using Amino,
which is required when signing this kind of messages using a Ledger.

### Gogoproto

Modules are encouraged to utilize Protobuf encoding for their respective types. In the SDK, we use the [Gogoproto](https://github.com/gogo/protobuf) specific implementation of the Protobuf spec that offers speed and DX improvements compared to the official [Google protobuf implementation](https://github.com/protocolbuffers/protobuf).
Expand Down
7 changes: 7 additions & 0 deletions x/auth/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the account interfaces and concrete types on the
Expand Down Expand Up @@ -45,4 +47,9 @@ var (
func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
14 changes: 12 additions & 2 deletions x/auth/vesting/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package types
import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the vesting interfaces and concrete types on the
Expand Down Expand Up @@ -58,9 +60,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var amino = codec.NewLegacyAmino()
var (
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
amino.Seal()
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
4 changes: 2 additions & 2 deletions x/auth/vesting/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (msg MsgCreateVestingAccount) ValidateBasic() error {
// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreateVestingAccount.
func (msg MsgCreateVestingAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(amino.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreateVestingAccount.
Expand Down Expand Up @@ -112,7 +112,7 @@ func (msg MsgCreateClawbackVestingAccount) GetSigners() []sdk.AccAddress {
// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreateClawbackVestingAccount.
func (msg MsgCreateClawbackVestingAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(amino.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// ValidateBasic Implements Msg.
Expand Down
12 changes: 6 additions & 6 deletions x/authz/authorization_grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// NewGrant returns new Grant
func NewGrant( /*blockTime time.Time, */ a Authorization, expiration time.Time) (Grant, error) {
// TODO: add this for 0.45
// if !expiration.After(blockTime) {
// return Grant{}, sdkerrors.ErrInvalidRequest.Wrapf("expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
// }
// NewGrant returns new Grant. It returns an error if the expiration is before
// the current block time, which is passed into the `blockTime` arg.
func NewGrant(blockTime time.Time, a Authorization, expiration time.Time) (Grant, error) {
if !expiration.After(blockTime) {
return Grant{}, sdkerrors.ErrInvalidRequest.Wrapf("expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
}
g := Grant{
Expiration: expiration,
}
Expand Down
10 changes: 4 additions & 6 deletions x/authz/authorization_grant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"
"time"

// banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/require"
)

Expand All @@ -18,7 +17,6 @@ func expecError(r *require.Assertions, expected string, received error) {
}

func TestNewGrant(t *testing.T) {
// ba := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123)))
a := NewGenericAuthorization("some-type")
var tcs = []struct {
title string
Expand All @@ -27,16 +25,16 @@ func TestNewGrant(t *testing.T) {
expire time.Time
err string
}{
// {"wrong expire time (1)", a, time.Unix(10, 0), time.Unix(8, 0), "expiration must be after"},
// {"wrong expire time (2)", a, time.Unix(10, 0), time.Unix(10, 0), "expiration must be after"},
{"wrong expire time (1)", a, time.Unix(10, 0), time.Unix(8, 0), "expiration must be after"},
{"wrong expire time (2)", a, time.Unix(10, 0), time.Unix(10, 0), "expiration must be after"},
{"good expire time (1)", a, time.Unix(10, 0), time.Unix(10, 1), ""},
{"good expire time (2)", a, time.Unix(10, 0), time.Unix(11, 0), ""},
}

for _, tc := range tcs {
tc := tc
t.Run(tc.title, func(t *testing.T) {
// _, err := NewGrant(tc.blockTime, tc.a, tc.expire)
_, err := NewGrant(tc.a, tc.expire)
_, err := NewGrant(tc.blockTime, tc.a, tc.expire)
expecError(require.New(t), tc.err, err)
})
}
Expand Down
2 changes: 1 addition & 1 deletion x/authz/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func NewCmdGrantAuthorization() *cobra.Command {
Use: "grant <grantee> <authorization_type=\"send\"|\"generic\"|\"delegate\"|\"unbond\"|\"redelegate\"> --from <granter>",
Short: "Grant authorization to an address",
Long: strings.TrimSpace(
fmt.Sprintf(`grant authorization to an address to execute a transaction on your behalf:
fmt.Sprintf(`create a new grant authorization to an address to execute a transaction on your behalf:

Examples:
$ %s tx %s grant cosmos1skjw.. send %s --spend-limit=1000stake --from=cosmos1skl..
Expand Down
5 changes: 3 additions & 2 deletions x/authz/client/rest/grpc_query_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build norace
// +build norace

package rest_test
Expand Down Expand Up @@ -61,7 +62,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.Require().Contains(out.String(), `"code":0`)

// grant authorization
out, err = authztestutil.ExecGrant(val, []string{
out, err = authztestutil.CreateGrant(val, []string{
newAddr.String(),
"send",
fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
Expand Down Expand Up @@ -177,7 +178,7 @@ func (s *IntegrationTestSuite) TestQueryGrantsGRPC() {
false,
"",
func() {
_, err := authztestutil.ExecGrant(val, []string{
_, err := authztestutil.CreateGrant(val, []string{
s.grantee.String(),
"generic",
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
Expand Down
4 changes: 2 additions & 2 deletions x/authz/client/testutil/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorizations() {
grantee := s.grantee
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -98,7 +98,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorization() {
grantee := s.grantee
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down
2 changes: 1 addition & 1 deletion x/authz/client/testutil/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/authz/client/cli"
)

func ExecGrant(val *network.Validator, args []string) (testutil.BufferWriter, error) {
func CreateGrant(val *network.Validator, args []string) (testutil.BufferWriter, error) {
cmd := cli.NewCmdGrantAuthorization()
clientCtx := val.ClientCtx
return clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
Expand Down
31 changes: 15 additions & 16 deletions x/authz/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
val := s.network.Validators[0]
grantee := s.grantee

twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()
pastHour := time.Now().Add(time.Minute * time.Duration(-60)).Unix()
twoHours := time.Now().Add(time.Minute * 120).Unix()
pastHour := time.Now().Add(-time.Minute * 60).Unix()

testCases := []struct {
name string
Expand Down Expand Up @@ -278,15 +278,14 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
}

for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
clientCtx := val.ClientCtx
out, err := ExecGrant(
out, err := CreateGrant(
val,
tc.args,
)
if tc.expectErr {
s.Require().Error(err)
s.Require().Error(err, out)
} else {
var txResp sdk.TxResponse
s.Require().NoError(err)
Expand All @@ -310,7 +309,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

// send-authorization
_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand All @@ -326,7 +325,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
s.Require().NoError(err)

// generic-authorization
_, err = ExecGrant(
_, err = CreateGrant(
val,
[]string{
grantee.String(),
Expand All @@ -342,7 +341,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
s.Require().NoError(err)

// generic-authorization used for amino testing
_, err = ExecGrant(
_, err = CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -455,7 +454,7 @@ func (s *IntegrationTestSuite) TestExecAuthorizationWithExpiration() {
grantee := s.grantee
tenSeconds := time.Now().Add(time.Second * time.Duration(10)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -495,7 +494,7 @@ func (s *IntegrationTestSuite) TestNewExecGenericAuthorized() {
grantee := s.grantee
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -597,7 +596,7 @@ func (s *IntegrationTestSuite) TestNewExecGrantAuthorized() {
grantee := s.grantee
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -682,7 +681,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
grantee := s.grantee
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -774,7 +773,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}

// test delegate no spend-limit
_, err = ExecGrant(
_, err = CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -851,7 +850,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}

// test delegating to denied validator
_, err = ExecGrant(
_, err = CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -886,7 +885,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

// granting undelegate msg authorization
_, err := ExecGrant(
_, err := CreateGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -995,7 +994,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
}

// grant undelegate authorization without limit
_, err = ExecGrant(
_, err = CreateGrant(
val,
[]string{
grantee.String(),
Expand Down
18 changes: 18 additions & 0 deletions x/authz/codec.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
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"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// 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/MsgGrant", nil)
cdc.RegisterConcrete(&MsgRevoke{}, "cosmos-sdk/MsgRevoke", nil)
cdc.RegisterConcrete(&MsgExec{}, "cosmos-sdk/MsgExec", nil)

cdc.RegisterInterface((*Authorization)(nil), nil)
cdc.RegisterConcrete(&GenericAuthorization{}, "cosmos-sdk/GenericAuthorization", nil)
}

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

msgservice.RegisterMsgServiceDesc(registry, MsgServiceDesc())
}
func init() {
// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
Loading