Skip to content

Fix: Change default encoding of CIDv1 to base32#77

Open
sumanjeet0012 wants to merge 3 commits into
ipld:masterfrom
sumanjeet0012:fix-66-default-base32
Open

Fix: Change default encoding of CIDv1 to base32#77
sumanjeet0012 wants to merge 3 commits into
ipld:masterfrom
sumanjeet0012:fix-66-default-base32

Conversation

@sumanjeet0012

@sumanjeet0012 sumanjeet0012 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Closes #66

Description

This PR addresses the issue where CIDv1.encode() incorrectly defaulted to base58btc. The CID specification officially recommends base32 as the default encoding for CIDv1 strings. This aligns py-cid with the spec and with other implementations such as Go's Cid.String().

Changes

  • Updated CIDv1.encode to use "base32" as the default encoding parameter.
  • Updated test_encode_default in tests/test_cid.py to assert against the base32 encoded string rather than base58btc.

Warning

Breaking Change: The default output of str(cid) and cid.encode() for all CIDv1 objects will now be base32 strings (which begin with b) instead of base58btc strings (which begin with z). Ensure you document this in the release notes so downstreams relying on default base58btc serialization can update their code to explicitly pass cid.encode("base58btc") if they rely on the older behavior.

@sumanjeet0012 sumanjeet0012 force-pushed the fix-66-default-base32 branch from 005aabd to e7bbd13 Compare July 12, 2026 18:52
@seetadev

Copy link
Copy Markdown
Contributor

@sumanjeet0012 : Great work on this PR! Thank you for identifying this specification mismatch and aligning the default CIDv1.encode() behavior with the CID specification, which recommends base32 as the canonical default encoding. The implementation is clean and focused, and I appreciate that you updated the associated tests and included a news fragment to document the change.

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 py-cid with other implementations, it is also a breaking API change because it alters the output of both str(cid) and cid.encode() for every existing CIDv1 object.

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.

@acul71

acul71 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Thanks @sumanjeet0012 — nice catch and clean fix. Aligning CIDv1.encode() with base32 matches the CID ecosystem (e.g. go-cid) and is the right call.

Two requests before we merge:

  1. Update the CIDv1.encode docstring so it explicitly documents that the default encoding is now "base32".
  2. Ship this as a breaking change in the next release (clear changelog / release notes, and bump accordingly). No deprecation window — callers that need the old behavior can pass encode("base58btc") explicitly. Please expand the newsfragment a bit to note that this also affects str(cid) and IPLD JSON serialization via to_json_dict / CIDJSONEncoder.

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CIDv1 default encoding is base58btc instead of base32

3 participants