Use core and alloc crates for no_std compatibility (Take 2)

[soareschen]

Part of informalsystems/ibc-rs#1158

This follows similar work in informalsystems/ibc-rs#1156 to move closer to supporting no_std in tendermint-rs.

This is a succession of #980 with the changes to replace the use of std::time::SystemTime being moved to another PR.

• Referenced an issue explaining the need for the change
• Updated all relevant documentation in docs
• Updated all code comments where relevant
• Wrote tests
• Added entry in .changelog/

[codecov-commenter]

Codecov Report

Merging #993 (604e631) into master (7637b2f) will decrease coverage by 0.0%.
The diff coverage is 93.3%.

❗ Current head 604e631 differs from pull request most recent head 4faa88d. Consider uploading reports for the commit 4faa88d to get more accurate results

@@           Coverage Diff            @@
##           master    #993     +/-   ##
========================================
- Coverage    72.8%   72.8%   -0.1%     
========================================
  Files         206     206             
  Lines       16635   16638      +3     
========================================
- Hits        12126   12121      -5     
- Misses       4509    4517      +8     

Impacted Files	Coverage Δ
config/src/config.rs	65.8% <ø> (+0.7%)	⬆️
config/src/lib.rs	100.0% <ø> (ø)
config/src/net.rs	92.3% <ø> (ø)
config/src/node_key.rs	56.5% <ø> (ø)
config/src/priv_validator_key.rs	57.8% <ø> (ø)
proto/src/lib.rs	100.0% <ø> (ø)
proto/src/serializers/bytes.rs	80.0% <ø> (ø)
proto/src/serializers/optional_from_str.rs	0.0% <ø> (ø)
proto/src/serializers/part_set_header_total.rs	100.0% <ø> (ø)
proto/src/serializers/time_duration.rs	87.5% <ø> (ø)
... and 128 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7637b2f...4faa88d. Read the comment docs.

[xla]

Quite some work, impressive. Would it be possible to consolidate the multiple preludes into a shared crate to ease with maintenance down the line?

[soareschen]

Would it be possible to consolidate the multiple preludes into a shared crate to ease with maintenance down the line?

I am a bit reluctant to expose the internal prelude as public interface. It will make it more difficult for us to remove it in the future.

It is also not clear that if we do want to expose it, which crate should it be in. The lowest common denominator of all tendermint-rs and ibc-rs crates is probably tendermint-proto. But it also doesn't feel right to put it there. I also don't think it is worth a hassle to publish it as a separate crate.

In any case, I think this is just a temporary measure with minimal content that we can manage with. Once the alloc::prelude crate is graduated from nightly at rust-lang/rust#58935, or in case the Rust team decide to go with different approaches, it is much easier for us to remove/alter the prelude.rs in the individual crates than having to do a major version release with deprecation and other hassles.

[thanethomson]

Should this not be the following?

Suggested change

	tendermint-config = { version = "0.23.0-internal", path = "../config" }
	tendermint-config = { version = "0.23.0-internal", path = "../config", default-features = false }

[soareschen]

Yeah that is about right. Actually the main reason why the option default-features = false is added everywhere is because there are always weird interaction between the feature flags of each crate that can somehow activate the use of std.

Worse yet it seems like other crates within the same project repository may also affect the feature flags enabled, so we still have to disable default-features in all crates so that they do not interfere with the features in the crates supporting no_std.

Technically, we could only selectively add default-features = false to specific crates that are found to interfere with the no_std features. But identifying the offending crates are too tedious, and so it is better off to just add that to every single dependency we use and not worry about it.

[thanethomson]

Following from the comment below regarding the tendermint-config dependency, should we not also be enabling the std feature of tendermint-config as follows?

Suggested change

	]
	"tendermint-config/std",
	]

[soareschen]

Actually with a clearer thoughts now, I think it is safe for us to remove all the "std" feature in each of tendermint-rs and ibc-rs crates. Currently the only use of the default "std" feature in our crates is to activate the flex-error/eyre_tracer feature so that the eyre error tracer is used for richer error information. But we can easily activate flex-error/eyre_tracer feature at the CLI crate like ibc-relayer-cli without having go through the individual dependencies, and still get the same benefit.

I have pushed a new commit to remove all use of "std" features in tendermint crates so that we do not need the same std feature boilerplate as used in Substrate.

[thanethomson]

One more question here: do these changes modify the API in any meaningful way? i.e. will they impact the way that users depend on/import our crates?

If so we should include a breaking changelog entry.

[thanethomson]

Also I see the tools build's failing: https://github.com/informalsystems/tendermint-rs/runs/4009051636?check_suite_focus=true

[soareschen]

One more question here: do these changes modify the API in any meaningful way? i.e. will they impact the way that users depend on/import our crates?

I think there shouldn't be any change in the API. I have changed the default features of each tendermint-rs crate to ["flex-error/std", "flex-error/eyre_tracer"] so that the eyre tracer in flex-error is used by default unless default-features = false is set. That way there should be less surprise when user try to debug errors when building tendermint with the default features.

One potential breaking change is the removal of the "std" feature of the tendermint crates. I'll also remove the std features on ibc-rs side so that we do not run into the same std boilerplate required in Substrate.

[thanethomson]

I'd say we're good to go here. Thanks @soareschen!