magic-wormhole icon indicating copy to clipboard operation
magic-wormhole copied to clipboard

Transit JSON is a bit inconsistent

Open vu3rdd opened this issue 6 years ago • 2 comments

While trying to write a parser and a generator for the transit message, I noticed this message between two clients running on the same machine (formatted with jq for readability):

{
  "transit": {
    "abilities-v1": [
      {
        "type": "direct-tcp-v1"
      },
      {
        "type": "relay-v1"
      }
    ],
    "hints-v1": [
      {
        "priority": 0,
        "hostname": "192.168.1.106",
        "type": "direct-tcp-v1",
        "port": 36199
      },
      {
        "type": "relay-v1",
        "hints": [
          {
            "priority": 0,
            "hostname": "transit.magic-wormhole.io",
            "type": "direct-tcp-v1",
            "port": 4001
          }
        ]
      }
    ]
  }
}

Inside the hints-v1 array, we have a direct hint and a relay hint. The relay hint has a type key and a hints key whereas the direct hint is just an object. Also, the object inside the hints for relay has another type key inside that says direct-tcp-v1. Is that indicative of a direct tcp connection to be made with the transit hostname?

I find the messages a bit confusing. The documentation seem to suggest that the inner type key is redundant? I was expecting something of this sort, as suggested by the documentation. i.e. The client learns the abilities first and for each of those abilities, we have the associated hints.

{
  "transit": {
    "abilities-v1": [
      {
        "type": "direct-tcp-v1"
      },
      {
        "type": "relay-v1"
      }
    ],
    "hints-v1": [
      {
        "type":"direct-tcp-v1",
        "hints": [
          {
            "priority": 0,
            "hostname": "192.168.1.106",
            "port": 36199
          }
        ]
      },
      {
        "type": "relay-v1",
        "hints": [
          {
            "priority": 0,
            "hostname": "transit.magic-wormhole.io",
            "port": 4001
          }
        ]
      }
    ]
  }
}

Or am I missing something?

Thanks

Addendum:

Also why not encode abilities-v1 as:

"abilities-v1": [ "direct-tcp-v1", "relay-v1" ]

I am guessing, it is too late. May be we should call any new design as V2 and also maintain compatibility with V1 (which is more work!)..

vu3rdd avatar Jun 28 '18 06:06 vu3rdd

I think I will also add my point that, this JSON could have been better defined. I struggled a lot in the rust code just to produce this form of JSON. After discussing with @vu3rdd I think both relay type and direct tcp type are having same content except additional type tag in direct-tcp-v1. But if we use type as shown in hints-v1 for relay then this could have been much much simplified.

This probably might not be very difficult to achieve in python but for typed language like rust or haskell it takes a lot of effort to arrive at final JSON of this form.

copyninja avatar Jun 28 '18 07:06 copyninja

Good questions.. let me see if I can remember what I was thinking when I wrote that :).

Each hint (and ability) has an inline type field, which tells you all the other fields to look for. In Rust's serde lingo, this is called Internally tagged.

The abilities-v1 key contains a list of abilities, each of which has a type and then some (possibly-empty) set of properties. None of the current ability types (direct-tcp-v1 and relay-v1) have any properties, but I wanted to leave room to add some in the future.

The hints-v1 key has a list of hints, each of which has a type and some additional set of properties.

When the hint type is relay-v1, one of the properties is hints, which is a list of ways you might connect to the relay. The idea here is that you might be able to get to the relay via multiple pathways or protocols (e.g. both plain TCP and also WebRTC), but they're all connecting to the same thing, so you shouldn't bother connecting via more than one of them. These days, it won't hurt if one side connects to the same relay multiple times (there's a "side" identifier to detect this), but the earlier code would get confused (two connections from the same side would be glued together).

Each member of the relay descriptor's hints field is a fully-fledged hint, so it has the same syntax as the members of hints-v1.

hope that helps!

warner avatar Oct 17 '18 18:10 warner