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

Events Tracking / Tendermint v0.32.0 Update #4541

Merged
merged 63 commits into from
Jun 26, 2019

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Jun 12, 2019

Refactor the SDK to use the ABCI events semantics:

  • Move x/{module}/tags/tags.go => x/{module}/types/events.go
  • Update docs/specs to reflect all new event types
  • Refactor tags in favor of new Event(s) type(s)
  • Update Context to use new EventManager
  • (Begin|End)Blocker no longer return tags, but rather uses new EventManager
  • Message handlers no longer return tags, but rather uses new EventManager

Any component (e.g. BeginBlocker, message handler, etc...) wishing to emit an event must do so
through ctx.EventManger().EmitEvent(s).

To reset or wipe emitted events: ctx = ctx.WithEventManager(sdk.NewEventManager())
To get all emitted events: events := ctx.EventManager().Events()


To perform the minimal review, simply review each spec's xx_events.md file as the structural code changes are trivial (but verbose).

closes: #4387


  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: clog add [section] [stanza] [message]

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alexanderbez alexanderbez added WIP T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Jun 12, 2019
go.mod Show resolved Hide resolved
types/events.go Outdated Show resolved Hide resolved
types/result.go Outdated Show resolved Hide resolved
alexanderbez and others added 8 commits June 20, 2019 13:35
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
@alexanderbez alexanderbez mentioned this pull request Jun 20, 2019
13 tasks
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

x/distribution/handler.go Show resolved Hide resolved
x/distribution/handler.go Show resolved Hide resolved
@alexanderbez alexanderbez changed the title Events Tracking Events Tracking / Tendermint v0.32.0 Update Jun 26, 2019
Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏 👏 stoked - few changes I'll just push the typo upgrades

docs/spec/governance/04_events.md Outdated Show resolved Hide resolved
docs/spec/staking/06_events.md Outdated Show resolved Hide resolved
x/staking/types/events.go Outdated Show resolved Hide resolved
@@ -156,6 +147,18 @@ func (keeper BaseSendKeeper) SendCoins(
if !amt.IsValid() {
return sdk.ErrInvalidCoins(amt.String())
}

ctx.EventManager().EmitEvents(sdk.Events{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emit a bit confusing here although it makes sense... Does AddEvents not make sense?

x/gov/types/events.go Outdated Show resolved Hide resolved
@rigelrozanski rigelrozanski merged commit 67f6b02 into master Jun 26, 2019
@rigelrozanski rigelrozanski deleted the bez/4387-events-tracking branch June 26, 2019 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Events Tracking
6 participants