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

Some examples/comments #583

Closed
wants to merge 3 commits into from
Closed

Some examples/comments #583

wants to merge 3 commits into from

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Aug 5, 2021

Do not merge 🚨

Some examples from SDK and comments.

I would like to see examples of flattened events. I am worried now this may open the door for fraud. 🤷

@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #583 (fc88359) into events-docs (657b427) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           events-docs     #583      +/-   ##
===============================================
+ Coverage        59.67%   59.71%   +0.03%     
===============================================
  Files               45       45              
  Lines             5208     5208              
===============================================
+ Hits              3108     3110       +2     
+ Misses            1875     1874       -1     
+ Partials           225      224       -1     
Impacted Files Coverage Δ
x/wasm/keeper/keeper.go 85.47% <0.00%> (+0.36%) ⬆️

@alpe alpe marked this pull request as draft August 5, 2021 15:02
@ethanfrey
Copy link
Member

I would like to see examples of flattened events. I am worried now this may open the door for fraud. 🤷

They are only available via Tendermint (Cosmos SDK does not flatten).

If you have:

[
  { "type": "wasm", "attributes": [
    {"key": "_contract_address", "value": "12345"}, 
    {"key": "transfer", "value": "failed"}]},
  { "type": "wasm", "attributes": [
    {"key": "_contract_address", "value": "76543"}, 
    {"key": "transfer", "value": "success"}]}
]

This will end up in the Tendermint queries as:

[
  { "type": "wasm", "attributes": [
    {"key": "_contract_address", "value": "12345"}, 
    {"key": "transfer", "value": "failed"}]},
    {"key": "_contract_address", "value": "76543"}, 
    {"key": "transfer", "value": "success"}]}
]

If we tag each on with the _contract_address, the info is there and the onus is on the client to carefully parse. (Not just contract address is there and transfer=success is there, but the order.

That said, CosmJS has long considered this field unreliable and only returns events from parsing the log field, which has all proper data.

Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Some response to your comments. Links to other parts of the sdk

EVENTS.md Outdated

TODO: what is added by the AnteHandlers (message.signer? auth?)
Copy link
Member

Choose a reason for hiding this comment

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

This is emitted on every transaction - https://github.com/cosmos/cosmos-sdk/blob/v0.42.9/x/auth/ante/sigverify.go#L103-L120 not so interesting for us but good to reference.

Also, it would make reading easier IMO if sdk.AttributeKeyModule was replaced by "module" or whatever. Anyone using events will need to find the strings for the client.

Copy link
Member

Choose a reason for hiding this comment

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

And THIS is what I was thinking of when I mentioned AnteHandlers: https://github.com/cosmos/cosmos-sdk/blob/6888de1d86026c25197c1227dae3d7da4d41a441/baseapp/baseapp.go#L746-L748

msgEvents := sdk.Events{
			sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, eventMsgName)),
		}
		msgEvents = msgEvents.AppendEvents(msgResult.GetEvents())

Unfortunately the calculation of that string changed between 0.42.9 and 0.43.0 (for new message service modules):

0.42.9: msgFqName = svcMsg.MethodName https://github.com/cosmos/cosmos-sdk/blob/v0.42.9/baseapp/baseapp.go#L709

0.43.0: eventMsgName = sdk.MsgTypeURL(msg) https://github.com/cosmos/cosmos-sdk/blob/v0.43.0-rc3/baseapp/baseapp.go#L720

It uses a longer URL. This did break relayers


They also emit one or more other custom type event in the module that describe the operation executed with more details or metadata.
```go
sdk.NewEvent(
Copy link
Member

Choose a reason for hiding this comment

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

This is the same as above? Did you intend to link something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ja, first example should have been the message type. sorry

@ethanfrey
Copy link
Member

Thanks for the input. Integrated feedback into #581 manually

@ethanfrey ethanfrey closed this Aug 6, 2021
@ethanfrey ethanfrey deleted the alex_event-docs branch August 6, 2021 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants