feat(`profile`): events #742

aljo242 · 2022-04-21T14:20:56Z

Closes #731

What does this PR does?

Emit typed events for the profile module.

The events currents are just Create* and Update*. This differs from the Tx that we expose, but the state is still fully covered by these events

codecov · 2022-04-21T14:27:35Z

Codecov Report

Merging #742 (4394016) into develop (10f3301) will decrease coverage by 0.08%.
The diff coverage is 7.83%.

❗ Current head 4394016 differs from pull request most recent head 5287a59. Consider uploading reports for the commit 5287a59 to get more accurate results

@@             Coverage Diff             @@
##           develop     #742      +/-   ##
===========================================
- Coverage    11.46%   11.37%   -0.09%     
===========================================
  Files          279      279              
  Lines        60803    61456     +653     
===========================================
+ Hits          6971     6993      +22     
- Misses       53667    54300     +633     
+ Partials       165      163       -2

Impacted Files	Coverage Δ
x/profile/types/events.pb.go	`0.93% <0.93%> (ø)`
...ofile/keeper/msg_add_validator_operator_address.go	`100.00% <100.00%> (ø)`
x/profile/keeper/msg_create_coordinator.go	`100.00% <100.00%> (ø)`
x/profile/keeper/msg_disable_coordinator.go	`66.66% <100.00%> (+3.70%)`	⬆️
x/profile/keeper/msg_update_coordinator_address.go	`74.35% <100.00%> (+2.13%)`	⬆️
...ofile/keeper/msg_update_coordinator_description.go	`71.42% <100.00%> (+3.68%)`	⬆️
...profile/keeper/msg_update_validator_description.go	`100.00% <100.00%> (ø)`
x/participation/simulation/store.go	`82.02% <0.00%> (-0.84%)`	⬇️
x/launch/simulation/simulation.go	`0.00% <0.00%> (ø)`
x/reward/simulation/simulation.go	`0.00% <0.00%> (ø)`
... and 7 more

lumtis

I'm honestly not sure about the create/updated events
This would require context to understand the state change.
For example, if you have a Coordinator Updated event, you can't determine directly from the event data if the address of the coordinator or the coordinator description have been changed, you need to get the previous state of the coordinator

What is the downside of having per-message events?

x/reward/keeper/reward_distribution_test.go

aljo242 · 2022-04-21T19:58:48Z

I'm honestly not sure about the create/updated events This would require context to understand the state change. For example, if you have a Coordinator Updated event, you can't determine directly from the event data if the address of the coordinator or the coordinator description have been changed, you need to get the previous state of the coordinator

What is the downside of having per-message events?

When any Msg is run in the Cosmos SDK, a basic "action" event is emitted, stating the type of the message. The code can be found here.

So, in spn when you run any Tx, we already emit this basic event. An example can he seen here:

spnd tx profile create-coordinator --from steve
{"body":{"messages":[{"@type":"/tendermint.spn.profile.MsgCreateCoordinator","address":"spn1mhyps2hlkm0nz6k2puumn69928cnvgg43zlrg7","description":{"identity":"","website":"","details":""}}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""}},"signatures":[]}

confirm transaction before signing and broadcasting [y/N]: y
code: 0
codespace: ""
data: 0A320A2C2F74656E6465726D696E742E73706E2E70726F66696C652E4D7367437265617465436F6F7264696E61746F7212020801
events:
- attributes:
  - index: true
    key: ZmVl
    value: ""
  type: tx
- attributes:
  - index: true
    key: YWNjX3NlcQ==
    value: c3BuMW1oeXBzMmhsa20wbno2azJwdXVtbjY5OTI4Y252Z2c0M3pscmc3LzA=
  type: tx
- attributes:
  - index: true
    key: c2lnbmF0dXJl
    value: RmNpZVRZNEdnbi9UUTdkNjlBR1hqdU9yZnB4TktNSHZTQzlILzlGamtWZC9VODRMVjlMdExVMHRUWFc5UERzTXNZUnNTakN0bXBkMjVNVXhYRisyNkE9PQ==
  type: tx
- attributes:
  - index: true
    key: YWN0aW9u
    value: Y3JlYXRlX2Nvb3JkaW5hdG9y
  type: message
