Event Subscription response cannot be deserialized correctly

[greg-szabo]

What happens
When receiving an event subscription response from Tendermint Go, Rust deserializes the incoming data into a GenericJSONEvent. This can be observed in the event_subscription test.
(Tendermint Go v0.33.4)

What should happen
The incoming data should be deserialized into a JsonRPCTransactionResult type correctly.

The reason seems to be that the defined Rust structs are not compatible with the incoming Go JSON.
Interesting addition: the Game of Zones compatible Gaia RPC sends the correct JSON and Rust can deserialize it correctly into JsonRPCTransactionResult.

One discrepancy found is that the JSON result_begin_block field is {} in Tendermint, while it's populated with an events: [...] field in Gaia. The Rust implementation considers the events field mandatory. There might be other field that don't behave well.

Relevant discussion here. Please note the TODO in the code above the discussion.

ref: #225

[greg-szabo]

The below files are example responses from Tendermint Go v0.33.4 and Gaia (commit ID cbc3321 which is the WIP code for GoZ phase 2, also using Tendermint v0.33.4)

gaia.json.log
tendermint.json.log

[liamsi]

How does it work in tendermint go?

Some context on the events; in tendermint there are these types:

Event data from subscriptions:

type ResultEvent struct {
	Query  string              `json:"query"`
	Data   types.TMEventData   `json:"data"`
	Events map[string][]string `json:"events"`
}

and the TMEventData in the Data field:

// TMEventData implements events.EventData.
type TMEventData interface {
	// empty interface
}

func RegisterEventDatas(cdc *amino.Codec) {
	cdc.RegisterInterface((*TMEventData)(nil), nil)
	cdc.RegisterConcrete(EventDataNewBlock{}, "tendermint/event/NewBlock", nil)
	cdc.RegisterConcrete(EventDataNewBlockHeader{}, "tendermint/event/NewBlockHeader", nil)
	cdc.RegisterConcrete(EventDataTx{}, "tendermint/event/Tx", nil)
	cdc.RegisterConcrete(EventDataRoundState{}, "tendermint/event/RoundState", nil)
	cdc.RegisterConcrete(EventDataNewRound{}, "tendermint/event/NewRound", nil)
	cdc.RegisterConcrete(EventDataCompleteProposal{}, "tendermint/event/CompleteProposal", nil)
	cdc.RegisterConcrete(EventDataVote{}, "tendermint/event/Vote", nil)
	cdc.RegisterConcrete(EventDataValidatorSetUpdates{}, "tendermint/event/ValidatorSetUpdates", nil)
	cdc.RegisterConcrete(EventDataString(""), "tendermint/event/ProposalString", nil)
}

https://github.com/tendermint/tendermint/blob/044f1bf288192cd49a400511189178270f02d48a/types/events.go#L45-L61

Currently, in rust each event type implements its own result, e.g.:

tendermint-rs/tendermint/src/rpc/event_listener.rs

Lines 186 to 192 in d28c4b1

	/// Block Results
	#[derive(Serialize, Deserialize, Debug, Clone)]
	pub struct RPCBlockResult {
	query: String,
	data: BlockResultData,
	events: HashMap<String, Vec<String>>,
	}

tendermint-rs/tendermint/src/rpc/event_listener.rs

Lines 129 to 135 in d28c4b1

	/// JSON RPC Result Type
	#[derive(Serialize, Deserialize, Debug, Clone)]
	pub struct RPCTxResult {
	query: String,
	data: Data,
	events: HashMap<String, Vec<String>>,
	}

Suggestion for the Rust code

Similarly, to what I suggested in #225 (comment), I think it would be a good first step refactor the rust code to match that structure: one ResultEvent and one enum, that has all those registered types as variants (it's almost there in form of the current enum Event already.
I'm confident that rather simple refactoring would fix the serialization issue at hand as well as make the code more readable.

[liamsi]

I think the above suggested refactoring is simple enough to do even if the tendermint team is potentially going to overhaul some of the jsonrpc types (see: tendermint/tendermint#4828).

Also linking the general serialzation issue here: #197

[liamsi]

I started implementing above in #279

[liamsi]

Fixed the serialization issue in #279

I have a few more suggestions regarding the subscription module but I'll add those to a separate issue (or rather in an ADR).