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

serialized_length_from_bytes() #355

Merged
merged 1 commit into from
Dec 20, 2023
Merged

serialized_length_from_bytes() #355

merged 1 commit into from
Dec 20, 2023

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Dec 19, 2023

This PR adds an implementation of serialized_length_from_bytes() where any back-references are validated. Rename the existing function to indicate that the input is trusted (i.e. not fully validated).

In support of this, it also adds:

  • a fuzzer ensuring that any buffer that's accepted by serialized_length_from_bytes() also parses correctly
  • a benchmark comparing parsing with- and without back-references, computing the serialized length with and without validation

objective

The objective is to ensure that the buffer is a valid serialization if it passes this function. That way, an invalid CLVM program fails early, at parse-time, rather than time-of-use. Currently, the Program class in chia_rs (the counterpart to SerializedProgram) is parsed in a relaxed manner, and so might fail later, in run().

This can also tie into trusted- and untrusted parsing of Streamable. So trusted programs don't need to be validated to the same extent.

benchmarks:

     Running benches/deserialize.rs (target/release/deps/deserialize-2f84100aeab5fe87)
deserialize/serialized_length_from_bytes
                        time:   [150.42 µs 150.99 µs 151.54 µs]
deserialize/serialized_length_from_bytes_trusted
                        time:   [44.187 µs 44.339 µs 44.522 µs]
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high mild
deserialize/node_from_bytes_backrefs
                        time:   [239.17 µs 240.30 µs 241.73 µs]
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high mild
deserialize/node_from_bytes
                        time:   [222.05 µs 222.99 µs 223.73 µs]
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) low severe

…ut is not trusted. i.e. back-references are validated. Rename the existing function to indicate that the input is trusted (i.e. not fully validated)
Copy link

coveralls-official bot commented Dec 19, 2023

Pull Request Test Coverage Report for Build 7261386983

  • 119 of 122 (97.54%) changed or added relevant lines in 1 file are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 93.804%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/serde/tools.rs 119 122 97.54%
Files with Coverage Reduction New Missed Lines %
src/serde/tools.rs 4 97.18%
Totals Coverage Status
Change from base Build 7261384226: 0.03%
Covered Lines: 5405
Relevant Lines: 5762

💛 - Coveralls

@arvidn arvidn closed this Dec 19, 2023
@arvidn arvidn reopened this Dec 19, 2023
@arvidn arvidn marked this pull request as ready for review December 19, 2023 12:08
Copy link
Contributor

@richardkiss richardkiss left a comment

Choose a reason for hiding this comment

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

Seems okay.

@arvidn arvidn merged commit 007b049 into main Dec 20, 2023
49 of 50 checks passed
@arvidn arvidn deleted the serialized-length-untrusted branch December 20, 2023 00:52
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