Add skeleton for the get_block_hash syscall.

[ArniStarkware]

This change is 

[noaov1]

Reviewed 1 of 6 files at r1.
Reviewable status: 1 of 6 files reviewed, 5 unresolved discussions (waiting on @ArniStarkware and @Yoni-Starkware)

---

crates/blockifier/feature_contracts/cairo1/test_contract.cairo line 48 at r1 (raw file):

    #[external]
    fn test_get_block_hash(block_number: felt252) -> felt252 {

Why is there a difference from here? (felt252 vs. u64)

Code quote:

block_number: felt252

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 317 at r1 (raw file):

    // TODO(Arni, 6/6/2023): Implement this logic.
    pub fn get_block_hash(&mut self, _block_number: StarkFelt) -> SyscallResult<StarkFelt> {
        Err(SyscallExecutionError::Unimplemented)

Why is it needed? Why can't you implement the logic directly in the file mod.rs? Do you need the syscall_handler for the implementation (why is it a method)?

Code quote:

    // TODO(Arni, 6/6/2023): Implement this logic.
    pub fn get_block_hash(&mut self, _block_number: StarkFelt) -> SyscallResult<StarkFelt> {
        Err(SyscallExecutionError::Unimplemented)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 317 at r1 (raw file):

    // TODO(Arni, 6/6/2023): Implement this logic.
    pub fn get_block_hash(&mut self, _block_number: StarkFelt) -> SyscallResult<StarkFelt> {
        Err(SyscallExecutionError::Unimplemented)

Why is it needed? Why can't you return some value?

Code quote:

Err(SyscallExecutionError::Unimplemented)

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r1 (raw file):

    };

    // TODO(spapini): Fix the "UNEXPECTED ERROR".

Why? WDYM?

Code quote:

// TODO(spapini): Fix the "UNEXPECTED ERROR".

---

crates/blockifier/src/fee/os_resources.rs line 52 at r1 (raw file):

                "n_memory_holes": 0,
                "n_steps": 0
            },

Out of curiosity - where do these numbers come from?

Code quote:

            "GetBlockHash": {
                "builtin_instance_counter": {},
                "n_memory_holes": 0,
                "n_steps": 0
            },

[noaov1]

Reviewable status: 0 of 7 files reviewed, 8 unresolved discussions (waiting on @ArniStarkware and @Yoni-Starkware)

---

crates/blockifier/feature_contracts/cairo1/test_contract.cairo line 46 at r2 (raw file):

        starknet::syscalls::emit_event_syscall(keys.span(), data.span()).unwrap_syscall();
    }

Compile the contract? :)

---

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

    let contract_address =
        ContractAddress::try_from(StarkFelt::from(constants::BLOCK_NUMBER_TO_BLOCK_HASH_ADDRESS))?;
    let block_hash = syscall_handler.state.get_storage_at(contract_address, key)?;

See get_contract_storage_at(). You need to update the accessed_keys and read_values values.

Code quote:

let block_hash = syscall_handler.state.get_storage_at(contract_address, key)?;

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

    };

    let _call_info = entry_point_call.execute_directly(&mut state).unwrap();

I don't know what should be the value. I think you will need to declare and deploy the contract at BLOCK_NUMBER_TO_BLOCK_HASH_ADDRESS (and initialize the key 1800).
If you don't want to do this in this PR, please add a TODO.

Suggestion:

  assert_eq!(entry_point_call.execute_directly(&mut state).unwrap(). execution, CallExecution::from_retdata(retdata![stark_felt!(value)]))

[ArniStarkware]

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why? WDYM?

Done. Removed.

[ArniStarkware]

Reviewable status: 0 of 9 files reviewed, 8 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

crates/blockifier/feature_contracts/cairo1/test_contract.cairo line 48 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is there a difference from here? (felt252 vs. u64)

Done. TY for sending an example.

---

crates/blockifier/feature_contracts/cairo1/test_contract.cairo line 46 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Compile the contract? :)

Done. Maybe we will want to compile again.

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 317 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is it needed? Why can't you implement the logic directly in the file mod.rs? Do you need the syscall_handler for the implementation (why is it a method)?

Done. Removed.

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 317 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is it needed? Why can't you return some value?

