Closed BasileiosKal closed 1 year ago
Thanks for raising this, it is a good opportunity for me to document the design decision.
The current I2OSP implementation is meant to return octets of the length of any arbitrary data. As BBS spec says max length can be 2^64-1 so in that case length can be encoded in 8 bytes that's why the function is defined to accept a length of type u64 and for u64 type, octets_length is of 8 bytes. We can relax the octets_length check to be any arbitrary size but I don't see any practical use for it in our code.
I thought of implementing I2OSP which would accept input
as bytes to cover all use cases(Scalars etc.) instead of u64 but then the caller has to convert u64 length themselves into bytes or use other programming constructs like enums to abstract input
to do that but with loss of simplicity.
As you already have seen the code that we get I2OSP for Scalar free from blst library, it made sense to me to support I2OSP only for u64. In future, if we change to use some other library or we see a need to support ourselves, we can add another version i2osp_scalar
for it.
That's good context! Thank you! I can't help but still be a little worried though. The reason is that if someone decides to slightly change some of the "constants" (namely XOF_NO_OF_BYTES
and OCTETS_MESSAGE_LENGTH_ENCODING_LENGTH
) the whole thing will potentially not work (they have reason to do so also, since it will improve efficiency).
I'm fine with keeping things as they are but maybe for now we should at least move those constants into the ciphersuite?? Having them in under "core\constants" is a little too inviting IMO to people to "experiment" with them :P. Having them in the ciphersuite means that we bind the implementation to specific ciphersuites (which is fine IMO) but not specific constants (it seems also more appropriate since those constants are suite-specific).
Also, the from_bytes_wide
operation is not from blst or blstr. It is just a leftover from the bls12-381 rust crate we where using previously. The blst from_bytes
operations don't have a requirement of 512 bits if I'm not mistaken?? So maybe another option is to describe a more generic Scalar::from_bytes
operation that continues to use blst but it's not "bound" to 512 bits input length. So no need for i2osp_scalar, and we get some flexibility when defining suites.
I would also update the naming from i2osp to i2osp_length or length_to_bytes or similar. So it is clear what the intend is (not a generic i2osp implementation but something more specific).
Closing as don't believe this to be applicable any longer.
The I2OSP operation as is defined now, does not accept an
octet_length
> 8 bytes. However it seems to me that this may not always be the case.More specifically, an
octet_length
of 8 bytes is what is defined for lengths of elements, however I2OSP is also used to serialize scalars in various places that will be 32 bytes for bls12-381.I know that right now this functionality is handled internally by blstr, that said, was wandering since we already have an I2OSP operation, if it would be better to "sever" a little the coupling with blstr with making I2OSP more flexible i.e., making it accept any
octet_length
, so we can handle some of those operations ourselves if need be.(For example, what if for efficiency we decide that XOF_NO_OF_BYTES = 384 bits instead of 512 that
Scallar::from_bytes_wide
expects?? Wouldn't we then have to use I2OSP for serialization of scalars??)