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

One-time command to upload the current idgraph to parachain #3131

Conversation

Kailai-Wang
Copy link
Collaborator

@Kailai-Wang Kailai-Wang commented Oct 16, 2024

Context

As topic.

We just stop the worker and do RUST_LOG=info ./litentry-worker upload-id-graph -m <shard> - it will not return before all txs are finalised on-chain.

-m <shard> is optional if the shard (mrenclave) is the same as where IDGraph is stored.

Locally tested ok

image

@Kailai-Wang Kailai-Wang self-assigned this Oct 16, 2024
Copy link

linear bot commented Oct 16, 2024

Copy link
Member

@kziemianek kziemianek left a comment

Choose a reason for hiding this comment

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

If we decide to keep it as it is we need to call it ultimately one more time once sidechai's IMP becomes immutable (trusted call are not touching idgraps) and before IMP pallet storage is removed so I assume there will be worker version supporting this command and disabling IMP's related trusted calls.

let local_state = state.clone();

io_handler.add_sync_method("omni_UploadIDGraph", move |params: Params| {
debug!("worker_api_direct rpc was called: omni_UploadIDGraph");
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest putting this in startup code, guarded by CLI argument.

Currently anybody can call this method - it will flood parachain.
Another thing is: upload should be triggered after sidechain's imp pallet becomes immutable and before parachain storage becomes mutable - otherwise there will be conflicts/outdated account stores.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't go for this as I wanted to save the communication between non-enclave and enclave, as it's a one-time thing. omni_UploadIDGraph could be called multiple times - e.g. right before we switch to the new on-chain IDGraph, the whole process should be done within 1 minute - like it's hard for any abusers to predict when we are going to do the upgrade.

Let me change it to start-up sub-command. In that way we'll stop worker, do the upload (so that nothing changes at the same time), and restart it with newer version that supports on-chain Graph

Copy link
Collaborator

Choose a reason for hiding this comment

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

start-up sub-command that would be a better solution. Completely offline. No worries for any potential hacks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated it

@@ -171,7 +172,7 @@ pub mod pallet {
who: omni_account,
result: result.map(|_| ()).map_err(|e| e.error),
});
Ok(())
Ok(Pays::No.into())
Copy link
Member

Choose a reason for hiding this comment

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

I've also noticed it missing but in my case I had to:
https://github.com/litentry/litentry-parachain/pull/3132/files#diff-960f61f16faf996e829d1decb06b54fb404525cc02cdc585b9e8db86d596014dR344

Otherwise all extrinsincs signed by newly generated account were rejected (low funds)

Copy link
Member

@kziemianek kziemianek left a comment

Choose a reason for hiding this comment

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

I've checked it locally - works fine for small amount of idgraphs.

@Kailai-Wang Kailai-Wang enabled auto-merge (squash) October 18, 2024 13:55
@Kailai-Wang Kailai-Wang merged commit 7ead80a into dev Oct 18, 2024
19 checks passed
@Kailai-Wang Kailai-Wang deleted the p-1085-one-time-command-to-upload-the-current-idgraph-to-parachain branch October 18, 2024 14:43
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