nikkolasg
commented
7 years ago I think this has proven to be pretty useful functionality as well
Do you have specific examples on that ? From my 2 years experience using/developing this library, I never used it explicitely. I'm always requiring a specific bitlength so min = 32 and max = 32.
I'm OK with the suggestion of moving the endianness as a parameters of the SetBytes and Bytes functions. I'm not sure with overloading them with a min and max parameter...
mod.Int has a limited degree of support for "endianness control"; i.e., serving clients who might want little-endian byte-slices as opposed to the default big-endian. However, two issues with this:
First, it's currently completely specific to mod.Int, and thus this (I think generally useful) endian-neutrality feature is not available in the Bytes/SetBytes calls to the general Scalar interface.
Second, I'm having second thoughts about whether the "stateful" approach to endianness control is the best idea here. I think that approach was motivated mainly by the desire to keep the Bytes/SetBytes method signatures consistent with those in the standard big.Int package, which does not provide endianness control at all. However, neither the Scalar nor Point interface is actually directly compatible with the big.Int package, anyway, so there doesn't seem to be a huge need to be dogged with that consistency. Further, as I've been experiencing, that extra state sometimes leads to unpleasant surprises, especially when the same Scalar object gets used sometimes big-endian and sometimes little-endian, e.g., during transitions between different endian representations. (This happens with ed25519 especially because ed25519 traditionally uses little-endian scalars.)
Thus, I propose that we move the ByteOrder, BigEndian, and LittleEndian names from the group/mod package to the top level, and define the Scalar.Bytes and Scalar.SetBytes to take a byteOrder parameter directly. Just make this standard functionality for all Scalars (of which there currently aren't that many different implementations anyway).
Another, related bit of API inconsistency comes from the fact that mod.Int also has LittleEndian() and BigEndian() methods, which act like Bytes() for the appropriate endianness but also take 'min' and 'max' parameters defining the minimum and maximum output length. I think this has proven to be pretty useful functionality as well - e.g., sometimes you just want the minimum-length slice that can represent an integer, sometimes you want a slice exactly some length. So for further API consolidation, I suggest we eliminate the separate BigEndian() and LittleEndian() methods from mod.Int and instead make Scalar.Bytes() take three paramters: min, max, and byteOrder.