Tracking issue for no-std support

[soareschen]

This is a tracking issue to track the progress of no-std support for ibc-rs to close #29. The change set for PR #1109 is pretty broad, and as a result it is difficult to review and merge the PR all in one go. I would suggest that we split the changes into multiple smaller PRs, so that the changes can be incremental with the goal of eventually supporting no-std.

Based on the changes in PR #1109, I'd suggest we have separate PRs for the following changes:

• Full no-std support for ibc crate #1846

• Tracking issue for no-std compliance for dependencies in ibc-rs and tendermint-rs #1359

• Full support of no_std in ibc crate #1390

• Full support of no_std in tendermint crate tendermint-rs#988

• Separate generated code from prost and tonic in proto-compiler cosmos/ibc-proto-rs#7

• Move out tendermint::config to tendermint-config crate tendermint-rs#986

• Split out verifier parts of tendermint-light-client to a separate no_std compliant crate tendermint-rs#1027

• Remove use of Time::now() in ibc crate #1612

• Add --build-tonic flag to proto-compiler, sync protobuf script, and generate protobuf for std and no_std mode #1439

• tendermint-proto uses std features in dependencies tendermint-rs#981

• Use core and alloc crates for no_std compatibility (Take 2) tendermint-rs#993

• Move config module out from tendermint into its own crate tendermint-rs#983

• url crate used by tendermint does not support no_std tendermint-rs#985

• tendermint crate uses std features in ed25519-dalek tendermint-rs#992

