Fix: Change default encoding of CIDv1 to base32#77
Conversation
005aabd to
e7bbd13
Compare
|
@sumanjeet0012 : Great work on this PR! Thank you for identifying this specification mismatch and aligning the default It is also encouraging to see that the implementation passes 23 out of 24 CI checks across the supported Python versions and Windows, with the remaining documentation job appearing to be unrelated to the functional changes. One point we should consider before merging is backward compatibility. While changing the default to base32 improves standards compliance and aligns Many downstream applications may implicitly depend on the current base58btc default, and changing that behavior in a patch release could introduce unexpected regressions. It would be worth exploring an approach that preserves backward compatibility, for example, retaining the existing default while encouraging explicit base32 encoding, introducing the new behavior behind a major version bump, or providing a migration path with deprecation notices. Overall, this is an excellent contribution and raises an important standards-compliance issue; with a backward-compatible migration strategy. CCing @acul71. |
|
Thanks @sumanjeet0012 — nice catch and clean fix. Aligning Two requests before we merge:
@seetadev — agreeing with your compatibility concern; we'll treat it as an intentional break rather than a silent behavior change in a patch. Once the docstring and newsfragment are updated, this LGTM from my side. |
Closes #66
Description
This PR addresses the issue where
CIDv1.encode()incorrectly defaulted tobase58btc. The CID specification officially recommendsbase32as the default encoding for CIDv1 strings. This alignspy-cidwith the spec and with other implementations such as Go'sCid.String().Changes
CIDv1.encodeto use"base32"as the default encoding parameter.test_encode_defaultintests/test_cid.pyto assert against thebase32encoded string rather thanbase58btc.Warning
Breaking Change: The default output of
str(cid)andcid.encode()for allCIDv1objects will now bebase32strings (which begin withb) instead ofbase58btcstrings (which begin withz). Ensure you document this in the release notes so downstreams relying on defaultbase58btcserialization can update their code to explicitly passcid.encode("base58btc")if they rely on the older behavior.