Skip to content

Commit

Permalink
Backport to v0.46.x: feat: Add x/authz SendAuthorization AllowList co…
Browse files Browse the repository at this point in the history
…smos#12648 (#368)

* feat: Add x/authz SendAuthorization AllowList (cosmos#12648)

Closes: cosmos#12609

---

*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/main/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/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/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)

* Fix a unit test that for whatever reason didn't get updated in the merge.

Co-authored-by: likhita-809 <[email protected]>
  • Loading branch information
SpicyLemon and likhita-809 committed Nov 28, 2022
1 parent c2622cd commit 0a88841
Show file tree
Hide file tree
Showing 18 changed files with 360 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Features

* Add functionality to update denom metadata via gov proposal [#270](https://github.com/provenance-io/cosmos-sdk/pull/270)
* (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.

### Improvements

Expand Down
6 changes: 6 additions & 0 deletions proto/cosmos/bank/v1beta1/authz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,10 @@ 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 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;
}
35 changes: 31 additions & 4 deletions x/authz/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
FlagAllowedValidators = "allowed-validators"
FlagDenyValidators = "deny-validators"
FlagAllowedAuthorizations = "allowed-authorizations"
FlagAllowList = "allow-list"
delegate = "delegate"
redelegate = "redelegate"
unbond = "unbond"
Expand Down Expand Up @@ -110,7 +111,18 @@ Examples:
return fmt.Errorf("spend-limit should be greater than zero")
}

authorization = bank.NewSendAuthorization(spendLimit)
allowList, err := cmd.Flags().GetStringSlice(FlagAllowList)
if err != nil {
return err
}

allowed, err := bech32toAccAddresses(allowList)
if err != nil {
return err
}

authorization = bank.NewSendAuthorization(spendLimit, allowed)

case "generic":
msgType, err := cmd.Flags().GetString(FlagMsgType)
if err != nil {
Expand Down Expand Up @@ -157,12 +169,12 @@ Examples:
delegateLimit = &spendLimit
}

allowed, err := bech32toValidatorAddresses(allowValidators)
allowed, err := bech32toValAddresses(allowValidators)
if err != nil {
return err
}

denied, err := bech32toValidatorAddresses(denyValidators)
denied, err := bech32toValAddresses(denyValidators)
if err != nil {
return err
}
Expand Down Expand Up @@ -201,6 +213,7 @@ Examples:
cmd.Flags().String(FlagSpendLimit, "", "SpendLimit for Send Authorization, an array of Coins allowed spend")
cmd.Flags().StringSlice(FlagAllowedValidators, []string{}, "Allowed validators addresses separated by ,")
cmd.Flags().StringSlice(FlagDenyValidators, []string{}, "Deny validators addresses separated by ,")
cmd.Flags().StringSlice(FlagAllowList, []string{}, "Allowed addresses grantee is allowed to send funds separated by ,")
cmd.Flags().Int64(FlagExpiration, 0, "Expire time as Unix timestamp. Set zero (0) for no expiry. Default is 0.")
cmd.Flags().Int32(FlagAllowedAuthorizations, 0, "Allowed authorizations for a Count Authorization")
return cmd
Expand Down Expand Up @@ -291,7 +304,8 @@ Example:
return cmd
}