Done. removed.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I don't know what should be the value. I think you will need to declare and deploy the contract at BLOCK_NUMBER_TO_BLOCK_HASH_ADDRESS (and initialize the key 1800).
If you don't want to do this in this PR, please add a TODO.

Done.

---

crates/blockifier/src/fee/os_resources.rs line 52 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Out of curiosity - where do these numbers come from?

Still needs to be done.

[ArniStarkware]

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

See get_contract_storage_at(). You need to update the accessed_keys and read_values values.

Do you mean I need to initialize the mapping? I agree - in another PR.

[Yoni-Starkware]

Reviewed 2 of 3 files at r4, all commit messages.
Reviewable status: 2 of 9 files reviewed, 9 unresolved discussions (waiting on @ArniStarkware and @noaov1)

---

crates/blockifier/src/abi/constants.rs line 40 at r4 (raw file):

// This contract stores the block number -> block hash mapping.
pub const BLOCK_HASH_CONTRACT_ADDRESS: u64 = 0x1;

Suggestion:

pub const BLOCK_HASH_CONTRACT_ADDRESS: u64 = 1;

---

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Do you mean I need to initialize the mapping? I agree - in another PR.

Actually no, it's different.

[ArniStarkware]

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Actually no, it's different.

We do not need to update accessed_keys and read_values. This is a read from one contract to "another contract" - which is not actually a contract, but a ledger for this particular history.

[ArniStarkware]

Reviewable status: 2 of 9 files reviewed, 9 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 40 at r4 (raw file):

// This contract stores the block number -> block hash mapping.
pub const BLOCK_HASH_CONTRACT_ADDRESS: u64 = 0x1;

Done.

[noaov1]

