frontport: Add CI check on no_std compliance for supported tendermint crates (#1087)

.github/workflows/no-std.yaml

@@ -0,0 +1,33 @@
+name: no_std check
+on:
+  pull_request: {}
+  push:
+    branches: master
+
+jobs:
+  check-no-std-panic-conflict:
+    name: Check no_std panic conflict
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          override: true
+      - run: |
+          cd tools/no-std-check
+          make check-panic-conflict
+
+  check-substrate:
+    name: Check no_std substrate support
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: nightly
+          target: wasm32-unknown-unknown
+          override: true
+      - run: |
+          cd tools/no-std-check
+          make check-substrate

Cargo.toml

@@ -17,7 +17,8 @@ members = [
 ]
 
 exclude = [
-    "proto-compiler"
+    "proto-compiler",
+    "tools/no-std-check"
 ]
 
 [profile.release.package.tendermint-light-client-js]

light-client-verifier/Cargo.toml

@@ -27,8 +27,6 @@ default = ["flex-error/std", "flex-error/eyre_tracer"]
 
 [dependencies]
 tendermint = { version = "0.24.0-pre.1", path = "../tendermint", default-features = false }
-tendermint-rpc = { version = "0.24.0-pre.1", path = "../rpc", default-features = false }
-
 derive_more = { version = "0.99.5", default-features = false, features = ["display"] }
 serde = { version = "1.0.106", default-features = false }
 time = { version = "0.3.5", default-features = false }

tools/Cargo.toml

@@ -6,3 +6,7 @@ members = [
     "proto-compiler",
     "rpc-probe"
 ]
+
+exclude = [
+    "no-std-check"
+]

tools/no-std-check/.gitignore

@@ -0,0 +1 @@
+target/

tools/no-std-check/Cargo.toml

@@ -0,0 +1,29 @@
+[package]
+name = "no-std-check"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+tendermint = { path = "../../tendermint", default-features = false }
+tendermint-proto = { path = "../../proto", default-features = false }
+tendermint-light-client-verifier = { path = "../../light-client-verifier", default-features = false }
+
+sp-core = { version = "4.0.0", default-features = false, optional = true }
+sp-io = { version = "4.0.0", default-features = false, optional = true }
+sp-runtime = { version = "4.0.0", default-features = false, optional = true }
+sp-std = { version = "4.0.0", default-features = false, optional = true }
+
+[features]
+panic-handler = []
+use-substrate = [
+  "sp-core",
+  "sp-io",
+  "sp-runtime",
+  "sp-std",
+]
+substrate-std = [
+  "sp-core/std",
+  "sp-io/std",
+  "sp-runtime/std",
+  "sp-std/std",
+]

tools/no-std-check/Makefile

@@ -0,0 +1,33 @@
+NIGHTLY_VERSION=nightly
+
+setup:
+	rustup install $(NIGHTLY_VERSION)
+	rustup target add wasm32-unknown-unknown --toolchain $(NIGHTLY_VERSION)
+
+build-substrate:
+	cargo build \
+		--no-default-features \
+		--features use-substrate,substrate-std
+
+check-panic-conflict:
+	cargo build \
+		--no-default-features \
+		--features panic-handler
+
+check-cargo-build-std:
+	rustup run $(NIGHTLY_VERSION) -- \
+		cargo build -Z build-std=core,alloc \
+		--no-default-features \
+		--target x86_64-unknown-linux-gnu
+
+check-wasm:
+	rustup run $(NIGHTLY_VERSION) -- \
+		cargo build \
+		--target wasm32-unknown-unknown
+
+check-substrate:
+	rustup run $(NIGHTLY_VERSION) -- \
+		cargo build \
+		--no-default-features \
+		--features use-substrate \
+		--target wasm32-unknown-unknown

tools/no-std-check/README.md

@@ -0,0 +1,111 @@
+# `no_std` Compliance Check
+
+This crate checks the `no_std` compliance of the supported crates in ibc-rs.
+
+## Make Recipes
+
+- `check-panic-conflict` - Check for `no_std` compliance by installing a panic handler, and any other crate importing `std` will cause a conflict. Runs on default target.
+
+- `check-cargo-build-std` - Check for `no_std` compliance using Cargo nightly's `build-std` feature. Runs on the target `x86_64-unknown-linux-gnu`.
+
+- `check-wasm` - Check for WebAssembly and `no_std` compliance by building on the target `wasm32-unknown-unknown` and installing a panic handler.
+
+- `check-substrate` - Check for Substrate, WebAssembly, and `no_std` compliance by importing Substrate crates and building on `wasm32-unknown-unknown`. Any crate using `std` will cause a conflict on the panic and out-of-memory (OOM) handlers installed by `sp-io`.
+
+## Conflict Detection Methods
+
+There are two methods that we use to detect `std` conflict:
+
+### Panic Handler Conflict
+
+This follows the outline of the guide by
+[Danilo Bargen](https://blog.dbrgn.ch/2019/12/24/testing-for-no-std-compatibility/)
+to register a panic handler in the `no-std-check` crate.
+Any crate imported `no-std-check` that uses `std` will cause a compile error that
+looks like follows:
+
+```
+$ cargo build
+    Updating crates.io index
+   Compiling no-std-check v0.1.0 (/data/development/informal/ibc-rs/no-std-check)
+error[E0152]: found duplicate lang item `panic_impl`
+  --> src/lib.rs:31:1
+   |
+31 | fn panic(_info: &PanicInfo) -> ! {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the lang item is first defined in crate `std` (which `prost` depends on)
+   = note: first definition in `std` loaded from /home/ubuntu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b6b48477bfa8c673.rlib
+   = note: second definition in the local crate (`no_std_check`)
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0152`.
+error: could not compile `no-std-check`
+```
+
+- Pros:
+  - Can be tested using Rust stable.
+- Cons:
+  - Crates must be listed on both `Cargo.toml` and `lib.rs`.
+  - Crates that are listed in `Cargo.toml` but not imported inside `lib.rs` are not checked.
+
+### Override std crates using Cargo Nightly
+
+This uses the unstable `build-std` feature provided by
+[Cargo Nightly](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-std).
+With this we can explicitly pass the std crates we want to support, `core` and `alloc`,
+via command line, and exclude the `std` crate.
+
+If any of the dependency uses `std`, they will fail to compile at all, albeit with
+confusing error messages:
+
+```
+$ rustup run nightly -- cargo build -j1 -Z build-std=core,alloc --target x86_64-unknown-linux-gnu
+   ...
+   Compiling bytes v1.0.1
+error[E0773]: attempted to define built-in macro more than once
+    --> /home/ubuntu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/macros/mod.rs:1201:5
+     |
+1201 | /     macro_rules! cfg {
+1202 | |         ($($cfg:tt)*) => {
+1203 | |             /* compiler built-in */
+1204 | |         };
+1205 | |     }
+     | |_____^
+     |
+note: previously defined here
+    --> /home/ubuntu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/macros/mod.rs:1201:5
+     |
+1201 | /     macro_rules! cfg {
+1202 | |         ($($cfg:tt)*) => {
+1203 | |             /* compiler built-in */
+1204 | |         };
+1205 | |     }
+     | |_____^
+
+error: duplicate lang item in crate `core` (which `std` depends on): `bool`.
+  |
+  = note: the lang item is first defined in crate `core` (which `bytes` depends on)
+  = note: first definition in `core` loaded from /data/development/informal/ibc-rs/no-std-check/target/x86_64-unknown-linux-gnu/debug/deps/libcore-c00d94870d25cd7e.rmeta
+  = note: second definition in `core` loaded from /home/ubuntu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-9924c22ae1efcf66.rlib
+
+error: duplicate lang item in crate `core` (which `std` depends on): `char`.
+  |
+  = note: the lang item is first defined in crate `core` (which `bytes` depends on)
+  = note: first definition in `core` loaded from /data/development/informal/ibc-rs/no-std-check/target/x86_64-unknown-linux-gnu/debug/deps/libcore-c00d94870d25cd7e.rmeta
+  = note: second definition in `core` loaded from /home/ubuntu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-9924c22ae1efcf66.rlib
+...
+```
+
+The above error are shown when building the `bytes` crate. This is caused by `bytes` using imports from `std`,
+which causes `std` to be included and produce conflicts with the `core` crate that is explicitly built by Cargo.
+This produces very long error messages, so we may want to use tools like `less` to scroll through the errors.
+
+Pros:
+  - Directly identify use of `std` in dependencies.
+  - Error is raised on the first dependency that imports `std`.
+
+Cons:
+  - Nightly-only feature that is subject to change.
+  - Confusing and long error messages.

tools/no-std-check/src/lib.rs

@@ -0,0 +1,50 @@
+// ensure_no_std/src/main.rs
+#![no_std]
+#![allow(unused_imports)]
+
+extern crate alloc;
+
+// Import the crates that we want to check if they are fully no-std compliance
+
+use tendermint;
+use tendermint_proto;
+use tendermint_light_client_verifier;
+
+#[cfg(feature = "sp-core")]
+use sp_core;
+
+#[cfg(feature = "sp-io")]
+use sp_io;
+
+#[cfg(feature = "sp-runtime")]
+use sp_runtime;
+
+#[cfg(feature = "sp-std")]
+use sp_std;
+
+use core::panic::PanicInfo;
+
+/*
+
+This function definition checks for the compliance of no-std in
+dependencies by causing a compile error if  this crate is
+linked with `std`. When that happens, you should see error messages
+such as follows:
+
+```
+error[E0152]: found duplicate lang item `panic_impl`
+  --> no-std-check/src/lib.rs
+   |
+12 | fn panic(_info: &PanicInfo) -> ! {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the lang item is first defined in crate `std` (which `offending-crate` depends on)
+```
+
+ */
+#[cfg(feature="panic-handler")]
+#[panic_handler]
+#[no_mangle]
+fn panic(_info: &PanicInfo) -> ! {
+    loop {}
+}