- attributes:
  - index: true
    key: Y29vcmRpbmF0b3I=
    value: eyJjb29yZGluYXRvcklEIjoiMSIsImFkZHJlc3MiOiJzcG4xbWh5cHMyaGxrbTBuejZrMnB1dW1uNjk5MjhjbnZnZzQzemxyZzciLCJkZXNjcmlwdGlvbiI6eyJpZGVudGl0eSI6IiIsIndlYnNpdGUiOiIiLCJkZXRhaWxzIjoiIn0sImFjdGl2ZSI6dHJ1ZX0=
  type: tendermint.spn.profile.EventCoordinatorCreated
gas_used: "62937"
gas_wanted: "200000"
height: "78"
info: ""
logs:
- events:
  - attributes:
    - key: action
      value: create_coordinator
    type: message
  - attributes:
    - key: coordinator
      value: '{"coordinatorID":"1","address":"spn1mhyps2hlkm0nz6k2puumn69928cnvgg43zlrg7","description":{"identity":"","website":"","details":""},"active":true}'
    type: tendermint.spn.profile.EventCoordinatorCreated
  log: ""
  msg_index: 0
raw_log: '[{"events":[{"type":"message","attributes":[{"key":"action","value":"create_coordinator"}]},{"type":"tendermint.spn.profile.EventCoordinatorCreated","attributes":[{"key":"coordinator","value":"{\"coordinatorID\":\"1\",\"address\":\"spn1mhyps2hlkm0nz6k2puumn69928cnvgg43zlrg7\",\"description\":{\"identity\":\"\",\"website\":\"\",\"details\":\"\"},\"active\":true}"}]}]}]'
timestamp: ""
tx: null
txhash: 88ED7C296C1359217C1BF2E4909BFB6BD14F4281D28EC540D6DCEB4C5ED61200

My thinking is that we are providing additional events that detail how the state on-chain is being modified. The "context" providing events are already there.

lumtis · 2022-04-22T07:05:33Z

When fetching or listening for events, can you get the hash of the tx where the event is emitted? If yes, we can actually associate the message name event and custom event to get the full state transition but that still seems more work from the client from my point of view.
If a coordinator's address is updated, for example, you would just need to have the previous address and new address to interpret the state transition, the description doesn't matter

giunatale

Considering we already emit events for messages, I agree with @aljo242's proposal of state-transition events. I would probably make them more informative of what changed though

proto/profile/events.proto

giunatale

I've changed my mind. I like the way things are already.

@lubtd as a clarification (hopefully) events are emitted as part of "raw_data" of transactions, which are comprised of a bunch of messages. Events are already bundled together per-message by the event manager, so they should be representing state-transitions that happens as the messages executes rather than be a recap of the message itself (since the data is included in the transaction anyway).

lumtis · 2022-04-22T15:44:35Z

so they should be representing state-transitions that happens as the messages executes rather than be a recap of the message itself (since the data is included in the transaction anyway).

Yes but in the case of CoordinatorUpdated we don't emit the state transition but the output state from the state transition. The state of the coordinator after the address or the description has been changed

giunatale · 2022-04-22T15:50:09Z

The state transition happened on the coordinator object so you show how the object has changed (IMHO)

Yes but in the case of CoordinatorUpdated we don't emit the state transition but the output state from the state transition. The state of the coordinator after the address or the description has been changed

Not sure I follow. could you clarify? what would practically be what you would emit in the event?

aljo242 · 2022-04-22T15:50:14Z

so they should be representing state-transitions that happens as the messages executes rather than be a recap of the message itself (since the data is included in the transaction anyway).

Yes but in the case of CoordinatorUpdated we don't emit the state transition but the output state from the state transition. The state of the coordinator after the address or the description has been changed

The state transmission is automatically already emitted with the Action event emitted for all transactions. So, if a client is subscribing to an event stream, they will see:

The type of message (already emitted per-tx)
The associated state change (my addition)

Events are emitted and associated with transactions already, so associating them with a specific Tx is natural.

giunatale · 2022-04-22T15:54:03Z

The state transmission is automatically already emitted with the Action event emitted for all transactions. So, if a client is subscribing to an event stream, they will see:

