-
Notifications
You must be signed in to change notification settings - Fork 79
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
Fix/core validation #648
base: dev
Are you sure you want to change the base?
Fix/core validation #648
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #648 +/- ##
==========================================
+ Coverage 54.35% 54.43% +0.07%
==========================================
Files 192 192
Lines 20474 20530 +56
==========================================
+ Hits 11129 11175 +46
- Misses 9345 9355 +10 ☔ View full report in Codecov by Sentry. |
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.
LGTM! just some questions
/// | ||
/// DO NOT use bit encodings which have less than 50 uniformly random bits, | ||
/// since the SSP < 40 bits is widely considered inadequate. | ||
const MAX_HEIGHT: usize = 10; |
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.
is there any reason why 10 is chosen?
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.
No specific reason other than we want to achieve SSP=40 and don't want to mandate that encodings are unnecessarily large. It seemes reasonable that there won't be > 1024 commitments. Although, now I'm realizing that with our json commit-to-everything strategy 1024 may be insufficient.
Let's tag @sinui0 to double check.
This PR adds validation to some of the core types.
Also renames
tree_len
toleaf_count
since "tree length" is too ambiguous.