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

Support for x86_64-fortanix-unknown-sgx target #252

Open
DragonDev1906 opened this issue Aug 16, 2023 · 3 comments
Open

Support for x86_64-fortanix-unknown-sgx target #252

DragonDev1906 opened this issue Aug 16, 2023 · 3 comments

Comments

@DragonDev1906
Copy link

Hello, I've tried to use Helios (to be more specific the consensus package) on the x86_64-fortanix-unknown-sgx target to run a light client in an Intel SGX enclave. Unfortunately this is currently not possible because the dependency socket2 cannot be compiled for that target. I don't mind having to implement a custom ConsensusRpc, but the reqwest dependency and thus the socket2 dependency are still compiled and cannot be disabled by a feature flag.

It would be awesome if we could use Helios (or at least the consensus package) on this target by disabling default features such that code using this dependency is not included. Here is a (likely incomplete) list of dependencies that would need to be disabled (or where some features would have to be optional):

  • reqwest
  • tokio feature "net"

I don't ask for the removal of those dependencies, just that they, their use and/or these features are behind a feature flag that is enabled by default.

Steps to reproduce the compile error for this target:

rustup +nightly target install x86_64-fortanix-unknown-sgx
cargo +nightly build --target x86_64-fortanix-unknown-sgx -p consensus --no-default-features

Why would we want light clients in SGX (a Trusted Execution Environment)?

This would allow following the Ethereum chain in a Trusted Execution Environment such that the TEE does not need to rely on an external full node for knowledge about the Ethereum chain.

If you know of other crates that could be useful for checking the Ethereum consensus in such an Environment please let me know.

@ncitron
Copy link
Collaborator

ncitron commented Aug 16, 2023

This seems like a reasonable feature. Right now we are working on a couple of big refactors to how Helios works internally which leaves Helios in a weird spot for the next few days, but once I have tightened that up I can take a look at this.

@zvolin
Copy link
Contributor

zvolin commented Aug 29, 2023

Not exactly the same but I think still relevant. As a background, we wrote an eth light client smart contract (canister) for the internet-computer at Eiger. Smart contracts there are executed in a custom wasm runtime and in general any 'non-browser' wasm environment don't work well with reqwest and the others (eg. wasi but also others which are still wasm32-unknown-unknown).
We've also been using the execution and client stuff and from our observations, also the ethers-providers crate imposed some troubles.

I think the correct solution for helios (library) would be to keep the current implementation of ConsensusRpc, ExecutionRpc and all their dependencies behind a (enabled by default) feature flag. The client builder should provide a way to inject the custom ones, and current options of providing just urls should either forward them to the injected ones or also been hiddden behind the same features.

The traits could look like this (with default impls for ergonomics):

#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
pub trait ConsensusRpc {
    type ConsensusRpcError;

    fn new(path: &str) -> Result<Self, Self::ConsensusRpcError>;

    fn request<P: AsRef<str>, R: DeserializeOwned>(
        &self,
        path: P,
    ) -> Result<R, Self::ConsensusRpcError>;

    async fn get_bootstrap(&self, block_root: &'_ [u8]) -> Result<Bootstrap, Self::ConsensusRpcError> {
        // provided default impl
    }

    // ...the rest of current trait methods
}

#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
pub trait ExecutionRpc: Send + Clone + Sync + 'static {
    type ExecutionRpcError;

    fn new(rpc: &str) -> Result<Self, Self::ExecutionRpcError>;

    fn request<T: Serialize + Send + Sync, R: DeserializeOwned>(
        &self,
        method: &str,
        params: T,
    ) -> Result<R, Self::ExecutionRpcError>;

    async fn get_proof(
        &self,
        address: &Address,
        slots: &[H256],
        block: u64,
    ) -> Result<EIP1186ProofResponse, Self::ExecutionRpcError> {
        // provided default impl
    }

    // ...the rest of current trait methods
}

The problem I can see with that is that the current Rpc implementations might need to live in a separate crate so that they can still have different dependencies for native and wasm if needed (not needed if just reqwest and ethers-providers are used).

@DragonDev1906
Copy link
Author

Right now we are working on a couple of big refactors

One more (related) thing, as you've mentioned refactors:
For now I've decided to not use Helios (instead using a custom implementation based on the specs) for two reasons (besides it not working for sgx, yet):

  • It is currently not possible to use the consensus checking parts without using async (i.e. in a way where I'm providing all the data in advance instead of letting Helios decide when it wants what). Async isn't problematic, but I don't see much use in adding an async runtime if the remainder of the application is fully synchronous. I assume it is similar in an ICP canister. It would be amazing if the consensus critical part (i.e. the part that comes from the specs) could be used completely without an async runtime.
  • Currently the bls signing library milagro_bls used is experimental (as helios is, too). It might be useful to be able to change the signing library (I'm currently using https://github.com/supranational/blst for example). Just some food for thought.

Those are out of scope for this issue of course, so feel free to put them in a separate issue if you see them as relevant.

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

No branches or pull requests

3 participants