{"key":"action","value":"create_coordinator"}

in the example above

lumtis · 2022-04-22T16:17:17Z

Events are emitted and associated with transactions already, so associating them with a specific Tx is natural.

Ok, this is what I wanted to ensure.
I may be a bit biased with Solidity where you subscribe for a specific event type.
In any case, we can have feedback from Front-End in the future

x/profile/keeper/msg_update_validator_description.go

lumtis · 2022-04-25T09:01:13Z

Seems like we have some test errors since last develop merge

aljo242 · 2022-04-25T11:39:24Z

Seems like we have some test errors since last develop merge

Tests pass now - just a CodeCov upload error

aljo242 added 3 commits April 21, 2022 10:17

add proto

89d8b38

compile proto

d5b2f1f

emit events

b4f3bec

aljo242 requested review from fadeev, ilgooz, lumtis, Pantani, ivanovpetr and giunatale as code owners April 21, 2022 14:20

aljo242 changed the title ~~Feat/profile events~~ feat(profile): emit events Apr 21, 2022

aljo242 changed the title ~~feat(profile): emit events~~ feat(profile): events Apr 21, 2022

Merge branch 'develop' into feat/profile-events

628d0c9

lumtis reviewed Apr 21, 2022

View reviewed changes

x/reward/keeper/reward_distribution_test.go Outdated Show resolved Hide resolved

giunatale suggested changes Apr 22, 2022

View reviewed changes

proto/profile/events.proto Show resolved Hide resolved

giunatale self-requested a review April 22, 2022 12:41

giunatale previously approved these changes Apr 22, 2022

View reviewed changes

lumtis suggested changes Apr 22, 2022

View reviewed changes

x/profile/keeper/msg_update_validator_description.go Outdated Show resolved Hide resolved

emit proper events

2d44c5d

aljo242 dismissed giunatale’s stale review via 2d44c5d April 22, 2022 18:54

Merge branch 'develop' into feat/profile-events

4394016

aljo242 requested review from lumtis and giunatale April 22, 2022 18:54

lumtis previously approved these changes Apr 24, 2022

View reviewed changes

Merge branch 'develop' into feat/profile-events

75ef375

fix error

5287a59

aljo242 dismissed lumtis’s stale review via 5287a59 April 25, 2022 11:28

aljo242 requested a review from lumtis April 25, 2022 11:29

lumtis merged commit 424657d into develop Apr 25, 2022

lumtis deleted the feat/profile-events branch April 25, 2022 13:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(`profile`): events #742

feat(`profile`): events #742

aljo242 commented Apr 21, 2022

codecov bot commented Apr 21, 2022 •

edited

Loading

lumtis left a comment

aljo242 commented Apr 21, 2022 •

edited

Loading

lumtis commented Apr 22, 2022

giunatale left a comment

giunatale left a comment •

edited

Loading

lumtis commented Apr 22, 2022

giunatale commented Apr 22, 2022

aljo242 commented Apr 22, 2022

giunatale commented Apr 22, 2022

lumtis commented Apr 22, 2022

lumtis commented Apr 25, 2022

aljo242 commented Apr 25, 2022

feat(profile): events #742

feat(profile): events #742

Conversation

aljo242 commented Apr 21, 2022

What does this PR does?

codecov bot commented Apr 21, 2022 • edited Loading

Codecov Report

lumtis left a comment

Choose a reason for hiding this comment

aljo242 commented Apr 21, 2022 • edited Loading

lumtis commented Apr 22, 2022

giunatale left a comment

Choose a reason for hiding this comment

giunatale left a comment • edited Loading

Choose a reason for hiding this comment

lumtis commented Apr 22, 2022

giunatale commented Apr 22, 2022

aljo242 commented Apr 22, 2022

giunatale commented Apr 22, 2022

lumtis commented Apr 22, 2022

lumtis commented Apr 25, 2022

aljo242 commented Apr 25, 2022

feat(`profile`): events #742

feat(`profile`): events #742

codecov bot commented Apr 21, 2022 •

edited

Loading

aljo242 commented Apr 21, 2022 •

edited

Loading

giunatale left a comment •

edited

Loading