NethermindEth / StarknetByExample

A collection of examples of Cairo smart contracts for Starknet.
https://starknet-by-example.voyager.online/
MIT License
101 stars 76 forks source link

bug: Solidity-compatible hashes #123

Open enitrat opened 7 months ago

enitrat commented 7 months ago

There is a problem with the example of solidity-compatible hashes. The way it's currently done is that it calls keccak_u256s_be_inputs to hash a span composed of u256 words - however this function expects only full u256 words!

Consider the following:

use debug::PrintTrait;
use keccak::keccak_u256s_be_inputs;

#[test]
fn test_full_world() {
    let input_data: Span<u256> = array![
        0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
    ]
        .span();
    let hashed = keccak_u256s_be_inputs(input_data);

    // Split the hashed value into two 128-bit segments
    let low: u128 = hashed.low;
    let high: u128 = hashed.high;

    // Reverse each 128-bit segment
    let reversed_low = integer::u128_byte_reverse(low);
    let reversed_high = integer::u128_byte_reverse(high);

    // Reverse merge the reversed segments back into a u256 value
    let compatible_hash = u256 { low: reversed_high, high: reversed_low };

    assert(
        compatible_hash == 0xa9c584056064687e149968cbab758a3376d22aedc6a55823d1b3ecbee81b8fb9,
        'wrong hash'
    )
}

#[test]
fn test_partial_word() {
    let input_data: Span<u256> = array![0xAA].span();
    let hashed = keccak_u256s_be_inputs(input_data);

    // Split the hashed value into two 128-bit segments
    let low: u128 = hashed.low;
    let high: u128 = hashed.high;

    // Reverse each 128-bit segment
    let reversed_low = integer::u128_byte_reverse(low);
    let reversed_high = integer::u128_byte_reverse(high);

    // Reverse merge the reversed segments back into a u256 value
    let compatible_hash = u256 { low: reversed_high, high: reversed_low };

    assert(
        compatible_hash == 0xdb81b4d58595fbbbb592d3661a34cdca14d7ab379441400cbfa1b78bc447c365,
        'wrong hash'
    )
}

If you run the tests, you'll notice that it works for the full u256 word (with all bits set) but not for the word that is not full.

This is a bit tricky, but basically what you should do instead is:

See how it's implemented in Alexandria for bytes inputs, or Herodotus for u64-words inputs.

julio4 commented 7 months ago

@julienbrs will work on it

fontanellag commented 6 months ago

Start working on it too.

IjayAbby commented 1 month ago

@julio4 kindly assign me this issue

IjayAbby commented 1 month ago

@julio4 where is the best place for me to test the bug? Can't seem to find it

julio4 commented 1 month ago

@julio4 where is the best place for me to test the bug? Can't seem to find it

You need to add a new test that specifically verify this case

IjayAbby commented 1 month ago

Should I add it under listings/testing_how_to directory?

julio4 commented 1 month ago

Should I add it under listings/testing_how_to directory?

This is a bug with the solidity-compatible hashes example, so you should fix the bug and add tests directly in this listing directory.

IjayAbby commented 2 weeks ago

Hello @julio4 I need little clarity Am I supposed to replace the existing code on tests.cairo with the code snippet provided above?