cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.08k stars 3.5k forks source link

Implement a query for `EndBlock` msg events #13703

Closed damiannolan closed 1 year ago

damiannolan commented 1 year ago

Summary

Implement a CLI query for EndBlocker emitted events. When debugging an integration issue associated with #13389 I faced the problem of having no simple way to query for events associated with msgs exec'd by x/gov.

Problem Definition

We currently have a means of querying msg events associated with transactions.

For example, we can query a tx by: hash|acc_seq|signature. We can also query for associated events like below:

simd q txs --events 'send_packet.packet_sequence=1'

However, for messages executed by modules such as x/gov or x/group we can't seem to use the same approach.

Proposal

Add a similar CLI query for events associated with those executed by x/gov and x/group.

alexanderbez commented 1 year ago

There's already a query for this -> https://cosmos-rpc.quickapi.com/block_search?query=_&page=_&per_page=_&order_by=_

We could add a proxy to this in the SDK's gRPC query layer, but just noting this already exists. In fact, tx searching also proxies directly to Tendermint (https://cosmos-rpc.quickapi.com/tx_search?query=_&prove=_&page=_&per_page=_&order_by=_)

damiannolan commented 1 year ago

Awesome, thanks for this @alexanderbez!

I figured this was the case with tx search alright, but just noticed the case of no way to get at events via the CLI for blocks. I think simd q block --events could be a good home for it if you guys choose to implement this.

alexanderbez commented 1 year ago

Yah, so we'd have to proxy BlockSearch to Tendermint in order to have a gRPC and CLI query variant. We can definitely do this, it's just not a massive priority. I'd be happy to review a PR for it if someone wants to work on it. They can use TxSearch as an example ;-)

Maybe @gsk967 has interest?

tac0turtle commented 1 year ago

If this is reliant on events we can't make guarantees that the events will be there in the long run. If events are removed from being stored in the node then it could break our api.

alexanderbez commented 1 year ago

Yes, but since we proxy TxSearch, we can also proxy BlockSearch. If the underlying node doesn't support it (i.e. it's using psql), it'll just return an error which is forwarded to the user/client invoking the query. So there's no harm in having a proxy for BlockSearch.

symtor commented 1 year ago

I'd like to upvote this topic, am also looking for a way of accessing the EndBlocker events (gRPC).

One caveat: The data returned by tendermint's block_search (both the RPC end point linked above and the implementation in ResultBlockSearch) does not currently include these events either. The block_results end point ( https://cosmos-rpc.quickapi.com/block_results?height=_ / ResultBlockResults ) does, except it apparently only returns the first 364 begin_block_events and no end_block_events.

alexanderbez commented 1 year ago

@symtor are you open to submitting a PR?

symtor commented 1 year ago

@symtor are you open to submitting a PR?

Not right now, sorry about that. We worked around the issue by using the block_results end point (which actually works well, my earlier worry about not seeing end_block_events was just me looking at the wrong block). This workaround also addresses an issue that we were having with the GetTxsEvent API, where for recently indexed blocks txs are sometimes not returned (though a proper proxy for block_results would likely also solve this).

ahm23 commented 1 year ago

I've been doing the same with the block_results RPC endpoint; however, that is very slow compared to a potential gRPC endpoint. GetTx returns the transaction result alongside the broadcasted transaction; meanwhile, GetBlockByHeight for blocks which has no events included in the query response.

cipherzzz commented 1 year ago

@alexanderbez - I can take a stab at it if you like...

alexanderbez commented 1 year ago

@cipherzzz yes please! TYSM

cipherzzz commented 1 year ago

@alexanderbez - you can assign it to me - I'll get started

cipherzzz commented 1 year ago

@alexanderbez - working through this right now. Could you give me some feedback on

Thanks!

cipherzzz commented 1 year ago

@tac0turtle - I think I have it mostly there. One issue I'm having with my make proto-gen is that I get compile errors unless I manually add the following to my generated structs in abci.pb.go - which is obviously a nonstarter.

// BlockResponse
type BlockResponse struct {
    // The block height
    Height         int64  `protobuf:"varint,1,opt,name=height,proto3" json:"height,omitempty"`
    Time           string `protobuf:"bytes,2,opt,name=time,proto3" json:"time,omitempty"`
    ChainId        string `protobuf:"bytes,3,opt,name=chain_id,json=chainId,proto3" json:"chain_id,omitempty"`
    LastCommit     string `protobuf:"bytes,4,opt,name=last_commit,json=lastCommit,proto3" json:"last_commit,omitempty"`
    Data           string `protobuf:"bytes,5,opt,name=data,proto3" json:"data,omitempty"`
    Validators     string `protobuf:"bytes,6,opt,name=validators,proto3" json:"validators,omitempty"`
    NextValidators string `protobuf:"bytes,7,opt,name=next_validators,json=nextValidators,proto3" json:"next_validators,omitempty"`
    App            string `protobuf:"bytes,8,opt,name=app,proto3" json:"app,omitempty"`
    Consensus      string `protobuf:"bytes,9,opt,name=consensus,proto3" json:"consensus,omitempty"`
    Results        string `protobuf:"bytes,10,opt,name=results,proto3" json:"results,omitempty"`
    Evidence       string `protobuf:"bytes,11,opt,name=evidence,proto3" json:"evidence,omitempty"`
    Proposer       string `protobuf:"bytes,12,opt,name=proposer,proto3" json:"proposer,omitempty"`
}

func (m *BlockResponse) Reset()      { *m = BlockResponse{} }
func (m *BlockResponse) String() string { return proto.CompactTextString(m) } //I had to manually add this
func (m *BlockResponse) Empty() bool { return len(proto.CompactTextString(m)) == 0 } //I had to manually add this

Any idea why I have to manually add the String and Empty methods?

alexanderbez commented 1 year ago

If you needed to add the String method, consider removing the stringer attribute from the proto message schema.

tac0turtle commented 1 year ago

thinking through this some more, I think we should have a general query method for events. With 0.38 of tendermint the notion of begin and end block events disappear and they become events. This is finalise block.

cipherzzz commented 1 year ago

@tac0turtle - so are you saying that you want to query for events in general regardless of whether they are associated with a tx or block? Or are you saying that we should just query for block events regardless of whether they are in a begin/end callback?

alexanderbez commented 1 year ago

There will be two sets of events with Tendermint v0.38 -- Block and Tx events, so essentially Begin|EndBlock events will be condensed into a single stream of events (txs will still be separate).

I can see a general events query interface where we take a type argument -- block or tx. But I actually don't know how Tendermint will expose event querying in 0.38