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

Fix no-std compatibility and add check on CI #104

Merged
merged 3 commits into from
Aug 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ jobs:
- run:
name: Run all tests
command: cargo test --all
- run:
name: Check no_std compatibility
command: cd no-std-check/; make setup; make all

lint-rust:
docker:
Expand Down
10 changes: 5 additions & 5 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,24 @@ name = "ics23"
version = "0.8.0"
authors = ["Ethan Frey <[email protected]>"]
edition = "2021"
exclude = ["codegen"]
exclude = ["codegen", "no-std-check"]
description = "Merkle proof verification library - implements Cosmos ICS23 Spec"
repository = "https://github.com/confio/ics23/tree/master/rust"
license = "Apache-2.0"
rust-version = "1.56.1"

[workspace]
members = ["codegen"]
members = ["codegen", "no-std-check"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
prost = { version = "0.11", default-features = false, features = ["prost-derive"] }
bytes = { version = "1.0.1", default-features = false }
hex = { version = "0.4.3", default-features = false, features = [ "alloc" ] }
anyhow = { version = "1.0.40", default-features = false }
sha2 = { version = "0.10.2", optional = true }
sha3 = { version = "0.10.2", optional = true }
ripemd = { version = "0.1.1", optional = true }
sha2 = { version = "0.10.2", optional = true, default-features = false }
sha3 = { version = "0.10.2", optional = true, default-features = false }
ripemd = { version = "0.1.1", optional = true, default-features = false }

[dev-dependencies]
sha2 = { version = "0.10.2" }
Expand Down
1 change: 1 addition & 0 deletions rust/no-std-check/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
target/
11 changes: 11 additions & 0 deletions rust/no-std-check/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[package]
name = "no-std-check"
version = "0.1.0"
edition = "2021"
resolver = "2"

[dependencies]
ics23 = { path = "../", default-features = false }

[features]
panic-handler = []
36 changes: 36 additions & 0 deletions rust/no-std-check/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
NIGHTLY_VERSION=nightly

.DEFAULT_GOAL := help

.PHONY: all setup check-panic-conflict check-cargo-build-std check-wasm help

all: ## Run all checks
$(MAKE) check-panic-conflict
$(MAKE) check-cargo-build-std
$(MAKE) check-wasm

setup: ## Setup the required nightly toolchain and the wasm32 target
rustup install $(NIGHTLY_VERSION)
rustup target add wasm32-unknown-unknown --toolchain $(NIGHTLY_VERSION)
rustup component add rust-src --toolchain $(NIGHTLY_VERSION)

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.
cargo build \
--no-default-features \
--features panic-handler

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`.
rustup run $(NIGHTLY_VERSION) -- \
cargo build -Z build-std=core,alloc \
--no-default-features \
--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.
rustup run $(NIGHTLY_VERSION) -- \
cargo build \
--features panic-handler \
--target wasm32-unknown-unknown

help: ## Show this help message
@grep -E '^[a-z.A-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

191 changes: 191 additions & 0 deletions rust/no-std-check/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
# `no_std` Compliance Check

This crate checks the `no_std` compliance of the `ics23` crate.

Based on the corresponding check in the [`ibc-rs`](https://github.com/informalsystems/ibc-rs) project.

## 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.

## Checking Single Unsupported Dependency

By default, the check scripts try to build all unsupported dependencies and will fail. To test if a particular crate still fails the no_std check, edit the `use-unsupported` list in [Cargo.toml](./Cargo.toml) to uncomment all crates except the crate that we are interested to check. For example, to check for only the `getrandom` crate:

```toml
use-unsupported = [
# "tonic",
# "socket2",
"getrandom",
# "serde",
# ...,
]
```

## Adding New Dependencies

For a crate named `my-package-1.2.3`, first try and add the crate in [Cargo.toml](./Cargo.toml) of this project as:

```toml
my-package = { version = "1.2.3" }
```

Then comment out the `use-unsupported` list in the `[features]` section of Cargo.toml and replace it with an empty list temporarily for testing:

```toml
[features]
...
use-unsupported = []
# use-unsupported = [
# "tonic",
# "socket2",
# "getrandom",
# ...
# ]
```

Then import the package in [src/lib.rs](./src/lib.rs):

```rust
use my_package
```

Note that you must import the package in `lib.rs`, otherwise Cargo will skip linking the crate and fail to check for the panic handler conflicts.

Then run all of the check scripts and see if any of them fails. If the check script fails, try and disable the default features and run the checks again:

```rust
my-package = { version = "1.2.3", default-features = false }
```

You may also need other tweaks such as enable custom features to make it run on Wasm.
At this point if the checks pass, we have verified the no_std compliance of `my-package`. Restore the original `use-unsupported` list and commit the code.

Otherwise if it fails, we have found a dependency that does not support `no_std`. Update Cargo.toml to make the crate optional:

```rust
my-package = { version = "1.2.3", optional = true, default-features = false }
```

Now we have to modify [lib.rs](./src/lib.rs) again and only import the crate if it is enabled:

```rust
#[cfg(feature = "my-package")]
use my_package;
```

Retore the original `use-unsupported` list, and add `my-package` to the end of the list:

```toml
use-unsupported = [
"tonic",
"socket2",
"getrandom",
...,
"my-package",
]
```

Commit the changes so that we will track if newer version of the crate would support no_std in the future.

## 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.

### Overrride 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.
36 changes: 36 additions & 0 deletions rust/no-std-check/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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 ics23;

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 {}
}