Skip to content

fix: Fix setting up a profile and immediately transferring to a second device #6657

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

Merged
merged 10 commits into from
Mar 17, 2025

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Mar 13, 2025

Found and fixed a bug while investigating #6656. It's not the same bug, though.

Steps to reproduce this bug:

  • Create a new profile
  • Transfer it to a second device
  • Send a message from the first device
  • -> It will never arrive on the second device, instead a warning will be printed that you are using DC on multiple devices.

The bug was that the key wasn't created before the backup transfer, so that the second device then created its own key instead of using the same key as the first device.

In order to regression-test, this PR now changes clone() to use "Add second device" instead of exporting and importing a backup. Exporting and importing a backup has enough tests already.

This PR also adds an unrelated test test_selfavatar_sync().

The bug was introduced by #6574 in v1.156.0

@Hocuri Hocuri changed the title fix: Fix setting up a device and immediately transferring the account [WIP] fix: Fix setting up a device and immediately transferring the account Mar 13, 2025
@Hocuri Hocuri requested review from link2xt and iequidoo and removed request for link2xt and iequidoo March 13, 2025 17:31
@Hocuri Hocuri changed the title [WIP] fix: Fix setting up a device and immediately transferring the account fix: Fix setting up a device and immediately transferring the account Mar 13, 2025
@Hocuri Hocuri requested review from link2xt and iequidoo March 13, 2025 17:50
@Hocuri Hocuri changed the title fix: Fix setting up a device and immediately transferring the account fix: Fix setting up a profile and immediately transferring to a second device Mar 13, 2025
@iequidoo
Copy link
Collaborator

Maybe add a Rust test instead which checks that the key is the same on devices after a backup export/import? We don't even need to check that synchronization works (if keys are different, it's a bug anyway)

@Hocuri
Copy link
Collaborator Author

Hocuri commented Mar 14, 2025

Maybe add a Rust test instead

I don't think it's possible to do iroh-add-second-device in Rust? And the issue only happened with iroh-add-second-device, not with export-backup

@link2xt
Copy link
Collaborator

link2xt commented Mar 16, 2025

I don't think it's possible to do iroh-add-second-device in Rust? And the issue only happened with iroh-add-second-device, not with export-backup

It is possible to do everything in Rust, but better not add online tests there. We have around 5 online tests in Rust and I don't like that they fail when offline, better keep them in Python if it requires being online.

@Hocuri Hocuri force-pushed the hoc/fix-setup-second-device branch from 7d8aebd to 7c55f9a Compare March 17, 2025 16:26
@Hocuri
Copy link
Collaborator Author

Hocuri commented Mar 17, 2025

In order to regression-test, this PR now changes clone() to use "Add second device" instead of exporting and importing a backup. I checked that when I revert the fix, then 5 tests fail now. Exporting and importing a backup has enough tests already.

This PR also adds an unrelated test test_selfavatar_sync().

@Hocuri Hocuri requested a review from link2xt March 17, 2025 16:39
@Hocuri Hocuri merged commit dc17f26 into main Mar 17, 2025
29 checks passed
@Hocuri Hocuri deleted the hoc/fix-setup-second-device branch March 17, 2025 17:12
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.

3 participants