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

Add import/export dexie db workflow to web client #740

Merged
merged 11 commits into from
Feb 27, 2025

Conversation

julian-demox
Copy link
Collaborator

Adds the ability to dump a user's entire dexie db into js. This can then be passed back and forth in a file for the user to re-import later.

Future additions can include specifying the tables / specific information pertaining to the user.

Attempted to use libraries like dexie-export-import to avoid writing the recursive traversal as seen, but something about the encoding of the blob that it outputted was not working with the serialization layers being passed from js to wasm to js.

records.map(recursivelyTransformForImport)
);

await table.bulkPut(transformedRecords);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are upserting all the exported entries in the db. This means that the data before importing will also be present. If we don't do some checks/extra logic before upserting there are some problems that might arise. Here are some that I can think of right now:

  • An imported account already existed in the db and the imported state is older. The latest account state would be overwritten and if the account was private it might not be recoverable.
  • The imported notes could be from the past/future relative to the client. In each case, the imported note needs different handling (for context here's the current import note logic).
  • I think the stateSync table in the web store will also be overwritten, basically moving the client to the past/future. This might also cause issues with already tracked notes/accounts as the sync request will depend on the current block of the client.

Basically it's not as straightforward to introduce new data to an existing client.

I'm not sure what is the intended use for this db import/export but it might be better we could change it so that the old db is deleted before inserting. Then, these problems would disapear. It would just be a way of copying and pasting your client state to another computer. The clear downside here is that the old data will be lost. It depends on the intended use, if we want this to be for new empty clients then it's ok, if not then we will need to add some additional logic to deal with the importing of new elements.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the idea is to use it for new clients then maybe the "import" process could be moved to a constructor, so we remove the possibility of a user deleting their existing data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes makes sense. The intended use is as you described, to simply move data from one computer to another or to for example backup a snapshot state, and restore from that state (destructively) if for example your local machine failed in some way. It's not designed to be "cleanly" applied on top of an existing db, at least not for its current use-case. The way we intend to allow chrome wallet users to access this ultimately is only when creating a wallet for the first time, offering the opportunity from a clean slate, to import from a file.

Totally understand what you're saying wrt providing a way for the users to shoot themselves in the foot. I can rename functions to make things clearer i.e. overwriteDb, forceImportDb . This plus some better documentation as @igamigo mentioned I think would cover safety of usage for majority of cases.

I don't think there's a straightforward way to make it a constructor option as you're presenting it, though I understand your thought process. Ultimately an sdk user can instantiate as many WebClient miden web client connections as they'd like. It utilizes the wasm to access the dexie db, but it can spin up as many independent connections as it wants as each client instantiates its own unique connection to the main, single, dexie db in the browser. I suppose we could track which access is the "first" and in that way enforce it, but that doesn't feel clean to me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of upserting data, could it somehow override the existing database?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately an sdk user can instantiate as many WebClient miden web client connections as they'd like. It utilizes the wasm to access the dexie db, but it can spin up as many independent connections as it wants as each client instantiates its own unique connection to the main, single, dexie db in the browser.

Is the dexie db shared between separate WebClient instances?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the import js call we could have a pre-process step and remove any existing data from all of the tables. Perhaps wrap the whole thing in an atomic txn call in case the bulkPuts fail. Is that in line what you're thinking?

Copy link
Collaborator Author

@julian-demox julian-demox Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the dexie db shared between separate WebClient instances?

@tomyrd Yes, that's mostly correct. Presuming they're part of the same browser session profile (i used session here, incorrectly... it persists across sessions) . If you went incognito for example, it will separate out the dbs even if using chrome for both.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's mostly correct. Presuming they're part of the same browser session. If you went incognito for example, it will separate out the dbs even if using chrome for both.

Oh, I didn't know this. So if we clear de db to import in one client it will delete the data for other separate instances?

On a separate note: How do separate clients deal with the syncState table where it's only one entry that gets updated? They also share it? Like a sync in one client will sync the others?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will delete the data in the Miden dexie db for that browser profile to be precise. All WebClient instances point to the same Miden dexie db. Which leads to the answer for the second question which is all operations work with the singular Miden dexie db (which has its single syncState table) in the browser profile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, in the wallet we're developing for example, you only have one instance of the client that's in charge of running the syncState operation. Other client instances (or the same client instance depending on the design) can then perform operations independent of that one.

Copy link
Collaborator

@igamigo igamigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Code looks good, but I agree with @tomyrd in that there could be unwanted consequences so depending on the usecase, a slightly more specific approach could be used in order to avoid these.

Copy link
Collaborator

@igamigo igamigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Collaborator

@SantiagoPittella SantiagoPittella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

@julian-demox julian-demox force-pushed the julian-demox-export-db branch from 528ecb5 to 11a2ab4 Compare February 25, 2025 21:47
@julian-demox julian-demox merged commit 91cbdb4 into next Feb 27, 2025
13 checks passed
@julian-demox julian-demox deleted the julian-demox-export-db branch February 27, 2025 01:09
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.

4 participants