Proto: remove incorrect functions from Reveal_hash
Context
Fixes: #4201 (closed)
Currently we include all the functions from Environment.S.Hash
in Sc_rollup.Reveal_hash.t
, and we borrow the implementation from
the underlying V0
module. However, many of these functions (to_bytes, of_bytes_opt, to_string, of_string_opt, etc...) are incorrect
as they do not prepend (remove) the tag byte when computing a byte sequence/string (hash).
Only one function that is used in the codebase, Reveal_hash.size
was found to be incorrect and to lead to incorrect behaviour when serializing payloads of large size.
We currently have hashes of size 31 + 1 byte (the extra byte for the tag, but we use the function size borrowed by the underlying hash module (which returns 31). This means that the dac encoding will not compute the correct number of hashes in a page: floor(4096/31) = 131, which will lead to hash pages having at most size 131 * 32 = 4192 bytes
This MR explicitly removes the inclusion of V0 functions in the Reveal_hash module, and instead specifies a minimal signature of safe-to-use functions
whose implementation is borrowed from the V0
hash module. The implementation of the size function is also changed to fix the bug discovered.
In the future, we may want to define a multihash interface to deal with Reveal_hash.
Manually testing the MR
Relying on CI passing.
Checklist
-
Document the interface of any function added or modified (see the coding guidelines) -
Document any change to the user interface, including configuration parameters (see node configuration) -
Provide automatic testing (see the testing guide). -
For new features and bug fixes, add an item in the appropriate changelog ( docs/protocols/alpha.rst
for the protocol and the environment,CHANGES.rst
at the root of the repository for everything else). -
Select suitable reviewers using the Reviewers
field below. -
Select as Assignee
the next person who should take action on that MR