Introduce capabilites #6482

konradkonrad · 2020-08-25T17:13:19Z

Add protocol capabilities to the transport handshake

sedes of capabilities are in a new module utils/capability.py https://github.com/raiden-network/raiden/pull/6482/files#diff-f02562dc4c1065f371e6994f4ba7c2c2
this add manipulation of the matrix avatar_url with capability strings of the format defined in Introduce protocol handshake #3283 (comment)
the predefined capabilities are in constants.py https://github.com/raiden-network/raiden/pull/6482/files#diff-fc15643f6fbeaa49665008430fe239bbR85-R94
the current client default settings are in settings.py https://github.com/raiden-network/raiden/pull/6482/files#diff-14a4915b775dfbab039e080a28553d78R130-R137
on client startup the default settings are published during login of matrix/utils.py https://github.com/raiden-network/raiden/pull/6482/files#diff-b10b960f92576b8e90998119485c75ebR704
on reachability changes, the PeerCapabilities are updated

This adds configuration, settings and parser for capabilities.

fredo

Over all looks very good to me. I commented some nitpicks/questions.

One topic I'd like to clarify before approve:

I wonder if the capabilities should be mapped to a user_id or an address. I noticed that question because some variables seem a bit confusing to me. While for reachability we usually use the prefix user_ or address_, when it comes to capabilities we have peer_, address_to_ and in the fixtures user_- capabilities.

Is there any rationale behind or rather what do you think to what the capabilites should be mapped to? And if there is a clear answer, a consistent naming would probably avoid confusion in the future.

raiden/network/transport/matrix/utils.py

raiden/utils/typing.py

konradkonrad · 2020-09-01T11:57:58Z

Over all looks very good to me. I commented some nitpicks/questions.

One topic I'd like to clarify before approve:

I wonder if the capabilities should be mapped to a user_id or an address. I noticed that question because some variables seem a bit confusing to me. While for reachability we usually use the prefix user_ or address_, when it comes to capabilities we have peer_, address_to_ and in the fixtures user_- capabilities.

Is there any rationale behind or rather what do you think to what the capabilites should be mapped to? And if there is a clear answer, a consistent naming would probably avoid confusion in the future.

As we discussed out-of-band, we do a mapping of address <-> capabilities now. We re-query the capabilities only on reachability changes, for the "best reachable" user_id, see https://github.com/raiden-network/raiden/pull/6482/files#diff-b10b960f92576b8e90998119485c75ebR366-R406

fredo · 2020-09-01T12:27:25Z

raiden/network/transport/matrix/utils.py

@@ -347,11 +366,12 @@ def get_reachability_from_matrix(self, user_ids: Iterable[str]) -> AddressReacha
    def _maybe_address_reachability_changed(self, address: Address) -> None:
        # A Raiden node may have multiple Matrix users, this happens when
        # Raiden roams from a Matrix server to another. This loop goes over all
-        # these users and uses the "best" presence. IOW, if there is a single
+        # these users and uses the "best" presence. IOW, if there is at least one


I just had a discussion with @ulope about the feasability of allowing multiple users of the same address online. This should actually be forbidden per definition because it would imply multiple usage of the same keystore. A raiden node could prevent starting if a user for the same address is found online.

The comment change was only to reflect the current code.

Then we should ask us if we should change the code. Why should we handle multiple users being online if this should be prevented anyway?
But on the other hand this could also be implemented in a different PR

We should create a follow up issue to discuss this:

for a canonical client, we can definitely try to forbid multiple logins

for the peer handling side (such as here), we have to deal with the possibility of multiple logins

I don't know how roaming user ids behave, when it comes to login/logout. We need to clarify if there are interleaved states, where the old user_id still appears online, while the new user_id already came up.

Good point. A user is set to be offline by the server whenever he did not longpoll sync() for 30s. This definitely leaves the possibility open to appear online with 2 user ids for the same address.

fredo · 2020-09-01T12:45:53Z

raiden/network/transport/matrix/utils.py

        # Matrix user that is reachable, then the Raiden node is considered
        # reachable.
        userids = self._address_to_userids[address].copy()
-        composite_presence = {self._userid_to_presence.get(uid) for uid in userids}
+        presence_to_uid = {self._userid_to_presence.get(uid): uid for uid in userids}
+        composite_presence = set(presence_to_uid.keys())


