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

* fix: remove time.now check from authz cosmos#11129 * fix: implement Amino serialization for x/authz and x/feegrant (cosmos#11224) * fix: Add RegisterLegacyAminoCodec for authz/feegrant * add module name * Fix GetSignByes, add tests * removed module names from registered messages to match other modules * added interfaces and concrete types registration * unseal amino instances to allow external grant and authorization registration * fixed messages tests * allow to register external types into authz modulecdc * use legacy.Cdc instead of ModuleCdc inside x/authz * move the legacy.Cdc initialization outside init function * added serialization docs * Update docs/core/encoding.md Co-authored-by: Amaury <[email protected]> * added the Ledger specification * fixed tests Co-authored-by: Amaury M <[email protected]> * revert: replace all ModuleCdc instances with legacy.Cdc (cosmos#11680) Reverts the usage of a singleton `legacy.Cdc` codec while (de)serializing `x/authz` messages. Closes: cosmos#11643 --- *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) Co-authored-by: Riccardo Montagnin <[email protected]> Co-authored-by: Amaury M <[email protected]>
osmosis-labs · May 19, 2022 · a4201aa · a4201aa
1 parent 949254a
commit a4201aa
Show file tree

Hide file tree

Showing 39 changed files with 411 additions and 169 deletions.
diff --git a/codec/legacy/codec.go b/codec/legacy/codec.go
@@ -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)
 }

diff --git a/docs/core/encoding.md b/docs/core/encoding.md
@@ -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).

diff --git a/x/auth/types/codec.go b/x/auth/types/codec.go
@@ -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
@@ -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)
 }
diff --git a/x/auth/vesting/types/codec.go b/x/auth/vesting/types/codec.go
@@ -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
@@ -63,9 +65,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)
 }
diff --git a/x/auth/vesting/types/msgs.go b/x/auth/vesting/types/msgs.go
@@ -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.
@@ -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.

diff --git a/x/authz/authorization_grant.go b/x/authz/authorization_grant.go
@@ -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,
 	}

diff --git a/x/authz/authorization_grant_test.go b/x/authz/authorization_grant_test.go
@@ -4,7 +4,6 @@ import (
 	"testing"
 	"time"
 
-	// banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/stretchr/testify/require"
 )
 
@@ -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
@@ -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)
 		})
 	}

diff --git a/x/authz/client/cli/tx.go b/x/authz/client/cli/tx.go
@@ -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..

diff --git a/x/authz/client/rest/grpc_query_test.go b/x/authz/client/rest/grpc_query_test.go
@@ -1,3 +1,4 @@
+//go:build norace
 // +build norace
 
 package rest_test
@@ -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),
@@ -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()),

diff --git a/x/authz/client/testutil/query.go b/x/authz/client/testutil/query.go
@@ -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(),
@@ -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(),

diff --git a/x/authz/client/testutil/test_helpers.go b/x/authz/client/testutil/test_helpers.go
@@ -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)

diff --git a/x/authz/client/testutil/tx.go b/x/authz/client/testutil/tx.go
@@ -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
@@ -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)
@@ -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(),
@@ -326,7 +325,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
 	s.Require().NoError(err)
 
 	// generic-authorization
-	_, err = ExecGrant(
+	_, err = CreateGrant(
 		val,
 		[]string{
 			grantee.String(),
@@ -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(),
@@ -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(),
@@ -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(),
@@ -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(),
@@ -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(),
@@ -774,7 +773,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
 	}
 
 	// test delegate no spend-limit
-	_, err = ExecGrant(
+	_, err = CreateGrant(
 		val,
 		[]string{
 			grantee.String(),
@@ -851,7 +850,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
 	}
 
 	// test delegating to denied validator
-	_, err = ExecGrant(
+	_, err = CreateGrant(
 		val,
 		[]string{
 			grantee.String(),
@@ -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(),
@@ -995,7 +994,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
 	}
 
 	// grant undelegate authorization without limit
-	_, err = ExecGrant(
+	_, err = CreateGrant(
 		val,
 		[]string{
 			grantee.String(),

diff --git a/x/authz/codec.go b/x/authz/codec.go
@@ -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),
@@ -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)
+}