add WebTransport multiaddr components#176
Conversation
a07348a to
2903efa
Compare
transcoders.go
Outdated
| } | ||
|
|
||
| func certHashBtS(b []byte) (string, error) { | ||
| return multibase.Encode(multibase.Base58BTC, b) |
There was a problem hiding this comment.
This choice is totally arbitrary. We could use any other multibase that's widely supported.
Is there any best practice for choosing one over the other?
There was a problem hiding this comment.
cc @lidel, but the general guidelines I know of are:
- Do you want compact -> choose base64URL (
u), although base58btc is probably fine too - Do you want lowercase -> choose base32 (
b)
There was a problem hiding this comment.
Switched to Base64url.
| var TranscoderCertHash = NewTranscoderFromFunctions(certHashStB, certHashBtS, nil) | ||
|
|
||
| func certHashStB(s string) ([]byte, error) { | ||
| _, data, err := multibase.Decode(s) |
There was a problem hiding this comment.
Are there any other requirements here? e.g. is it a multihash, is it just 32 bytes, etc.
There was a problem hiding this comment.
There are. First of all, it needs to be a multihash. Do you want to check that here, just to make sure we're not encoding garbage?
Currently browsers only support SHA256, but I'd say that the WebTransport implementation would be the better place to handle this.
There was a problem hiding this comment.
I don't live in this repo much, I was mostly copying from what we do with the p2p multiaddr
Line 314 in f5adc3b
This seems to indicate that if adding some basic checking is relatively cheap then we might as well put it here. If web transport isn't specifically 32 bytes (plus the extra couple multihash bytes) then leaving that part defined in the transport seems reasonable.
There was a problem hiding this comment.
I added the validation on encoding.
2903efa to
db19e64
Compare
| P_IPCIDR = 0x002B | ||
| P_QUIC = 0x01CC | ||
| P_WEBTRANSPORT = 0x01D1 | ||
| P_CERTHASH = 0x01D2 |
There was a problem hiding this comment.
Can you claim the code on https://github.com/multiformats/multiaddr/blob/master/protocols.csv?
There was a problem hiding this comment.
Based on libp2p/specs#404.