Reviewable status: 2 of 9 files reviewed, 5 unresolved discussions (waiting on @ArniStarkware and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 40 at r4 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

Or maybe pub const BLOCK_HASH_CONTRACT_ADDRESS: &str = "0x1";

---

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

We do not need to update accessed_keys and read_values. This is a read from one contract to "another contract" - which is not actually a contract, but a ledger for this particular history.

You are right, we don't need to update the those mappings.

---

crates/blockifier/src/execution/syscalls/mod.rs line 322 at r5 (raw file):

    request: GetBlockHashRequest,
    _vm: &mut VirtualMachine,
    syscall_handler: &mut SyscallHintProcessor<'_>,

I wonder why it wasn't indicated by the compiler

Suggestion:

pub fn get_block_hash(
    request: GetBlockHashRequest,
    _vm: &mut VirtualMachine,
    syscall_handler: &mut SyscallHintProcessor<'_>,
    _remaining_gas: &mut Felt252,

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

I wonder what will enforce that the BLOCK_HASH_CONTRACT is deployed and that the queried block number was initialized. Do we want that?

[ArniStarkware]

Reviewable status: 2 of 9 files reviewed, 5 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 309 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

You are right, we don't need to update the those mappings.

Done.

---

crates/blockifier/src/execution/syscalls/mod.rs line 322 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I wonder why it wasn't indicated by the compiler

It was. You are right. Done.

[ArniStarkware]

crates/blockifier/src/execution/syscalls/mod.rs line 315 at r6 (raw file):

// Returns the block hash of the block at given block_number.
// Raises an error if latest - block_number < 10.

Suggestion:

// Raises an error if latest_block_number - block_number < 10.

[ArniStarkware]

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I wonder what will enforce that the BLOCK_HASH_CONTRACT is deployed and that the queried block number was initialized. Do we want that?

Regarding the "Is the queried block was initialized" question: This is somewhat addressed in the TODO in crates/blockifier/src/execution/syscalls/mod.rs.
Regarding the enforcement that BLOCK_HASH_CONTRACT is deployed, this is @dafna Matsry's domain.

[noaov1]

Reviewable status: 2 of 9 files reviewed, 6 unresolved discussions (waiting on @ArniStarkware, @dafna, and @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 290 at r7 (raw file):

#[derive(Debug, Eq, PartialEq)]

// TODO(Arni, 8/6/2023): Consider replacing with block_number: u64 or with BlockNumber.

Suggestion:

// TODO(Arni, 8/6/2023): Consider replacing with `BlockNumber`.

---

crates/blockifier/src/execution/syscalls/mod.rs line 304 at r7 (raw file):

#[derive(Debug, Eq, PartialEq)]
pub struct GetBlockHashResponse {
    pub block_hash: StarkFelt,

Suggestion:

BlockHash

---

crates/blockifier/src/execution/syscalls/mod.rs line 318 at r7 (raw file):

// Returns the expected block hash if 10 <= latest_block_hash - block_number < 1024.
// Returns unexpected value, otherwise (Most likly - returns an uninitialized value = 0).
// TODO(Arni, 11/6/2023): Implement according to the documentation above.

What is the latest_block_number? The last one that was created?

Suggestion:

// Returns the block hash of a given block_number.
// Returns the expected block hash if the given block was created at most 1024 blocks before the latest block and at least 10 blocks before.
// Otherwise, returns a default value or an error respectively.
// TODO(Arni, 11/6/2023): Implement according to the documentation above.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Regarding the "Is the queried block was initialized" question: This is somewhat addressed in the TODO in crates/blockifier/src/execution/syscalls/mod.rs.
Regarding the enforcement that BLOCK_HASH_CONTRACT is deployed, this is @dafna Matsry's domain.

I don't follow :) Will you check whether the returned value is different from 0?

---

crates/blockifier/src/fee/os_resources.rs line 52 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Still needs to be done.

Add a TODO?

[Yoni-Starkware]

Reviewed 2 of 5 files at r8, all commit messages.
Reviewable status: 3 of 9 files reviewed, 8 unresolved discussions (waiting on @ArniStarkware, @dafna, and @noaov1)

---

crates/blockifier/src/abi/constants.rs line 64 at r8 (raw file):

// This contract stores the block number -> block hash mapping.
pub const BLOCK_HASH_CONTRACT_ADDRESS: u64 = 1;

u64 -> Address type or whatever it is called here

Code quote:

 const BLOCK_HASH_CONTRACT_ADDRESS: u64 = 1;

---

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 75 at r8 (raw file):

            b"Deploy" => Ok(Self::Deploy),
            b"EmitEvent" => Ok(Self::EmitEvent),
            b"GetBlockHash" => Ok(Self::GetBlockHash),

Why? it's the deprecated handler

Code quote:

  b"GetBlockHash" => Ok(Self::GetBlockHash),

[ArniStarkware]

Reviewable status: 3 of 9 files reviewed, 8 unresolved discussions (waiting on @dafna, @giladchase, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 64 at r8 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

u64 -> Address type or whatever it is called here

You wish that the type of the constant will be: ContractAddress.

Rust complaints:

error[E0015]: cannot call non-const fn `<starknet_api::hash::StarkFelt as std::convert::From<u64>>::from` in constants
  --> crates/blockifier/src/execution/syscalls/mod.rs:30:43
   |
30 | pub const BLOCK_HASH_ADDRESS: StarkFelt = StarkFelt::from(1u64);
   |                                           ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants

Spoke with @giladchase. Solving this may be overkill. here is some code, allowing for similar behavior using lazy.

This change requires adding crates. Will try to do that if I have time.

---

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 75 at r8 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why? it's the deprecated handler

IIUC, this is a unified list for both deprecated and not deprecated syscalls. Some sort of Hack. @noaov1 - assured me this is the correct thing to do.

---

crates/blockifier/src/execution/syscalls/mod.rs line 290 at r7 (raw file):

#[derive(Debug, Eq, PartialEq)]

// TODO(Arni, 8/6/2023): Consider replacing with block_number: u64 or with BlockNumber.

Done.

---

crates/blockifier/src/execution/syscalls/mod.rs line 304 at r7 (raw file):

#[derive(Debug, Eq, PartialEq)]
pub struct GetBlockHashResponse {
    pub block_hash: StarkFelt,

Done.

---

crates/blockifier/src/execution/syscalls/mod.rs line 318 at r7 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What is the latest_block_number? The last one that was created?

Done.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 117 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Please check a non-trivial value instead.

Testing a non-trivial case.
Done.

---

crates/blockifier/src/fee/os_resources.rs line 52 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

To start with, copy the values of storage_read, which is quite similar, and add a TODO.

Done.

[Yoni-Starkware]

Reviewed 1 of 5 files at r8, all commit messages.
Reviewable status: 4 of 9 files reviewed, 7 unresolved discussions (waiting on @dafna and @noaov1)

---

crates/blockifier/src/abi/constants.rs line 64 at r8 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

You wish that the type of the constant will be: ContractAddress.

Rust complaints:

error[E0015]: cannot call non-const fn `<starknet_api::hash::StarkFelt as std::convert::From<u64>>::from` in constants
  --> crates/blockifier/src/execution/syscalls/mod.rs:30:43
   |
30 | pub const BLOCK_HASH_ADDRESS: StarkFelt = StarkFelt::from(1u64);
   |                                           ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants

Spoke with @giladchase. Solving this may be overkill. here is some code, allowing for similar behavior using lazy.

This change requires adding crates. Will try to do that if I have time.

No need, I see that they use u64 also for selectors, so let's go with their convention (u64 is fine)

[noaov1]

Reviewable status: 4 of 9 files reviewed, 8 unresolved discussions (waiting on @ArniStarkware, @dafna, and @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 333 at r9 (raw file):

            syscall_handler.state.get_storage_at(block_hash_contract_address, key)?,
        ),
    })

Suggestion:

    let block_hash = BlockHash(syscall_handler.state.get_storage_at(block_hash_contract_address, key)?)
    Ok(GetBlockHashResponse {
        block_hash
    })

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 133 at r9 (raw file):

    // Initialize block number -> block hash entry.
    let block_number = stark_felt!(1800_u64);
    let block_hash = stark_felt!(0x42_u64);

Suggestion:

"0x42"

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 147 at r9 (raw file):

    };

    // Run entry point and test result.

I think you can remove it.

Code quote:

 // Run entry point and test result.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 151 at r9 (raw file):

        entry_point_call.execute_directly(&mut state).unwrap().execution,
        CallExecution {
            gas_consumed: stark_felt!(0x3d86_u128),

Can you please convert it to an integer? As in the rest of the tests (for consistency)

Code quote:

0x3d86_u128

The base branch was changed.

[ArniStarkware]

Reviewable status: 0 of 23 files reviewed, 8 unresolved discussions (waiting on @dafna, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 333 at r9 (raw file):

            syscall_handler.state.get_storage_at(block_hash_contract_address, key)?,
        ),
    })

Done.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 133 at r9 (raw file):

    // Initialize block number -> block hash entry.
    let block_number = stark_felt!(1800_u64);
    let block_hash = stark_felt!(0x42_u64);

Done.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 147 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I think you can remove it.

Done.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 151 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can you please convert it to an integer? As in the rest of the tests (for consistency)

Done.

[Yoni-Starkware]

Reviewed 4 of 12 files at r13, all commit messages.
Reviewable status: 4 of 23 files reviewed, 9 unresolved discussions (waiting on @ArniStarkware, @dafna, and @noaov1)

---

crates/blockifier/src/execution/syscalls/mod.rs line 294 at r14 (raw file):

// TODO(Arni, 8/6/2023): Consider replacing `BlockNumber`.
pub struct GetBlockHashRequest {
    pub block_number: StarkFelt,

Why not use u64? Like in Cairo 1.

Code quote:

// TODO(Arni, 8/6/2023): Consider replacing `BlockNumber`.
pub struct GetBlockHashRequest {
    pub block_number: StarkFelt,

[Yoni-Starkware]

Reviewable status: 4 of 23 files reviewed, 10 unresolved discussions (waiting on @ArniStarkware, @dafna, and @noaov1)

---

crates/blockifier/src/execution/syscalls/mod.rs line 318 at r14 (raw file):

// Returns the block hash of a given block_number.
// Returns the expected block hash if the given block was created at most 1024 blocks before the
// latest block and at least 10 blocks before. Otherwise, returns a default value or an error

We support the entire history.
Replace 10 with a constant (next PR)

Suggestion:

// Returns the expected block hash if the given block was created at least 10 blocks before the current block. Otherwise, returns a default value or an error

[Yoni-Starkware]

Reviewable status: 4 of 23 files reviewed, 11 unresolved discussions (waiting on @ArniStarkware, @dafna, and @noaov1)

---

crates/blockifier/src/abi/constants.rs line 71 at r14 (raw file):

        ContractAddress::try_from(stark_felt!(1_u64))
            .unwrap_or_else(|_| panic!("Failed to convert 1_u64 to StarkFelt"))
    });

Okay, I think that this is the definition of overkill. @giladchase wdyt?

Code quote:

pub static BLOCK_HASH_CONTRACT_ADDRESS: once_cell::sync::Lazy<ContractAddress> =
    once_cell::sync::Lazy::new(|| {
        ContractAddress::try_from(stark_felt!(1_u64))
            .unwrap_or_else(|_| panic!("Failed to convert 1_u64 to StarkFelt"))
    });

[ArniStarkware]

crates/blockifier/src/abi/constants.rs line 70 at r14 (raw file):

    once_cell::sync::Lazy::new(|| {
        ContractAddress::try_from(stark_felt!(1_u64))
            .unwrap_or_else(|_| panic!("Failed to convert 1_u64 to StarkFelt"))

Suggestion:

.unwrap_or_else(|_| panic!("Failed to convert 1_u64 to ContractAddress."))

[giladchase]

Reviewable status: 4 of 23 files reviewed, 12 unresolved discussions (waiting on @ArniStarkware, @dafna, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 71 at r14 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Okay, I think that this is the definition of overkill. @giladchase wdyt?

A bit overkill yes, also the stark_felt! and panic are problematic; the former is a testing feature and the later is confusing (this will never really panic so it's confusing).

We could add a const fn one() to ContractAddress (similar toconst fn Felt252::one for example), does it make sense for it to be in starknet API though? (does it have special meaning for everyone implementing the API?)

[Yoni-Starkware]

Reviewable status: 4 of 23 files reviewed, 12 unresolved discussions (waiting on @ArniStarkware, @dafna, @giladchase, and @noaov1)

---

crates/blockifier/src/abi/constants.rs line 71 at r14 (raw file):

Previously, giladchase wrote…

A bit overkill yes, also the stark_felt! and panic are problematic; the former is a testing feature and the later is confusing (this will never really panic so it's confusing).

We could add a const fn one() to ContractAddress (similar toconst fn Felt252::one for example), does it make sense for it to be in starknet API though? (does it have special meaning for everyone implementing the API?)

I don't think so. 1 is a specific contract address.
I'm okay with eiyher u64 or str for now.

[giladchase]

Reviewable status: 4 of 23 files reviewed, 12 unresolved discussions (waiting on @ArniStarkware, @dafna, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 71 at r14 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

I don't think so. 1 is a specific contract address.
I'm okay with eiyher u64 or str for now.

Ya, that's pretty much our go-to solution at this point as well :)

[ArniStarkware]

Reviewable status: 1 of 23 files reviewed, 11 unresolved discussions (waiting on @dafna, @giladchase, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/abi/constants.rs line 71 at r14 (raw file):

Previously, giladchase wrote…

Ya, that's pretty much our go-to solution at this point as well :)

Reverted.

---

crates/blockifier/src/execution/syscalls/mod.rs line 294 at r14 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why not use u64? Like in Cairo 1.

BlockNumber is just an extension of u64, and is more correct in this case.
There is an issue with converting from Felt to an integer type in both cases. I started thinking about it, and even created a PR in the starknet-api repo. I now see there is a conversion that passes through usize. Note usize may be a u32 on some hardware.

In any case - it is not straightforward.

---

crates/blockifier/src/execution/syscalls/mod.rs line 318 at r14 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

We support the entire history.
Replace 10 with a constant (next PR)

Done.

[ArniStarkware]

crates/blockifier/src/execution/syscalls/mod.rs line 294 at r14 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

BlockNumber is just an extension of u64, and is more correct in this case.
There is an issue with converting from Felt to an integer type in both cases. I started thinking about it, and even created a PR in the starknet-api repo. I now see there is a conversion that passes through usize. Note usize may be a u32 on some hardware.

In any case - it is not straightforward.

See #604 .

[ArniStarkware]

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 126 at r15 (raw file):

}

// TODO(Arni, 11/6/2023): Initialize the "contract" at BLOCK_HASH_CONTRACT_ADDRESS.

This is done.

[ArniStarkware]

Reviewable status: 1 of 23 files reviewed, 11 unresolved discussions (waiting on @dafna, @giladchase, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 333 at r9 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

un-reverted.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 133 at r9 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

un-reverted.

[Yoni-Starkware]

Reviewed 5 of 12 files at r13, 10 of 13 files at r14, 5 of 7 files at r15, 2 of 2 files at r16, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @dafna and @noaov1)

[noaov1]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)

[noaov1]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware)

[Yoni-Starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArniStarkware)