add no-std

[claravanstaden]

• Adds no-std support by adding std module that imports from std or core/alloc depending if std feature is toggled.
• Removes thiserror crate, no support for no-std yet, but in the works. It depends on Error being included in the core crate: Move Error trait into core rust-lang/project-error-handling#3 and thiserror issue: add #![no_std] support dtolnay/thiserror#196

[ralexstokes]

it looks like some of the changes in here were done in #29; please rebase main when you get a chance -- very excited to dig into this PR :)

[ralexstokes]

this is amazing! nice work :)

I left some questions/comments on an initial pass

At a high-level, I think this direction makes sense -- the only thing I'd suggest is to take inspiration from the serde library which also supports std vs no-std in this way but they have a central lib that does the import logic once and then can simply use lib::*; everywhere else

see here: https://github.com/serde-rs/serde/blob/master/serde/src/lib.rs#L151

and here for usage: https://github.com/serde-rs/serde/blob/master/serde/src/ser/mod.rs#L110

I'll want to make another pass to look more deeply into the thiserror issue and also investigate some of the dependency changes but I like the direction this is going in and think no-std is a great addition to this library

[claravanstaden]

@ralexstokes thanks for the review! I have made the requested individual changes as well as the std lib imports as modelled by the serde crate in f29b3fe. Let me know if this is what you've had in mind. I can also move the crate std mod into lib, if you prefer (but I guess then the import would just be crate::*, which is less clear)? Or rename it something else. 😄

[ralexstokes]

nice updates!

things are looking good, I left a few questions

[ralexstokes]

Or rename it something else.

I like how serde does it and just calls it lib

can you update the mod name and then the imports so that it is crate::lib::* etc? thank you!

[ralexstokes]

also the fact that the tests are passing even though we are 'ignoring' some errors suggests the code coverage is not ideal -- I'll leave a note here to investigate further

[claravanstaden]

I like how serde does it and just calls it lib

can you update the mod name and then the imports so that it is crate::lib::* etc? thank you!

Done in d776326. I removed the std mod and created a lib mod in lib.rs, since lib.rs exists already.

[claravanstaden]

also the fact that the tests are passing even though we are 'ignoring' some errors suggests the code coverage is not ideal -- I'll leave a note here to investigate further

This is a little worrying. The dropped error check I introduced was in pack_bytes, used by hash_tree_root, which the individual mod tests don't cover. But it is still concerning that the container tests didn't pick them up.

I had a quick look but must admit I couldn't quickly see how to add test cases that would trigger those error conditions.

[dharjeezy]

Is this still being worked on? Is there any way i can contribute to this?

[claravanstaden]

Is this still being worked on? Is there any way i can contribute to this?

@dharjeezy it's ready for review! @ralexstokes will probably get to it as soon as he can. 😄 You are welcome to also review and test if you are keen.

[ralexstokes]

a few comments to resolve, but I think we are only a round or two of back/forth before we can merge :)

[claravanstaden]

@ralexstokes thanks for the review! 😄 Comments actioned.

[ralexstokes]

reviewed the Cargo manifest and I think we can tighten it up some

I'd like to get to a point where the default thing is the status quo and only if you do --no-default-features will you get a no_std compatible library

[ralexstokes]

another round of questions/comments :)