Updating production Least Authority address #172
Conversation
| @@ -207,15 +207,15 @@ const String ERR_INTERRUPTION_CANCELLATION_SENDER = | |||
|
|
|||
| final Config magicWormholeIO = Config( | |||
| rendezvousUrl: "ws://relay.magic-wormhole.io:4000/v1", | |||
| transitRelayUrl: "tcp:transit.magic-wormhole.io:4001", | |||
| transitRelayUrl: "tcp://transit.magic-wormhole.io:4001", | |||
There was a problem hiding this comment.
Is "tcp" a valid URI scheme?
Usually the scheme specifies the protocol...
(The other way of spelling it without the // looks like a Twisted "endpoint string" .. but this is dart, so who knows if that was just copy-pasted from Python code, or what).
There was a problem hiding this comment.
This was updated based on this PR from w-w: LeastAuthority/wormhole-william#72
There was a problem hiding this comment.
That's a pretty weird, arbitrary change. tcp:host is just as valid a URL as tcp://host (and AFAIK "tcp" isn't a scheme that's standard anyway). Usually the scheme is the protocol, not the transport. (e.g. transit:host:post would make way more sense).
So now we've made a backwards-incompatible change that doesn't really change anything... :/
There was a problem hiding this comment.
(Is there a ticket describing the "why"?)
There was a problem hiding this comment.
For destiny the why is that wormhole-william requires this syntax now.
From what I gather about the wormhole-william PR it seems to try to accept everything as a valid URI, which would not allow tcp:host:port. Given that indeed tcp is not a valid URI scheme, maybe we could have had it as transit://host:port but still we'd need to add to the URI scheme something to say if it's over WS or raw TCP. So I'm not sure what's the best way here that doesn't deviate from what a URL is.
There was a problem hiding this comment.
At the risk of further deviation, maybe we can indicate the transport as a query param and specify the transit URI scheme in detail in the specs. Although I never saw such usage of a URL before.
There was a problem hiding this comment.
Making things more ambiguous, WebSocket has a "subprtocol" feature -- which might indicate that we should be a "transit" sub-protocol of a WebSocket connection. That would make the URL still ws://.../v1/.. but headers during protocol-switching indicate the subprotocol... :/ ...so in "Web" speak, you connect to a particular endpoint (including the path, query-parts, fragment etc) via HTTP, then upgrade to speak WebSocket plus a "transit" subprotocol.
So, that might make it fine to think of a "WebSocket URI" to a "transit subprotocol speaking endpoint" in that case, versus a "transit URI" that is the "transit" scheme spoken directly to a TCP endpoint.
(The "switching" feature of WebSocket means it isn't really like other TCP streaming protocols -- you have the whole Web stack and HTTP standards involved too .. so you speak HTTP briefly, and then switch to "websocket + subprotocol" if you both agree).
There was a problem hiding this comment.
I agree tcp:host:port isn't a valid URI, but neither is tcp://host:port (for exactly the same reason, "tcp" isn't a registered scheme) .. so the "why" I was asking about was for the WW fork's PR.
There was a problem hiding this comment.
For me, as newcomer User for Magic-Wormhole and Wormhole-William was unclear and unintuitive to use in some case ws://host:port in some case tcp:host:port. From user perspective seem more reasonable to have common pattern (IMO). However, after more detail explanation here, I don't see problem to use also tcp:host:port to.
As for PR, it ensures backward compatibility: LeastAuthority/wormhole-william#72
There was a problem hiding this comment.
As I said elsewhere, the real solution is to specify something like transit: since tcp is just the transport, not the protocol.
Updating production Least Authority addresses
p.s. Readme will be adjusted with another PR
Code Review Checklist