CodecToStr and Codecs in v0.3.1 could cause data corruption about go-cid HOT 3 CLOSED

Ah hum ... mb Ill revert, open pr for 0.3.2 and fix Kubo's dep

from go-cid.

Well, as I said in #142, the main break is experienced via CodecToStr, I don't really care that much about Codecs as I haven't seen any difficulty with that. The main source of difficulty is in the way go-libp2p performed some major upgrades at the same time as adapting to this change. So to pull an updated go-cid, a package that's also using go-libp2p has to go through the full upgrade there as well just to get a latest version. Because go-cid is so core to multiformats and ipld, you can't get an updated go-ipld-prime or related packages without going through all of these hoops.

So, in hindsight, I still think we made the wrong call in #137 and should have gone through a longer deprecation cycle, using the deprecation tools afforded by Go—which aren't awesome, but they exist.

More discussion in #142 would have been nice to work through this a bit further.

from go-cid.

Just to be clear (and reiterate #137), both Codecs AND CodecToStr had invalid entries.

• Codec table had invalid mapping cbor → 0x71 and protobuf → 0x70
• CodecToStr table had invalid mapping 0x71 → cbor and 0x70 → protobuf

In my mind, this is not something we can "silently fix" like you did in #142.
Silently changing mappings in #142 introduced silent breaking changes in disguise of "deprecation".

My position is:

• Invalid mapping in go-cid means downstream software could be producing invalid CIDs and DAGs and we would not even know its happening.
• Given we talk about content-addressing software stack, this is not something one deprecates, this is a critical bug that had to be fixed ASAP.
  • Silently fixing and changing mapping is as bad or worse than having invalid one.
    Removal and forcing refactor is the only safe way.

Digression about work in Q1 and potential shortcuts:

I am deeply aware how core go-cid is, and how painful bubbling this up is 😿 Kubo team went over this in IPFS in Q1: had to open multiple surgical PRs to fix this mess in Kubo 0.13.0 and go-libp2p, and change many commands in a way that does not silently break existing users (see summary of BREAKING CHANGES in release notes).

Note the strategy around #137 was to replace use of Codecs and CodecToStr in places that are not impacted by invalid mappings WITHOUT bumping go-cid (e.g., libp2p/go-libp2p-core#240). The actual go.mod bump happened in go-libp2p 0.21, two months later.

Initially, we only bumped go-cid to v0.2.0 in Kubo to force refactor of impacted code paths and redesign of end user commands. Sure, it is a lot of tedious work, but the end result in Kubo 0.13.0 avoided exactly the type of silent data corruption described in my comment at the top (and briefly re-introduced in v0.3.1).

This was done months ago. Since then, unrelated refactors of go-libp2p landed (such as repo consolidation), and now you have even more technical debt to pay off.

You could re-introduce Codecs AND CodecToStr and then manually censor tables and remove entries with invalid mappings. This will ensure go-cid won't produce invalid data, and save you from bubbling up refactors, but will make also make some apps error randomly on cbor/protobuf/dag-pb.

I did not go that route because it felt like dealing with the fallout will be more work than doing things properly and refactoring away from go-cid.

from go-cid.