Ensure tx search returns useful error messages

[cwgoes]

We should make sure searching for transactions, if the node wouldn't have indexed a transaction with the tag being searched for, returns an error instead of just returning no transactions.

[zramsay]

related to #1021

[tnachen]

@cwgoes Is this already fixed? Wonder if we can close this

[cwgoes]

@cwgoes Is this already fixed? Wonder if we can close this

I'm not sure - this issue is more than a year old - but not specifically to my knowledge.

[alexanderbez]

You'll get the following response when querying for txs by bogus or non-existing events:

{
    "count": "0",
    "limit": "30",
    "page_number": "1",
    "page_total": "0",
    "total_count": "0",
    "txs": []
}

What exactly are you looking to add and under which scenarios @cwgoes ?

[cwgoes]

Ah, the question was not that, but rather what happens when you query for an event that a node is not configured to index, and so will return no transactions even if transactions would have matched had the node indeed been indexing that event (tag).

[alexanderbez]

@cwgoes I see. Sounds like you want a friendly error returned signaling that the underlying RPC node either has indexing disabled or not enabled for those events types? Correct me if I'm wrong.

[cwgoes]

@cwgoes I see. Sounds like you want a friendly error returned signaling that the underlying RPC node either has indexing disabled or not enabled for those events types? Correct me if I'm wrong.

Precisely!

[alexanderbez]

Got it. One immediate way to do this would be to read/load the Tendermint TxIndex config into the CLIContext and use this info when processing requests in QueryTxsByEvents.

[cipherzzz]

@alexanderbez @tac0turtle - you can assign this to me. I will work on it when I finish - #13703

[tac0turtle]

done, let us know if you need any help

[cwgoes]

done, let us know if you need any help

wow, amazing long-term issue management 🎉

[cipherzzz]

Got it. One immediate way to do this would be to read/load the Tendermint TxIndex config into the CLIContext and use this info when processing requests in QueryTxsByEvents.

Can you explain this to me a little more? Not sure where to read the TxConfig in from Comet and how to add it to the Context @alexanderbez

[alexanderbez]

So this would probably happen somewhere in the server package. You already have access to the entire CometBFT config in interceptConfigs. There you could grab the CometBFT's indexing config and push that into the Context.

I will note however, I'm not sure what CometBFT's long-term plans are on the indexer.

[cipherzzz]

Ok, so I think that I'm grabbing the list of events to index correctly here @alexanderbez

Query.go

// QueryTxsByEventsCmd returns a command to search through transactions by events.
func QueryTxsByEventsCmd() *cobra.Command {
....
			clientCtx, err := client.GetClientQueryContext(cmd)
			if err != nil {
				return err
			}

                         # Is this correct?
			cfg, err := config.GetConfig(clientCtx.Viper)
			if err != nil {
				return err
			}
			fmt.Println("indexEvents", cfg.IndexEvents)
...

The issue is that the slice is empty - which I guess according to this line would mean that all events are indexed?

Config.go

// BaseConfig defines the server's basic configuration
type BaseConfig struct {
...

	// IndexEvents defines the set of events in the form {eventType}.{attributeKey},
	// which informs CometBFT what to index. If empty, all events will be indexed.
	IndexEvents []string `mapstructure:"index-events"`
...

Is it reasonable to assume that if no events are returned for a given query and the IndexEvents are empty that

• The event does not exist
• It was removed in pruned tx(s)

Also, if there are events defined, we can check against them to verify they are an indexed event...

[alexanderbez]

@cipherzzz I know you spent some time thinking about this problem, but I simply don't know if tackling this is worth it. First, the indexer config relates to Comet's (now legacy) proprietary indexer. It can also change over time and not watch what has been indexed thus far. Simply put, the config can change all the time and can sometimes not relate to what's actually indexed.

[cipherzzz]

Yeah, that's fine - thanks for the response. I'm definitely not looking to waste anyone's time if this is a moving target. I guess we can close this issue

[tac0turtle]

now that we pass everything to comet, we shouldnt do extra wrapping. thanks for looking into the issue ❤️