Use peer's relay hints

[piegamesde]

If peer sends a relay server in their transit hints, that hint should be followed. Otherwise, clients with different configured relay servers cannot find each other over a relay connection. Conversely, the own configured relay server should be sent over as hint (not sure if this is done currently).

[psanford]

Aren't we already doing that here? https://github.com/psanford/wormhole-william/blob/master/wormhole/file_transport.go#L187

[piegamesde]

Hm, something definitely is fishy in there. Especially since I can reproduce it only while receiving, and not while sending.

Another thing that I noticed by looking at the code (not sure if related, but maybe): Why is the code checking on the type field of the inner hints and especially for a direct-tcp-v1 value? As far as I know, this is not part of the (original) protocol.

[psanford]

The python code sets the inner message type to direct-tcp-v1 here: https://github.com/magic-wormhole/magic-wormhole/blob/master/src/wormhole/_hints.py#L130-L136

wormhole-william checks this on the assumption that the inner message could support alternative types in the future.

I can't reproduce this and don't see anything incorrect in the code. Can you provide steps to reproduce the issue you are seeing?

[piegamesde]

The python code sets the inner message type to direct-tcp-v1 here

Oh, [expletive deleted].

The thing is, I didn't know that and it wasn't in the specification. In magic-wormhole/magic-wormhole-protocols@af046df (#16), I therefore added a type but with a default value of tcp instead of direct-tcp-v1 (because, frankly, "direct"-something-something makes no sense for a relay hint). Also, I incremented the version number to make sure because of backwards compatibility (which, turns out, may not have been necessary if all clients already check type.).

Can you provide steps to reproduce the issue you are seeing?

I think I can now. As before, use two clients that have different relay servers configured, but also the sender side must not provide a type field in its inner relay hints.

---

So probably the behavior of the code is mostly "correct" and it is the specification that requires fixing.

[piegamesde]

Aren't we already doing that here?

Apparently not, or at least not when receiving. On the sender side, I get the following JSON: {"transit":{"abilities-v1":[{"type":"direct-tcp-v1"},{"type":"relay-v1"}],"hints-v1":[]}}

[psanford]

Please provide the steps to reproduce the issue.

[piegamesde]

In one terminal:

git clone https://github.com/magic-wormhole/magic-wormhole.rs
cd magic-wormhole.rs
cargo run -- send -v --force-relay --relay-server="tcp://lodestone.piegames.de:4001" <somefile>

And in the other:

git clone https://github.com/psanford/wormhole-william
cd wormhole-william
go build
./wormhole-william receive <code>

In the second terminal, it will fail with "Receive file error: failed to establish connection". In the first terminal, you will eventually see the following log message, indicating that the go implementation did not send any hints:

[2022-03-21T11:00:38Z DEBUG magic_wormhole::transfer::v1] Received transit message: TransitV1 { abilities_v1: Abilities { direct_tcp_v1: true, relay_v1: true, relay_v2: false }, hints_v1: Hints { direct_tcp: {}, relay: [] } }

If you want to see the raw JSON messages passing around to make sure that this is not a deserialization error, apply the following patch to the Rust implementation:

diff --git a/src/core.rs b/src/core.rs
index 8a2c695..da8c4d1 100644
--- a/src/core.rs
+++ b/src/core.rs
@@ -316,6 +316,7 @@ impl Wormhole {
         T: for<'a> serde::Deserialize<'a>,
     {
         self.receive().await.map(|data: Vec<u8>| {
+            dbg!(std::str::from_utf8(&data));
             serde_json::from_slice(&data).map_err(|e| {
                 log::error!(
                     "Received invalid data from peer: '{}'",

[psanford]

Does the issue occur for you if you use the python implementation instead of the rust implementation? I tested with the python implementation yesterday and didn't have any issues with using an alternative tcp relay.

[piegamesde]

The issue does not occur when testing against the Python implementation because as long as at least one one side sends the hints correctly, they are able to find each other. Things only break down because the Rust implementation has a bug too (it doesn't add the type field, so the Go implementation ignores the hints).

[psanford]

The receive behavior sounds like its another instance of #57, that wormhole-william doesn't listen on the receiver side for incoming connections today, including via a transit relay. Thus it doesn't publish a transit relay hint since it isn't listening for incoming connections via a relay.

[piegamesde]

Tentatively closing as #57. I still think that "not sending the hints" is an issue distinct to "not following the hints", but they may have a common root cause.

[psanford]

I still think that "not sending the hints" is an issue distinct to "not following the hints"

In the current implementation of wormhole-william, the receiver chooses not to connect to any tcp relay except for the one sent in the senders hints. Thus is does not send any tcp relay hint of its own. If you think this is an issue, its not that "the reciever isn't sending the tcp relay host in the hint", its that the receiver has no opinion of what tcp relay to use other than what the sender says in its hints.

Its really not clear to me why having both the sender and receiver choose tcp relays and then doing a cross product of connections to all those relays is necessary or useful. It seems that it only would have useful to you to mask a bug in the rust implementation.