Closed thomaseizinger closed 11 months ago
Now that multihash
is basically sorted, here is a list of things we should consider for this crate:
unsigned_varint
from the public API (it is currently exposed through the Error::InvalidUvar
variantError
completely opaque like we did for multihash::Error
?from_url
module: Whilst useful, it can easily live in a separate crateRemove
unsigned_varint
from the public API (it is currently exposed through theError::InvalidUvar
variant
I vaguely remember an in-person discussion about this. Given that unsigned_varint
is unlikely to change in the near future, do you consider this a hard requirement for v0.18
?
Consider making
Error
completely opaque like we did formultihash::Error
?
Error
is already non_exhaustive
. Again, do you consider this a hard requirement for v0.18
?
Remove
from_url
module: Whilst useful, it can easily live in a separate crate
Given that from_url
is behind a feature flag and given that its only dependency (url
) is optional
, I would consider this low priority. Agreed @thomaseizinger?
Remove
unsigned_varint
from the public API (it is currently exposed through theError::InvalidUvar
variantI vaguely remember an in-person discussion about this. Given that
unsigned_varint
is unlikely to change in the near future, do you consider this a hard requirement forv0.18
?Consider making
Error
completely opaque like we did formultihash::Error
?
Error
is alreadynon_exhaustive
. Again, do you consider this a hard requirement forv0.18
?
I'd strongly prefer if we make the Error
type opaque. That will give us much more flexibility in the kind of changes we can make further down the line without breaking users.
It is not a hard requirement for v0.18
.
Remove
from_url
module: Whilst useful, it can easily live in a separate crateGiven that
from_url
is behind a feature flag and given that its only dependency (url
) isoptional
, I would consider this low priority. Agreed @thomaseizinger?
Not a hard requirement for v0.18
either but should be done in the next release then. I opened https://github.com/multiformats/rust-multiaddr/issues/94 and added it to a new v0.19
milestone.
I think we can close this now :)
A
Multiaddr
is a core primitive of libp2p and thus an almost guaranteed dependency of everything libp2p. Every breaking change in this library will trigger a cascade of breaking changes across the ecosystem becauseMultiaddr
constantly appears in public type signatures.Just within our own crates, a breaking change here causes a breaking change in:
libp2p-core
->libp2p-swarm
->libp2p-identify
(for example) ->libp2p
. There are several users of libp2p that build their own libraries on top which continues the chain.We should do everything possible to:
After a quick survey, I see the following problems:
Protocol
is notnon_exhaustive
: There will always be more protocols, we should future proof the API for that.multihash
is re-exported and part of the public API despite being < 1.0: This is a problem. A fundamental crate likemultiaddr
should only have stable dependencies in its API.I am not sure how to deal with
multihash
. It is useful to representCerthash
andP2p
in a type-safe way. One thing we can do is just not update to the latest version as quickly unless we actually need a specific feature.Update: After looking at
multihash
in more detail and opening and issue there, I think the best way forward would be to splitmultihash
into two crates: one for the definition ofmultihash
and one for all the implementations of hash algorithms, the custom derive, etc.