Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of borsh::maybestd #373

Closed
wants to merge 3 commits into from
Closed

Get rid of borsh::maybestd #373

wants to merge 3 commits into from

Conversation

kpp
Copy link
Contributor

@kpp kpp commented Apr 11, 2024

Description

This is a part of upgrade to a new version of borsh.

Remove line pub use borsh::maybestd::{borrow, boxed, collections, format, io, string, vec}; and use alloc crate whenever possible. Instead of borsh::maybestd::hashmap use hashbrown. Instead of borsh::maybestd::io use no_std_io crate.

With a new version of borsh it will be possible to use borsh::io module instead of no_std_io crate.

Testing

make test

@kpp kpp added the T - enhancement New feature or request label Apr 11, 2024
@kpp kpp requested a review from eyusufatik April 11, 2024 20:43
@kpp kpp self-assigned this Apr 11, 2024
@eyusufatik
Copy link
Member

I think using std::Vec etc. might break zk vm compatability. Are you sure about your changes?

@kpp
Copy link
Contributor Author

kpp commented Apr 12, 2024

Alloc::vec::Vec is designed for no_std. Borsh::maybestd reexports Vec from alloc, I did it transparently to reduce code dependency from borsh. In version 1 borsh doesnt have that maybestd module.
And we have ‘make check-nostd’ which performs checks in a nostd env for risc0. Those particular std vecs are okay because they are used along with std Result. It would break on Result first way before vecs.

@kpp kpp marked this pull request as draft April 19, 2024 08:08
@kpp kpp marked this pull request as ready for review April 19, 2024 08:53
@kpp kpp marked this pull request as draft April 19, 2024 12:37
@kpp
Copy link
Contributor Author

kpp commented Jul 9, 2024

Fixed by #851

@kpp kpp closed this Jul 9, 2024
@kpp kpp deleted the kpp/borsh_maybestd branch July 11, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T - enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants