Remove deprecated packet_data and packet_ack attributes

[romac]

Summary

Remove the packet_data and packet_ack attributes which were deprecated in ibc-go v1 because they are incompatible with the requirements introduced in CometBFT v0.37.

Problem Definition

The packet_data and packet_ack attributes have been deprecated in ibc-go v1 in favor of packet_data_hex and packet_ack_hex but never removed.

The difference between these two sets of attributes is that the former two hold raw binary data (ie. they are byte arrays holding potentially non-UTF-8 data) whereas the latter two hold hex-encoded data (ie. they are valid UTF-8 strings).

This was all fine until CometBFT v0.37, as previously these event attributes were defined as Protobuf bytes and could therefore hold arbitrary data.

However, since CometBFT v0.37, event attributes are defined as Protobuf string:

message EventAttribute {
  string key   = 1;
  string value = 2;
  bool   index = 3;  // nondeterministic
}

As per the Protobuf specification:

A string must always contain UTF-8 encoded or 7-bit ASCII text

The fact that these attributes which may hold arbitrary data are encoded to Protobuf strings may not cause any problem when encoded from Go because Go strings, for better or worse, are allowed to hold arbitrary data:

It’s important to state right up front that a string holds arbitrary bytes. It is not required to hold Unicode text, UTF-8 text, or any other predefined format. As far as the content of a string is concerned, it is exactly equivalent to a slice of bytes.

However, this will fail at decoding time because proto.Unmarshal validates that string fields hold valid UTF-8 data:

failed to decode Protobuf message: EventAttribute.value: Event.attributes: Result.events: SimulateResponse.result: invalid string value: data is not UTF-8 encoded

Moreover, in Rust:

• A value of the String type is required to hold valid UTF-8 data
• Protobuf code generators will generate Rust fields of type String for Protobuf fields of type string
• Therefore potentially causing an error at both encoding and decoding time

This has seldom caused issue in practice in Hermes, because most apps use JSON-encoded data (ie. valid UTF-8 data) in ICS-04 packet payloads.

Nevertheless, we have seen one instance of this problem, both on-chain and in Hermes, where an app was using Protobuf-encoded data for ICS-04 packet payloads, which is not valid UTF-8 data.

Proposal

Remove the packet_data and packet_ack attributes in ibc-go v9.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[DimitrisJim]

Ref #3897 and #3896. Thanks for opening! Agreed they should be removed.

[romac]

Ah sorry about that! I did a search prior to opening this but didn't find these two issues because I was searching for packet_data/packet_ack instead of AttributeKeyData/AttributeKeyAck.

[DimitrisJim]

All good. The clear justification very easily makes up for it!

[ValarDragon]

This is of notable performance benefit as well, so really happy this is getting in!