-
Notifications
You must be signed in to change notification settings - Fork 1
Create a Rust runtime for LB Prelude #105
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bladyjoker
reviewed
Oct 10, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great freaking job! Left some comments, let me know wdyt.
bladyjoker
reviewed
Oct 10, 2023
bladyjoker
approved these changes
Oct 17, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is lovely!
Let me know if you need help syncing this with main.
…-buffers into szg251/rustfmt
Custom pre-commit hooks for our monorepo setup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #103
Some notes on the implementaton:
nix integration
I used nix-cargo-integration (which builds on dream2nix). This hides a lot of complexity with reasonable default configurations. It also uses flake-parts, so making the
build.nix
a flake module seemed like a natural choice, but it diverges from the current status quo, so there's a separate issue to bring everything to the same conventions.Types
Integer: num_bigint is used in cardano_serialisation_lib
ByteString:
&[u8]
orVec<u8>
are the obvious choices, I'm not sure if there's too much difference from the end-user standpoint.Json serialisers
I followed the same principles that we used in Hs and Ps packages:
to_json
instances serialise to aValue
(intermediary JSON representation). However I'm not sure, if this is the most convenient way from the end-user perspective, we will have to experiment with this later.Another question mark is error messages: I'm still not familiar with the messages
serde
returns, so I basically copied the error messages from the Haskell implementation. To make this library feel more natural, I think these should be adjusted in the future.Property testing
There are two popular options in Rust for property testing:
quickcheck
andproptest
.The former is a simple library, mostly following the Haskell implementation and design principles.
proptest
is a more robust (and a little bit slower) tool, with a lot of nice features.I decided to go with the latter one, as it allowed me to write simply implement BigInt generators. Also, I simply liked the API of the library.
One area which is still not clear to me is
proptest-regressions
. When a proptest fails, the input values are stored in the project directory, and used in later tests. I decided to put this into.gitignore
, I'm not exactly sure if this is working exactly as I desired (e.g. these tests logs remain even if I change the tests).Links to related issues