RPC client fails to decode block_results with non-nil validator_updates

[ancazamfir]

RPC client fails to decode block_results response with non empty validator_updates.
@mankenavenkatesh has reported this problem, thanks Venkatesh! It shows on a number of chains when hermes tries to look for begin/end blocker IBC events by calling the block_results RPC.
Decoding the response fails when validator_updates is not empty. This is reported for some chains at tm-go version v0.34.14. I believe the format is different than what we expect, here is one example of validator_updates from one chain:

    "validator_updates": [
      {
        "pub_key": {
          "Sum": {
            "type": "tendermint.crypto.PublicKey_Ed25519",
            "value": {
              "ed25519": "VqJCr3vjQdffcLIG6RMBl2MgXDFYNY6b3Joaa43gV3o="
            }
          }
        },
        "power": "573929"
      }
    ],

I don't think there were recent changes in the crypto PubKey proto definition, maybe this never worked.
Full output of curl http://18.191.147.51:26657/block_results?height=4555980 can be found here.

Steps to reproduce
Use the above structure for validator_updates in the RPC block_results test fixture. The output is:

called `Result::unwrap()` on an `Err` value: serde parse error

Caused by:
    missing field `type` at line 103 column 9

What's the definition of "done" for this issue?
Be able to decode the block_results response with non-nil validator updates.

[thanethomson]

I find this part of the JSON to be rather odd:

          "Sum": {
            "type": "tendermint.crypto.PublicKey_Ed25519",
            "value": {
              "ed25519": "VqJCr3vjQdffcLIG6RMBl2MgXDFYNY6b3Joaa43gV3o="
            }
          }

That doesn't look right.

From a cursory look at our deserializer, it appears as though we expect something of the form:

            "type": "tendermint.crypto.PublicKey_Ed25519",
            "value": {
              "ed25519": "VqJCr3vjQdffcLIG6RMBl2MgXDFYNY6b3Joaa43gV3o="
            }

without the Sum wrapper.

Is this perhaps a bug in Tendermint? I can modify the RPC on our side - maybe there's a way I can accommodate both formats.

[ancazamfir]

Where did we get that structure from in our serializer? I saw an example in the test fixture but not clear where those are coming from. Maybe this was different a while ago, I looked back a few releases and nothing seemed to have changed there.

The .go generated file from the .proto of PublicKey is:

// PublicKey defines the keys available for use with Tendermint Validators
type PublicKey struct {
	// Types that are valid to be assigned to Sum:
	//	*PublicKey_Ed25519
	//	*PublicKey_Secp256K1
	Sum isPublicKey_Sum `protobuf_oneof:"sum"`
}

[thanethomson]

I think when our PublicKey struct was written (2-3 years ago, from the looks of our commit history) it was assumed that Tendermint would treat such structs as sum types (like Rust enum types). In the Tendermint crypto package it even shows examples of what a PublicKey struct should serialize to: https://github.com/tendermint/tendermint/tree/master/crypto#json-encoding

From https://github.com/tendermint/tendermint/blob/master/crypto/encoding/codec.go#L16 it looks like the tendermint.crypto.PublicKey_Ed25519 type (the raw Protobuf struct) is being returned directly in the JSON, instead of Tendermint's "domain type" (whose serialization is registered here: https://github.com/tendermint/tendermint/blob/c5cc3c8d3fbb4636922864105e98162012ae0162/crypto/ed25519/ed25519.go#L59).

It looks like the raw ValidatorUpdate Protobuf struct is being returned in the RPC block_results request as opposed to a "domain type": https://github.com/tendermint/tendermint/blob/master/rpc/coretypes/responses.go#L67

Technically, the serialized PublicKey is actually supposed to look like:

"type": "tendermint/PubKeyEd25519",
"value": {
  "ed25519": "VqJCr3vjQdffcLIG6RMBl2MgXDFYNY6b3Joaa43gV3o="
}

(note the difference in the type field, without the Sum wrapper)

It's serialized that way in many other RPC responses.

[ancazamfir]

I see. So it looks like this is a bug in Tendermint Go. Should we open there an issue for this? And can we have a temporary fix in tendermint-rs until the fix gets on the IBC chains? This would help Hermes.

[thanethomson]

I'd say this is done now. I've logged #1091 as a reminder to remove this workaround in future once Tendermint fixes the block_results endpoint response.

Please reopen if this issue comes up again.