address review comments

CHANGELOG.md

@@ -74,7 +74,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
-* (x/authz) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) Add an allow list, an optional list of addresses allowed to receive bank assests via authz MsgSend grant.
+* (x/bank) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) `NewSendAuthorization` takes a new argument of  an optional list of addresses allowed to receive bank assests via authz MsgSend grant. You can pass `nil` for the same behavior as before, i.e. any recipient is allowed.
 * (x/bank) [\#12593](https://github.com/cosmos/cosmos-sdk/pull/12593) Add `SpendableCoin` method to `BaseViewKeeper`
 * (x/slashing) [#12581](https://github.com/cosmos/cosmos-sdk/pull/12581) Remove `x/slashing` legacy querier.
 * (types) [\#12355](https://github.com/cosmos/cosmos-sdk/pull/12355) Remove the compile-time `types.DBbackend` variable. Removes usage of the same in server/util.go

proto/cosmos/bank/v1beta1/authz.proto

@@ -17,7 +17,9 @@ message SendAuthorization {
   repeated cosmos.base.v1beta1.Coin spend_limit = 1
       [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
 
-  // allow_list specifies the list of addresses to whom the grantee can send tokens on behalf of granter's 
-  // account.
+  // allow_list specifies an optional list of addresses to whom the grantee can send tokens on behalf of the 
+  // granter. If omitted, any recipient is allowed. 
+  //
+  // Since: cosmos-sdk 0.47
   repeated string allow_list = 2;
 }

types/address.go

@@ -677,29 +677,3 @@ func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey
 	cache.Add(cacheKey, bech32Addr)
 	return bech32Addr
 }
-
-// Bech32toValAddresses returns []ValAddress from a list of Bech32 string addresses.
-func Bech32toValAddresses(validators []string) ([]ValAddress, error) {
-	vals := make([]ValAddress, len(validators))
-	for i, validator := range validators {
-		addr, err := ValAddressFromBech32(validator)
-		if err != nil {
-			return nil, err
-		}
-		vals[i] = addr
-	}
-	return vals, nil
-}
-
-// Bech32toAccAddresses returns []AccAddress from a list of Bech32 string addresses.
-func Bech32toAccAddresses(accAddrs []string) ([]AccAddress, error) {
-	addrs := make([]AccAddress, len(accAddrs))
-	for i, addr := range accAddrs {
-		accAddr, err := AccAddressFromBech32(addr)
-		if err != nil {
-			return nil, err
-		}
-		addrs[i] = accAddr
-	}
-	return addrs, nil
-}

x/authz/client/cli/tx.go

@@ -99,15 +99,12 @@ Examples:
 					return err
 				}
 
-				allowed, err := sdk.Bech32toAccAddresses(allowList)
+				allowed, err := bech32toAccAddresses(allowList)
 				if err != nil {
 					return err
 				}
 
-				authorization, err = bank.NewSendAuthorization(allowed, spendLimit)
-				if err != nil {
-					return err
-				}
+				authorization = bank.NewSendAuthorization(spendLimit, allowed)
 
 			case "generic":
 				msgType, err := cmd.Flags().GetString(FlagMsgType)
@@ -155,12 +152,12 @@ Examples:
 					delegateLimit = &spendLimit
 				}
 
-				allowed, err := sdk.Bech32toValAddresses(allowValidators)
+				allowed, err := bech32toValAddresses(allowValidators)
 				if err != nil {
 					return err
 				}
 
-				denied, err := sdk.Bech32toValAddresses(denyValidators)
+				denied, err := bech32toValAddresses(denyValidators)
 				if err != nil {
 					return err
 				}
@@ -288,3 +285,29 @@ Example:
 
 	return cmd
 }
+
+// bech32toValAddresses returns []ValAddress from a list of Bech32 string addresses.
+func bech32toValAddresses(validators []string) ([]sdk.ValAddress, error) {
+	vals := make([]sdk.ValAddress, len(validators))
+	for i, validator := range validators {
+		addr, err := sdk.ValAddressFromBech32(validator)
+		if err != nil {
+			return nil, err
+		}
+		vals[i] = addr
+	}
+	return vals, nil
+}
+
+// bech32toAccAddresses returns []AccAddress from a list of Bech32 string addresses.
+func bech32toAccAddresses(accAddrs []string) ([]sdk.AccAddress, error) {
+	addrs := make([]sdk.AccAddress, len(accAddrs))
+	for i, addr := range accAddrs {
+		accAddr, err := sdk.AccAddressFromBech32(addr)
+		if err != nil {
+			return nil, err
+		}
+		addrs[i] = accAddr
+	}
+	return addrs, nil
+}

x/authz/keeper/keeper_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	tmtime "github.com/tendermint/tendermint/libs/time"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -126,8 +125,7 @@ func (s *TestSuite) TestKeeperIter() {
 	granteeAddr := addrs[1]
 	granter2Addr := addrs[2]
 	e := ctx.BlockTime().AddDate(1, 0, 0)
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, coins100)
-	require.NoError(s.T(), err)
+	sendAuthz := banktypes.NewSendAuthorization(coins100, nil)
 
 	s.authzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAuthz, &e)
 	s.authzKeeper.SaveGrant(ctx, granteeAddr, granter2Addr, sendAuthz, &e)
