Skip to content

Conversation

ntninja
Copy link
Contributor

@ntninja ntninja commented Sep 1, 2019

Inspired by the Python code I wrote with lots of references to relevant specs.

@ntninja
Copy link
Contributor Author

ntninja commented Oct 3, 2019

@Stebalien @lgierth: Bump. What trick would I have to use to realistically get some discussion/feedback on this with everybody being so busy? 🙃

@ShadowJonathan
Copy link

Bump, I believe this is a useful addition to the repo.


Encodes a libp2p node address.

TBD: Is this really always a base58btc encoded string of at least 5 characters in length!?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's anything that parses as a peer ID (the canonical encoding is now actually a libp2p key CID): https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least the go implementation does not follow that spec at all but just expects everything to be base58btc: https://github.com/multiformats/go-multiaddr/blob/master/transcoders.go#L293-L315

If your saying that is just a bug to be fixed, I'll be happy to update this text and py-multiaddr accordingly. 🙂

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That spec also fails to mention that some IDs are still encoded with 1 as first character, which is also CIDv0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexander255 correct (well, not a bug so much as something we need to implement).

@ShadowJonathan also correct. I'll take a stab at fixing the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stebalien: I just tried to implement that spec in Python and there is no guidance in there what to do with binary CIDv0 IDs. I assume they are detected (similar to their string format) using len(buf) == 34 and buf.startswith(b"\x12\x20")?

@ntninja ntninja mentioned this pull request Jan 12, 2020
…ecified and add wording about path delimiter conversion for Windows
@ntninja
Copy link
Contributor Author

ntninja commented Aug 14, 2020

@Stebalien: I've updated the description of fspath to leave the binary encoding completely unspecified. Can we merge this now?

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.

3 participants