Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for webrtc and certhash (#261) #262

Merged
merged 7 commits into from
Aug 30, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -167,6 +167,7 @@
"dns-over-http-resolver": "^2.1.0",
"err-code": "^3.0.1",
"is-ip": "^4.0.0",
"multibase": "^4.0.6",
"multiformats": "^9.4.5",
"uint8arrays": "^3.0.0",
"varint": "^6.0.0"
11 changes: 11 additions & 0 deletions src/convert.ts
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ import { getProtocol } from './protocols-table.js'
import { CID } from 'multiformats/cid'
import { base32 } from 'multiformats/bases/base32'
import { base58btc } from 'multiformats/bases/base58'
import * as MB from 'multibase'
import * as Digest from 'multiformats/hashes/digest'
import varint from 'varint'
import { toString as uint8ArrayToString } from 'uint8arrays/to-string'
@@ -51,6 +52,8 @@ export function convertToString (proto: number | string, buf: Uint8Array) {
return bytes2onion(buf)
case 445: // onion3
return bytes2onion(buf)
case 466: //certhash
return bytes2mh(buf)
default:
return uint8ArrayToString(buf, 'base16') // no clue. convert to hex
}
@@ -84,6 +87,8 @@ export function convertToBytes (proto: string | number, str: string) {
return onion2bytes(str)
case 445: // onion3
return onion32bytes(str)
case 466: //certhash
return mb2bytes(str)
default:
return uint8ArrayFromString(str, 'base16') // no clue. convert from hex
}
@@ -148,6 +153,12 @@ function mh2bytes (hash: string) {
return uint8ArrayConcat([size, mh], size.length + mh.length)
}

function mb2bytes(mbstr: string) {
let mb = MB.decode(mbstr)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to handle the case where decoding the multibase fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

decode would throw in some failure cases

https://multiformats.github.io/js-multibase/#decode
https://github.com/multiformats/js-multibase/blob/dde00682ed1a2539677c1e2b17dc6a3c112c8e3a/src/base.js#L40-L47

That said I don't think multibase is aware of what kind of data it's decoding, so it should not be able to detect things like the field being truncated. I don't think the existing analogous multihash functions do validation of this kind in all cases either, though.

https://github.com/multiformats/js-multiaddr/blob/master/src/convert.ts#L140-L142

const size = Uint8Array.from(varint.encode(mb.length))
return uint8ArrayConcat([size, mb], size.length + mb.length)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any verification required that the decoded multibase contains a valid multihash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure there's an easy way to verify that. The existing multihash function does one validation I did not replicate here,

https://github.com/multiformats/js-multiaddr/blob/master/src/convert.ts#L158-L160

That it reports a size that is correct. I could (maybe should) do that. But to go beyond that I think we'd have to be aware of all the supported encryption schemes, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@achingbrain - thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, otherwise as you say it would have to know about all the available hash algorithms.

}

/**
* Converts bytes to bas58btc string
*/
2 changes: 2 additions & 0 deletions src/protocols-table.ts
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@ export const table: Array<[number, number, string, boolean?, boolean?]> = [
[275, 0, 'p2p-webrtc-star'],
[276, 0, 'p2p-webrtc-direct'],
[277, 0, 'p2p-stardust'],
[280, 0, 'webrtc'],
[290, 0, 'p2p-circuit'],
[301, 0, 'udt'],
[302, 0, 'utp'],
@@ -40,6 +41,7 @@ export const table: Array<[number, number, string, boolean?, boolean?]> = [
[445, 296, 'onion3'],
[446, V, 'garlic64'],
[460, 0, 'quic'],
[466, V, 'certhash'],
[477, 0, 'ws'],
[478, 0, 'wss'],
[479, 0, 'p2p-websocket-star'],