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

Expose the sgx-pallets constants on parachain #1569

Open
Kailai-Wang opened this issue Apr 4, 2023 · 7 comments
Open

Expose the sgx-pallets constants on parachain #1569

Kailai-Wang opened this issue Apr 4, 2023 · 7 comments
Assignees
Labels
D3-chore tasks that need to be completed but don’t provide any additional features/functionality I2-medium should be completed within 10 working days Stale

Comments

@Kailai-Wang
Copy link
Collaborator

Context

Some constants are hardcoded in sgx-runtime pallets, e.g.: https://github.com/litentry/litentry-parachain/blob/03f681ce63bbaff015a6d69bff8aa245b539f7cb/tee-worker/app-libs/sgx-runtime/src/lib.rs#L276

F/E can't query it.

It's better to make it more transparent by exposing it on parachain and allowing sidechain to read it.

Medium prio.


✔️ Please set appropriate labels and assignees if applicable.

@Kailai-Wang Kailai-Wang added I2-medium should be completed within 10 working days D3-chore tasks that need to be completed but don’t provide any additional features/functionality labels Apr 4, 2023
@Kailai-Wang
Copy link
Collaborator Author

Kailai-Wang commented May 9, 2023

An alternative is using PublicGetter in direct invocation

But personally I think these constants are better to make public and more transparent, so I'm favoring option 1 at the moment

@kziemianek kziemianek self-assigned this May 16, 2023
@Kailai-Wang
Copy link
Collaborator Author

I discussed this with @kziemianek and I logged down some results/thoughts.

The issue is about the necessity of exposing the current sidechain constants (e.g. MaxVerificationDelay) on parachain: I believe it's about striking a balance between efficiency, transparency and decentralisation.

I try to make a table for comparison: making it a parachain constant/storage vs sidechain storage. I welcome thoughts/feedback from all sides.

Even if we eventually decide not to expose the constant on parachain, it's not time wasted as it helps better understand the architecture and decision-making process.

parachain storage sidechain storage comment
Transparency(publicity) ++ + It's public in both cases, maybe parachain is more F/E friendly as people already know how to query them
Performance + +++ If it's stored on parachain, we need a way to retrieve it (see below)
Setter permission ++ + It's about who can update it. On parachain it's defined in runtime - eventually it's (mostly) down to root/democracy/council, while in sidechain it's the enclave author if we keep using MRSIGNER
Upgradability ++ + If it's easy to modify it. Sidechain would need an enclave upgrade. This is not as important because the value should rarely change

In light of retrieving parachain storage, there're also two ways:

  1. via parachain block sync (*)
    Similar to what we've done to scheduled enclave. Upon parachain block syncing, we take down the values and all upstream changes and seal them in a local file. We read the value from the sealed file before execution of the corresponding trusted call.
  2. do a live storage query via get_storage_hashes_to_update
    It's a built-in entry point to query the parachain state via ocall_api. It's guaranteed to query the state at the specific block height (= not always the latest)
via parachain block sync live storage query comment
performance ++/+++ + Reading local files vs network query. Normally the former wins
development complexity + ++ Little effort for option 2. For option 1 we need to write extrinsic parser and IO persistence
scalability + ++ It's related to the previous point. Imagine we'll have several such constants in the future. Option 1 is significantly less scalable - even though you can group/traitify it
robustness + ++ Most likely we'll need to back up the file for option 1(similar to light_client.bin) as the node could be terminated anytime - also halfway when writing the file
  • a variant of option 1 is to store the value in the sidechain state. But since they share the same way of getting the data (from parachain), only option 1 is listed.

@kziemianek
Copy link
Member

kziemianek commented May 18, 2023

I'll add another let's call it hybrid way of syncing sidechain state which is a mix of via parachain block sync and live query. If we add constants to parachain pallet as #[pallet::constant] then we could query them from sidechain after receiving CodeUpdated event. It would greatly reduce amount of queries.

@kziemianek
Copy link
Member

kziemianek commented May 19, 2023

Current decision is to move selected constants to parachain and sync them through live storage queries. Once we add metrics we will have more insights how this solution affects performance and stability. In case of any problems or doubts we will rethink design in the future.

@kziemianek
Copy link
Member

One last thing regarding this querying topic. I think it might query more than once per block the same storage_hash , so at least this kind optimization might be desirable.

@kziemianek
Copy link
Member

I created issue in integritee-network/worker repo:
integritee-network/worker#1322

@github-actions
Copy link
Contributor

❗ This issue is stale because it has been open for 60 days with no activity.
Please take proper action against it.
@litentry/parachain

@github-actions github-actions bot added the Stale label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D3-chore tasks that need to be completed but don’t provide any additional features/functionality I2-medium should be completed within 10 working days Stale
Projects
None yet
Development

No branches or pull requests

2 participants