• Use flex-error to define error types (Use flex-error to define errors #988)

  • This is to replace anomaly for fine grained error handling. While not directly related to no-std, the PR removes majority use of std::error::Error and allows different error tracers such as anyhow, eyre, or naive StringTracer to be used, depending on their support in no-std environment.

• Use core and alloc crates in place of std crate whenever possible (Use core and alloc crates for no_std compatibility #1156)

  • The crates core and alloc can be accessed by both std and no-std code, and therefore do not require feature flag to switch between importing from core/alloc or std.
  • To use alloc in std mode, we just have to add an extern crate alloc in the root module, and that is automatically linked by Rust in std mode.
  • There are still a small number of imports from std that are not available in core nor alloc. In that case, the PR leaves the use of std in those cases, and have them fixed in follow up PRs.
  • The changes to use core and alloc can be done in a separate PR independent of other changes. While we cannot ensure that the change covers everything, this will reduce the number of changes required in subsequent PRs and make it easier for review.

• Use a wrapper struct for Box<dyn std::error::Error> for remaining errors not updated by Use flex-error to define errors #988.

  • PR Use flex-error to define errors #988 have removed most of the uses of Box<dyn std::error::Error>, however there are still a few places remaining that are still using std::error::Error.
  • PR Add no-std support to ibc #1109 attempts to polyfill std::error::Error by aliasing it to a DummyError struct in no-std mode. However doing so does not prevent dependencies from constructing Box<dyn std::error::Error>, which would prevent no-std from being supported. On the other hand, if we have control of all places that produce Box<dyn std::error::Error>, we can also modify them to return something other than Box<dyn std::error::Error>.
  • I'd propose that we define a type like LegacyError, which is either a newtype wrapper to Box<dyn std::error::Error> in std mode, or a newtype wrapper to String in no-std mode. By always using a newtype wrapper, we can avoid accidental implicit coercion from Box<dyn std::error::Error> to LegacyError in std mode, which will prevent no-std mode to work.
  • There should be a Cargo separate feature flag to switch between the two modes ot LegacyError. The purpose is to be able to test the two modes of LegacyError without fully activating no-std.
  • This change can be done inside Add no-std support to ibc #1109 Use flex-error to define errors #988, or a follow up PR to Add no-std support to ibc #1109 Use flex-error to define errors #988. It is also possible to make this change against master, although there will be much more changes needed.
  • Update: This is no longer needed as all use of std::error::Error are now removed.

• Find alternative crates that replace missing functionalities in core and alloc.

  • There are still a small number of essential types like SystemTime that are provided by std, but not provided by core nor alloc.
  • PR Add no-std support to ibc #1109 attempts to polyfill the types like SystemTime inside no-std mode. However this makes it difficult to maintain two version of the code depending on the std/no-std mode.
  • Ideally we should instead try and find alternative crates that can work in both std and no-std mode.
  • For example, we may want to investigate whether it is possible to use chrono::DateTime directly instead of SystemTime. If we can, then it would reduce our burden of having to maintain the code in different modes.

• Update Cargo.toml to use updated dependencies that have been published and support no-std, including tendermint-rs crates.

  • PR Add no-std support to ibc #1109 has a [patch.crates-io] section that refers to upstream dependencies like tendermint-rs that are also being patched to support no-std. This makes it difficult to review and merge the PR, because we cannot easily see what is being changed in the upstream dependencies. Even if the reviews pass, the PR cannot be merged because we cannot publish Cargo crates with git dependencies inside.
  • We should first make changes in ibc-rs that do not require upstream dependencies to be updated. Even if that do not result in supporting no-std in one go, making independent changes will help reduce the number of changes necessary when the dependencies are updated.
  • The PR for updating upstream dependencies should be done after the other PRs for no-std have been merged, and also when the upstream dependencies have been published. Ideally this PR should have minimal changes, other than updating Cargo.toml.

• Add Cargo feature flags to activate #![no_std].

  • Other than the issues listed above, there might be other blockers that prevent no-std mode from being enabled. In such case, it is still better to merge the other PRs first, and only activate no-std when everything is fixed.
  • I am listing this as a separate step, so that it does not become a blocker for the other PRs. The point is we do not need to have one or few big PRs that immediately pass when #![no_std] is enabled.
  • The priority for the other PRs is to continue to have ibc-rs work in std mode, and not be concerned with whether they compiles in no-std. We can still maintain our own private branches that activate no-std during development.
  • Ideally, this final PR should have just few line changes to introduce the feature flags and #![no_std].
  • Note that as mentioned in Use #![no_std] for ibc-proto crate #1164, #![no_std] alone is still not sufficient for true no-std support, as dependencies may still use std behind the scene. The current known way to verify for true no-std support is to compile the crate with the target wasm32-unknown-unknown.
  • This is also the PR that enables the CI checks to ensure that no-std continues to be supported in the future.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[DaviRain-Su]

Hi, I'm glad to see you've written this in such detail, and I agree with what you've written when you list this. After all the skimming through the third one listed, this one I think can go for it. For the penultimate one, I don't know who is in charge of the tendermint-rs. over there. I would like someone to review my pr to merge the code in and fix this issue.

[soareschen]

For the penultimate one, I don't know who is in charge of the tendermint-rs. over there. I would like someone to review my pr to merge the code in and fix this issue.

I am also responsible to having no_std supported in tendemint-rs. I am still looking at informalsystems/tendermint-rs#890, but got side tracked by something else, sorry for haven't giving any feedback yet.

That being said, I think the work need to be done on tendermint-rs side should be similar with what's mentioned in this issue, and I will create a separate tracking issue for that. I also think that the two developments can be done in parallel, and we'd have a separate PR to update tendermint-rs in ibc-rs once the changes have been merged on tendermint-rs side.

[soareschen]

I found this issue on substrate (paritytech/substrate#4043), which mentions that the std crate is in fact partially supported in WebAssembly. However Substrate decides to not support std because the std implementation is not stable. This means that the need for no-std is more of a Substrate-specific issue, rather than a WebAssembly-specific issue.

Because of this, I think we should refine the acceptance criteria of what we are aiming for here:

• Crate-local no-std compliance: our own crates can be built with #![no_std] enabled, but the dependencies may still be using std. Example: Use #![no_std] for ibc-proto crate #1164 enables no-std on ibc-proto, but its dependent prost is still using std.
• Full no-std compliance: Our own crates and all dependencies can be built with #![no_std] enabled. This can be verified following this guide.
• WebAssembly compliance: Our crates can be built with WebAssembly targets such as wasm32-unknown-unknown and wasm32-wasi. Since std is still partially available there, either our crates or dependencies may use some of the std features available.
• Substrate compliance: Our crates can be built and run on the Substrate platform. The exact step to verify this needs to be clarified.

The first three acceptance criteria are rather straightforward to verify. However I have not been able to find good source of verifying for no-std compliance on Substrate. The only information available is on the guide which shows how to configure Cargo.toml feature flags to switch between std and no_std modes. It is important to note that feature flags alone cannot verify that crates compiled without the std features are truly no-std compliance, and the Substrate documentation do not explain further on how to verify on this.

What I found instead is that Substrate crates like sp-io seems to also register panic handlers when the std feature flag is absent, which conflicts with the panic handler in std. So the question becomes: Is the acceptance criteria for Substrate compliance such that it can import crates like sp-io without the std feature flags?

[soareschen]

Related issue on checking for no_std support: rust-embedded/wg#64

[romac]

Thanks for the write-up, @soareschen!

My understanding is that at the moment, we want both WebAssembly (wasm32-unknown-unknown ) and Substrate compliance (the latter seems to require the former).

The first three acceptance criteria are rather straightforward to verify. However I have not been able to find good source of verifying for no-std compliance on Substrate. The only information available is on the guide which shows how to configure Cargo.toml feature flags to switch between std and no_std modes. It is important to note that feature flags alone cannot verify that crates compiled without the std features are truly no-std compliance, and the Substrate documentation do not explain further on how to verify on this.

What I found instead is that Substrate crates like sp-io seems to also register panic handlers when the std feature flag is absent, which conflicts with the panic handler in std. So the question becomes: Is the acceptance criteria for Substrate compliance such that it can import crates like sp-io without the std feature flags?

@DaviRain-Su Can you please chime in here to provide with more details on this? Especially as to how we can test that the ibc and ibc-proto crates are compatible with Substrate?

[DaviRain-Su]

Substrate compliance: Our crates can be built and run on the Substrate platform. The exact step to verify this needs to be clarified.
@DaviRain-Su Can you please chime in here to provide with more details on this? Especially as to how we can test that the ibc and ibc-proto crates are compatible with Substrate?

Regarding the validation on substrate, we use ibc to implement pallet (a term used in substrate) and the use of pallet is registered to the runtime, the final compilation output in substrate is native binary, and wasm files, these pallet files will be compiled into wasm files. files will also be compiled into wasm files. So the conclusion here is that the verification result on substrate is to set the compilation target to the WebAssembly target.

relate toutials: https://substrate.dev/docs/en/tutorials/add-a-pallet/import-a-pallet

his is important to enable the Substrate runtime to compile to both native binary, which supports Rust std, and Wasm binary, which do not (see: no_std).
The use of Wasm runtime binaries is one of Substrate's defining features. It allows the runtime code to become a part of a blockchain's evolving state; it also means that the definition of the runtime itself is subject to the cryptographic consensus mechanisms that ensure the security of the blockchain network. The usage of Wasm runtimes enables one of Substrate's most innovative features: forkless runtime upgrades, which means that Substrate blockchain nodes can stay up-to-date and even acquire new features without needing to be replaced with an updated application binary.

This means that the substrate platform can also be applicable as long as the requirements proposed in the previous three are suitable.

[adizere]

@DaviRain-Su many thanks for your support with no_std so far!
I just wanted to check with you on the timeline for no_std support in ibc-rs: If we're targeting end of September to close this issue, would that align with Octopus Network plans?

[DaviRain-Su]

@DaviRain-Su many thanks for your support with no_std so far!
I just wanted to check with you on the timeline for no_std support in ibc-rs: If we're targeting end of September to close this issue, would that align with Octopus Network plans?

Oh, thanks, this time is in line with our expectations

[DaviRain-Su]

@soareschen Hi, I would like to know how this issue is progressing now.

[soareschen]

@DaviRain-Su I have some time to work on this for the next few weeks. Many changes have been merged and moving us closer to no_std support. However there are still quite a number of blockers, in particular on the no_std support from the dependencies. I have identified some of the main issues that still need to be addressed and updated the list in the description.

At the moment it is not clear how much time is needed to resolve the current issues, and how many new issues will still be uncovered.

[DaviRain-Su]

@DaviRain-Su I have some time to work on this for the next few weeks. Many changes have been merged and moving us closer to no_std support. However there are still quite a number of blockers, in particular on the no_std support from the dependencies. I have identified some of the main issues that still need to be addressed and updated the list in the description.

At the moment it is not clear how much time is needed to resolve the current issues, and how many new issues will still be uncovered.

For substrate-ibc that depends on ibc-rs now compiles through in substrate, I think it can be merged into the main branch.

[DaviRain-Su]

Hi @soareschen , I encountered some errors when integrating substrate-ibc to substrate-parachain-template , can you help me to take a look at it, my previous node-template on substrate was tested and passed, the difference between the two is the version of substrate. The version of substrate-parachain-template template is polkadot-v0.9.12.

This is error:

    Checking pallet-vesting v4.0.0-dev (https://github.com/paritytech/substrate?branch=polkadot-v0.9.12#d76f3999)
error[E0277]: the trait bound `ics23::ProofSpec: informalsystems_prost::Message` is not satisfied
  --> /Users/davirain/.cargo/git/checkouts/ibc-rs-b37d925b86d85d59/8da1b06/modules/src/ics23_commitment/specs.rs:40:36
   |
40 |             prost::Message::encode(ds, &mut encoded).unwrap();
   |             ---------------------- ^^ the trait `informalsystems_prost::Message` is not implemented for `ics23::ProofSpec`
   |             |
   |             required by a bound introduced by this call
   |
note: required by `informalsystems_prost::Message::encode`
  --> /Users/davirain/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/informalsystems-prost-0.8.1/src/message.rs:47:5
   |
47 | /     fn encode<B>(&self, buf: &mut B) -> Result<(), EncodeError>
48 | |     where
49 | |         B: BufMut,
50 | |         Self: Sized,
   | |____________________^

substrate-ibc: https://github.com/octopus-network/substrate-ibc.git and branch is dv-ibc-dev-0.9.12
substrate-parachain-template: https://github.com/octopus-network/substrate-parachain-template.git and branch is master

[mzabaluev]

@DaviRain-Su Looks to me like informalsystems-prost version used to build the component does not match the version that is used in the generated impls. Try checking with cargo tree.

[DaviRain-Su]

Looks to me like informalsystems-prost version used to build the component does not match the version that is used in the generated impls. Try checking with cargo tree.

Thx,I'll give it a try

[soareschen]

Yes, I am updating this branch so that it will be using the same prost version as master.

[DaviRain-Su]

Yes, I am updating this branch so that it will be using the same prost version as master.

Okay, when you fix it, you can @DaviRain-Su

[DaviRain-Su]

@DaviRain-Su Looks to me like informalsystems-prost version used to build the component does not match the version that is used in the generated impls. Try checking with cargo tree.

After I run this command, I see that all the versions of this library(informalsystems-prost)us
ed are the same.

[soareschen]

@DaviRain-Su the crate informalsystems-prost was a temporary fork we used to patch up bugs in prost. Since then new versions of prost have been released, and we have switched back to directly use prost instead. You'll need to remove the use of package = "informal-systems-prost" in Cargo.toml and update prost and tonic to the latest version.

I have updated #1390 to keep up to date to master. But now the no_std support is broken, with various new issues such as tendermint-rs/issues/1027 and #1612.

[soareschen]

@DaviRain-Su please use the branch in #1613, which I have updated the original branch with new versions of prost and tonic.

As mentions, the support for no_std #1390 is currently broken. It may take a while before we can fix everything again. Just be aware that #1613 has been lagging behind master since October, and there have been many breaking changes made on master since then.

[DaviRain-Su]

@DaviRain-Su please use the branch in #1613, which I have updated the original branch with new versions of prost and tonic.

As mentions, the support for no_std #1390 is currently broken. It may take a while before we can fix everything again. Just be aware that #1613 has been lagging behind master since October, and there have been many breaking changes made on master since then.

Yes because of this problem, I found out for a long time a few days ago because of this problem that the upper dependency of tendemint-rs has been modified, no-std has not compiled over, and now I have fixed tendermint-rs to a rev that can be run at that time, as a temporary development now.

[soareschen]

Update: the full support for no_std with the latest version of ibc-rs is now available at #1846. There are just some minor fixes need to be upstreamed to ics23 and safe-regex. Once that is done we can get full support for no_std on master.

[soareschen]

The master branch now fully supports no_std, albeit with some patches needed for the dependencies. This issue can now be closed with #1858 tracking the last bit of requirements for full support of no_std without requiring the patches.