Skip to content

Conversation

@achingbrain
Copy link
Member

@achingbrain achingbrain commented Jan 8, 2025

To allow use cases like fetching IPNS records in a way compatible with go-libp2p we need to send binary as fetch identifiers.

JavaScript strings are UTF-16 so we can't round-trip binary reliably since some byte sequences are interpreted as multi-byte or otherwise non-printable characters.

Instead we need to accept Uint8Arrays and send them over the wire as-is.

This is a backwards compatible change as far as interop goes since protobuf bytes and string types are identical on the wire, but it's breaking for API consumers in that the lookup function now needs to accept a Uint8Array identifier instead of a string.

Refs: libp2p/specs#656

BREAKING CHANGE: registered lookup functions now receive a Uint8Array identifier instead of a string

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

To allow use cases like fetching IPNS records in a way compatible
with go-libp2p we need to send binary as fetch identifiers.

JavaScript strings are UTF-16 so we can't round-trip binary
reliably since some byte sequences are interpreted as multi-byte
characters.

Instead we need to accept Uint8Arrays and send them over the wire
as-is.

Refs: libp2p/specs#656

BREAKING CHANGE: registered lookup functions now receive a Uint8Array identifier instead of a string
@achingbrain achingbrain requested a review from a team as a code owner January 8, 2025 10:37
@achingbrain achingbrain merged commit b56d918 into main Jan 9, 2025
36 checks passed
@achingbrain achingbrain deleted the fix/accept-uint8arrays-as-fetch-keys branch January 9, 2025 10:18
@achingbrain achingbrain mentioned this pull request Jan 9, 2025
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.

2 participants