keep-starknet-strange / snos

Rust Library for running the Starknet OS via the Cairo VM
MIT License
57 stars 27 forks source link

bug: Computed compiled_class_hash is inconsistent with the hash in the os_input #379

Closed whichqua closed 1 month ago

whichqua commented 1 month ago

This issue is encountered during proving blocks for the following blocks:

The error log:

inner_exc error: Got an exception while executing a hint: Computed compiled_class_hash is inconsistent with the hash in the os_input. Computed hash = 2485633680350787828726933984920527634476219774484559009917585374531416244260, Expected hash = 2608700897848576865126422581545291943880363901435711603201682741980244957638.

The issue happens in this deprecated hint:

    %{
        from starkware.python.utils import from_bytes

        computed_hash = ids.compiled_class_fact.hash
        expected_hash = compiled_class_hash
        assert computed_hash == expected_hash, (
            "Computed compiled_class_hash is inconsistent with the hash in the os_input. "
            f"Computed hash = {computed_hash}, Expected hash = {expected_hash}.")

        vm_load_program(compiled_class.program, ids.compiled_class.bytecode_ptr)
    %}

This hint is also executed in the same function and may be related:

    %{
        from starkware.starknet.core.os.contract_class.deprecated_class_hash import (
            get_deprecated_contract_class_struct,
        )

        compiled_class_hash, compiled_class = next(compiled_class_facts)

        cairo_contract = get_deprecated_contract_class_struct(
            identifiers=ids._context.identifiers, contract_class=compiled_class)
        ids.compiled_class = segments.gen_arg(cairo_contract)
    %}

Not yet figured out why.

whichqua commented 1 month ago

This issue relates to a bug in our block context where an in correct deprecated_contract_hash is used.

ftheirs commented 1 month ago

After some research, this issue only applies for Cairo0 contracts and we've identified 3 different scenarios

class_hash and recomputed_class_hash are equals (block 204936)

[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Contract address: 0x41a78e741e5af2fec34b695679bc6891742439f7afb8484ecd7766661ad02bf
[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Class hash: 0x7b3e05f48f0c69e4a65ce5e076a66271a527aff2c34ce1083ec6e1526997a69
[WARN  prove_block::state_utils] Recomputed Class hash: 0x7b3e05f48f0c69e4a65ce5e076a66271a527aff2c34ce1083ec6e1526997a69

class_hash and recomputed_class_hash are different but the last one matches OS (cairo code) recomputed class hash (block 30000)

[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Contract address: 0x7a3c142b1ef242f093642604c2ac2259da0efa3a0517715c34a722ba2ecd048
[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Class hash: 0x5c478ee27f2112411f86f207605b2e2c58cdb647bac0df27f660ef2252359c6
[WARN  prove_block::state_utils] Recomputed Class hash: 0x7c373e188d0f701559ed725ef6c7bc10fca3441ef3399d5f6aee49bba5e2d4e
[WARN  starknet_os::hints::deprecated_compiled_class] Computed_hash: 0x7c373e188d0f701559ed725ef6c7bc10fca3441ef3399d5f6aee49bba5e2d4e

class_hash and recomputed_class_hash are different and the last one also doesn't match the OS computed class hash (block 159674)

[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Contract address: 0x7a3c142b1ef242f093642604c2ac2259da0efa3a0517715c34a722ba2ecd048
[WARN  prove_block::state_utils] ------------------------------------
[WARN  prove_block::state_utils] Class hash: 0x5c478ee27f2112411f86f207605b2e2c58cdb647bac0df27f660ef2252359c6
[WARN  prove_block::state_utils] Recomputed Class hash: 0x7c373e188d0f701559ed725ef6c7bc10fca3441ef3399d5f6aee49bba5e2d4e
[WARN  prove_block::state_utils] CAIRO_0
[WARN  starknet_os::hints::deprecated_compiled_class] Computed_hash: 0x57ed19d5ae9c505530da5f2ac62f90633a2f3cfd0cda8a94948b11909744024

The recomputed class_hash can be checked here while the comparison with the computed by Cairo code is here

ftheirs commented 1 month ago

Once we have a clearer view of the issue, we focus on the type conversions between Starknet and Pathfinder types. In order to simplify this process, some tests were added to the project. The first step was to check that the conversion between CompressedLegacyContractClass and LegacyContractClass is correct and we can get the correct class_hash

Some fixes were made to getting the right class_hash in this commit and here

ftheirs commented 1 month ago

With the latest fix, we are able to recompute the correct class_hash but the third scenario is still pending since the OS computation is not correct

ftheirs commented 1 month ago

After making a deeper debugging, we've realized that the information provided to Cairo code was different and that was the root cause of the error on the hash computation. The information that is sent to Cairo code is loaded here

ftheirs commented 1 month ago

All the fields but hinted_class_hash were correct, so we've compared in pathfinder code how that hash was computed. This function is the one that computes the class_hash but if you pay attention, it applies some changes on the input Json object to keep compatibility for old Cairo contracts. On these blocks 166705, 159746, 159752, 161371, 164180, the issue was that there was no compiler version on the object and this part was missing. So the solution was to encapsulate the pre-processing that is done to the Json object in this function and make it public, so we are able to use it from SNOS.

Once we have this done, the fix is pretty straightforward to apply as you can check here