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

Data root missing when linking a second device #420

Closed
bgins opened this issue Sep 7, 2022 · 5 comments
Closed

Data root missing when linking a second device #420

bgins opened this issue Sep 7, 2022 · 5 comments
Assignees

Comments

@bgins
Copy link
Member

bgins commented Sep 7, 2022

Summary

Problem

When a user links their account on a second device immediately after registering, the data root for their filesystem might not be available.

Impact

The linked device may not be able to load the filesystem right away. The linked device will need to poll until the data root is available.

Solution

One possibility would be moving the necessary polling into the account linking code. This option is not ideal because we should keep the account linking code decoupled from filesystem concerns.

Another possibility would be to move the polling into loadFilesystem: https://github.com/fission-codes/webnative/blob/01b57e2d6616631b2b4a484a46fd14e385aa8a0b/src/filesystem.ts#L35-L103

Detail

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Comment out https://github.com/fission-codes/webnative-app-template/blob/05b70fd1596c89204a45a54f4eef81aed45c41ba/src/lib/common/webnative.ts#L111 in the Webnative app template
  2. Register a new user
  3. Follow the linking flow
  4. The filesystem should fail to load and a Could not decode CID: undefined error will appear in the console

Expected behavior

The filesystem should load without throwing an exception.

Screenshots

The decode CID exception looks like

decode-cid

Note that the exception is not a direct representation of the missing data root. It occurs because no data root exists and we instead look for a root CID in the CID log, which does not exist yet.

Desktop (please complete the following information):

  • OS: macOS
  • Browser: Chrome, Firefox, Safari

Additional context

The Webnative app template works around this issue by polling the data root: https://github.com/fission-codes/webnative-app-template/blob/05b70fd1596c89204a45a54f4eef81aed45c41ba/src/lib/common/webnative.ts#L126-L149

@icidasset
Copy link
Contributor

I was also looking at this error message earlier today, we're actually calling decodeCID at the wrong time.
That line the error points to in loadFileSystem should be something like:

// No DNS CID yet
cid = await cidLog.newest()
cid = cid ? decodeCID(cid) : null

@icidasset
Copy link
Contributor

I should also point out that if the loadFileSystem function worked as it should (ie. with the above fix), it would create a new file system in this situation. Which is obviously also not the correct thing to do.

That said, we could reuse waitForRootDid from the channel code here.

@bgins
Copy link
Member Author

bgins commented Sep 20, 2022

I should also point out that if the loadFileSystem function worked as it should (ie. with the above fix), it would create a new file system in this situation. Which is obviously also not the correct thing to do.

Yep, agreed. 💯

That said, we could reuse waitForRootDid from the channel code here.

Would we want to poll the data root instead of the root DID? We are checking the data root in the workaround in the Webnative app template. Would root DID also work?

@icidasset
Copy link
Contributor

Would we want to poll the data root instead of the root DID? We are checking the data root in the workaround in the Webnative app template. Would root DID also work?

Sorry should've been more clear. I meant reusing the retry mechanism of that function, we can easily swap out the DID lookup with the data root lookup.

@bgins
Copy link
Member Author

bgins commented Sep 20, 2022

Sorry should've been more clear. I meant reusing the retry mechanism of that function, we can easily swap out the DID lookup with the data root lookup.

Yep, I think this is the right way to go. I'll implement it and put in a PR with it. 👌

@bgins bgins mentioned this issue Sep 26, 2022
7 tasks
@walkah walkah assigned jessmartin and icidasset and unassigned bgins and jessmartin Oct 18, 2022
@walkah walkah closed this as completed Nov 29, 2022
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 a pull request may close this issue.

4 participants