integritee-network / pallets

Other
10 stars 14 forks source link

Remove sgx feature where not strictly necessary #66

Closed haerdib closed 1 year ago

haerdib commented 2 years ago

We have now primitives crate that include sgx_tstd and thiserror_sgx but only to be able to use thiserror. However, this makes the crate by default not no_std compatible, as either sgx or std must be enabled.

Question: Is the thiserror feature important enough to keep the whole crate non no_std compatible, or is the From derivation enough? -> No, it's not. Since core2 does not seem to be a good option, remove thiserror completely and use Format or similiar instead.

Example crate: https://github.com/integritee-network/pallets/blob/master/primitives/utils/Cargo.toml

Possible thiserror_sgx alternative: https://crates.io/crates/thiserror_core2

clangenb commented 2 years ago

If it is only this error, I'd vote for making the crate fully no_std compatible. We lose some backtrace information, but I think this is acceptable.

Personally, I am having doubts about core2. Sounds like a wonderful feature, but it does not seem to be used widely, which concerns me a little. However, this is only my first impression, and I am very open to other opinions.

Regardless, independently of that, I believe using core2 just because of the error is a massive overkill. It brings in many deps into the runtime, which we should avoid. This could also be a reason, why it is not used widely, as no_std applications are usually running on constrained hardware and demands small and efficient binaries.

clangenb commented 1 year ago

I think this issue should be in the worker, doesn't it?

haerdib commented 1 year ago

Heh, yeah, you're right. 🙊

clangenb commented 1 year ago

This is in the wrong repository and will be obsolete once we switch to teaclave v2. I will close the issue.