Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Don't assume wasm in no_std env in runtime-interface #5547

Open
h4x3rotab opened this issue Apr 6, 2020 · 10 comments
Open

Don't assume wasm in no_std env in runtime-interface #5547

h4x3rotab opened this issue Apr 6, 2020 · 10 comments
Assignees
Labels
J0-enhancement An additional feature request.

Comments

@h4x3rotab
Copy link
Contributor

h4x3rotab commented Apr 6, 2020

Currently sp-io implements external functions for the runtime. It uses runtime-interface macros to automatically generate the functions for native and wasm build.

Specifically, it is handled by:

As written in the code, the macros determines if the generated interface function is for wasm or native code by checking std feature.

It usually works, until someone wants use the runtime natively in a no_std environment. runtime-interface will try to generate a wasm external call when it detects a no_std environment, but there's no wasm interface because it's actually a native build. As a result, it will trigger a link error.

The problem lies in the way runtime-interface detects if it's a wasm or native build. I suggest to add a new rust feature as a switch (e.g. runtime_wasm), instead of checking std or no_std. With that I can simply build the runtime in a native no_std target.

Why native no_std?

I'm working on a Substrate light client in TEE. The TEE enclave code is in no_std because it's required by the TEE SDK.

In the light client, there are a few places calculating the hash of the block header (header.hash()), which eventually calls sp_io::hashing::blake2_256(). The hash functions in sp_io is defined by runtime-interface. So if any code invokes block.hash(), it will try to find the wasm interface and thus fails to link.

Currently my walk-around is to replace header.hash() by serializing the header and call sp_core::hashing::blake2_256() directly, as written in wasm_hacks. If this issue can be fixed, it will be much easier to make the light client a separate package for better code reuse. I believe this cloud also help substraTEE team.

@brenzi
Copy link
Contributor

brenzi commented Apr 6, 2020

We solved this by replacing/patching sr-io:
https://github.com/scs/substraTEE-worker/tree/master/substrate-sgx/sr-io
(! this is pre-2.0.0)

@bkchr bkchr self-assigned this Apr 6, 2020
@bkchr bkchr added the J0-enhancement An additional feature request. label Apr 6, 2020
@NikVolf
Copy link
Contributor

NikVolf commented Apr 7, 2020

I think the problem might be broader than just runtime-interface? We assume no-std as wasm pretty much everywhere across codebase.

@bkchr
Copy link
Member

bkchr commented Apr 7, 2020

Yep ;)

@h4x3rotab
Copy link
Contributor Author

I think the problem might be broader than just runtime-interface? We assume no-std as wasm pretty much everywhere across codebase.

At least dozens of of packages under primitives/ can work well in native no_std.

@recmo
Copy link

recmo commented Apr 11, 2020

I do a non-wasm no_std build as part of the CI to check for accidental inclusion of std. Currently sp-io fails because:

/// A default panic handler for WASM environment.
#[cfg(all(not(feature = "disable_panic_handler"), not(feature = "std")))]
#[panic_handler]
#[no_mangle]
pub fn panic(info: &core::panic::PanicInfo) -> ! {
	unsafe {
		let message = sp_std::alloc::format!("{}", info);
		logging::log(LogLevel::Error, "runtime", message.as_bytes());
		core::arch::wasm32::unreachable();
	}
}

/// A default OOM handler for WASM environment.
#[cfg(all(not(feature = "disable_oom"), not(feature = "std")))]
#[alloc_error_handler]
pub fn oom(_: core::alloc::Layout) -> ! {
	unsafe {
		logging::log(LogLevel::Error, "runtime", b"Runtime memory exhausted. Aborting");
		core::arch::wasm32::unreachable();
	}
}

assumes that no-std means core::arch::wasm32 is available. this should be gated behind cfg(target_arch = "wasm32").

This work around, together #5607, makes my runtime compile in no-std on a 32-bit target (on 64-bit targets there are further issues):

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
sp-io = { version = "2.0.0-alpha.5", default-features = false, features = [ "disable_oom", "disable_panic_handler" ] }

@h4x3rotab
Copy link
Contributor Author

@recmo The lang items can be disabled by features:

[features]
default = [
  "sp-io/disable_panic_handler",
  "sp-io/disable_oom",
  "sp-io/disable_allocator",
]

But of course it’s better to detect the wasm environment by something else instead of std. Arch may work.

@brenzi
Copy link
Contributor

brenzi commented Aug 30, 2020

This problem still has no nice solution. Any plans?
My remaining problem with our workaround (patch sp-io) is that we can't see debug info if we instantiate a runtime inside sgx. We'd need all the types to implement Debug, which is scattered over too many crates to do patching

@bkchr
Copy link
Member

bkchr commented Aug 30, 2020

Hmm. So we already have support for runtime-wasm feature in wasm-builder. I "always" planed to make more use of this, maybe we should start using it for this purpose.

@brenzi
Copy link
Contributor

brenzi commented Sep 1, 2020

@bkchr would that mean we can basically replace all

#[cfg(feature = "std")]

by

#[cfg(not(feature = "runtime-wasm"))]

(and the logical opposite)
?

If its that easy I'd volounteer to propose a PR

@bkchr
Copy link
Member

bkchr commented Oct 7, 2020

@brenzi sorry for the late reply. I have commented on the pr, it is not that easy. Otherwise I would have done it already :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
None yet
5 participants