-
Notifications
You must be signed in to change notification settings - Fork 101
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, test, and enable custom domain tags. #116
Conversation
261d5c9
to
ba88a2f
Compare
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.
In my opinion, it is doing the right thing, but another pair of eyes would be great.
ba88a2f
to
9e3bf89
Compare
Looks good to me |
9e3bf89
to
00a87a7
Compare
HashType::Encryption => with_strength(pow2::<F>(32)), | ||
// identifier * 2^40 | ||
// identifier * 2^(40 + strength) -- where standard strength is 0. |
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.
The formula is a bit wrong I think,
it should be identifier * 2^40 + strength * 2^32
.
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.
Yes, I think that's correct.
Fill in the blanks of the custom domain tag implementation, and enable it — with support for 256 unique tags. Although it would be possible to add more, I wanted to be conservative, and this should be plenty.
The way the tag space is divided up and
Standard
/Strengthened
variants are supported makes this already a little crowded and confusing. Let's minimize the extent to which we paint our future selves into a corner by annexing only what we need for now.