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

Support no_std #87

Merged
merged 11 commits into from
Apr 7, 2023
Merged

Support no_std #87

merged 11 commits into from
Apr 7, 2023

Conversation

preston-evans98
Copy link
Collaborator

@preston-evans98 preston-evans98 commented Apr 1, 2023

This commit allows the JMT to be used in no_std environments. The trie still requires alloc.

To implement this functionality, the following non-trivial changes are made:

  1. Replace all of the old serialization with Borsh derives. This is a breaking change which alters the state roots computed by the JMT. Correction: serialization is only used for the database, so this change does not alter state roots
  2. Remove support for multithreaded testing from the provided mock tree store. This functionality was unused, but if desired it can be re-implemented behind a feature-gate.
  3. Remove the (unused) LegacyInternal NodeType.

In addition, there are numerous minor changes to imports. Finally, the trie's built-in metrics are gated behind the "metrics" feature.

Copy link
Contributor

@plaidfinch plaidfinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good!

The one blocker I think is the replacement of RwLock with RefCell in the MockTreeStore: I think we don't want this, because it limits the utility of the MockTreeStore. More on that below and possible different approach that preserves no_std.

A couple of thoughts about backwards compatibility: I think we do not need to preserve it, and we should break it while we can (i.e. while there are not downstream users other than ourselves). What do you think?

src/mock.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
src/node_type.rs Show resolved Hide resolved
src/node_type.rs Outdated Show resolved Hide resolved
This commit allows the JMT to be used in no_std environments. The trie
still requires alloc.

To implement this functionality, two non-trivial changes are made.
1. Replace all custom serialization with Borsh. **This is a breaking
   change which alters the state roots computed by the JMT**.
2. Remove support for multithreaded testing from the provided mock tree
   store. This functionality was unused, but if desired it can be easily
   re-implemented behind a feature-gate.

In addition, there are numerous minor changes to imports. Finally, the
tries built-in metrics are gated behind the "metrics" feature.
A bug in the previous commit disabled all code when building with std.
This commit removes the erroneous #![cfg(...)] and fixes some imports
in the std version which caused builds to fail when the bug was fixed.
Remove test vector dep on std
Copy link
Contributor

@plaidfinch plaidfinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Feel free to merge once CI has passed.

@preston-evans98 preston-evans98 merged commit 6a44cc1 into main Apr 7, 2023
@preston-evans98 preston-evans98 deleted the nostd branch April 7, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants