peerIdFromString doesn't accept CID encoded PeerIDs

[2color]

Description

Since #2660, an undocumented breaking change was introduced, whereby the peerIdFromString function in @libp2p/peer-id throws an error when a base36 encoded PeerID CID is passed.

This can be attributed specifically to https://github.com/libp2p/js-libp2p/pull/2660/files#diff-03404c4c2b9d68fdd3cc5903cbdc0958a10b3a89c8070f1b498e60492fe6a20dL264.

There are to parts to this problem:

1. The default composed decoders for from multiformats have been removed.
2. Even when a base36 decoder is passed in, the function throws for base36 encoded PeerID CIDs, since it attempts to parse it as a multihash (rather than a CID)

The older version would first try to decode the binary digest as a multihash and if that fails, attempt to decode it as a CID:

js-libp2p/packages/peer-id/src/index.ts

Lines 264 to 303 in e42da78

	export function peerIdFromString (str: string, decoder?: MultibaseDecoder<any>): Ed25519PeerId | Secp256k1PeerId | RSAPeerId | URLPeerId {
	decoder = decoder ?? baseDecoder
	if (str.charAt(0) === '1' || str.charAt(0) === 'Q') {
	// identity hash ed25519/secp256k1 key or sha2-256 hash of
	// rsa public key - base58btc encoded either way
	const multihash = Digest.decode(base58btc.decode(`z${str}`))
	if (str.startsWith('12D')) {
	return new Ed25519PeerIdImpl({ multihash })
	} else if (str.startsWith('16U')) {
	return new Secp256k1PeerIdImpl({ multihash })
	} else {
	return new RSAPeerIdImpl({ multihash })
	}
	}
	return peerIdFromBytes(baseDecoder.decode(str))
	}
	export function peerIdFromBytes (buf: Uint8Array): Ed25519PeerId | Secp256k1PeerId | RSAPeerId | URLPeerId {
	try {
	const multihash = Digest.decode(buf)
	if (multihash.code === identity.code) {
	if (multihash.digest.length === MARSHALLED_ED225519_PUBLIC_KEY_LENGTH) {
	return new Ed25519PeerIdImpl({ multihash })
	} else if (multihash.digest.length === MARSHALLED_SECP256K1_PUBLIC_KEY_LENGTH) {
	return new Secp256k1PeerIdImpl({ multihash })
	}
	}
	if (multihash.code === sha256.code) {
	return new RSAPeerIdImpl({ multihash })
	}
	} catch {
	return peerIdFromCID(CID.decode(buf))
	}
	throw new Error('Supplied PeerID CID is invalid')

Proposed solutions & suggestions

1. Update the example to be working
2. Update the function to handle CIDs

[achingbrain]

Ah, that's interesting. Where this function is used in the libp2p codebase, the string that's being decoded is a always a multibase encoded multihash, not a CID.

Do you have a situation where the strings you have might be CIDs or might be multibase encoded multihashes?

[2color]

Do you have a situation where the strings you have might be CIDs or might be multibase encoded multihashes?

This is where this popped up:
https://github.com/ipfs/helia-verified-fetch/blob/aede17f0a0941722f6b1d2bc3582551592d33f40/packages/verified-fetch/src/utils/parse-url-string.ts#L180

But it's also in the example:

js-libp2p/packages/peer-id/src/index.ts

Line 10 in c5bbb25

* const peer = peerIdFromString('k51qzi5uqu5dkwkqm42v9j9kqcam2jiuvloi16g72i4i4amoo2m8u3ol3mqu6s')

[achingbrain]

IPNS is a bit wonky like that, the spec for the value field is very loose.

The question here is, do we want to push that complexity down to the libp2p level or can we contain it somewhere IPNS-related?

One of the goals of #2660 was to reduce the complexity and guesswork in the peerIdFromXXX methods - trying to be explicit in what XXX is (for example, replacing ...FromBytes with ...FromMultihash, ...FromPublicKey, ...FromCID, etc) - perhaps it didn't go far enough with ...FromString.

it's also in the example

Yes, I think this is a symptom of the problem - when you're dealing with opaque strings, the compiler cannot help you.

[2color]

The question here is, do we want to push that complexity down to the libp2p level

Probably not. But we should update the upgrade guide to account for this breaking change.