func bech32toValidatorAddresses(validators []string) ([]sdk.ValAddress, error) {
// 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)
Expand All @@ -302,3 +316,16 @@ func bech32toValidatorAddresses(validators []string) ([]sdk.ValAddress, error) {
}
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
}
2 changes: 1 addition & 1 deletion x/authz/client/testutil/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (s *IntegrationTestSuite) TestQueryGranterGrantsGRPC() {
fmt.Sprintf("%s/cosmos/authz/v1beta1/grants/granter/%s", val.APIAddress, val.Address.String()),
false,
"",
7,
8,
},
}
for _, tc := range testCases {
Expand Down
15 changes: 13 additions & 2 deletions x/authz/client/testutil/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,18 @@ func (s *IntegrationTestSuite) TestQueryAuthorization() {
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
false,
`{"@type":"/cosmos.bank.v1beta1.SendAuthorization","spend_limit":[{"denom":"stake","amount":"100"}]}`,
`{"@type":"/cosmos.bank.v1beta1.SendAuthorization","spend_limit":[{"denom":"stake","amount":"100"}],"allow_list":[]}`,
},
{
"Valid txn with allowed list (json)",
[]string{
val.Address.String(),
s.grantee[3].String(),
typeMsgSend,
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
false,
fmt.Sprintf(`{"@type":"/cosmos.bank.v1beta1.SendAuthorization","spend_limit":[{"denom":"stake","amount":"88"}],"allow_list":["%s"]}`, s.grantee[4]),
},
}
for _, tc := range testCases {
Expand Down Expand Up @@ -221,7 +232,7 @@ func (s *IntegrationTestSuite) TestQueryGranterGrants() {
},
false,
"",
7,
8,
},
{
"valid case with pagination",
Expand Down
142 changes: 140 additions & 2 deletions x/authz/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.Require().NoError(err)

val := s.network.Validators[0]
s.grantee = make([]sdk.AccAddress, 3)
s.grantee = make([]sdk.AccAddress, 6)

// Send some funds to the new account.
// Create new account in the keyring.
Expand Down Expand Up @@ -86,7 +86,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.grantee[2] = s.createAccount("grantee3")

// grant send authorization to grantee3
out, err = CreateGrant(val, []string{
_, err = CreateGrant(val, []string{
s.grantee[2].String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
Expand All @@ -98,6 +98,30 @@ func (s *IntegrationTestSuite) SetupSuite() {
})
s.Require().NoError(err)

// Create new accounts in the keyring.
s.grantee[3] = s.createAccount("grantee4")
s.msgSendExec(s.grantee[3])

s.grantee[4] = s.createAccount("grantee5")
s.grantee[5] = s.createAccount("grantee6")

// grant send authorization with allow list to grantee4
out, err = CreateGrant(
val,
[]string{
s.grantee[3].String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, time.Now().Add(time.Minute*time.Duration(120)).Unix()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, s.grantee[4]),
},
)
s.Require().NoError(err)

err = s.network.WaitForNextBlock()
s.Require().NoError(err)

Expand Down Expand Up @@ -404,6 +428,40 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
false,
"",
},
{
"Valid tx send authorization with allow list",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, s.grantee[1]),
},
0,
false,
"",
},
{
"Invalid tx send authorization with duplicate allow list",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, fmt.Sprintf("%s,%s", s.grantee[1], s.grantee[1])),
},
0,
true,
"duplicate entry",
},
{
"Valid tx generic authorization",
[]string{
Expand Down Expand Up @@ -875,6 +933,86 @@ func (s *IntegrationTestSuite) TestNewExecGrantAuthorized() {
}
}

func (s *IntegrationTestSuite) TestExecSendAuthzWithAllowList() {
val := s.network.Validators[0]
grantee := s.grantee[3]
allowedAddr := s.grantee[4]
notAllowedAddr := s.grantee[5]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
val,
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, allowedAddr),
},
)
s.Require().NoError(err)

tokens := sdk.NewCoins(
sdk.NewCoin("stake", sdk.NewInt(12)),
)

validGeneratedTx, err := banktestutil.MsgSendExec(
val.ClientCtx,
val.Address,
allowedAddr,
tokens,
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
)
s.Require().NoError(err)
execMsg := testutil.WriteToNewTempFile(s.T(), validGeneratedTx.String())

invalidGeneratedTx, err := banktestutil.MsgSendExec(
val.ClientCtx,
val.Address,
notAllowedAddr,
tokens,
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
)
s.Require().NoError(err)
execMsg1 := testutil.WriteToNewTempFile(s.T(), invalidGeneratedTx.String())

// test sending to allowed address
args := []string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
}
var response sdk.TxResponse
cmd := cli.NewCmdExecAuthorization()
out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String())

// test sending to not allowed address
args = []string{
execMsg1.Name(),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
}
out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
s.Require().NoError(err)
s.Contains(out.String(), fmt.Sprintf("cannot send to %s address", notAllowedAddr))
}