What is the difference here to the old code?

We preserve the mapping presence <-> user_id in order to pull the avatar_url for a user_id that has the new_presence.

The old code was only concerned with ordering presences.

fredo

thanks for the changes! lgtm

konradkonrad · 2020-09-02T08:43:52Z

@ulope you wanted to discuss the serialization format of the capabilities, no?

This ports protocol handshake (#3283 / #6503) capabilities from #6482 / #6572 back to the release branch.

Changelog for Capabilities

konradkonrad force-pushed the introduce_capabilites branch 2 times, most recently from db70c20 to bae4c2d Compare August 31, 2020 09:10

konradkonrad added 4 commits August 31, 2020 14:00

Introduce capabilities

104b222

This adds configuration, settings and parser for capabilities.

Set our capabilities on matrix login

743e57f

Add minimal integration test for capabilities

c6c52b5

Set capabilities on reachability changes

080a596

konradkonrad force-pushed the introduce_capabilites branch from f38e752 to 080a596 Compare August 31, 2020 12:16

fredo suggested changes Sep 1, 2020

View reviewed changes

raiden/network/transport/matrix/utils.py Outdated Show resolved Hide resolved

raiden/utils/typing.py Outdated Show resolved Hide resolved

konradkonrad added 2 commits September 1, 2020 13:47

Query peer capabilities on reachability changes

b2d0f29

Tighten the PeerCapabilities type

f1850db

konradkonrad marked this pull request as ready for review September 1, 2020 12:44

fredo reviewed Sep 1, 2020

View reviewed changes

fredo approved these changes Sep 1, 2020

View reviewed changes

Prepare more exhaustive capability search

eb29f76

konradkonrad changed the title ~~[WIP] Introduce capabilites~~ Introduce capabilites Sep 2, 2020

konradkonrad mentioned this pull request Sep 2, 2020

Discuss format of capabilities #6503

Closed

konradkonrad merged commit dad9c59 into raiden-network:develop Sep 2, 2020

konradkonrad deleted the introduce_capabilites branch September 2, 2020 12:16

ulope mentioned this pull request Sep 2, 2020

Remove dependency of non-protocol required confirmation messages (Delivered, Processed) #3254

Open

3 tasks

karlb pushed a commit that referenced this pull request Oct 9, 2020

Backport Capabilities for release

8189c11

This ports protocol handshake (#3283 / #6503) capabilities from #6482 / #6572 back to the release branch.

fredo added a commit that referenced this pull request Nov 6, 2020

Update Changelog for #6482

1a20f86

Changelog for Capabilities

karlb pushed a commit that referenced this pull request Nov 6, 2020

Update Changelog for #6482

9fbfc4a

Changelog for Capabilities

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Introduce capabilites #6482

Introduce capabilites #6482

Uh oh!

konradkonrad commented Aug 25, 2020 •

edited

Loading

Uh oh!

fredo left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

konradkonrad commented Sep 1, 2020

Uh oh!

fredo Sep 1, 2020

Uh oh!

konradkonrad Sep 1, 2020

Uh oh!

fredo Sep 1, 2020

Uh oh!

konradkonrad Sep 1, 2020

Uh oh!

fredo Sep 1, 2020

Uh oh!

fredo Sep 1, 2020

Uh oh!

konradkonrad Sep 1, 2020 •

edited

Loading

Uh oh!

konradkonrad Sep 1, 2020

Uh oh!

fredo left a comment

Uh oh!

konradkonrad commented Sep 2, 2020 •

edited

Loading

Uh oh!

Uh oh!

Introduce capabilites #6482

Introduce capabilites #6482

Uh oh!

Conversation

konradkonrad commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add protocol capabilities to the transport handshake

Uh oh!

fredo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

konradkonrad commented Sep 1, 2020

Uh oh!

fredo Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

konradkonrad Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fredo Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

konradkonrad Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fredo Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fredo Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

konradkonrad Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konradkonrad Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

fredo left a comment

Choose a reason for hiding this comment

Uh oh!

konradkonrad commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

konradkonrad commented Aug 25, 2020 •

edited

Loading

fredo left a comment •

edited

Loading

konradkonrad Sep 1, 2020 •

edited

Loading

konradkonrad commented Sep 2, 2020 •

edited

Loading