refactor(x/authz): audit x/authz changes (backport #21251) (#21254)

Co-authored-by: son trinh <trinhleson2000@gmail.com>
cosmos · Aug 12, 2024 · 8c83145 · 8c83145
1 parent 5bd3049
commit 8c83145
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 10 deletions.
diff --git a/x/authz/CHANGELOG.md b/x/authz/CHANGELOG.md
@@ -31,6 +31,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#20161](https://github.com/cosmos/cosmos-sdk/pull/20161) Added `RevokeAll` method to revoke all grants at once.
 * [#20687](https://github.com/cosmos/cosmos-sdk/pull/20687) Prevent user to grant authz MsgGrant to other accounts. Preventing user from accidentally authorizing their entire account to a different account.
 
+### Improvements 
+
+* [#18070](https://github.com/cosmos/cosmos-sdk/pull/18070) Use clientCtx adress codecs in cli.
+
 ### API Breaking Changes
 
 * [#20502](https://github.com/cosmos/cosmos-sdk/pull/20502) `Accept` on the `Authorization` interface now expects the authz environment in the `context.Context`. This is already done when `Accept` is called by `k.DispatchActions`, but should be done manually if `Accept` is called directly.

diff --git a/x/authz/authorization_grant_test.go b/x/authz/authorization_grant_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 	"time"
 
+	any "github.com/cosmos/gogoproto/types/any"
 	"github.com/stretchr/testify/require"
 )
 
@@ -42,6 +43,33 @@ func TestNewGrant(t *testing.T) {
 	}
 }
 
+func TestValidateBasic(t *testing.T) {
+	validAuthz, err := any.NewAnyWithCacheWithValue(NewGenericAuthorization("some-type"))
+	require.NoError(t, err)
+	invalidAuthz, err := any.NewAnyWithCacheWithValue(&Grant{})
+	require.NoError(t, err)
+	tcs := []struct {
+		title         string
+		authorization *any.Any
+		err           string
+	}{
+		{"valid grant", validAuthz, ""},
+		{"invalid authorization", invalidAuthz, "invalid type"},
+		{"empty authorization", nil, "authorization is nil"},
+	}
+
+	for _, tc := range tcs {
+		tc := tc
+		t.Run(tc.title, func(t *testing.T) {
+			grant := Grant{
+				Authorization: tc.authorization,
+			}
+			err := grant.ValidateBasic()
+			expecError(require.New(t), tc.err, err)
+		})
+	}
+}
+
 func unixTime(s, ns int64) *time.Time {
 	t := time.Unix(s, ns)
 	return &t

diff --git a/x/authz/client/cli/tx_test.go b/x/authz/client/cli/tx_test.go
@@ -6,8 +6,6 @@ import (
 	"testing"
 	"time"
 
-	abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
-	rpcclientmock "github.com/cometbft/cometbft/rpc/client/mock"
 	"github.com/stretchr/testify/suite"
 
 	_ "cosmossdk.io/api/cosmos/authz/v1beta1"
@@ -59,7 +57,6 @@ func (s *CLITestSuite) SetupSuite() {
 		WithKeyring(s.kr).
 		WithTxConfig(s.encCfg.TxConfig).
 		WithCodec(s.encCfg.Codec).
-		WithClient(clitestutil.MockCometRPC{Client: rpcclientmock.Client{}}).
 		WithAccountRetriever(client.MockAccountRetriever{}).
 		WithOutput(io.Discard).
 		WithChainID("test-chain").
@@ -71,9 +68,7 @@ func (s *CLITestSuite) SetupSuite() {
 
 	ctxGen := func() client.Context {
 		bz, _ := s.encCfg.Codec.Marshal(&sdk.TxResponse{})
-		c := clitestutil.NewMockCometRPC(abci.QueryResponse{
-			Value: bz,
-		})
+		c := clitestutil.NewMockCometRPCWithValue(bz)
 		return s.baseCtx.WithClient(c)
 	}
 	s.clientCtx = ctxGen()

diff --git a/x/authz/go.mod b/x/authz/go.mod
@@ -14,7 +14,7 @@ require (
 	cosmossdk.io/x/staking v0.0.0-00010101000000-000000000000
 	cosmossdk.io/x/tx v0.13.4-0.20240802064046-23fac2f1b8ab // main
 	github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
-	github.com/cometbft/cometbft v1.0.0-rc1
+	github.com/cometbft/cometbft v1.0.0-rc1 // indirect
 	github.com/cosmos/cosmos-proto v1.0.0-beta.5
 	github.com/cosmos/cosmos-sdk v0.52.0
 	github.com/cosmos/gogoproto v1.6.0
@@ -51,7 +51,7 @@ require (
 	github.com/cockroachdb/pebble v1.1.0 // indirect
 	github.com/cockroachdb/redact v1.1.5 // indirect
 	github.com/cometbft/cometbft-db v0.12.0 // indirect
-	github.com/cometbft/cometbft/api v1.0.0-rc.1
+	github.com/cometbft/cometbft/api v1.0.0-rc.1 // indirect
 	github.com/cosmos/btcutil v1.0.5 // indirect
 	github.com/cosmos/cosmos-db v1.0.2 // indirect
 	github.com/cosmos/crypto v0.1.2 // indirect

diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go
@@ -58,7 +58,7 @@ func (k Keeper) getGrant(ctx context.Context, skey []byte) (grant authz.Grant, f
 	return grant, true
 }
 
-func (k Keeper) update(ctx context.Context, grantee, granter sdk.AccAddress, updated authz.Authorization) error {
+func (k Keeper) updateGrant(ctx context.Context, grantee, granter sdk.AccAddress, updated authz.Authorization) error {
 	skey := grantStoreKey(grantee, granter, updated.MsgTypeURL())
 	grant, found := k.getGrant(ctx, skey)
 	if !found {
@@ -133,7 +133,7 @@ func (k Keeper) DispatchActions(ctx context.Context, grantee sdk.AccAddress, msg
 				if !ok {
 					return nil, fmt.Errorf("expected authz.Authorization but got %T", resp.Updated)
 				}
-				err = k.update(ctx, grantee, granter, updated)
+				err = k.updateGrant(ctx, grantee, granter, updated)
 			}
 			if err != nil {
 				return nil, err