Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: emit cached context events #13063

Merged
merged 19 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (x/authz) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) Add an allow list, an optional list of addresses allowed to receive bank assests via authz MsgSend grant.
* (x/authz) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) Add an allow list, an optional list of addresses allowed to receive bank assets via authz MsgSend grant.
* (sdk.Coins) [#12627](https://github.com/cosmos/cosmos-sdk/pull/12627) Make a Denoms method on sdk.Coins.
* (testutil) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Add generic `testutil.RandSliceElem` function which selects a random element from the list.
* (client) [#12936](https://github.com/cosmos/cosmos-sdk/pull/12936) Add capability to preprocess transactions before broadcasting from a higher level chain.
Expand Down Expand Up @@ -93,6 +93,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (context) [#13063](https://github.com/cosmos/cosmos-sdk/pull/13063) Update `Context#CacheContext` to automatically emit all events on the parent context's `EventManager`.
* (x/bank) [#12706](https://github.com/cosmos/cosmos-sdk/pull/12706) Removed the `testutil` package from the `x/bank/client` package.
* (simapp) [#12747](https://github.com/cosmos/cosmos-sdk/pull/12747) Remove `simapp.MakeTestEncodingConfig`. Please use `moduletestutil.MakeTestEncodingConfig` (`types/module/testutil`) in tests instead.
* (x/bank) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) `NewSendAuthorization` takes a new argument of an optional list of addresses allowed to receive bank assests via authz MsgSend grant. You can pass `nil` for the same behavior as before, i.e. any recipient is allowed.
Expand Down
11 changes: 9 additions & 2 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,18 @@ func (c Context) TransientStore(key storetypes.StoreKey) KVStore {

// CacheContext returns a new Context with the multi-store cached and a new
// EventManager. The cached context is written to the context when writeCache
// is called.
// is called. Note, events are automatically emitted on the parent context's
// EventManager when the caller executes the write.
func (c Context) CacheContext() (cc Context, writeCache func()) {
cms := c.MultiStore().CacheMultiStore()
cc = c.WithMultiStore(cms).WithEventManager(NewEventManager())
return cc, cms.Write

writeCache = func() {
c.EventManager().EmitEvents(cc.EventManager().Events())
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
cms.Write()
}

return cc, writeCache
}

var _ context.Context = Context{}
Expand Down
5 changes: 5 additions & 0 deletions types/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,18 @@ func (s *contextTestSuite) TestCacheContext() {
s.Require().Equal(v1, cstore.Get(k1))
s.Require().Nil(cstore.Get(k2))

// emit some events
cctx.EventManager().EmitEvent(types.NewEvent("foo", types.NewAttribute("key", "value")))
cctx.EventManager().EmitEvent(types.NewEvent("bar", types.NewAttribute("key", "value")))

cstore.Set(k2, v2)
s.Require().Equal(v2, cstore.Get(k2))
s.Require().Nil(store.Get(k2))

write()

s.Require().Equal(v2, store.Get(k2))
s.Require().Len(ctx.EventManager().Events(), 2)
}

func (s *contextTestSuite) TestLogContext() {
Expand Down
6 changes: 0 additions & 6 deletions x/gov/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
tagValue = types.AttributeValueProposalPassed
logMsg = "passed"

// The cached context is created with a new EventManager. However, since
// the proposal handler execution was successful, we want to track/keep
// any events emitted, so we re-emit to "merge" the events into the
// original Context's EventManager.
ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())

alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
// write state to the underlying multi-store
writeCache()
} else {
Expand Down
8 changes: 1 addition & 7 deletions x/group/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,19 +754,13 @@ func (k Keeper) Exec(goCtx context.Context, req *group.MsgExec) (*group.MsgExecR
return nil, err
}

results, err := k.doExecuteMsgs(cacheCtx, k.router, proposal, addr)
if err != nil {
if _, err := k.doExecuteMsgs(cacheCtx, k.router, proposal, addr); err != nil {
proposal.ExecutorResult = group.PROPOSAL_EXECUTOR_RESULT_FAILURE
logs = fmt.Sprintf("proposal execution failed on proposal %d, because of error %s", id, err.Error())
k.Logger(ctx).Info("proposal execution failed", "cause", err, "proposalID", id)
} else {
proposal.ExecutorResult = group.PROPOSAL_EXECUTOR_RESULT_SUCCESS
flush()

for _, res := range results {
// NOTE: The sdk msg handler creates a new EventManager, so events must be correctly propagated back to the current context
ctx.EventManager().EmitEvents(res.GetEvents())
}
}
}

Expand Down