Tendermint version (use tendermint version or git rev-parse --verify HEAD if installed from source):

v0.34.10

ABCI app (name for built-in, URL for self-written if it's publicly available):

Cosmos SDK v0.42.4 (and master)

Environment:

• OS (e.g. from /etc/os-release):
• Install tools:
• Others:

What happened:

A custom IBC app dared to actually return binary data in the acknowledgement, which was converted to a string string(rawBytes) for the cosmos sdk event, then back to []byte for abci, and then into the tendermint rpc event system. Somewhere along the way it got mangled and a few bytes changed - which caused the protobuf data to not decode.

Relevant issue and fix (hex-encode binary before passing to event system) here: cosmos/ibc-go#144

What you expected to happen:

If the abci.Event takes key and value as []byte it should support them, no?

Have you tried the latest version: yes

How to reproduce it (as minimally and precisely as possible):

See cosmos/ibc-go#144

Suggestion

Just make the KVPair used for returning Events over abci take key string, value string. There is no real reason for this and lots of code unsafely converts between strings and bytes. (I doubt any indexer handles raw binary well).

Once it used to be a nicely typed string -> string | int mapping

message KVPair {
  string key = 1;
  enum Type {
    STRING = 0;
    INT = 1;
  }
  Type value_type = 2;
  string value_string = 3;
  int64 value_int = 4;
}

But it was later converted to

message KVPair {
  bytes key = 1;
  bytes value = 2;
}

in tendermint/abci#167. Although the arguments don't really convince me (as no processing code knows what to do with []byte besides try to cast it):

-> Protobuf3 strings are supposed to be only utf8 or 7-bit ansii but for something called "KVPair", we don't want to be restricted to unicode or ansii characters.
-> Perhaps using []byte makes us more vigilant in validating the input before accepting it, than be lulled into complacency w/ a string.

This actually broke all searches in the event system, as it didn't know how to handle anything anymore. You can see a desire to revert this change here: #1369 (particularly comment #1369 (comment)) and it seemed to have been decided to use strings again: #1369 (comment)

Although the only fix was that the event system expects all tags to be strings #1498. It uses map[string]string to store the data that is abci-encoded as []byte. WHY?

Please update the abci interface to use strings. This inspired refactoring is causing issues more than 3 years later.