Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

non-blocking group QR joins #2508

Merged
merged 15 commits into from
Oct 26, 2021
Merged

non-blocking group QR joins #2508

merged 15 commits into from
Oct 26, 2021

Conversation

r10s
Copy link
Contributor

@r10s r10s commented Jun 20, 2021

targets "dc_join_securejoin() should always return immediately" of #2507

  • make background processing basically work
  • make sure, a stale join-process does not block new joins. as a minimum, check that a new handshake really (!) aborts existing ones (as stated in the docs). group-joins would have the same state as setup-contact then, we can improve that by allowing concurrent joins in a later pr.
    EDIT: with the fix of fix "QR process failed" error and add a test #2725, things should be fine
  • add an info-messages after scanning about what is going on and show the other info-messages in the destination chat
  • avoid empty one-to-one-chat being created

this is how a scan looks like, the fist screen is shown instantly, even if both devices are offline:

usually, there are only a few seconds between the screenshots, however, if things are in slow networks, nothing blocks using the app as before. the second message does not appear when the contacts already know their keys (shortcut in https://countermitm.readthedocs.io/en/latest/new.html#setup-contact-protocol)

in practise, for UIs, DC_EVENT_SECUREJOIN_JOINER_PROGRESS is no longer needed with this change, however, for now, still existent.

for review: half of the changes result from streamlining the use of chat_id: to not mix chats for info-messages/handshake-message/group-chats that may be blocked/unblocked several "semi-global" chat_ids are removed and the decision is done deeper in the code, closer to the final purpose. also some refactoring was done on that way, see the corresponding commits for details.
it may make some sense to factor out Alice's part (as already done for Bob), however, that to not make the diff even larger, i avoided that for now; this can be done at some later point, at best without functionality changes then.

@r10s r10s force-pushed the instant-grp-qr-join branch 2 times, most recently from 8ef8090 to 98c6a49 Compare June 26, 2021 14:10
r10s added a commit that referenced this pull request Sep 23, 2021
this is needed for targeting "non-blocking group QR joins"
as create_multiuser_record() would also be needed from other places.

this will make rebasing/rewriting and finally reviewing #2508 easier.
@r10s
Copy link
Contributor Author

r10s commented Sep 23, 2021

this needs a rewrite/rebase, to make things easier focusable, i factored out some easy things to #2706 that should be merged first. EDIT: done.

r10s added a commit that referenced this pull request Sep 27, 2021
this is needed for targeting "non-blocking group QR joins"
as create_multiuser_record() would also be needed from other places.

this will make rebasing/rewriting and finally reviewing #2508 easier.
@r10s r10s force-pushed the instant-grp-qr-join branch from 98c6a49 to 16804a5 Compare September 27, 2021 21:48
@r10s r10s mentioned this pull request Sep 29, 2021
3 tasks
@r10s r10s force-pushed the instant-grp-qr-join branch 2 times, most recently from e2b2517 to e5cf462 Compare September 29, 2021 19:00
@r10s r10s changed the title [wip] non-blocking group QR joins non-blocking group QR joins Sep 29, 2021
@r10s r10s marked this pull request as ready for review September 29, 2021 21:40
link2xt
link2xt previously approved these changes Oct 3, 2021
@link2xt link2xt dismissed their stale review October 3, 2021 15:13

Misclick

@r10s r10s force-pushed the instant-grp-qr-join branch from dbcd294 to db68ab2 Compare October 3, 2021 20:03
@r10s r10s force-pushed the instant-grp-qr-join branch 7 times, most recently from e1d9d55 to 6fd2a07 Compare October 16, 2021 17:07
r10s added 4 commits October 25, 2021 23:14
- the function takes a contact_id directly now.
  before it consumes the first contact of a one-to-one chat -
  which may be easily confused with the group-chat in creation.
  moreover, the conversion contact_id -> chat_id -> contact_id
  is unneeded overhead.
@r10s r10s force-pushed the instant-grp-qr-join branch from f1ed3e4 to 41d373f Compare October 25, 2021 21:15
@r10s r10s force-pushed the instant-grp-qr-join branch from a13e9e9 to 43c6d3b Compare October 25, 2021 21:34
@r10s r10s requested review from flub and link2xt October 26, 2021 09:34
@r10s r10s merged commit 3b7b8ea into master Oct 26, 2021
@r10s r10s deleted the instant-grp-qr-join branch October 26, 2021 14:34
r10s added a commit that referenced this pull request Nov 5, 2021
not totally sure if that change in #2508 was on-purpose,
however, all yet released versions
did create the one-to-one chat also on the Inviter's (Alice) side,
so, let's stay with that,
i do not see many reasons to change that.
r10s added a commit that referenced this pull request Nov 6, 2021
* test one-to-one chats on setup-contact/secure-join

only one chat is created after scanning a QR code:

- on setup-contact, one-to-ones are created on both sided

- on secure-join, the joined group chat is created;
  one-to-ones are not created intitally,
  but should become visible on receiving messages

* make sure, Alice creates the chat with Bob on setup-contact

not totally sure if that change in #2508 was on-purpose,
however, all yet released versions
did create the one-to-one chat also on the Inviter's (Alice) side,
so, let's stay with that,
i do not see many reasons to change that.

* unblock hidden (Blocked::Yes) one-to-one chats

one-to-one chats may be hidden by secure-join,
in case someone later writes a message to it
(not unlikely), the chat needs to be shown.

before, messages are just not shown,
the corresponding chat did not appear.

the 'Blocked' wording of a 'Chat' must not be mixed with the
'Blocking' of a contact. 'Chat-Blocking' is mostly a visibility thing,
that may change as messages come in.

this change should not affect _really_ blocked contacts -
they are filtered out already before
and their messages are usually not even downloaded.
also, before allow_creation is checked,
that may disallow chat creation for show_emails reasons.

all in all, it just does the same
as if the user has manualy deleted the chat before and it would be created.

* simplify test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants