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
Add two Poseidon Filecoin variant multihashes #171
Conversation
The above is a good summary of our long discussion earlier today. |
@@ -425,6 +425,8 @@ skein1024-1000, multihash, 0xb3dd, | |||
skein1024-1008, multihash, 0xb3de, | |||
skein1024-1016, multihash, 0xb3df, | |||
skein1024-1024, multihash, 0xb3e0, | |||
poseidon-bls12_381-a2-fc1, multihash, 0xb401, Poseidon using BLS12-381 and arity of 2 with Filecoin parameters |
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.
I use some automatic conversion in JS libs in regards to dashes and underscores. I meant to document those, but haven't really. But the rules are (IIRC) only use alphanumeric characters and dashes. Would it be ok to switch this to the slightly less readable `poseidon-bls12-381-a2-fc1?
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 that able to be changed? the reason that change bothers me is that you can no longer split it by logical sections. If we end up with 50 of these, -
is no longer a useful separator without additional logic. Replacements for BLS12-381 include bn
and ed
for now, possibly more in the future, including other BLS12-XXX variants. But you can see with bn
how the scheme would break down. poseidon-bn-a32-boop1?
. bls12381
is an option too, just less clear I think.
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.
Feedback from FIL team is bls12381
would not be the worst thing in the world, bls12q381
is an option too, but still not optimal; a clear separator is preferred.
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.
To explain what I'm doing. I create constants out of the names. Constants (in many languages) are all uppercase with underscore as separator. Converting it one way is easy poseidon-bls12_381-a2-fc1
to POSEIDON_BLS12_381_A2_FC1
. Probably you would keep the original around in order to use it in error message.
I just though it would be nice to have the option to have a two-way conversion back to the original name, but that isn't a must. I thought restricting the characters would make things simpler, but if it makes things harder to use/read, we can surely have more characters.
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.
Ah yeah, I can feel the awkwardness of that. And the significance of -
vs _
would get lost in the conversion because they'd all be _
anyway. Maybe we just need to force bls12q381
on them.
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.
One quick thought, in case it can rescue this: what if you convert _ to __ (double underscore)? As long as we also established the rule that source names will never contain two of either - or _ in a row, then that would be unambiguous and fairly legible — preserving the distinction visually as well.
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.
yeah, although __
can get out of hand really quickly if people aren't conscious of it in their naming - if @vmx is the one responsible for converting and the people adding names here aren't consuming those constants then they're not going to feel the pain of FOO__BAR_2_3__456__BLIP_BORK
. I'll leave the call to @vmx because it'd be a policy change.
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.
Given that the conversion from a constant back into the name is rather hypothetical (I checked the JS code again, I don't think I ever do that), I think it's fine to have deterministic rules to convert from name into an all uppercase constant name. Hence I suggest that we just allow underscored, which will stay underscores. Allowed characters would then be [a-z0-9-_]
.
So I would expect in most languages poseidon-bls12_381-a2-fc1
being used as a constant named POSEIDON_BLS12_381_A2_FC1
.
Give a thumbs up if that sounds like a plan.
Reserving the 0xb400 range for Poseidon variants, allowing FIL to iterate on the `fcX` extension of the name where they stay with BLS12-381 and arity=2. High security variant is for extra circuits that are usable in case new attacks arise from the standard variant. Ref: #161 Ref: https://eprint.iacr.org/2019/458.pdf
* sha2-256-trunc254-padded * poseidon-bls12_381-a2-fc1 Ref: multiformats/multicodec#161 Ref: multiformats/multicodec#171 Ref: multiformats/multicodec#170
Reserving the 0xb400 range for Poseidon variants. These entries allow FIL to iterate on the
fcX
extension of the name where they stay with BLS12-381 and arity=2. High security variant is for extra circuits that are usable in case new attacks arise from the standard variant.fc1
.fcX
extension.sc
addition. This may or may not be used, but will exist alongside the standard variant and be deployable if new attacks render the standard variant less secure.Implementation: https://github.com/filecoin-project/neptune
Ref: #161
Ref: https://eprint.iacr.org/2019/458.pdf
R= @vmx @mikeal @dignifiedquire @porcuquine @Stebalien