paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

`core/trie` has not impl for `no_std` #3901

Closed yjhmelody closed 4 years ago

yjhmelody commented 4 years ago

I run following cmd to build core/trie

cargo +nightly build --no-default-features
warning: unused import: `core as std`
   --> core/primitives/src/sr25519.rs:133:5
    |
133 | use core as std;
    |     ^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default

   Compiling substrate-trie v2.0.0 (/mnt/d/code/my-github/substrate/core/trie)
error[E0277]: the trait bound `error::Error: std::error::Error` is not satisfied
  --> core/trie/src/node_codec.rs:43:17
   |
43 | impl<H: Hasher> NodeCodecT<H> for NodeCodec<H> {
   |                 ^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `error::Error`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `substrate-trie`.
warning: build failed, waiting for other jobs to finish...
error: build failed

I find that core/trie has not impl for no_std.

See https://github.com/paritytech/substrate/blob/master/core/trie/src/error.rs#L11

The trie-db has not reexport the no_std Error. And I open a PR to fix it. https://github.com/paritytech/trie/pull/31

yjhmelody commented 4 years ago

BTW, I fix all of above in my local. But there's still many other errors when built in --no-default-features for core/trie.

bkchr commented 4 years ago
  1. We have tests that build substrate-trie for wasm in test-runtime.
  2. We use substrate-trie in Cumulus in wasm.

So, I don't understand your problems.

bkchr commented 4 years ago

Okay, I checked this. I get the same error when I build just the crate. This is probably again some Cargo bug, we can not hack around.

Your linked pr to re-export the no_std error also is not required. The no_std error is implemented for every type. Furthermore the rust error message also says that std::error::Error is not implemented.

cheme commented 4 years ago

It looks like trie crate imports std somewhere, then Error get cast to std error. I would expect following code from 'trie' crate to be enough:

#[cfg(not(feature = "std"))]
impl<T> Error for T {}
bkchr commented 4 years ago

It looks like trie crate imports std somewhere, then Error get cast to std error. I would expect following code from 'trie' crate to be enough:

#[cfg(not(feature = "std"))]
impl<T> Error for T {}

The problem is the imports of trie-bench etc, they import trie-db without disabling the std feature. Cargo merges all features of dev-dependencies and dependencies and then we compile trie-db with std enabled, while substrate-trie is compiled without std.

bkchr commented 4 years ago

Did you read my last comment? That explains it.

yjhmelody commented 4 years ago

Well, I try to understand it.

bkchr commented 4 years ago

The question is still, what do you want to achieve here? Why do you build just the substrate-trie crate?

yjhmelody commented 4 years ago

The client relies heavily on it. Can easy to build some other client functions if it can built in no_std.

bkchr commented 4 years ago

What? Again, substrate-trie compiles for Wasm. We already use it in Wasm.

Given that the issue can not be solved by us and that substrate-trie compiles for Wasm, I will close this issue.