@@ -147,8 +145,7 @@ func (s *TestSuite) TestDispatchAction() {
 	granterAddr := addrs[0]
 	granteeAddr := addrs[1]
 	recipientAddr := addrs[2]
-	a, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, coins100)
-	require.NoError(err)
+	a := banktypes.NewSendAuthorization(coins100, nil)
 
 	require.NoError(banktestutil.FundAccount(s.bankKeeper, s.ctx, granterAddr, coins1000))
 

x/authz/migrations/v046/store_test.go

@@ -35,8 +35,7 @@ func TestMigration(t *testing.T) {
 	blockTime := ctx.BlockTime()
 	oneDay := blockTime.AddDate(0, 0, 1)
 	oneYear := blockTime.AddDate(1, 0, 0)
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, coins100)
-	require.NoError(t, err)
+	sendAuthz := banktypes.NewSendAuthorization(coins100, nil)
 
 	grants := []struct {
 		granter       sdk.AccAddress

x/authz/module/abci_test.go

@@ -44,8 +44,7 @@ func TestExpiredGrantsQueue(t *testing.T) {
 	expiration := ctx.BlockTime().AddDate(0, 1, 0)
 	expiration2 := expiration.AddDate(1, 0, 0)
 	smallCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 10))
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, smallCoins)
-	require.NoError(t, err)
+	sendAuthz := banktypes.NewSendAuthorization(smallCoins, nil)
 
 	save := func(grantee sdk.AccAddress, exp *time.Time) {
 		err := authzKeeper.SaveGrant(ctx, grantee, granter, sendAuthz, exp)

x/authz/msgs_test.go

@@ -174,8 +174,7 @@ func TestAminoJSON(t *testing.T) {
 	require.NoError(t, err)
 	grant, err := authz.NewGrant(blockTime, authz.NewGenericAuthorization(typeURL), &expiresAt)
 	require.NoError(t, err)
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))))
-	require.NoError(t, err)
+	sendAuthz := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))), nil)
 	sendGrant, err := authz.NewGrant(blockTime, sendAuthz, &expiresAt)
 	require.NoError(t, err)
 	valAddr, err := sdk.ValAddressFromBech32("cosmosvaloper1xcy3els9ua75kdm783c3qu0rfa2eples6eavqq")

x/authz/simulation/decoder_test.go

@@ -26,8 +26,7 @@ func TestDecodeStore(t *testing.T) {
 
 	now := time.Now().UTC()
 	e := now.Add(1)
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, sdk.NewCoins(sdk.NewInt64Coin("foo", 123)))
-	require.NoError(t, err)
+	sendAuthz := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123)), nil)
 	grant, _ := authz.NewGrant(now, sendAuthz, &e)
 	grantBz, err := cdc.Marshal(&grant)
 	require.NoError(t, err)

x/authz/simulation/genesis.go

@@ -2,7 +2,6 @@ package simulation
 
 import (
 	"math/rand"
-	"testing"
 	"time"
 
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -12,7 +11,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/authz"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
-	"github.com/stretchr/testify/require"
 )
 
 // genGrant returns a slice of authorization grants.
@@ -26,8 +24,7 @@ func genGrant(r *rand.Rand, accounts []simtypes.Account, genT time.Time) []authz
 			e := genT.AddDate(1, 0, 0)
 			expiration = &e
 		}
-		grant, err := generateRandomGrant(r)
-		require.NoError(&testing.T{}, err)
+		grant := generateRandomGrant(r)
 		authorizations[i] = authz.GrantAuthorization{
 			Granter:       granter.Address.String(),
 			Grantee:       grantee.Address.String(),
@@ -39,16 +36,13 @@ func genGrant(r *rand.Rand, accounts []simtypes.Account, genT time.Time) []authz
 	return authorizations
 }
 
-func generateRandomGrant(r *rand.Rand) (*codectypes.Any, error) {
+func generateRandomGrant(r *rand.Rand) *codectypes.Any {
 	authorizations := make([]*codectypes.Any, 2)
-	sendAuthz, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))))
-	if err != nil {
-		return nil, err
-	}
+	sendAuthz := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))), nil)
 	authorizations[0] = newAnyAuthorization(sendAuthz)
 	authorizations[1] = newAnyAuthorization(authz.NewGenericAuthorization(sdk.MsgTypeURL(&v1.MsgSubmitProposal{})))
 
