Discuss format of capabilities

[konradkonrad]

Abstract

Handshake protocol capabilities are introduced with #6482 / #3283

With matrix, the feature (ab)uses the user's avatar_url for broadcasting it. The current serialization format was implemented first by raiden-network/light-client#1210 and informally specified in #3283 (comment)

We should discuss the format, before we release a new client version to the mainnet.

Areas to discuss

• Negating/Opt-out flags
• Hand-rolled (de-)serialization

Negating/Opt-out flags

The current incarnation of capabilities knows three negating flags:

• NoReceive
• NoMediate
• NoDelivery
  and one positive flag:
• WebRTC

The serialization on the other hand defines that the absence of a flag translates to a boolean False. So a capability handshake message for NoDelivery+WebRTC will be serialized as

capabilities_serialized = "NoDelivery,WebRTC"

and de-serialized as

capabilities_deserialized = {"NoDelivery": True, "NoMediate": False, "NoDelivery": False, "WebRTC": True}

We should discuss, whether or not we could swap the defaults around to simplify the semantics.

Hand-rolled (de-)serialization

Capabilities are typed as

Dict[str, Union[str, bool]]

During serialization, the keys are ',' joined for all True boolean values, keys for False boolean values are dropped; all non-boolean values are serialized as key="value" and also ',' joined.

However, the existing proposal suggests a type of Dict[str, Any]. During deserialization that will need casting to expected types.

Judging from history, hand-rolled (de-)serializations are not necessarily a good idea.

We should discuss alternative encoding options, e.g. base64 + yaml.

Ping: @ulope @andrevmatos @karlb

[andrevmatos]

The negative/positive flags are due to how we needed to differentiate from the Python client: the negative flags specify we don't need/do/support things the way of the python client, while the positive flags mean we do support something that you don't. Since these are all per convention, we did it like that, but agree we should discuss on how to improve the semantics of it.

The serialization don't get me very happy too, but avatar_url is limited in size, and we went for one approach which would serialize to the shortest string. yml/json/json5 would all be cluttered with control characters we would not needed, because of our limited usecase. Again, if anyone finds a nice lib/standard which serializes to smaller enough strings, I'm all for ditching the custom parser as well. Some discussions on the topic were made here: raiden-network/light-client#1975

[ulope]

I'd suggest to get rid of the 'no' prefixes and instead define a default set of capabilities for nodes that don't publish any.
These would be:

• WebRTC: False
• Receive: True
• Mediate: True
• Delivery: True

Nodes that know about capabilities would publish them normally.
The current most 'modern' set (for a full featured node) would be:

• WebRTC: True
• Receive: True
• Mediate: True
• Delivery: False

As for the encoding I'm very much against using hand rolled ad-hoc parsers.
Also I don't think we need to be too stingy with the length. The avatar_url field is limited to 1000 chars.

Using base64 encoded bson the above capabilities would need 60 bytes.
That seems small enough to allow for future growth.

Example:

>>> capabilities = {"WebRTC": True, "Receive": True, "Mediate": True, "Delivery": False}
>>> len(base64.urlsafe_b64encode(bson.dumps(capabilities)))
60

[andrevmatos]

This is a very nice proposal, @ulope . The only problem I see with it is that not defaulting to some value (falsy, in the case of no* caps) may give us trouble when new caps are introduced. Your proposal would require us to always set all flags, and relay on being in sync with some 'global shared defaults' for the unset keys/empty caps mapping. My original idea is that any cap missing on our client or on partner client could still fallback to some implicit default which wouldn't need to be shared: all of them would be opt-in, and anything (e.g. newer caps) missing, would make the clients still able to work in a backwards compatible manner.

About the serialization, that definitely would be enough, the only cons I see is it's not as readable (I'm used to inspecting capabilities for nodes on Riot client), but I suppose this isn't a priority compared to ditching hand-rolled serialization logic.

[ulope]

@andrevmatos Not necessarily.
To expand a bit on the logic:

• No capabilites are set (e.g. avatar_url is missing or the empty string):
  Use a hard coded default (as defined above)
• Capabilities are set (e.g. avatar_url is valid base64 encoded bson, possibly an empty mapping):
  Use the given capabilities, any missing keys will default to False.
  This way we could effectively drop the delivery capability except in the defaults for legacy nodes.
  Of course this means most nodes would (currently) have to publish 'WebRTC', 'Receive' and possibly 'Mediate'. But this seems more explicit than to rely on some default rule set (as you pointed out).

As for human readability of the capabilities I'd say that's a very very niche use case that shouldn't drive our decision, esp. since decoding them is essentially a one-liner on the terminal.

[karlb]

In the example given by @ulope, the JSON encoding is not that much larger (assuming we don't need to escape any JSON characters):

>>> capabilities = {"WebRTC": True, "Receive": True, "Mediate": True, "Delivery": False}
>>> len(json.dumps(capabilities))
69

I would not introduce a new encoding for 15% space savings.

Regarding the topic of skipping capabilities that are false: If we do that, all new capabilities will have to default to false. That's not too bad, bad it will lead to negative capabilities like "NoMediate".

[ulope]

@karlb Sine the avatar_url field we're 'abusing' is supposed to contain an URL we would need to check if it's possible to store plain json in there. However, since we need to remain forward compatible with future Synapse upgrades I would simply assume that we have to provide a URL-safe value.

URL-encoding the JSON would result in a bigger size than base64 and is almost as unreadable:

>>> import base64
>>> import json
>>> from urllib.parse import quote_plus

>>> len(quote_plus(json.dumps({'WebRTC': True, 'Receive': True, 'Mediate': True})))
76

>>> len(base64.b64encode(json.dumps({'WebRTC': True, 'Receive': True, 'Mediate': True}).encode()))
68

I would not introduce a new encoding for 15% space savings.

Yeah fair point, an advantage of bson though is that it encodes True/False in a single byte.

Regarding the topic of skipping capabilities that are false: If we do that, all new capabilities will have to default to false. That's not too bad, bad it will lead to negative capabilities like "NoMediate".

Yes true. I'd guess that's mostly a naming issue though.

[konradkonrad]

OUTDATED, please see update below

Here is my proposal for moving forward:

Encoding/Serialization

Capabilities published in the synapse avatar_url are serialized as base64 encoded json. There is no constraint on the order of the json fields.

Negated naming

With introduction of capabilities across the py and ts client, we swap the semantics of the negated values. For the so far known capabilities that means:

• noMediate becomes Mediate (with True meaning transfer mediation is supported)
• noReceive becomes Receive (with True meaning receiving payments is supported)
• noProcessed is dropped (see Remove dependency of non-protocol required confirmation messages (Delivered, Processed) #3254 -- Processed messages are still required in the protocol)
• webRTC stays webRTC (with True meaning webRTC messaging is supported)
• Delivered is introduced (with True meaning the client implementation sends Delivered messages)

Newly introduced capabilities should follow this approach and avoid the double negation encoding for legacy features (i.e. a hypothetical new capability for signalling that refund transfers are ignored should be introduced as "Refund": False).

Implicit values / versioning

Implicit values / omission of False-y values is postponed until there is a need for truncating the capabilities. Instead, in this first version the field "version": "0" is sent.

At a later time it is possible to define common protocol versions that imply a common set of flags/values that can also be omitted. Clients on such a newer protocol version will have to act backwards compatible.

edit after
Client implementations are free to introduce new capabilities. That implies that

• unknown capabilities must not break an existing client implementation
• the absence of a capability needs to be handled in a backwards compatible manner
• client implementations should update their default capabilities to include the previously unknown "default" as soon as possible

/edit

Rationale

Explicit is better than implicit. There is currently no space scarcity on the serialized capabilities string. The explicit definition avoids ambiguities moving forward.

Example

The python client (after #6532) would send the following capabilities:

capabilities_encoded = b'eyJ2ZXJzaW9uIjogIjAiLCAiTWVkaWF0ZSI6IHRydWUsICJSZWNlaXZlIjogdHJ1ZSwgIndlYlJUQyI6IHRydWUsICJEZWxpdmVyZWQiOiBmYWxzZX0='  # len = 116
capabilities_decoded = {
    'Delivered': False,
    'Mediate': True,
    'Receive': True,
    'version': '0',
    'webRTC': True
}

[ulope]

I very much like this!

(I assume the 'Processed': True in the example at the end is a left-over? Since you wrote above that this capability will be dropped.)

[konradkonrad]

I very much like this!

(I assume the 'Processed': True in the example at the end is a left-over? Since you wrote above that this capability will be dropped.)

Oh boy, yes...I'll correct the post above!