bug: Cairo `if else` statement removal in `keccak_add_uint256` modifies the assignment of `high` and `low`

[TAdev0]

keccak_add_uint256 function is as follows:

func keccak_add_uint256{range_check_ptr, bitwise_ptr: BitwiseBuiltin*, inputs: felt*}(
    num: Uint256, bigend: felt
) {
    if (bigend != 0) {
        let (num_reversed) = uint256_reverse_endian(num=num);
        tempvar bitwise_ptr = bitwise_ptr;
        tempvar high = num_reversed.high;
        tempvar low = num_reversed.low;
    } else {
        tempvar bitwise_ptr = bitwise_ptr;
        tempvar high = num.high;
        tempvar low = num.low;
    }

    %{
        segments.write_arg(ids.inputs, [ids.low % 2 ** 64, ids.low // 2 ** 64])
        segments.write_arg(ids.inputs + 2, [ids.high % 2 ** 64, ids.high // 2 ** 64])
    %}

    assert inputs[1] * 2 ** 64 + inputs[0] = low;
    assert inputs[3] * 2 ** 64 + inputs[2] = high;

    let inputs = inputs + 4;
    return ();
}

when running integration tests in a situation where bigend != 0 , the execution of the else statement is not working properly, inducing the same reference assigned to high and low.

Modifying this file for :

func keccak_add_uint256{range_check_ptr, bitwise_ptr: BitwiseBuiltin*, inputs: felt*}(
    num: Uint256, bigend: felt
) {
      tempvar bitwise_ptr = bitwise_ptr;
      tempvar high = num.high;
      tempvar low = num.low;

    %{
        segments.write_arg(ids.inputs, [ids.low % 2 ** 64, ids.low // 2 ** 64])
        segments.write_arg(ids.inputs + 2, [ids.high % 2 ** 64, ids.high // 2 ** 64])
    %}

    assert inputs[1] * 2 ** 64 + inputs[0] = low;
    assert inputs[3] * 2 ** 64 + inputs[2] = high;

    let inputs = inputs + 4;
    return ();
}

solves the issue and our VM works normally.

@rodrigo-pino @cicr99

[MKVEERENDRA]

It seems like the issue you're encountering involves the behavior of the keccak_add_uint256 function in your Cairo codebase, specifically when bigend != 0. Let's break down the problem and solution:

Problem Description: The keccak_add_uint256 function behaves unexpectedly when bigend != 0. The original implementation uses an if-else statement to determine whether to reverse the endianness of num. When bigend != 0, it reverses the endianness of num before assigning high and low. However, despite the condition, high and low are assigned incorrectly in the else block when bigend == 0, leading to incorrect behavior. Solution Implemented: The solution involves simplifying the function by removing the if-else statement entirely. Instead of conditionally assigning high and low, the function unconditionally assigns them directly from num. This modification ensures that high and low are consistently assigned correctly regardless of the value of bigend. Modified Function: cairo Copy code func keccak_add_uint256{range_check_ptr, bitwise_ptr: BitwiseBuiltin, inputs: felt}( num: Uint256, bigend: felt ) { tempvar bitwise_ptr = bitwise_ptr; tempvar high = num.high; tempvar low = num.low;

%{
    segments.write_arg(ids.inputs, [ids.low % 2 ** 64, ids.low // 2 ** 64])
    segments.write_arg(ids.inputs + 2, [ids.high % 2 ** 64, ids.high // 2 ** 64])
%}

assert inputs[1] * 2 ** 64 + inputs[0] = low;
assert inputs[3] * 2 ** 64 + inputs[2] = high;

let inputs = inputs + 4;
return ();

} Explanation: Direct Assignment: By directly assigning num.high to high and num.low to low, the function ensures that high and low are correctly populated with the values from num. Removed Conditional Logic: The conditional logic (if-else) that previously determined whether to reverse the endianness of num based on bigend has been removed. Verification: The assertions verify that low and high match the expected values in inputs, ensuring correctness after the assignment. Testing: After implementing this change, ensure to run your integration tests to verify that the function behaves correctly across different scenarios, including cases where bigend != 0 and bigend == 0. By following these steps, you should resolve the issue where keccak_add_uint256 was not functioning as expected in certain conditions, ensuring consistent behavior across your VM.

[TAdev0]

please do not spam the repo with AI generated content