bug: BytesTrait keccak computation mismatch

[JordyRo1]

Bug Report

Alexandria version:

This bug appears for the most recent Alexandria version cairo-v2.6.0

Current behavior:
The BytesTrait keccak function sometimes output incorrect hash, notably when 0-padding is involved. Let me explain with an example.
The objective was to compute the keccak hash of the felt252 element: 0x48595045524c414e455f414e4e4f554e43454d454e54. The python implementation provided on tests
was used to check if the result matches the expectation. The expected result from the python script was 0xda15cbb01a970c130a1bf6ba8b35fcff0fabd4695bfb03f4454482023f2ae84c. The function keccak from BytesTrait and from the python script have inverse endianness, so the expected result should be 0x4CE82A3F02824445F403FB5B69D4AB0FFFFC358BBAF61B0A130C971AB0CB15DA.
However, the output result from the bytes.keccak() is 0x93F59F2604EE54DE51A2DFCFB3E0A6D74EDE53826CF2D1CBA9422A06AF66BC65. After further investigation, the input argument that could possibly lead to this hash is 0000000000000000000048595045524c414e455f414e4e4f554e43454d454e54.
This is why I assume the error might be linked to the padding , unless I am missing something.

Expected behavior:

The output hash of the keccak function should match the one computed using the python script.

Steps to reproduce:

You can just append the following lines to the test_bytes_keccak function for example to test.

Related code:

let mut bytes : Bytes = BytesTrait::new_empty();
bytes.append_felt252(0x48595045524c414e455f414e4e4f554e43454d454e54);
let res = bytes.keccak();
assert_eq!(res, 0x4CE82A3F02824445F403FB5B69D4AB0FFFFC358BBAF61B0A130C971AB0CB15DA);

from Crypto.Hash import keccak
k = keccak.new(digest_bits=256)
k.update(bytes.fromhex('48595045524c414e455f414e4e4f554e43454d454e54'))
print(k.hexdigest()) 
# output: da15cbb01a970c130a1bf6ba8b35fcff0fabd4695bfb03f4454482023f2ae84c

[gaetbout]

Gm gm,
Could only have a very quick look so far so I'll take a look more in depth later when I can.
Are you sure it didn't happen before v2.6.0?
Because the changes done in that diff are only syntactic changes.

[JordyRo1]

Hey, no sorry I meant that the version that I used for the test was 2.6.0. But it occurs with previous versions too. Thank you for your time.

[gaetbout]

GM @zkcarter
You are the one that did this file, would you have any idea :) ?

[gaetbout]

This code should be removed thanks to:
starkware-libs/cairo#5877
When they release it

[gaetbout]

@JordyRo1 do you still think it needs to be addressed given that it'll be part of the standard library?
Don't close this issue, we'll keep it as a reminder to remove that piece of code :)

[JordyRo1]

@gaetbout No it's not necessary anymore, no problem I will not delete it !

[zkcarter]

Hi @gaetbout BytesTrait is a supplement to the standard library that has been developed for some time. I believe that once these functions are implemented in the standard library, this code should be removed, and developers should be encouraged to use the standard library.

[gaetbout]

We'll do it, thanks @zkcarter !

[github-actions]

There hasn't been any activity on this issue recently, and in order to prioritize active issues, it will be marked as stale.
Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by leaving a 👍
Because this issue is marked as stale, it will be closed and locked in 7 days if no further activity occurs.
Thank you for your contributions!

[gaetbout]

Stay alive

[gaetbout]

Should be solved with #321

[github-actions]

There hasn't been any activity on this issue recently, and in order to prioritize active issues, it will be marked as stale.
Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by leaving a 👍
Because this issue is marked as stale, it will be closed and locked in 7 days if no further activity occurs.
Thank you for your contributions!

[gaetbout]

Solved with #321

[gaetbout]

@JordyRo1 Can you close this :) ?