Packet data unmarshaling doesn't error when there is an unknown field

[srdtrk]

Summary of Bug

Whether or not this is a bug needs discussion. A recently made commit (#5778) has the unintended consequence of allowing unknown fields in transfer packet data.

Description

In #5778, we have removed ModuleCdc in favor of encoding/json. (This PR hasn't made its way to any releases yet). The module codec used protojson to decode the packet data which has the default behavior of returning an error if it sees an unknown field. However, encoding/json ignores the unknown fields and proceeds with deserialization.

Expected Behavior

Consider the following transfer packet data:

{
  "denom": "denom",
  "amount": "100",
  "sender": "%s",
  "receiver": "%s",
  "memo": "memo",
  "new_field": "value"
}

This cannot be deserialized in release versions of ibc-go. But can be deserialized in main.

Version

Only main. This hasn't made its way back to any releases yet.

Steps to Reproduce

See the tests I wrote here

---

For Admin Use

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

[colin-axner]

Oh my, nice find! Tagging bug, as allowing unmarshaling of unknown fields sounds problematic. We often make use of new fields to introduce new features. If I send a packet to chain X using a new field it doesn't understand (memo for example), I'd expect it to fail, not succeed ignoring my addition input

I suppose there's no easy way to error on unknown fields with std json lib?

[srdtrk]

We can override the unmarshaling behaviour of encoding/json for FungibleTokenPacketData by implementing json.Unmarshaler on it. We would write our custom UnmarshalJSON function. Wouldn't be too difficult

[colin-axner]

We have decided to implement the json.Unmarshaler with our custom function. If anything goes awry we can fallback to protojson for the time being (so as not to block v9 release)