-	return authorizations[r.Intn(len(authorizations))], nil
+	return authorizations[r.Intn(len(authorizations))]
 }
 
 func newAnyAuthorization(a authz.Authorization) *codectypes.Any {

x/authz/simulation/operations.go

@@ -152,10 +152,7 @@ func SimulateMsgGrant(cdc *codec.ProtoCodec, ak authz.AccountKeeper, bk authz.Ba
 
 func generateRandomAuthorization(r *rand.Rand, spendLimit sdk.Coins) (authz.Authorization, error) {
 	authorizations := make([]authz.Authorization, 2)
-	sendAuthz, err := banktype.NewSendAuthorization([]sdk.AccAddress{}, spendLimit)
-	if err != nil {
-		return nil, err
-	}
+	sendAuthz := banktype.NewSendAuthorization(spendLimit, nil)
 	authorizations[0] = sendAuthz
 	authorizations[1] = authz.NewGenericAuthorization(sdk.MsgTypeURL(&banktype.MsgSend{}))
 

x/authz/simulation/operations_test.go

@@ -161,11 +161,10 @@ func (suite *SimTestSuite) TestSimulateRevoke() {
 
 	granter := accounts[0]
 	grantee := accounts[1]
-	a, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, initCoins)
-	suite.Require().NoError(err)
+	a := banktypes.NewSendAuthorization(initCoins, nil)
 	expire := time.Now().Add(30 * time.Hour)
 
-	err = suite.authzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire)
+	err := suite.authzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire)
 	suite.Require().NoError(err)
 
 	// execute operation
@@ -197,11 +196,10 @@ func (suite *SimTestSuite) TestSimulateExec() {
 
 	granter := accounts[0]
 	grantee := accounts[1]
-	a, err := banktypes.NewSendAuthorization([]sdk.AccAddress{}, initCoins)
-	suite.Require().NoError(err)
+	a := banktypes.NewSendAuthorization(initCoins, nil)
 	expire := suite.ctx.BlockTime().Add(1 * time.Hour)
 
-	err = suite.authzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire)
+	err := suite.authzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire)
 	suite.Require().NoError(err)
 
 	// execute operation

x/bank/types/send_authorization.go

@@ -13,19 +13,17 @@ const gasCostPerIteration = uint64(10)
 var _ authz.Authorization = &SendAuthorization{}
 
 // NewSendAuthorization creates a new SendAuthorization object.
-func NewSendAuthorization(allowed []sdk.AccAddress, spendLimit sdk.Coins) (*SendAuthorization, error) {
+func NewSendAuthorization(spendLimit sdk.Coins, allowed []sdk.AccAddress) *SendAuthorization {
 	allowedAddrs := toBech32Addresses(allowed)
 
 	a := SendAuthorization{}
-	if allowedAddrs != nil {
-		a.AllowList = allowedAddrs
-	}
+	a.AllowList = allowedAddrs
 
 	if spendLimit != nil {
 		a.SpendLimit = spendLimit
 	}
 
-	return &a, nil
+	return &a
 }
 
 // MsgTypeURL implements Authorization.MsgTypeURL.
@@ -35,12 +33,11 @@ func (a SendAuthorization) MsgTypeURL() string {
 
 // Accept implements Authorization.Accept.
 func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptResponse, error) {
-	var toAddr string
 	mSend, ok := msg.(*MsgSend)
 	if !ok {
 		return authz.AcceptResponse{}, sdkerrors.ErrInvalidType.Wrap("type mismatch")
 	}
-	toAddr = mSend.ToAddress
+	toAddr := mSend.ToAddress
 	limitLeft, isNegative := a.SpendLimit.SafeSub(mSend.Amount...)
 	if isNegative {
 		return authz.AcceptResponse{}, sdkerrors.ErrInsufficientFunds.Wrapf("requested amount is more than spend limit")

x/bank/types/send_authorization_test.go

@@ -21,8 +21,7 @@ var (
 func TestSendAuthorization(t *testing.T) {
 	app := simapp.Setup(t, false)
 	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
-	authorization, err := types.NewSendAuthorization([]sdk.AccAddress{}, coins1000)
-	require.NoError(t, err)
+	authorization := types.NewSendAuthorization(coins1000, nil)
 
 	t.Log("verify authorization returns valid method name")
 	require.Equal(t, authorization.MsgTypeURL(), "/cosmos.bank.v1beta1.MsgSend")
@@ -37,8 +36,7 @@ func TestSendAuthorization(t *testing.T) {
 	require.True(t, resp.Delete)
 	require.Nil(t, resp.Updated)
 
-	authorization, err = types.NewSendAuthorization([]sdk.AccAddress{}, coins1000)
-	require.NoError(t, err)
+	authorization = types.NewSendAuthorization(coins1000, nil)
 	require.Equal(t, authorization.MsgTypeURL(), "/cosmos.bank.v1beta1.MsgSend")
 	require.NoError(t, authorization.ValidateBasic())
 	send = types.NewMsgSend(fromAddr, toAddr, coins500)
@@ -49,8 +47,7 @@ func TestSendAuthorization(t *testing.T) {
 	require.NoError(t, err)
 	require.False(t, resp.Delete)
 	require.NotNil(t, resp.Updated)
-	sendAuth, err := types.NewSendAuthorization([]sdk.AccAddress{}, coins500)
-	require.NoError(t, err)
+	sendAuth := types.NewSendAuthorization(coins500, nil)
 	require.Equal(t, sendAuth.String(), resp.Updated.String())
 
 	t.Log("expect updated authorization nil after spending remaining amount")