TM34 Events can't be Parsed when the Value is Binary Data

Question

TM34 Events can't be Parsed when the Value is Binary Data

ryanrossiter opened this issue 6 months ago · 8 comments

What went wrong?

When querying for tx/block results from a TM34 chain (i.e. events are base64-encoded), the parsing fails when the event data is binary and not utf8 because the deserialize_to_string conversion uses String.from_utf8 which asserts the data is valid utf8 here.

Steps to reproduce

Parse an event containing binary data formatted as base64, using the TM34 compatability mode. This event from the Canto chain as an example:

{
    "type": "block_bloom",
    "attributes": [
        {
            "key": "Ymxvb20=",
            "value": "ACAAABAAAAgAAAAAAAAAABIAAAIAABAAAAAAIgAAAQAEAAAAAAIAAIBAAAAAACAAAAQAgAAAAIACAAAEASAkiACAAIAAAAAgCAAADAAAACAAAAAAEAIAABQAABAAAACAAAAAAAAEAEAAQEAgABAAAAAAAAEACAAAAAAEEAAAAAAAAEAAAAQAAACAAAAAABAAAQAQAAAAAAAAAABABAAgAAIAAAAAAAAAQAAAAAAACAABBAQAAAAAAAKAAAAAAggIAQAAAgAAAAAAAAAEAAAAAAAAAAAAAAAAAAgAAAAAAAAAEQgAAAAAAAAAQAAAAAAAEAACAgAQAAABAAAAAAQAQA==",
            "index": true
        }
    ]
}

Definition of "done"

Parsing should not fail when the encoded data is binary. Perhaps it should leave any values which aren't utf8 as base64 strings.

Answer 1 · 2024-03-16T21:46:10.000Z

Thanks for bringing this to our attention!

Can you please post a minimal reproduction of the problem for us to take a look at?

Answer 2 · 2024-03-17T16:20:23.000Z

It seems indeed that we are wrongly modeling EventAttribute's key and value as String instead of Vec for Tendermint/CometBFT v0.34, whereas those are only required to be valid UTF-8 encoded strings since Tendermint/CometBFT v0.37.

Answer 3 · 2024-03-28T15:45:18.000Z

Yes indeed, this is also breaking on early osmosis blocks. I can't fetch block 1423377 (v0.34) because some EventAttribute have :

      {
        "type": "distribution",
        "attributes": [
          {
            "key": "Z2F1Z2VfaWQ=",
            "value": "MTQ1Mg==",
            "index": true
          },
          {
            "key": "cmVjZWl2ZXI=",
            "value": "V0RfatpWKg/BJHrErklu9FjLhBM=",
            "index": true
          },
          {
            "key": "YW1vdW50",
            "value": "Mzg2dW9zbW8=",
            "index": true
          }
        ]
      }

and V0RfatpWKg/BJHrErklu9FjLhBM= isn't base64 encoded UTF8.

Answer 4 · 2024-03-28T18:50:27.000Z

We'll fix this in v0.36, would gladly accept a PR if anyone has time to put one together otherwise I'll get to it next week together with the release.

Answer 5 · 2024-03-28T19:51:31.000Z

We'll fix this in v0.36, would gladly accept a PR if anyone has time to put one together otherwise I'll get to it next week together with the release.

How do you expect to fix this, use Vec<u8> only for v0.34 and String for later versions?

Answer 6 · 2024-03-28T20:41:35.000Z

Yes exactly!

Answer 7 · 2024-03-29T17:32:15.000Z

Yes exactly!

I'm not really familiar with tendermint and the code so I looked quickly out of curiosity since I need this to fetch early Osmosis blocks. Not sure how I'd get to do this in a small change.

Answer 8 · 2024-03-29T19:54:18.000Z

We'll fix this in v0.36, would gladly accept a PR if anyone has time to put one together otherwise I'll get to it next week together with the release.

Tried my luck, would love feedback. Never contributed to tendermint so not sure that's the right way to fix it.