get_custody_atoms applies modulo operation to a negative dividend: (-len(bytez) % BYTES_PER_CUSTODY_ATOM).
Modulo operation implementations differ from language to language, when dividend (or divisor) is negative. Therefore that may lead to a bug in a language, which implements modulo operation in a different way than python does (i.e. returning a negative result when a dividend is negative).
@protolambda in a personal communication has suggested that such an ambiguity is better to be avoided in the spec.
This is a very useful shorthand to get the number of missing elements to pad to a certain length. It's certainly unfortunate if we can't use it :(
I have mixed feelings about it. An alternative is definitely bulky and this padding approach is short and elegant.
However, we encounter more and more cases when concise is not clear (e.g. #1746 or #1924).
I.e. a short form may be ambiguous and, as a result, unsafe, in the sense that client implementers may implement it in different ways, potentially leading to a consensus break.
In the particular case, the modulo operation is clearly defined in Python, which is great. However, people are people and make mistakes, the particular case is not obvious to humans. On the other side, bugs here are easy to reveal with appropriate tests.
As a part of overall efforts to make the spec more formal and less ambiguous, my goal is at least to inform EF team about such ambiguities (and suggest a solution, when possible).
We used to have this problem with Java and JavaScript is affected as well. Both, Java and JavaScript by default does it in the following way:
-4 % 3 = -1
While Python follows the canonical math approach:
In mathematics, the result of the modulo operation is an equivalence class, and any member of the class may be chosen as representative; however, the usual representative is the least positive residue, the smallest non-negative integer that belongs to that class, i.e. the remainder of the Euclidean division.
EDT:
Thus -4 % 3 = 2 in Python and Ruby. And some other languages. There is a helper method in Java SDK My bad, this this the wrong method (thanks to @ericsson49). We've spent a time to learn this ambiguity and make a workaround. And I think that modulo operation should at least be explicitly defined in the spec and covered with test that will cut this ambiguity off on the early stage.Long.remainderUnsigned that does the calculation in the same way as Python
Java spec also have this operation description: https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.17.3 -- in Java % is for remainder, not for a modulo operation.
Many languages, including C, Rust, Java Nim, have the mod or % operator be the remainder, which is the same sign as the dividend, while Python, R and solidity uses the sign of the divisor (https://en.wikipedia.org/wiki/Modulo_operation#In_programming_languages)
Either we define modulo like @mkalinin sugested, or we use explicit sign(b) * (abs(a) % abs(b)) in the spec to remove ambiguities, which is the approach taken by IETF specs. This makes it clear to implementers that expected sign.
A couple of things:
I suggest we avoid this all together
Most helpful comment
We used to have this problem with Java and JavaScript is affected as well. Both, Java and JavaScript by default does it in the following way:
While Python follows the canonical math approach:
EDT:
Thus
-4 % 3 = 2in Python and Ruby. And some other languages.There is a helper method in Java SDKMy bad, this this the wrong method (thanks to @ericsson49). We've spent a time to learn this ambiguity and make a workaround. And I think that modulo operation should at least be explicitly defined in the spec and covered with test that will cut this ambiguity off on the early stage.Long.remainderUnsignedthat does the calculation in the same way as PythonJava spec also have this operation description: https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.17.3 -- in Java
%is for remainder, not for amodulooperation.