Skip to content

Commit

Permalink
feat: add message index event attribute to authz message execution (#…
Browse files Browse the repository at this point in the history
…12668)

## Description

It's impossible currently for operators or clients to understand what events emitted from messages executed via `MsgExec` correspond to exactly what messages. This PR adds a message index attribute to each event emitted by a message via `MsgExec`.

---

### 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/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

### 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)

(cherry picked from commit afab2f3)

# Conflicts:
#	CHANGELOG.md
#	x/authz/keeper/keeper.go
#	x/authz/keeper/msg_server.go
  • Loading branch information
alexanderbez authored and mergify[bot] committed Jul 21, 2022
1 parent 155d9ec commit 884e2dc
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 3 deletions.
49 changes: 49 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,55 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Features

* (upgrade) [#12603](https://github.com/cosmos/cosmos-sdk/pull/12603) feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces
<<<<<<< HEAD
=======

### Improvements

* [#12668](https://github.com/cosmos/cosmos-sdk/pull/12668) Add `authz_msg_index` event attribute to message events emitted when executing via `MsgExec` through `x/authz`.
* [#12634](https://github.com/cosmos/cosmos-sdk/pull/12634) Move `sdk.Dec` to math package.
* [#12596](https://github.com/cosmos/cosmos-sdk/pull/12596) Remove all imports of the non-existent gogo/protobuf v1.3.3 to ease downstream use and go workspaces.
* [#12589](https://github.com/cosmos/cosmos-sdk/pull/12589) Allow zero gas in simulation mode.
* [#12576](https://github.com/cosmos/cosmos-sdk/pull/12576) Remove dependency on cosmos/keyring and upgrade to 99designs/keyring v1.2.1
* [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Mark the `TipDecorator` as beta, don't include it in simapp by default.
* [#12153](https://github.com/cosmos/cosmos-sdk/pull/12153) Add a new `NewSimulationManagerFromAppModules` constructor, to simplify simulation wiring.
* [#12187](https://github.com/cosmos/cosmos-sdk/pull/12187) Add batch operation for x/nft module.
* [#12453](https://github.com/cosmos/cosmos-sdk/pull/12453) Add `NewInMemoryWithKeyring` function which allows the creation of in memory `keystore` instances with a specified set of existing items.
* [#11390](https://github.com/cosmos/cosmos-sdk/pull/11390) `LatestBlockResponse` & `BlockByHeightResponse` types' `Block` filed has been deprecated and they now contains new field `sdk_block` with `proposer_address` as `string`
* [#12626](https://github.com/cosmos/cosmos-sdk/pull/12626) Upgrade IAVL to v0.19.0 with fast index and error propagation. NOTE: first start will take a while to propagate into new model.

### State Machine Breaking

* (x/bank) [#12610](https://github.com/cosmos/cosmos-sdk/pull/12610) `MsgMultiSend` now allows only a single input.
* (x/bank) [#12630](https://github.com/cosmos/cosmos-sdk/pull/12630) Migrate `x/bank` to self-managed parameters and deprecate its usage of `x/params`.
* (x/auth) [#12475](https://github.com/cosmos/cosmos-sdk/pull/12475) Migrate `x/auth` to self-managed parameters and deprecate its usage of `x/params`.
* (x/slashing) [#12399](https://github.com/cosmos/cosmos-sdk/pull/12399) Migrate `x/slashing` to self-managed parameters and deprecate its usage of `x/params`.
* (x/mint) [#12363](https://github.com/cosmos/cosmos-sdk/pull/12363) Migrate `x/mint` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) Migrate `x/distribution` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) Migrate `x/crisis` to self-managed parameters and deprecate it's usage of `x/params`.

### API Breaking Changes

* (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
* (x/gov) [#12368](https://github.com/cosmos/cosmos-sdk/pull/12369) Gov keeper is now passed by reference instead of copy to make post-construction mutation of Hooks and Proposal Handlers possible at a framework level.
* (simapp) [#12270](https://github.com/cosmos/cosmos-sdk/pull/12270) Remove `invCheckPeriod uint` attribute from `SimApp` struct as per migration of `x/crisis` to app wiring
* (simapp) [#12334](https://github.com/cosmos/cosmos-sdk/pull/12334) Move `simapp.ConvertAddrsToValAddrs` and `simapp.CreateTestPubKeys ` to respectively `simtestutil.ConvertAddrsToValAddrs` and `simtestutil.CreateTestPubKeys` (`testutil/sims`)
* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Move `simapp.EmptyAppOptions` to `simtestutil.EmptyAppOptions` (`testutil/sims`)
* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Remove `skipUpgradeHeights map[int64]bool` and `homePath string` from `NewSimApp` constructor as per migration of `x/upgrade` to app-wiring.
* (testutil) [#12278](https://github.com/cosmos/cosmos-sdk/pull/12278) Move all functions from `simapp/helpers` to `testutil/sims`
* (testutil) [#12233](https://github.com/cosmos/cosmos-sdk/pull/12233) Move `simapp.TestAddr` to `simtestutil.TestAddr` (`testutil/sims`)
* (x/staking) [#12102](https://github.com/cosmos/cosmos-sdk/pull/12102) Staking keeper now is passed by reference instead of copy. Keeper's SetHooks no longer returns keeper. It updates the keeper in place instead.
* (linting) [#12141](https://github.com/cosmos/cosmos-sdk/pull/12141) Fix usability related linting for database. This means removing the infix Prefix from `prefix.NewPrefixWriter` and such so that it is `prefix.NewWriter` and making `db.DBConnection` and such into `db.Connection`
* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) `x/distribution` module `SetParams` keeper method definition is now updated to return `error`.
* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) `x/crisis` module `SetConstantFee` keeper method definition is now updated to return `error`.

### CLI Breaking Changes

* (x/group) [#12551](https://github.com/cosmos/cosmos-sdk/pull/12551) read the decision policy from disk in group CLI commands.

>>>>>>> afab2f348 (feat: add message index event attribute to authz message execution (#12668))
### Bug Fixes

* (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation.
Expand Down
26 changes: 23 additions & 3 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package keeper

import (
"fmt"
"strconv"
"time"

"github.com/gogo/protobuf/proto"

abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/baseapp"
Expand Down Expand Up @@ -74,13 +75,25 @@ 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) {
results := make([][]byte, len(msgs))
<<<<<<< HEAD
=======
now := ctx.BlockTime()

>>>>>>> afab2f348 (feat: add message index event attribute to authz message execution (#12668))
for i, msg := range msgs {
signers := msg.GetSigners()
if len(signers) != 1 {
return nil, sdkerrors.ErrInvalidRequest.Wrap("authorization can be given to msg with only one signer")
}

granter := signers[0]
<<<<<<< HEAD
// if granter != grantee then check authorization.Accept, otherwise we implicitly accept.
=======

// If granter != grantee then check authorization.Accept, otherwise we
// implicitly accept.
>>>>>>> afab2f348 (feat: add message index event attribute to authz message execution (#12668))
if !granter.Equals(grantee) {
authorization, _ := k.GetCleanAuthorization(ctx, grantee, granter, sdk.MsgTypeURL(msg))
if authorization == nil {
Expand All @@ -90,6 +103,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
if err != nil {
return nil, err
}

if resp.Delete {
err = k.DeleteGrant(ctx, grantee, granter, sdk.MsgTypeURL(msg))
} else if resp.Updated != nil {
Expand All @@ -98,6 +112,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
if err != nil {
return nil, err
}

if !resp.Accept {
return nil, sdkerrors.ErrUnauthorized
}
Expand All @@ -112,14 +127,19 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
if err != nil {
return nil, sdkerrors.Wrapf(err, "failed to execute message; message %v", msg)
}

results[i] = msgResp.Data

// emit the events from the dispatched actions
events := msgResp.Events
sdkEvents := make([]sdk.Event, 0, len(events))
for i := 0; i < len(events); i++ {
sdkEvents = append(sdkEvents, sdk.Event(events[i]))
for _, event := range events {
e := event
e.Attributes = append(e.Attributes, abci.EventAttribute{Key: "authz_msg_index", Value: strconv.Itoa(i)})

sdkEvents = append(sdkEvents, sdk.Event(e))
}

ctx.EventManager().EmitEvents(sdkEvents)
}

Expand Down
3 changes: 3 additions & 0 deletions x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,12 @@ func (s *TestSuite) TestDispatchedEvents() {
result, err := app.AuthzKeeper.DispatchActions(s.ctx, granteeAddr, executeMsgs)
require.NoError(err)
require.NotNil(result)

events := s.ctx.EventManager().Events()

// get last 5 events (events that occur *after* the grant)
events = events[len(events)-5:]

requiredEvents := map[string]bool{
"coin_spent": false,
"coin_received": false,
Expand Down
15 changes: 15 additions & 0 deletions x/authz/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
if err != nil {
return nil, err
}
<<<<<<< HEAD
=======

// create the account if it is not in account state
granteeAcc := k.authKeeper.GetAccount(ctx, grantee)
if granteeAcc == nil {
granteeAcc = k.authKeeper.NewAccountWithAddress(ctx, grantee)
k.authKeeper.SetAccount(ctx, granteeAcc)
}

>>>>>>> afab2f348 (feat: add message index event attribute to authz message execution (#12668))
granter, err := sdk.AccAddressFromBech32(msg.Granter)
if err != nil {
return nil, err
Expand All @@ -26,6 +37,7 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
if authorization == nil {
return nil, sdkerrors.ErrUnpackAny.Wrap("Authorization is not present in the msg")
}

t := authorization.MsgTypeURL()
if k.router.HandlerByTypeURL(t) == nil {
return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "%s doesn't exist.", t)
Expand Down Expand Up @@ -66,13 +78,16 @@ func (k Keeper) Exec(goCtx context.Context, msg *authz.MsgExec) (*authz.MsgExecR
if err != nil {
return nil, err
}

msgs, err := msg.GetMessages()
if err != nil {
return nil, err
}

results, err := k.DispatchActions(ctx, grantee, msgs)
if err != nil {
return nil, err
}

return &authz.MsgExecResponse{Results: results}, nil
}

0 comments on commit 884e2dc

Please sign in to comment.