Refactor left_pad_zeros_up_to_size to a parameter in the FixedSizeBytes constructor

[marioevz]

Changes left_pad_zeros_up_to_size to be instead an optional parameter in all classes that inherit FixedSizeBytes.

So for example Hash(left_pad_zeros_up_to_size("0x00", 32)) is equivalent to Hash("0x00", left_padding=True).

If neither left_padding or right_padding are specified, and the input is shorter than the expected size, an exception is raised.

[winsvega]

That undo the very same idea we wanted to ibtroduce is to restrict assigment of fixed bytes of different sizes

The point was to make it explicit, so we think about bytes size when writing the tests

[marioevz]

The point was to make it explicit, so we think about bytes size when writing the tests

But isn't the parameter explicit enough? if no parameter is passed and the byte length is shorter, an error will be raised, so the tester has to explicitly:

• Before this PR: Use the left_pad_zeros_up_to_size.
• After this PR: Pass either the left_padding or the right_padding parameter as True.

I guess my point is that both are explicit ways to ensure that the tester knows what they are doing, just that the second one is a bit more convenient.

[winsvega]

again the number 32 is implicit

[marioevz]

Yes, but I think at this point the tester has already selected Hash so they know they needed a 32-byte constructor.

Another entirely different discussion could be that the hashes become longer in the future, but in that case I think we need a completely new class.

[winsvega]

I see, so to conclude:

1. any fixedBytes can be initialized from the string "0x11" of a smaller size if we provide padding flag.
2. fixedBytes can be init with int, which will mean it is left padded by 00 as hash

it is a bit confusing if we provide no padding flag, we can write code that will throw an exception without realizing it.
I usually try to put there enum as a required parameter in such cases, but here there is a scenario when str does not need padding.

I agree that it is handy compared to importing a converter function.

Shall the unit tests be rewritten back with the flag? because I put explicit long strings there

[winsvega]

Ah, can we overload constructor so if it takes only str, it will throw if string byte representation has incorrect size
But we can use another constructor where it takes str and enum.
so

Hash("0x01", ZEROPAD::LEFT) = Hash(0x01) will make Hash 0x00..00001
Hash("0x01") throws an exception 0x01 has incorrect size

ah I see python has no overload constuctors.
so the default value of padding will be ZEROPAD.NONE

[marioevz]

Shall the unit tests be rewritten back with the flag? because I put explicit long strings there

I think they should be fine with the longer strings. We might need to add a new unit test that uses the padding flags if we decide to incorporate them.

[marioevz]

Ah, can we overload constructor so if it takes only str, it will throw if string byte representation has incorrect size But we can use another constructor where it takes str and enum. so

Hash("0x01", ZEROPAD::LEFT) = Hash(0x01) will make Hash 0x00..00001 Hash("0x01") throws an exception 0x01 has incorrect size

ah I see python has no overload constuctors. so the default value of padding will be ZEROPAD.NONE

Yes exactly, the magic happens here (this is in the FixedSizeBytes class so applies to all subclasses):

    def __new__(
        cls,
        input_bytes: FixedSizeBytesConvertible | T,
        *,
        left_padding: bool = False,
        right_padding: bool = False,
    ):

The asterisk on the fourth line means that no other positional arguments to the function are accepted, so if you do Hash(0x1), the 0x1 is taken as the single and only positional argument input_bytes.

The arguments after the asterisk are keyword arguments, which means they have to be explicitly named when the function called, so Hash(0x1, True) will not work, it has to be either Hash(0x1, left_padding=True) or Hash(0x1, right_padding=True).

Also since they have a default of False it means that the function can be called without them.

I thought about doing perhaps Hash(0x1, padding="left") or Hash(0x1, padding="right") but I don't really like letting the user have to write values to a string input. With an enum like Hash(0x1, padding=FixedSizeBytesPadding.Left) the downside is that FixedSizeBytesPadding would have to be imported.

[winsvega]

lets name Hash Hash32 because in tests we have Hash8 (block nonce) and Hash20 (address)