func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
val := s.network.Validators[0]
grantee := s.grantee[0]
Expand Down
9 changes: 5 additions & 4 deletions x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ func (s *TestSuite) TestKeeperIter() {
granteeAddr := addrs[1]
granter2Addr := addrs[2]
e := ctx.BlockTime().AddDate(1, 0, 0)
sendAuthz := banktypes.NewSendAuthorization(coins100, nil)

s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), &e)
s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granter2Addr, banktypes.NewSendAuthorization(coins100), &e)
s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAuthz, &e)
s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granter2Addr, sendAuthz, &e)

app.AuthzKeeper.IterateGrants(ctx, func(granter, grantee sdk.AccAddress, grant authz.Grant) bool {
s.Require().Equal(granteeAddr, grantee)
Expand All @@ -128,7 +129,7 @@ func (s *TestSuite) TestDispatchAction() {
granterAddr := addrs[0]
granteeAddr := addrs[1]
recipientAddr := addrs[2]
a := banktypes.NewSendAuthorization(coins100)
a := banktypes.NewSendAuthorization(coins100, nil)

require.NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, coins1000))

Expand Down Expand Up @@ -374,7 +375,7 @@ func (s *TestSuite) TestGetAuthorization() {

genAuthMulti := authz.NewGenericAuthorization(sdk.MsgTypeURL(&banktypes.MsgMultiSend{}))
genAuthSend := authz.NewGenericAuthorization(sdk.MsgTypeURL(&banktypes.MsgSend{}))
sendAuth := banktypes.NewSendAuthorization(coins10)
sendAuth := banktypes.NewSendAuthorization(coins10, nil)

start := s.ctx.BlockHeader().Time
expired := start.Add(time.Duration(1) * time.Second)
Expand Down
5 changes: 3 additions & 2 deletions x/authz/migrations/v046/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestMigration(t *testing.T) {
blockTime := ctx.BlockTime()
oneDay := blockTime.AddDate(0, 0, 1)
oneYear := blockTime.AddDate(1, 0, 0)
sendAuthz := banktypes.NewSendAuthorization(coins100, nil)

grants := []struct {
granter sdk.AccAddress
Expand All @@ -44,7 +45,7 @@ func TestMigration(t *testing.T) {
grantee1,
sendMsgType,
func() authz.Grant {
any, err := codectypes.NewAnyWithValue(banktypes.NewSendAuthorization(coins100))
any, err := codectypes.NewAnyWithValue(sendAuthz)
require.NoError(t, err)
return authz.Grant{
Authorization: any,
Expand All @@ -57,7 +58,7 @@ func TestMigration(t *testing.T) {
grantee2,
sendMsgType,
func() authz.Grant {
any, err := codectypes.NewAnyWithValue(banktypes.NewSendAuthorization(coins100))
any, err := codectypes.NewAnyWithValue(sendAuthz)
require.NoError(t, err)
return authz.Grant{
Authorization: any,
Expand Down
3 changes: 2 additions & 1 deletion x/authz/module/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ 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 := banktypes.NewSendAuthorization(smallCoins, nil)

save := func(grantee sdk.AccAddress, exp *time.Time) {
err := app.AuthzKeeper.SaveGrant(ctx, grantee, granter, banktypes.NewSendAuthorization(smallCoins), exp)
err := app.AuthzKeeper.SaveGrant(ctx, grantee, granter, sendAuthz, exp)
require.NoError(t, err, "Grant from %s", grantee.String())
}
save(grantee1, &expiration)
Expand Down
3 changes: 2 additions & 1 deletion x/authz/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ func TestAminoJSON(t *testing.T) {
require.NoError(t, err)
grant, err := authz.NewGrant(blockTime, authz.NewGenericAuthorization(typeURL), &expiresAt)
require.NoError(t, err)
sendGrant, err := authz.NewGrant(blockTime, banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000)))), &expiresAt)
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")
require.NoError(t, err)
Expand Down
Loading

0 comments on commit 0a88841

Please sign in to comment.