From c1b6ace7d542925b526cf3eef6df38a206eab8d8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 24 Mar 2022 19:27:23 +0100 Subject: [PATCH] feat!: make grant expiration optional (#11060) ## Description Closes: #11095 --- ### Author Checklist *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 ### Reviewers Checklist *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) --- CHANGELOG.md | 4 + api/cosmos/authz/v1beta1/authz.pulsar.go | 53 ++++++----- proto/cosmos/authz/v1beta1/authz.proto | 11 ++- x/authz/authorization_grant.go | 20 ++-- x/authz/authorization_grant_test.go | 16 +++- x/authz/authz.pb.go | 116 +++++++++++++---------- x/authz/client/cli/tx.go | 26 +++-- x/authz/keeper/genesis_test.go | 6 +- x/authz/keeper/grpc_query_test.go | 53 ++++------- x/authz/keeper/keeper.go | 57 ++++++----- x/authz/keeper/keeper_test.go | 38 +++++--- x/authz/keeper/keys.go | 8 +- x/authz/migrations/v046/store.go | 7 +- x/authz/migrations/v046/store_test.go | 12 ++- x/authz/module/abci_test.go | 55 +++++++---- x/authz/msgs.go | 2 +- x/authz/msgs_test.go | 56 +++++++---- x/authz/simulation/decoder_test.go | 3 +- x/authz/simulation/genesis.go | 7 +- x/authz/simulation/operations.go | 43 ++++----- x/authz/simulation/operations_test.go | 26 +++-- 21 files changed, 358 insertions(+), 261 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56a6aaae84e4..79985a51dfb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#11234](https://github.com/cosmos/cosmos-sdk/pull/11234) Add `GRPCClient` field to Client Context. If `GRPCClient` field is set to nil, the `Invoke` method would use ABCI query, otherwise use gprc. * [\#10962](https://github.com/cosmos/cosmos-sdk/pull/10962) ADR-040: Add state migration from iavl (v1Store) to smt (v2Store) * (types) [\#10948](https://github.com/cosmos/cosmos-sdk/issues/10948) Add `app-db-backend` to the `app.toml` config to replace the compile-time `types.DBbackend` variable. +* (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) Support grant with no expire time. ### API Breaking Changes @@ -142,6 +143,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/authz) [\#10447](https://github.com/cosmos/cosmos-sdk/pull/10447) authz `NewGrant` takes a new argument: block time, to correctly validate expire time. * [\#10961](https://github.com/cosmos/cosmos-sdk/pull/10961) Support third-party modules to add extension snapshots to state-sync. * [\#11274](https://github.com/cosmos/cosmos-sdk/pull/11274) `types/errors.New` now is an alias for `types/errors.Register` and should only be used in initialization code. +* (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) `authz.NewMsgGrant` `expiration` is now a pointer. When `nil` is used then no expiration will be set (grant won't expire). ### Client Breaking Changes @@ -161,6 +163,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Rename `--fee-account` CLI flag to `--fee-granter` * [\#10684](https://github.com/cosmos/cosmos-sdk/pull/10684) Rename `edit-validator` command's `--moniker` flag to `--new-moniker` * [\#11116](https://github.com/cosmos/cosmos-sdk/pull/11116) `software-upgrade` and `cancel-software-upgrade` gov proposal commands have changed to `legacy-software-upgrade` and `legacy-cancel-software-upgrade`. +* (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) Changed the default value of the `--expiration` `tx grant` CLI Flag: was now + 1year, update: null (no expire date). ### Improvements @@ -250,6 +253,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#11019](https://github.com/cosmos/cosmos-sdk/pull/11019) Add `MsgCreatePermanentLockedAccount` and CLI method for creating permanent locked account * (x/feegrant) [\#10830](https://github.com/cosmos/cosmos-sdk/pull/10830) Expired allowances will be pruned from state. * (x/authz,x/feegrant) [\#11214](https://github.com/cosmos/cosmos-sdk/pull/11214) Fix Amino JSON encoding of authz and feegrant Msgs to be consistent with other modules. +* (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) Support grant with no expire time. ### Deprecated diff --git a/api/cosmos/authz/v1beta1/authz.pulsar.go b/api/cosmos/authz/v1beta1/authz.pulsar.go index eb75eff56b78..eb34ad4a6db1 100644 --- a/api/cosmos/authz/v1beta1/authz.pulsar.go +++ b/api/cosmos/authz/v1beta1/authz.pulsar.go @@ -2132,8 +2132,11 @@ type Grant struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Authorization *anypb.Any `protobuf:"bytes,1,opt,name=authorization,proto3" json:"authorization,omitempty"` - Expiration *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=expiration,proto3" json:"expiration,omitempty"` + Authorization *anypb.Any `protobuf:"bytes,1,opt,name=authorization,proto3" json:"authorization,omitempty"` + // time when the grant will expire and will be pruned. If null, then the grant + // doesn't have a time expiration (other conditions in `authorization` + // may apply to invalidate the grant) + Expiration *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=expiration,proto3" json:"expiration,omitempty"` } func (x *Grant) Reset() { @@ -2294,8 +2297,8 @@ var file_cosmos_authz_v1beta1_authz_proto_rawDesc = []byte{ 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x42, 0x08, 0xc8, 0xde, 0x1f, - 0x00, 0x90, 0xdf, 0x1f, 0x01, 0x52, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, - 0x6e, 0x22, 0x91, 0x02, 0x0a, 0x12, 0x47, 0x72, 0x61, 0x6e, 0x74, 0x41, 0x75, 0x74, 0x68, 0x6f, + 0x01, 0x90, 0xdf, 0x1f, 0x01, 0x52, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, + 0x6e, 0x22, 0x8d, 0x02, 0x0a, 0x12, 0x47, 0x72, 0x61, 0x6e, 0x74, 0x41, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x32, 0x0a, 0x07, 0x67, 0x72, 0x61, 0x6e, 0x74, 0x65, 0x72, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x18, 0xd2, 0xb4, 0x2d, 0x14, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x53, 0x74, 0x72, @@ -2308,29 +2311,29 @@ var file_cosmos_authz_v1beta1_authz_proto_rawDesc = []byte{ 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x41, 0x6e, 0x79, 0x42, 0x11, 0xca, 0xb4, 0x2d, 0x0d, 0x41, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x0d, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x12, - 0x44, 0x0a, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x04, 0x20, + 0x40, 0x0a, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x42, - 0x08, 0xc8, 0xde, 0x1f, 0x00, 0x90, 0xdf, 0x1f, 0x01, 0x52, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, - 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x22, 0x34, 0x0a, 0x0e, 0x47, 0x72, 0x61, 0x6e, 0x74, 0x51, 0x75, - 0x65, 0x75, 0x65, 0x49, 0x74, 0x65, 0x6d, 0x12, 0x22, 0x0a, 0x0d, 0x6d, 0x73, 0x67, 0x5f, 0x74, - 0x79, 0x70, 0x65, 0x5f, 0x75, 0x72, 0x6c, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x0b, - 0x6d, 0x73, 0x67, 0x54, 0x79, 0x70, 0x65, 0x55, 0x72, 0x6c, 0x73, 0x42, 0xe0, 0x01, 0x0a, 0x18, - 0x63, 0x6f, 0x6d, 0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x61, 0x75, 0x74, 0x68, 0x7a, - 0x2e, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x42, 0x0a, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x50, - 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x42, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, - 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, - 0x2d, 0x73, 0x64, 0x6b, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, - 0x61, 0x75, 0x74, 0x68, 0x7a, 0x2f, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x3b, 0x61, 0x75, - 0x74, 0x68, 0x7a, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xa2, 0x02, 0x03, 0x43, 0x41, 0x58, - 0xaa, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x2e, - 0x56, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xca, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, - 0x5c, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x5c, 0x56, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xe2, 0x02, - 0x20, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x5c, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x5c, 0x56, 0x31, - 0x62, 0x65, 0x74, 0x61, 0x31, 0x5c, 0x47, 0x50, 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, - 0x61, 0xea, 0x02, 0x16, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x3a, 0x3a, 0x41, 0x75, 0x74, 0x68, - 0x7a, 0x3a, 0x3a, 0x56, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xc8, 0xe1, 0x1e, 0x00, 0x62, 0x06, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x04, 0x90, 0xdf, 0x1f, 0x01, 0x52, 0x0a, 0x65, 0x78, 0x70, 0x69, 0x72, 0x61, 0x74, 0x69, 0x6f, + 0x6e, 0x22, 0x34, 0x0a, 0x0e, 0x47, 0x72, 0x61, 0x6e, 0x74, 0x51, 0x75, 0x65, 0x75, 0x65, 0x49, + 0x74, 0x65, 0x6d, 0x12, 0x22, 0x0a, 0x0d, 0x6d, 0x73, 0x67, 0x5f, 0x74, 0x79, 0x70, 0x65, 0x5f, + 0x75, 0x72, 0x6c, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x0b, 0x6d, 0x73, 0x67, 0x54, + 0x79, 0x70, 0x65, 0x55, 0x72, 0x6c, 0x73, 0x42, 0xe0, 0x01, 0x0a, 0x18, 0x63, 0x6f, 0x6d, 0x2e, + 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x61, 0x75, 0x74, 0x68, 0x7a, 0x2e, 0x76, 0x31, 0x62, + 0x65, 0x74, 0x61, 0x31, 0x42, 0x0a, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x50, 0x72, 0x6f, 0x74, 0x6f, + 0x50, 0x01, 0x5a, 0x42, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, + 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2d, 0x73, 0x64, 0x6b, + 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x61, 0x75, 0x74, 0x68, + 0x7a, 0x2f, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x3b, 0x61, 0x75, 0x74, 0x68, 0x7a, 0x76, + 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xa2, 0x02, 0x03, 0x43, 0x41, 0x58, 0xaa, 0x02, 0x14, 0x43, + 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x2e, 0x56, 0x31, 0x62, 0x65, + 0x74, 0x61, 0x31, 0xca, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x5c, 0x41, 0x75, 0x74, + 0x68, 0x7a, 0x5c, 0x56, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xe2, 0x02, 0x20, 0x43, 0x6f, 0x73, + 0x6d, 0x6f, 0x73, 0x5c, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x5c, 0x56, 0x31, 0x62, 0x65, 0x74, 0x61, + 0x31, 0x5c, 0x47, 0x50, 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x16, + 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x3a, 0x3a, 0x41, 0x75, 0x74, 0x68, 0x7a, 0x3a, 0x3a, 0x56, + 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0xc8, 0xe1, 0x1e, 0x00, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, + 0x6f, 0x33, } var ( diff --git a/proto/cosmos/authz/v1beta1/authz.proto b/proto/cosmos/authz/v1beta1/authz.proto index c9b571eec5ad..06ce288ab019 100644 --- a/proto/cosmos/authz/v1beta1/authz.proto +++ b/proto/cosmos/authz/v1beta1/authz.proto @@ -22,8 +22,11 @@ message GenericAuthorization { // Grant gives permissions to execute // the provide method with expiration time. message Grant { - google.protobuf.Any authorization = 1 [(cosmos_proto.accepts_interface) = "Authorization"]; - google.protobuf.Timestamp expiration = 2 [(gogoproto.stdtime) = true, (gogoproto.nullable) = false]; + google.protobuf.Any authorization = 1 [(cosmos_proto.accepts_interface) = "Authorization"]; + // time when the grant will expire and will be pruned. If null, then the grant + // doesn't have a time expiration (other conditions in `authorization` + // may apply to invalidate the grant) + google.protobuf.Timestamp expiration = 2 [(gogoproto.stdtime) = true, (gogoproto.nullable) = true]; } // GrantAuthorization extends a grant with both the addresses of the grantee and granter. @@ -33,11 +36,11 @@ message GrantAuthorization { string grantee = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; google.protobuf.Any authorization = 3 [(cosmos_proto.accepts_interface) = "Authorization"]; - google.protobuf.Timestamp expiration = 4 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true]; + google.protobuf.Timestamp expiration = 4 [(gogoproto.stdtime) = true]; } // GrantQueueItem contains the list of TypeURL of a sdk.Msg. message GrantQueueItem { // msg_type_urls contains the list of TypeURL of a sdk.Msg. - repeated string msg_type_urls = 1; + repeated string msg_type_urls = 1; } diff --git a/x/authz/authorization_grant.go b/x/authz/authorization_grant.go index 2c2244660f82..bd94977ab67b 100644 --- a/x/authz/authorization_grant.go +++ b/x/authz/authorization_grant.go @@ -9,27 +9,25 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// 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) { +// NewGrant returns new Grant. Expiration is optional and noop if null. +// 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 != nil && !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, - } msg, ok := a.(proto.Message) if !ok { return Grant{}, sdkerrors.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", a) } - any, err := cdctypes.NewAnyWithValue(msg) if err != nil { return Grant{}, err } - g.Authorization = any - - return g, nil + return Grant{ + Expiration: expiration, + Authorization: any, + }, nil } var ( diff --git a/x/authz/authorization_grant_test.go b/x/authz/authorization_grant_test.go index ece0668c2f64..2501b5de1dba 100644 --- a/x/authz/authorization_grant_test.go +++ b/x/authz/authorization_grant_test.go @@ -23,13 +23,14 @@ func TestNewGrant(t *testing.T) { title string a Authorization blockTime time.Time - expire time.Time + 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"}, - {"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), ""}, + {"wrong expire time (1)", a, time.Unix(10, 0), unixTime(8, 0), "expiration must be after"}, + {"wrong expire time (2)", a, time.Unix(10, 0), unixTime(10, 0), "expiration must be after"}, + {"good expire time (1)", a, time.Unix(10, 0), unixTime(10, 1), ""}, + {"good expire time (2)", a, time.Unix(10, 0), unixTime(11, 0), ""}, + {"good expire time (nil)", a, time.Unix(10, 0), nil, ""}, } for _, tc := range tcs { @@ -41,3 +42,8 @@ func TestNewGrant(t *testing.T) { } } + +func unixTime(s, ns int64) *time.Time { + t := time.Unix(s, ns) + return &t +} diff --git a/x/authz/authz.pb.go b/x/authz/authz.pb.go index 2c8aac50f41c..7ffbadd83512 100644 --- a/x/authz/authz.pb.go +++ b/x/authz/authz.pb.go @@ -73,7 +73,10 @@ var xxx_messageInfo_GenericAuthorization proto.InternalMessageInfo // the provide method with expiration time. type Grant struct { Authorization *types.Any `protobuf:"bytes,1,opt,name=authorization,proto3" json:"authorization,omitempty"` - Expiration time.Time `protobuf:"bytes,2,opt,name=expiration,proto3,stdtime" json:"expiration"` + // time when the grant will expire and will be pruned. If null, then the grant + // doesn't have a time expiration (other conditions in `authorization` + // may apply to invalidate the grant) + Expiration *time.Time `protobuf:"bytes,2,opt,name=expiration,proto3,stdtime" json:"expiration,omitempty"` } func (m *Grant) Reset() { *m = Grant{} } @@ -115,7 +118,7 @@ type GrantAuthorization struct { Granter string `protobuf:"bytes,1,opt,name=granter,proto3" json:"granter,omitempty"` Grantee string `protobuf:"bytes,2,opt,name=grantee,proto3" json:"grantee,omitempty"` Authorization *types.Any `protobuf:"bytes,3,opt,name=authorization,proto3" json:"authorization,omitempty"` - Expiration time.Time `protobuf:"bytes,4,opt,name=expiration,proto3,stdtime" json:"expiration"` + Expiration *time.Time `protobuf:"bytes,4,opt,name=expiration,proto3,stdtime" json:"expiration,omitempty"` } func (m *GrantAuthorization) Reset() { *m = GrantAuthorization{} } @@ -200,33 +203,34 @@ func init() { func init() { proto.RegisterFile("cosmos/authz/v1beta1/authz.proto", fileDescriptor_544dc2e84b61c637) } var fileDescriptor_544dc2e84b61c637 = []byte{ - // 410 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xac, 0x52, 0x3d, 0x6f, 0xda, 0x40, - 0x18, 0xf6, 0x41, 0xbf, 0x38, 0x44, 0xd5, 0x5a, 0x1e, 0x80, 0xc1, 0x20, 0xab, 0x03, 0x0b, 0xb6, - 0xa0, 0x9d, 0xda, 0x09, 0xab, 0x12, 0xea, 0xd0, 0xa1, 0x2e, 0x5d, 0xba, 0x20, 0x1b, 0x2e, 0x87, - 0x15, 0xce, 0x67, 0xdd, 0x9d, 0x23, 0xcc, 0xaf, 0x20, 0x7b, 0x7e, 0x06, 0x3f, 0x02, 0x65, 0x42, - 0x99, 0x32, 0xe5, 0x03, 0xfe, 0x48, 0xc4, 0x9d, 0xad, 0x40, 0x58, 0xa2, 0x28, 0x93, 0xef, 0x7d, - 0xee, 0x79, 0x9e, 0x7b, 0xdf, 0xc7, 0x2f, 0x6c, 0x8e, 0x28, 0x27, 0x94, 0x3b, 0x7e, 0x22, 0x26, - 0x73, 0xe7, 0xac, 0x13, 0x20, 0xe1, 0x77, 0x54, 0x65, 0xc7, 0x8c, 0x0a, 0xaa, 0x1b, 0x8a, 0x61, - 0x2b, 0x2c, 0x63, 0xd4, 0x6b, 0x0a, 0x1d, 0x4a, 0x8e, 0x93, 0x51, 0x64, 0x51, 0x6f, 0x60, 0x4a, - 0xf1, 0x14, 0x39, 0xb2, 0x0a, 0x92, 0x13, 0x47, 0x84, 0x04, 0x71, 0xe1, 0x93, 0x38, 0x23, 0x18, - 0x98, 0x62, 0xaa, 0x84, 0xbb, 0x53, 0x86, 0xd6, 0x9e, 0xca, 0xfc, 0x28, 0x55, 0x57, 0xd6, 0x0f, - 0x68, 0xf4, 0x51, 0x84, 0x58, 0x38, 0xea, 0x25, 0x62, 0x42, 0x59, 0x38, 0xf7, 0x45, 0x48, 0x23, - 0xfd, 0x13, 0x2c, 0x12, 0x8e, 0xab, 0xa0, 0x09, 0x5a, 0x25, 0x6f, 0x77, 0xfc, 0xfe, 0xf9, 0x72, - 0xd9, 0xae, 0x1c, 0x90, 0xac, 0x0b, 0x00, 0xdf, 0xf6, 0x99, 0x1f, 0x09, 0xfd, 0x37, 0xac, 0xf8, - 0xfb, 0x57, 0x52, 0x58, 0xee, 0x1a, 0xb6, 0x7a, 0xd9, 0xce, 0x5f, 0xb6, 0x7b, 0x51, 0xea, 0x1e, - 0x3b, 0x79, 0x87, 0x6a, 0xfd, 0x27, 0x84, 0x68, 0x16, 0x87, 0x4c, 0x79, 0x15, 0xa4, 0x57, 0xfd, - 0xc8, 0x6b, 0x90, 0x0f, 0xef, 0x7e, 0x58, 0xdd, 0x34, 0xb4, 0xc5, 0x6d, 0x03, 0x78, 0x7b, 0x3a, - 0xeb, 0xbc, 0x00, 0x75, 0xd9, 0xde, 0xe1, 0x68, 0x5d, 0xf8, 0x1e, 0xef, 0x50, 0xc4, 0xd4, 0x78, - 0x6e, 0xf5, 0x6a, 0xd9, 0xce, 0x7f, 0x45, 0x6f, 0x3c, 0x66, 0x88, 0xf3, 0xbf, 0x82, 0x85, 0x11, - 0xf6, 0x72, 0xe2, 0xa3, 0x06, 0xc9, 0x6e, 0x9e, 0xa1, 0x41, 0xc7, 0x99, 0x14, 0x5f, 0x31, 0x93, - 0x37, 0x2f, 0xcc, 0xe4, 0x1b, 0xfc, 0x28, 0x23, 0xf9, 0x93, 0xa0, 0x04, 0xfd, 0x12, 0x88, 0xe8, - 0x16, 0xac, 0x10, 0x8e, 0x87, 0x22, 0x8d, 0xd1, 0x30, 0x61, 0x53, 0x5e, 0x05, 0xcd, 0x62, 0xab, - 0xe4, 0x95, 0x09, 0xc7, 0x83, 0x34, 0x46, 0xff, 0xd8, 0x94, 0xbb, 0xee, 0xea, 0xde, 0xd4, 0x56, - 0x1b, 0x13, 0xac, 0x37, 0x26, 0xb8, 0xdb, 0x98, 0x60, 0xb1, 0x35, 0xb5, 0xf5, 0xd6, 0xd4, 0xae, - 0xb7, 0xa6, 0xf6, 0xff, 0x0b, 0x0e, 0xc5, 0x24, 0x09, 0xec, 0x11, 0x25, 0xd9, 0xba, 0x66, 0x9f, - 0x36, 0x1f, 0x9f, 0x3a, 0x33, 0xb5, 0xf2, 0xc1, 0x3b, 0xd9, 0xe3, 0xd7, 0x87, 0x00, 0x00, 0x00, - 0xff, 0xff, 0x86, 0xd2, 0xd7, 0xaf, 0x17, 0x03, 0x00, 0x00, + // 417 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xa4, 0x52, 0xbd, 0xae, 0xda, 0x30, + 0x18, 0x8d, 0x81, 0xfe, 0x60, 0x44, 0xd5, 0x46, 0x19, 0x80, 0x21, 0xa0, 0xa8, 0x03, 0x0b, 0x89, + 0xa0, 0x9d, 0xda, 0xa5, 0x44, 0x95, 0x50, 0x87, 0x0e, 0x4d, 0xe9, 0xd2, 0x05, 0x25, 0xe0, 0x9a, + 0xa8, 0x38, 0x8e, 0x6c, 0xa7, 0x22, 0xbc, 0x43, 0x25, 0x1e, 0xa0, 0x8f, 0xc1, 0x43, 0xa0, 0x4e, + 0xa8, 0x53, 0xa7, 0xfe, 0xc0, 0x8b, 0x5c, 0x61, 0x27, 0xba, 0xe4, 0x66, 0xb8, 0x57, 0xba, 0x53, + 0xfc, 0x1d, 0x9f, 0x73, 0xfc, 0x7d, 0x27, 0x1f, 0xec, 0xcd, 0x29, 0x27, 0x94, 0x3b, 0x7e, 0x22, + 0x96, 0x1b, 0xe7, 0xdb, 0x30, 0x40, 0xc2, 0x1f, 0xaa, 0xca, 0x8e, 0x19, 0x15, 0x54, 0x37, 0x14, + 0xc3, 0x56, 0x58, 0xc6, 0xe8, 0xb4, 0x15, 0x3a, 0x93, 0x1c, 0x27, 0xa3, 0xc8, 0xa2, 0xd3, 0xc5, + 0x94, 0xe2, 0x15, 0x72, 0x64, 0x15, 0x24, 0x5f, 0x1c, 0x11, 0x12, 0xc4, 0x85, 0x4f, 0xe2, 0x8c, + 0x60, 0x60, 0x8a, 0xa9, 0x12, 0x9e, 0x4f, 0x19, 0xda, 0xbe, 0x29, 0xf3, 0xa3, 0x54, 0x5d, 0x59, + 0xaf, 0xa1, 0x31, 0x41, 0x11, 0x62, 0xe1, 0x7c, 0x9c, 0x88, 0x25, 0x65, 0xe1, 0xc6, 0x17, 0x21, + 0x8d, 0xf4, 0xa7, 0xb0, 0x4a, 0x38, 0x6e, 0x81, 0x1e, 0xe8, 0xd7, 0xbd, 0xf3, 0xf1, 0xd5, 0xb3, + 0x9f, 0xbb, 0x41, 0xb3, 0x40, 0xb2, 0x7e, 0x00, 0xf8, 0x60, 0xc2, 0xfc, 0x48, 0xe8, 0xef, 0x61, + 0xd3, 0xbf, 0xbc, 0x92, 0xc2, 0xc6, 0xc8, 0xb0, 0xd5, 0xcb, 0x76, 0xfe, 0xb2, 0x3d, 0x8e, 0x52, + 0xb7, 0xec, 0xe4, 0x15, 0xd5, 0xfa, 0x5b, 0x08, 0xd1, 0x3a, 0x0e, 0x99, 0xf2, 0xaa, 0x48, 0xaf, + 0x4e, 0xc9, 0x6b, 0x9a, 0x0f, 0xef, 0x3e, 0xde, 0xff, 0xe9, 0x82, 0xed, 0xdf, 0x2e, 0xf0, 0x2e, + 0x74, 0xd6, 0xf7, 0x0a, 0xd4, 0x65, 0x7b, 0xc5, 0xd1, 0x46, 0xf0, 0x11, 0x3e, 0xa3, 0x88, 0xa9, + 0xf1, 0xdc, 0xd6, 0xaf, 0xdd, 0x20, 0xff, 0x15, 0xe3, 0xc5, 0x82, 0x21, 0xce, 0x3f, 0x0a, 0x16, + 0x46, 0xd8, 0xcb, 0x89, 0xd7, 0x1a, 0x24, 0xbb, 0xb9, 0x83, 0x06, 0x95, 0x33, 0xa9, 0xde, 0x2b, + 0x93, 0x37, 0x85, 0x4c, 0x6a, 0xb7, 0x66, 0x52, 0x2b, 0xe5, 0xf1, 0x12, 0x3e, 0x91, 0x71, 0x7c, + 0x48, 0x50, 0x82, 0xde, 0x09, 0x44, 0x74, 0x0b, 0x36, 0x09, 0xc7, 0x33, 0x91, 0xc6, 0x68, 0x96, + 0xb0, 0x15, 0x6f, 0x81, 0x5e, 0xb5, 0x5f, 0xf7, 0x1a, 0x84, 0xe3, 0x69, 0x1a, 0xa3, 0x4f, 0x6c, + 0xc5, 0x5d, 0x77, 0xff, 0xdf, 0xd4, 0xf6, 0x47, 0x13, 0x1c, 0x8e, 0x26, 0xf8, 0x77, 0x34, 0xc1, + 0xf6, 0x64, 0x6a, 0x87, 0x93, 0xa9, 0xfd, 0x3e, 0x99, 0xda, 0xe7, 0xe7, 0x38, 0x14, 0xcb, 0x24, + 0xb0, 0xe7, 0x94, 0x64, 0xab, 0x9a, 0x7d, 0x06, 0x7c, 0xf1, 0xd5, 0x59, 0xab, 0x75, 0x0f, 0x1e, + 0xca, 0xfe, 0x5e, 0x5c, 0x05, 0x00, 0x00, 0xff, 0xff, 0xd3, 0xb3, 0xf3, 0x46, 0x13, 0x03, 0x00, + 0x00, } func (m *GenericAuthorization) Marshal() (dAtA []byte, err error) { @@ -279,14 +283,16 @@ func (m *Grant) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - n1, err1 := github_com_gogo_protobuf_types.StdTimeMarshalTo(m.Expiration, dAtA[i-github_com_gogo_protobuf_types.SizeOfStdTime(m.Expiration):]) - if err1 != nil { - return 0, err1 + if m.Expiration != nil { + n1, err1 := github_com_gogo_protobuf_types.StdTimeMarshalTo(*m.Expiration, dAtA[i-github_com_gogo_protobuf_types.SizeOfStdTime(*m.Expiration):]) + if err1 != nil { + return 0, err1 + } + i -= n1 + i = encodeVarintAuthz(dAtA, i, uint64(n1)) + i-- + dAtA[i] = 0x12 } - i -= n1 - i = encodeVarintAuthz(dAtA, i, uint64(n1)) - i-- - dAtA[i] = 0x12 if m.Authorization != nil { { size, err := m.Authorization.MarshalToSizedBuffer(dAtA[:i]) @@ -322,14 +328,16 @@ func (m *GrantAuthorization) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - n3, err3 := github_com_gogo_protobuf_types.StdTimeMarshalTo(m.Expiration, dAtA[i-github_com_gogo_protobuf_types.SizeOfStdTime(m.Expiration):]) - if err3 != nil { - return 0, err3 + if m.Expiration != nil { + n3, err3 := github_com_gogo_protobuf_types.StdTimeMarshalTo(*m.Expiration, dAtA[i-github_com_gogo_protobuf_types.SizeOfStdTime(*m.Expiration):]) + if err3 != nil { + return 0, err3 + } + i -= n3 + i = encodeVarintAuthz(dAtA, i, uint64(n3)) + i-- + dAtA[i] = 0x22 } - i -= n3 - i = encodeVarintAuthz(dAtA, i, uint64(n3)) - i-- - dAtA[i] = 0x22 if m.Authorization != nil { { size, err := m.Authorization.MarshalToSizedBuffer(dAtA[:i]) @@ -425,8 +433,10 @@ func (m *Grant) Size() (n int) { l = m.Authorization.Size() n += 1 + l + sovAuthz(uint64(l)) } - l = github_com_gogo_protobuf_types.SizeOfStdTime(m.Expiration) - n += 1 + l + sovAuthz(uint64(l)) + if m.Expiration != nil { + l = github_com_gogo_protobuf_types.SizeOfStdTime(*m.Expiration) + n += 1 + l + sovAuthz(uint64(l)) + } return n } @@ -448,8 +458,10 @@ func (m *GrantAuthorization) Size() (n int) { l = m.Authorization.Size() n += 1 + l + sovAuthz(uint64(l)) } - l = github_com_gogo_protobuf_types.SizeOfStdTime(m.Expiration) - n += 1 + l + sovAuthz(uint64(l)) + if m.Expiration != nil { + l = github_com_gogo_protobuf_types.SizeOfStdTime(*m.Expiration) + n += 1 + l + sovAuthz(uint64(l)) + } return n } @@ -650,7 +662,10 @@ func (m *Grant) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - if err := github_com_gogo_protobuf_types.StdTimeUnmarshal(&m.Expiration, dAtA[iNdEx:postIndex]); err != nil { + if m.Expiration == nil { + m.Expiration = new(time.Time) + } + if err := github_com_gogo_protobuf_types.StdTimeUnmarshal(m.Expiration, dAtA[iNdEx:postIndex]); err != nil { return err } iNdEx = postIndex @@ -833,7 +848,10 @@ func (m *GrantAuthorization) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - if err := github_com_gogo_protobuf_types.StdTimeUnmarshal(&m.Expiration, dAtA[iNdEx:postIndex]); err != nil { + if m.Expiration == nil { + m.Expiration = new(time.Time) + } + if err := github_com_gogo_protobuf_types.StdTimeUnmarshal(m.Expiration, dAtA[iNdEx:postIndex]); err != nil { return err } iNdEx = postIndex diff --git a/x/authz/client/cli/tx.go b/x/authz/client/cli/tx.go index 6857c9796cea..e6f37343ffbc 100644 --- a/x/authz/client/cli/tx.go +++ b/x/authz/client/cli/tx.go @@ -75,11 +75,6 @@ Examples: return err } - exp, err := cmd.Flags().GetInt64(FlagExpiration) - if err != nil { - return err - } - var authorization authz.Authorization switch args[1] { case "send": @@ -160,7 +155,12 @@ Examples: return fmt.Errorf("invalid authorization type, %s", args[1]) } - msg, err := authz.NewMsgGrant(clientCtx.GetFromAddress(), grantee, authorization, time.Unix(exp, 0)) + expire, err := getExpireTime(cmd) + if err != nil { + return err + } + + msg, err := authz.NewMsgGrant(clientCtx.GetFromAddress(), grantee, authorization, expire) if err != nil { return err } @@ -173,10 +173,22 @@ 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().Int64(FlagExpiration, time.Now().AddDate(1, 0, 0).Unix(), "The Unix timestamp. Default is one year.") + cmd.Flags().Int64(FlagExpiration, 0, "Expire time as Unix timestamp. Set zero (0) for no expiry. Default is 0.") return cmd } +func getExpireTime(cmd *cobra.Command) (*time.Time, error) { + exp, err := cmd.Flags().GetInt64(FlagExpiration) + if err != nil { + return nil, err + } + if exp == 0 { + return nil, nil + } + e := time.Unix(exp, 0) + return &e, nil +} + func NewCmdRevokeAuthorization() *cobra.Command { cmd := &cobra.Command{ Use: "revoke [grantee] [msg-type-url] --from=[granter]", diff --git a/x/authz/keeper/genesis_test.go b/x/authz/keeper/genesis_test.go index ecf6051deb20..077d2a162bd1 100644 --- a/x/authz/keeper/genesis_test.go +++ b/x/authz/keeper/genesis_test.go @@ -39,12 +39,14 @@ var ( func (suite *GenesisTestSuite) TestImportExportGenesis() { coins := sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(1_000))) - now := suite.ctx.BlockHeader().Time + now := suite.ctx.BlockTime() + expires := now.Add(time.Hour) grant := &bank.SendAuthorization{SpendLimit: coins} - err := suite.keeper.SaveGrant(suite.ctx, granteeAddr, granterAddr, grant, now.Add(time.Hour)) + err := suite.keeper.SaveGrant(suite.ctx, granteeAddr, granterAddr, grant, &expires) suite.Require().NoError(err) genesis := suite.keeper.ExportGenesis(suite.ctx) + // TODO, recheck! // Clear keeper suite.keeper.DeleteGrant(suite.ctx, granteeAddr, granterAddr, grant.MsgTypeURL()) diff --git a/x/authz/keeper/grpc_query_test.go b/x/authz/keeper/grpc_query_test.go index 52a1012d2788..d2f0b09e8b19 100644 --- a/x/authz/keeper/grpc_query_test.go +++ b/x/authz/keeper/grpc_query_test.go @@ -14,7 +14,7 @@ import ( ) func (suite *TestSuite) TestGRPCQueryAuthorization() { - app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs + queryClient, addrs := suite.queryClient, suite.addrs var ( req *authz.QueryGrantsRequest expAuthorization authz.Authorization @@ -58,11 +58,7 @@ func (suite *TestSuite) TestGRPCQueryAuthorization() { { "Success", func(require *require.Assertions) { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - expAuthorization = &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[0], addrs[1], expAuthorization, now.Add(time.Hour)) - require.NoError(err) + expAuthorization = suite.createSendAuthorization(addrs[0], addrs[1]) req = &authz.QueryGrantsRequest{ Granter: addrs[1].String(), Grantee: addrs[0].String(), @@ -97,7 +93,7 @@ func (suite *TestSuite) TestGRPCQueryAuthorization() { } func (suite *TestSuite) TestGRPCQueryAuthorizations() { - app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs + queryClient, addrs := suite.queryClient, suite.addrs var ( req *authz.QueryGrantsRequest expAuthorization authz.Authorization @@ -129,11 +125,7 @@ func (suite *TestSuite) TestGRPCQueryAuthorizations() { { "Success", func() { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - expAuthorization = &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[0], addrs[1], expAuthorization, now.Add(time.Hour)) - suite.Require().NoError(err) + expAuthorization = suite.createSendAuthorization(addrs[0], addrs[1]) req = &authz.QueryGrantsRequest{ Granter: addrs[1].String(), Grantee: addrs[0].String(), @@ -166,7 +158,7 @@ func (suite *TestSuite) TestGRPCQueryAuthorizations() { func (suite *TestSuite) TestGRPCQueryGranterGrants() { require := suite.Require() - app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs + queryClient, addrs := suite.queryClient, suite.addrs testCases := []struct { msg string @@ -185,11 +177,7 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() { { "valid case, single authorization", func() { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - authorization := &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[1], addrs[0], authorization, now.Add(time.Hour)) - require.NoError(err) + suite.createSendAuthorization(addrs[1], addrs[0]) }, false, authz.QueryGranterGrantsRequest{ @@ -200,11 +188,7 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() { { "valid case, multiple authorization", func() { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - authorization := &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[2], addrs[0], authorization, now.Add(time.Hour)) - require.NoError(err) + suite.createSendAuthorization(addrs[2], addrs[0]) }, false, authz.QueryGranterGrantsRequest{ @@ -242,7 +226,7 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() { func (suite *TestSuite) TestGRPCQueryGranteeGrants() { require := suite.Require() - app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs + queryClient, addrs := suite.queryClient, suite.addrs testCases := []struct { msg string @@ -261,11 +245,7 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() { { "valid case, single authorization", func() { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - authorization := &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[0], addrs[1], authorization, now.Add(time.Hour)) - require.NoError(err) + suite.createSendAuthorization(addrs[0], addrs[1]) }, false, authz.QueryGranteeGrantsRequest{ @@ -276,11 +256,7 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() { { "valid case, multiple authorization", func() { - now := ctx.BlockHeader().Time - newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) - authorization := &banktypes.SendAuthorization{SpendLimit: newCoins} - err := app.AuthzKeeper.SaveGrant(ctx, addrs[0], addrs[2], authorization, now.Add(time.Hour)) - require.NoError(err) + suite.createSendAuthorization(addrs[0], addrs[2]) }, false, authz.QueryGranteeGrantsRequest{ @@ -315,3 +291,12 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() { }) } } + +func (suite *TestSuite) createSendAuthorization(a1, a2 sdk.AccAddress) authz.Authorization { + exp := suite.ctx.BlockHeader().Time.Add(time.Hour) + newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) + authorization := &banktypes.SendAuthorization{SpendLimit: newCoins} + err := suite.app.AuthzKeeper.SaveGrant(suite.ctx, a1, a2, authorization, &exp) + suite.Require().NoError(err) + return authorization +} diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go index c9fa0311f9ab..a9917cf12119 100644 --- a/x/authz/keeper/keeper.go +++ b/x/authz/keeper/keeper.go @@ -83,6 +83,7 @@ func (k Keeper) update(ctx sdk.Context, grantee sdk.AccAddress, granter sdk.AccA // grants from the message signer to the grantee. func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []sdk.Msg) ([][]byte, error) { var results = make([][]byte, len(msgs)) + now := ctx.BlockTime() for i, msg := range msgs { signers := msg.GetSigners() if len(signers) != 1 { @@ -97,7 +98,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] return nil, sdkerrors.ErrUnauthorized.Wrap("authorization not found") } - if grant.Expiration.Before(ctx.BlockTime()) { + if grant.Expiration != nil && grant.Expiration.Before(now) { return nil, sdkerrors.ErrUnauthorized.Wrap("authorization expired") } @@ -149,31 +150,34 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] // SaveGrant method grants the provided authorization to the grantee on the granter's account // with the provided expiration time and insert authorization key into the grants queue. If there is an existing authorization grant for the // same `sdk.Msg` type, this grant overwrites that. -func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, authorization authz.Authorization, expiration time.Time) error { +func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, authorization authz.Authorization, expiration *time.Time) error { store := ctx.KVStore(k.storeKey) - skey := grantStoreKey(grantee, granter, authorization.MsgTypeURL()) - oldGrant, found := k.getGrant(ctx, skey) + msgType := authorization.MsgTypeURL() + skey := grantStoreKey(grantee, granter, msgType) grant, err := authz.NewGrant(ctx.BlockTime(), authorization, expiration) if err != nil { return err } - bz := k.cdc.MustMarshal(&grant) - store.Set(skey, bz) - - if found { - // if expiration is not the same, remove old key and add the new key to queue - if !oldGrant.Expiration.Equal(expiration) { - if err := k.removeFromGrantQueue(ctx, skey, oldGrant.Expiration, granter, grantee); err != nil { - return err - } - - k.insertIntoGrantQueue(ctx, granter, grantee, authorization.MsgTypeURL(), expiration) + var oldExp *time.Time + if oldGrant, found := k.getGrant(ctx, skey); found { + oldExp = oldGrant.Expiration + } + if oldExp != nil && (expiration == nil || !oldExp.Equal(*expiration)) { + if err = k.removeFromGrantQueue(ctx, skey, granter, grantee, *oldExp); err != nil { + return err } - } else { - k.insertIntoGrantQueue(ctx, granter, grantee, authorization.MsgTypeURL(), expiration) } + // If the expiration didn't change, then we don't remove it and we should not insert again + if expiration != nil && (oldExp == nil || !oldExp.Equal(*expiration)) { + if err = k.insertIntoGrantQueue(ctx, granter, grantee, msgType, *expiration); err != nil { + return err + } + } + + bz := k.cdc.MustMarshal(&grant) + store.Set(skey, bz) return ctx.EventManager().EmitTypedEvent(&authz.EventGrant{ MsgTypeUrl: authorization.MsgTypeURL(), @@ -193,8 +197,12 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee sdk.AccAddress, granter sdk } store.Delete(skey) - if err := k.removeFromGrantQueue(ctx, skey, grant.Expiration, granter, grantee); err != nil { - return err + + if grant.Expiration != nil { + err := k.removeFromGrantQueue(ctx, skey, granter, grantee, *grant.Expiration) + if err != nil { + return err + } } return ctx.EventManager().EmitTypedEvent(&authz.EventRevoke{ @@ -232,6 +240,7 @@ func (k Keeper) GetAuthorizations(ctx sdk.Context, grantee sdk.AccAddress, grant // IterateGrants iterates over all authorization grants // This function should be used with caution because it can involve significant IO operations. // It should not be used in query or msg services without charging additional gas. +// The iteration stops when the handler function returns true or the iterator exhaust. func (k Keeper) IterateGrants(ctx sdk.Context, handler func(granterAddr sdk.AccAddress, granteeAddr sdk.AccAddress, grant authz.Grant) bool) { store := ctx.KVStore(k.storeKey) @@ -251,11 +260,10 @@ func (k Keeper) IterateGrants(ctx sdk.Context, func (k Keeper) ExportGenesis(ctx sdk.Context) *authz.GenesisState { var entries []authz.GrantAuthorization k.IterateGrants(ctx, func(granter, grantee sdk.AccAddress, grant authz.Grant) bool { - exp := grant.Expiration entries = append(entries, authz.GrantAuthorization{ Granter: granter.String(), Grantee: grantee.String(), - Expiration: exp, + Expiration: grant.Expiration, Authorization: grant.Authorization, }) return false @@ -266,9 +274,10 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *authz.GenesisState { // InitGenesis new authz genesis func (k Keeper) InitGenesis(ctx sdk.Context, data *authz.GenesisState) { + now := ctx.BlockTime() for _, entry := range data.Authorization { // ignore expired authorizations - if entry.Expiration.Before(ctx.BlockTime()) { + if entry.Expiration != nil && entry.Expiration.Before(now) { continue } @@ -341,12 +350,12 @@ func (keeper Keeper) insertIntoGrantQueue(ctx sdk.Context, granter, grantee sdk. } // removeFromGrantQueue removes a grant key from the grant queue -func (keeper Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, expiration time.Time, granter, grantee sdk.AccAddress) error { +func (keeper Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, granter, grantee sdk.AccAddress, expiration time.Time) error { store := ctx.KVStore(keeper.storeKey) key := GrantQueueKey(expiration, granter, grantee) bz := store.Get(key) if bz == nil { - return sdkerrors.ErrLogic.Wrap("grant key not found") + return sdkerrors.ErrLogic.Wrap("can't remove grant from the expire queue, grant key not found") } var queueItem authz.GrantQueueItem diff --git a/x/authz/keeper/keeper_test.go b/x/authz/keeper/keeper_test.go index 7cb6a1b8461b..de57acf71c10 100644 --- a/x/authz/keeper/keeper_test.go +++ b/x/authz/keeper/keeper_test.go @@ -63,7 +63,8 @@ func (s *TestSuite) TestKeeper() { s.T().Log("verify save, get and delete") sendAutz := &banktypes.SendAuthorization{SpendLimit: coins100} - err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, now.AddDate(1, 0, 0)) + expire := now.AddDate(1, 0, 0) + err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, &expire) require.NoError(err) authorizations, err = app.AuthzKeeper.GetAuthorizations(ctx, granteeAddr, granterAddr) @@ -78,7 +79,7 @@ func (s *TestSuite) TestKeeper() { require.Len(authorizations, 0) s.T().Log("verify granting same authorization overwrite existing authorization") - err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, now.AddDate(1, 0, 0)) + err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, &expire) require.NoError(err) authorizations, err = app.AuthzKeeper.GetAuthorizations(ctx, granteeAddr, granterAddr) @@ -86,7 +87,7 @@ func (s *TestSuite) TestKeeper() { require.Len(authorizations, 1) sendAutz = &banktypes.SendAuthorization{SpendLimit: coins1000} - err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, now.AddDate(1, 0, 0)) + err = app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, sendAutz, &expire) require.NoError(err) authorizations, err = app.AuthzKeeper.GetAuthorizations(ctx, granteeAddr, granterAddr) require.NoError(err) @@ -107,9 +108,10 @@ func (s *TestSuite) TestKeeperIter() { granterAddr := addrs[0] granteeAddr := addrs[1] granter2Addr := addrs[2] + e := ctx.BlockTime().AddDate(1, 0, 0) - s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), ctx.BlockTime().AddDate(1, 0, 0)) - s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granter2Addr, banktypes.NewSendAuthorization(coins100), ctx.BlockTime().AddDate(1, 0, 0)) + s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), &e) + s.app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granter2Addr, banktypes.NewSendAuthorization(coins100), &e) app.AuthzKeeper.IterateGrants(ctx, func(granter, grantee sdk.AccAddress, grant authz.Grant) bool { s.Require().Equal(granteeAddr, grantee) @@ -127,6 +129,7 @@ func (s *TestSuite) TestDispatchAction() { granterAddr := addrs[0] granteeAddr := addrs[1] recipientAddr := addrs[2] + a := banktypes.NewSendAuthorization(coins100) require.NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, coins1000)) @@ -168,7 +171,8 @@ func (s *TestSuite) TestDispatchAction() { true, "authorization expired", func() sdk.Context { - err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), now.AddDate(0, 0, 1)) + e := now.AddDate(0, 0, 1) + err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, a, &e) require.NoError(err) return s.ctx.WithBlockTime(s.ctx.BlockTime().AddDate(0, 0, 2)) }, @@ -186,7 +190,8 @@ func (s *TestSuite) TestDispatchAction() { true, "requested amount is more than spend limit", func() sdk.Context { - err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), now.AddDate(0, 1, 0)) + e := now.AddDate(0, 1, 0) + err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, a, &e) require.NoError(err) return s.ctx }, @@ -204,7 +209,8 @@ func (s *TestSuite) TestDispatchAction() { false, "", func() sdk.Context { - err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), now.AddDate(0, 1, 0)) + e := now.AddDate(0, 1, 0) + err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, a, &e) require.NoError(err) return s.ctx }, @@ -229,7 +235,8 @@ func (s *TestSuite) TestDispatchAction() { false, "", func() sdk.Context { - err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, banktypes.NewSendAuthorization(coins100), now.AddDate(0, 1, 0)) + e := now.AddDate(0, 1, 0) + err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, a, &e) require.NoError(err) return s.ctx }, @@ -281,7 +288,7 @@ func (s *TestSuite) TestDispatchedEvents() { }) // grant authorization - err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: coins10}, expiration) + err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: coins10}, &expiration) require.NoError(err) authorizations, err := app.AuthzKeeper.GetAuthorizations(s.ctx, granteeAddr, granterAddr) require.NoError(err) @@ -319,18 +326,21 @@ func (s *TestSuite) TestDequeueAllGrantsQueue() { grantee := addrs[1] grantee1 := addrs[2] exp := s.ctx.BlockTime().AddDate(0, 0, 1) + a := banktypes.SendAuthorization{SpendLimit: coins100} // create few authorizations - err := app.AuthzKeeper.SaveGrant(s.ctx, grantee, granter, &banktypes.SendAuthorization{SpendLimit: coins100}, exp) + err := app.AuthzKeeper.SaveGrant(s.ctx, grantee, granter, &a, &exp) require.NoError(err) - err = app.AuthzKeeper.SaveGrant(s.ctx, grantee1, granter, &banktypes.SendAuthorization{SpendLimit: coins100}, exp) + err = app.AuthzKeeper.SaveGrant(s.ctx, grantee1, granter, &a, &exp) require.NoError(err) - err = app.AuthzKeeper.SaveGrant(s.ctx, granter, grantee1, &banktypes.SendAuthorization{SpendLimit: coins100}, exp.AddDate(0, 1, 0)) + exp2 := exp.AddDate(0, 1, 0) + err = app.AuthzKeeper.SaveGrant(s.ctx, granter, grantee1, &a, &exp2) require.NoError(err) - err = app.AuthzKeeper.SaveGrant(s.ctx, granter, grantee, &banktypes.SendAuthorization{SpendLimit: coins100}, exp.AddDate(2, 0, 0)) + exp2 = exp.AddDate(2, 0, 0) + err = app.AuthzKeeper.SaveGrant(s.ctx, granter, grantee, &a, &exp2) require.NoError(err) newCtx := s.ctx.WithBlockTime(exp.AddDate(1, 0, 0)) diff --git a/x/authz/keeper/keys.go b/x/authz/keeper/keys.go index 9414a84408b8..9a935e5b2778 100644 --- a/x/authz/keeper/keys.go +++ b/x/authz/keeper/keys.go @@ -83,10 +83,10 @@ func parseGrantQueueKey(key []byte) (time.Time, sdk.AccAddress, sdk.AccAddress, return exp, granter, grantee, nil } -// GrantQueueKey - return grant queue store key -// Key format is -// -// - 0x02: GrantQueueItem +// GrantQueueKey - return grant queue store key. If a given grant doesn't have a defined +// expiration, then it should not be used in the pruning queue. +// Key format is: +// 0x02: GrantQueueItem func GrantQueueKey(expiration time.Time, granter sdk.AccAddress, grantee sdk.AccAddress) []byte { exp := sdk.FormatTimeBytes(expiration) granter = address.MustLengthPrefix(granter) diff --git a/x/authz/migrations/v046/store.go b/x/authz/migrations/v046/store.go index 4db5649b8963..16837e1b3c25 100644 --- a/x/authz/migrations/v046/store.go +++ b/x/authz/migrations/v046/store.go @@ -31,6 +31,7 @@ func addExpiredGrantsIndex(ctx sdk.Context, store storetypes.KVStore, cdc codec. defer grantsIter.Close() queueItems := make(map[string][]string) + now := ctx.BlockTime() for ; grantsIter.Valid(); grantsIter.Next() { var grant authz.Grant bz := grantsIter.Value() @@ -39,11 +40,13 @@ func addExpiredGrantsIndex(ctx sdk.Context, store storetypes.KVStore, cdc codec. } // delete expired authorization - if grant.Expiration.Before(ctx.BlockTime()) { + // before 0.46 Expiration was required so it's safe to dereference + if grant.Expiration.Before(now) { grantsStore.Delete(grantsIter.Key()) } else { granter, grantee, msgType := ParseGrantKey(grantsIter.Key()) - key := GrantQueueKey(grant.Expiration, granter, grantee) + // before 0.46 expiration was not a pointer, so now it's safe to dereference + key := GrantQueueKey(*grant.Expiration, granter, grantee) queueItem, ok := queueItems[conv.UnsafeBytesToStr(key)] if !ok { diff --git a/x/authz/migrations/v046/store_test.go b/x/authz/migrations/v046/store_test.go index 8067a6d1d361..1142c115cb0d 100644 --- a/x/authz/migrations/v046/store_test.go +++ b/x/authz/migrations/v046/store_test.go @@ -29,7 +29,9 @@ func TestMigration(t *testing.T) { sendMsgType := banktypes.SendAuthorization{}.MsgTypeURL() genericMsgType := sdk.MsgTypeURL(&govtypes.MsgVote{}) coins100 := sdk.NewCoins(sdk.NewInt64Coin("atom", 100)) - oneDay := ctx.BlockTime().AddDate(0, 0, 1) + blockTime := ctx.BlockTime() + oneDay := blockTime.AddDate(0, 0, 1) + oneYear := blockTime.AddDate(1, 0, 0) grants := []struct { granter sdk.AccAddress @@ -46,7 +48,7 @@ func TestMigration(t *testing.T) { require.NoError(t, err) return authz.Grant{ Authorization: any, - Expiration: oneDay, + Expiration: &oneDay, } }, }, @@ -59,7 +61,7 @@ func TestMigration(t *testing.T) { require.NoError(t, err) return authz.Grant{ Authorization: any, - Expiration: oneDay, + Expiration: &oneDay, } }, }, @@ -72,7 +74,7 @@ func TestMigration(t *testing.T) { require.NoError(t, err) return authz.Grant{ Authorization: any, - Expiration: oneDay.AddDate(1, 0, 0), + Expiration: &oneYear, } }, }, @@ -85,7 +87,7 @@ func TestMigration(t *testing.T) { require.NoError(t, err) return authz.Grant{ Authorization: any, - Expiration: ctx.BlockTime(), + Expiration: &blockTime, } }, }, diff --git a/x/authz/module/abci_test.go b/x/authz/module/abci_test.go index 1dc8d5afdf6b..5858476aeac5 100644 --- a/x/authz/module/abci_test.go +++ b/x/authz/module/abci_test.go @@ -2,6 +2,7 @@ package authz_test import ( "testing" + "time" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/simapp" @@ -16,37 +17,49 @@ import ( func TestExpiredGrantsQueue(t *testing.T) { app := simapp.Setup(t, false) ctx := app.BaseApp.NewContext(false, types.Header{}) - addrs := simapp.AddTestAddrsIncremental(app, ctx, 4, sdk.NewInt(30000000)) + addrs := simapp.AddTestAddrsIncremental(app, ctx, 5, sdk.NewInt(30000000)) granter := addrs[0] grantee1 := addrs[1] grantee2 := addrs[2] grantee3 := addrs[3] + grantee4 := addrs[4] expiration := ctx.BlockTime().AddDate(0, 1, 0) + expiration2 := expiration.AddDate(1, 0, 0) smallCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 10)) - app.AuthzKeeper.SaveGrant(ctx, grantee1, granter, banktypes.NewSendAuthorization(smallCoins), expiration) - app.AuthzKeeper.SaveGrant(ctx, grantee2, granter, banktypes.NewSendAuthorization(smallCoins), expiration) - app.AuthzKeeper.SaveGrant(ctx, grantee3, granter, banktypes.NewSendAuthorization(smallCoins), expiration.AddDate(1, 0, 0)) + var save = func(grantee sdk.AccAddress, exp *time.Time) { + err := app.AuthzKeeper.SaveGrant(ctx, grantee, granter, banktypes.NewSendAuthorization(smallCoins), exp) + require.NoError(t, err, "Grant from %s", grantee.String()) + } + save(grantee1, &expiration) + save(grantee2, &expiration) + save(grantee3, &expiration2) + save(grantee4, nil) queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry()) authz.RegisterQueryServer(queryHelper, app.AuthzKeeper) queryClient := authz.NewQueryClient(queryHelper) - authzmodule.BeginBlocker(ctx, app.AuthzKeeper) - - res, err := queryClient.GranterGrants(ctx.Context(), &authz.QueryGranterGrantsRequest{ - Granter: granter.String(), - }) - require.NoError(t, err) - require.NotNil(t, res) - require.Len(t, res.Grants, 3) - - ctx = ctx.WithBlockTime(expiration.AddDate(0, 2, 0)) - authzmodule.BeginBlocker(ctx, app.AuthzKeeper) - res, err = queryClient.GranterGrants(ctx.Context(), &authz.QueryGranterGrantsRequest{ - Granter: granter.String(), - }) - require.NoError(t, err) - require.NotNil(t, res) - require.Len(t, res.Grants, 1) + var checkGrants = func(ctx sdk.Context, expectedNum int) { + authzmodule.BeginBlocker(ctx, app.AuthzKeeper) + + res, err := queryClient.GranterGrants(ctx.Context(), &authz.QueryGranterGrantsRequest{ + Granter: granter.String(), + }) + require.NoError(t, err) + require.NotNil(t, res) + require.Equal(t, expectedNum, len(res.Grants)) + } + + checkGrants(ctx, 4) + + // expiration is exclusive! + ctx = ctx.WithBlockTime(expiration) + checkGrants(ctx, 4) + + ctx = ctx.WithBlockTime(expiration.AddDate(0, 0, 1)) + checkGrants(ctx, 2) + + ctx = ctx.WithBlockTime(expiration2.AddDate(0, 0, 1)) + checkGrants(ctx, 1) } diff --git a/x/authz/msgs.go b/x/authz/msgs.go index b2b530ad0f8a..877932b75e11 100644 --- a/x/authz/msgs.go +++ b/x/authz/msgs.go @@ -28,7 +28,7 @@ var ( // NewMsgGrant creates a new MsgGrant //nolint:interfacer -func NewMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a Authorization, expiration time.Time) (*MsgGrant, error) { +func NewMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a Authorization, expiration *time.Time) (*MsgGrant, error) { m := &MsgGrant{ Granter: granter.String(), Grantee: grantee.String(), diff --git a/x/authz/msgs_test.go b/x/authz/msgs_test.go index b22a6f8b9b42..7f85584121ed 100644 --- a/x/authz/msgs_test.go +++ b/x/authz/msgs_test.go @@ -68,35 +68,52 @@ func TestMsgRevokeAuthorization(t *testing.T) { } } +// add time interval to a time object and returns a pointer +func addDatePtr(t *time.Time, months, days int) *time.Time { + t2 := t.AddDate(0, months, days) + return &t2 +} + func TestMsgGrantAuthorization(t *testing.T) { + now := time.Now() tests := []struct { - title string + name string granter, grantee sdk.AccAddress authorization authz.Authorization - expiration time.Time + expiration *time.Time expectErr bool - expectPass bool + valBasic bool }{ - {"nil granter address", nil, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false}, - {"nil grantee address", granter, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false}, - {"nil granter and grantee address", nil, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false}, - {"nil authorization", granter, grantee, nil, time.Now(), true, false}, - {"valid test case", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 1, 0), false, true}, - {"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), true, true}, + {"nil granter address", + nil, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, &now, false, false}, + {"nil grantee address", + granter, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, &now, false, false}, + {"nil granter and grantee address", + nil, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, &now, false, false}, + {"nil authorization should fail", + granter, grantee, nil, &now, true, false}, + {"valid test case", + granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, addDatePtr(&now, 1, 0), false, true}, + {"valid test case with nil expire time", + granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, nil, false, true}, + // we don't access the block time / nor time.Now, so we don't know if it's in the past at this level. + {"past expire time should not fail", + granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, addDatePtr(&now, 0, -1), false, true}, } - for i, tc := range tests { + for _, tc := range tests { msg, err := authz.NewMsgGrant( tc.granter, tc.grantee, tc.authorization, tc.expiration, ) if !tc.expectErr { - require.NoError(t, err) + require.NoError(t, err, "test: %v", tc.name) } else { + require.Error(t, err, "test: %v", tc.name) continue } - if tc.expectPass { - require.NoError(t, msg.ValidateBasic(), "test: %v", i) + if tc.valBasic { + require.NoError(t, msg.ValidateBasic(), "test: %v", tc.name) } else { - require.Error(t, msg.ValidateBasic(), "test: %v", i) + require.Error(t, msg.ValidateBasic(), "test: %v", tc.name) } } } @@ -126,20 +143,21 @@ func TestMsgGrantGetAuthorization(t *testing.T) { func TestAminoJSON(t *testing.T) { tx := legacytx.StdTx{} var msg legacytx.LegacyMsg - someDate := time.Date(1, 1, 1, 1, 1, 1, 1, time.UTC) + blockTime := time.Date(1, 1, 1, 1, 1, 1, 1, time.UTC) + expiresAt := blockTime.Add(time.Hour) msgSend := banktypes.MsgSend{FromAddress: "cosmos1ghi", ToAddress: "cosmos1jkl"} typeURL := sdk.MsgTypeURL(&msgSend) msgSendAny, err := cdctypes.NewAnyWithValue(&msgSend) require.NoError(t, err) - grant, err := authz.NewGrant(someDate, authz.NewGenericAuthorization(typeURL), someDate.Add(time.Hour)) + grant, err := authz.NewGrant(blockTime, authz.NewGenericAuthorization(typeURL), &expiresAt) require.NoError(t, err) - sendGrant, err := authz.NewGrant(someDate, banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000)))), someDate.Add(time.Hour)) + sendGrant, err := authz.NewGrant(blockTime, banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000)))), &expiresAt) require.NoError(t, err) valAddr, err := sdk.ValAddressFromBech32("cosmosvaloper1xcy3els9ua75kdm783c3qu0rfa2eples6eavqq") require.NoError(t, err) stakingAuth, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{valAddr}, nil, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, &sdk.Coin{Denom: "stake", Amount: sdk.NewInt(1000)}) require.NoError(t, err) - delegateGrant, err := authz.NewGrant(someDate, stakingAuth, someDate.Add(time.Hour)) + delegateGrant, err := authz.NewGrant(blockTime, stakingAuth, nil) require.NoError(t, err) // Amino JSON encoding has changed in authz since v0.46. @@ -166,7 +184,7 @@ func TestAminoJSON(t *testing.T) { msg = &authz.MsgGrant{Granter: "cosmos1abc", Grantee: "cosmos1def", Grant: delegateGrant} tx.Msgs = []sdk.Msg{msg} require.Equal(t, - `{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/MsgGrant","value":{"grant":{"authorization":{"type":"cosmos-sdk/StakeAuthorization","value":{"Validators":{"type":"cosmos-sdk/StakeAuthorization/AllowList","value":{"allow_list":{"address":["cosmosvaloper1xcy3els9ua75kdm783c3qu0rfa2eples6eavqq"]}}},"authorization_type":1,"max_tokens":{"amount":"1000","denom":"stake"}}},"expiration":"0001-01-01T02:01:01.000000001Z"},"grantee":"cosmos1def","granter":"cosmos1abc"}}],"sequence":"1","timeout_height":"1"}`, + `{"account_number":"1","chain_id":"foo","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[{"type":"cosmos-sdk/MsgGrant","value":{"grant":{"authorization":{"type":"cosmos-sdk/StakeAuthorization","value":{"Validators":{"type":"cosmos-sdk/StakeAuthorization/AllowList","value":{"allow_list":{"address":["cosmosvaloper1xcy3els9ua75kdm783c3qu0rfa2eples6eavqq"]}}},"authorization_type":1,"max_tokens":{"amount":"1000","denom":"stake"}}}},"grantee":"cosmos1def","granter":"cosmos1abc"}}],"sequence":"1","timeout_height":"1"}`, string(legacytx.StdSignBytes("foo", 1, 1, 1, legacytx.StdFee{}, []sdk.Msg{msg}, "memo", nil)), ) diff --git a/x/authz/simulation/decoder_test.go b/x/authz/simulation/decoder_test.go index cb6e1502fb4f..c169e84137f6 100644 --- a/x/authz/simulation/decoder_test.go +++ b/x/authz/simulation/decoder_test.go @@ -21,7 +21,8 @@ func TestDecodeStore(t *testing.T) { dec := simulation.NewDecodeStore(cdc) now := time.Now().UTC() - grant, _ := authz.NewGrant(now, banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123))), now.Add(1)) + e := now.Add(1) + grant, _ := authz.NewGrant(now, banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123))), &e) grantBz, err := cdc.Marshal(&grant) require.NoError(t, err) kvPairs := kv.Pairs{ diff --git a/x/authz/simulation/genesis.go b/x/authz/simulation/genesis.go index 2defb7ee5c21..8c4609b7bb5c 100644 --- a/x/authz/simulation/genesis.go +++ b/x/authz/simulation/genesis.go @@ -19,11 +19,16 @@ func genGrant(r *rand.Rand, accounts []simtypes.Account, genT time.Time) []authz for i := 0; i < len(accounts)-1; i++ { granter := accounts[i] grantee := accounts[i+1] + var expiration *time.Time + if i%3 != 0 { // generate some grants with no expire time + e := genT.AddDate(1, 0, 0) + expiration = &e + } authorizations[i] = authz.GrantAuthorization{ Granter: granter.Address.String(), Grantee: grantee.Address.String(), Authorization: generateRandomGrant(r), - Expiration: genT.AddDate(1, 0, 0), + Expiration: expiration, } } diff --git a/x/authz/simulation/operations.go b/x/authz/simulation/operations.go index 3247a0735309..1879522b0ce1 100644 --- a/x/authz/simulation/operations.go +++ b/x/authz/simulation/operations.go @@ -2,6 +2,7 @@ package simulation import ( "math/rand" + "time" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" @@ -9,12 +10,10 @@ import ( "github.com/cosmos/cosmos-sdk/simapp/helpers" simappparams "github.com/cosmos/cosmos-sdk/simapp/params" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/authz" - - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/authz/keeper" - banktype "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/cosmos-sdk/x/simulation" ) @@ -108,9 +107,10 @@ func SimulateMsgGrant(ak authz.AccountKeeper, bk authz.BankKeeper, _ keeper.Keep return simtypes.NoOpMsg(authz.ModuleName, TypeMsgGrant, "spend limit is nil"), nil, nil } - expiration := simtypes.RandTimestamp(r) - if expiration.Before(ctx.BlockTime()) { - return simtypes.NoOpMsg(authz.ModuleName, TypeMsgGrant, "past time"), nil, nil + var expiration *time.Time + t1 := simtypes.RandTimestamp(r) + if !t1.Before(ctx.BlockTime()) { + expiration = &t1 } msg, err := authz.NewMsgGrant(granter.Address, grantee.Address, generateRandomAuthorization(r, spendLimit), expiration) if err != nil { @@ -203,7 +203,7 @@ func SimulateMsgRevoke(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Kee _, _, err = app.SimDeliver(txCfg.TxEncoder(), tx) if err != nil { - return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "unable to deliver tx"), nil, err + return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "unable to execute tx: "+err.Error()), nil, err } return simtypes.NewOperationMsg(&msg, true, "", nil), nil, nil @@ -215,19 +215,27 @@ func SimulateMsgExec(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Keepe return func( r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string, ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { - hasGrant := false - var targetGrant authz.Grant var granterAddr sdk.AccAddress var granteeAddr sdk.AccAddress + var sendAuth *banktype.SendAuthorization + var err error k.IterateGrants(ctx, func(granter, grantee sdk.AccAddress, grant authz.Grant) bool { - targetGrant = grant granterAddr = granter granteeAddr = grantee - hasGrant = true - return true + var a authz.Authorization + a, err = grant.GetAuthorization() + if err != nil { + return true + } + var ok bool + sendAuth, ok = a.(*banktype.SendAuthorization) + return ok }) - if !hasGrant { + if err != nil { + return simtypes.NoOpMsg(authz.ModuleName, TypeMsgExec, err.Error()), nil, err + } + if sendAuth == nil { return simtypes.NoOpMsg(authz.ModuleName, TypeMsgExec, "no grant found"), nil, nil } @@ -249,15 +257,6 @@ func SimulateMsgExec(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Keepe } msg := []sdk.Msg{banktype.NewMsgSend(granterAddr, granteeAddr, coins)} - authorization, err := targetGrant.GetAuthorization() - if err != nil { - return simtypes.NoOpMsg(authz.ModuleName, TypeMsgExec, err.Error()), nil, err - } - - sendAuth, ok := authorization.(*banktype.SendAuthorization) - if !ok { - return simtypes.NoOpMsg(authz.ModuleName, TypeMsgExec, "not a send authorization"), nil, nil - } _, err = sendAuth.Accept(ctx, msg[0]) if err != nil { diff --git a/x/authz/simulation/operations_test.go b/x/authz/simulation/operations_test.go index 996345319b72..c1cfe27cc26b 100644 --- a/x/authz/simulation/operations_test.go +++ b/x/authz/simulation/operations_test.go @@ -37,8 +37,7 @@ func (suite *SimTestSuite) TestWeightedOperations() { appParams := make(simtypes.AppParams) weightedOps := simulation.WeightedOperations(appParams, cdc, suite.app.AccountKeeper, - suite.app.BankKeeper, suite.app.AuthzKeeper, cdc, - ) + suite.app.BankKeeper, suite.app.AuthzKeeper, cdc) s := rand.NewSource(3) r := rand.New(s) @@ -54,14 +53,19 @@ func (suite *SimTestSuite) TestWeightedOperations() { {simulation.WeightRevoke, simulation.TypeMsgRevoke}, } + require := suite.Require() for i, w := range weightedOps { - operationMsg, _, _ := w.Op()(r, suite.app.BaseApp, suite.ctx, accs, "") + op, _, err := w.Op()(r, suite.app.BaseApp, suite.ctx, accs, "") + require.NoError(err) // the following checks are very much dependent from the ordering of the output given // by WeightedOperations. if the ordering in WeightedOperations changes some tests // will fail - suite.Require().Equal(expected[i].weight, w.Weight(), "weight should be the same") - suite.Require().Equal(expected[i].opMsgRoute, operationMsg.Route, "route should be the same") - suite.Require().Equal(expected[i].opMsgRoute, operationMsg.Name, "operation Msg name should be the same") + require.Equal(expected[i].weight, w.Weight(), + "weight should be the same. %v", op.Comment) + require.Equal(expected[i].opMsgRoute, op.Route, + "route should be the same. %v", op.Comment) + require.Equal(expected[i].opMsgRoute, op.Name, + "operation Msg name should be the same %v", op.Comment) } } @@ -131,9 +135,10 @@ func (suite *SimTestSuite) TestSimulateRevoke() { granter := accounts[0] grantee := accounts[1] - authorization := banktypes.NewSendAuthorization(initCoins) + a := banktypes.NewSendAuthorization(initCoins) + expire := time.Now().Add(30 * time.Hour) - err := suite.app.AuthzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, authorization, time.Now().Add(30*time.Hour)) + err := suite.app.AuthzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire) suite.Require().NoError(err) // execute operation @@ -166,9 +171,10 @@ func (suite *SimTestSuite) TestSimulateExec() { granter := accounts[0] grantee := accounts[1] - authorization := banktypes.NewSendAuthorization(initCoins) + a := banktypes.NewSendAuthorization(initCoins) + expire := suite.ctx.BlockTime().Add(1 * time.Hour) - err := suite.app.AuthzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, authorization, suite.ctx.BlockTime().Add(1*time.Hour)) + err := suite.app.AuthzKeeper.SaveGrant(suite.ctx, grantee.Address, granter.Address, a, &expire) suite.Require().NoError(err) // execute operation