fix: add check for address 1 in gateway:get_storage_at

[Yael-Starkware]

When handling a get_storage_at request in the gateway, need to check first if the contract address is 1,
the contract address for the block hash table.
If it is, we need to return a value and not a ContractNotFound error.

Pull Request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build-related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

• Yes
• No

Other information

---

This change is 

[OmriEshhar1]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @DvirYo-starkware and @Yael-Starkware)

a discussion (no related file):
I see that there are other places in the code that check is a contract exists (e.g. fn verify_contract_exists() in papyrus_execution).
Is the check of "contract1" relevant for it too?
If it is, we should probably put this logic in one common place instead of doing it in multiple places.

---

[DvirYo-starkware]

Reviewable status: 0 of 4 files reviewed, 5 unresolved discussions (waiting on @OmriEshhar1 and @Yael-Starkware)

a discussion (no related file):

Previously, OmriEshhar1 wrote…

I see that there are other places in the code that check is a contract exists (e.g. fn verify_contract_exists() in papyrus_execution).
Is the check of "contract1" relevant for it too?
If it is, we should probably put this logic in one common place instead of doing it in multiple places.

This is irrelevant to the execution.

---

---

-- commits line 2 at r2:
Add () after the" fix" and consider changing "address != 1" to "address != 0x1"

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 57 at r2 (raw file):

    ContinuationTokenAsStruct,
};
pub const BLOCK_HASH_TABLE_CONTRACT_ADDRESS: &str = "0x1";

Instead of creating the ContractAddress from this string every time, create it only one time. Try to create this using a const function or using the lazy_static macro.

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 137 at r2 (raw file):

            .get_storage_at(state, &contract_address, &key)
            .map_err(internal_server_error);
        if res == Ok(StarkFelt::default()) {

Document what is special in this contract.
In addition, consider using only one if statement with two conditions.

---

crates/papyrus_gateway/src/v0_3_0/api/test.rs line 1116 at r2 (raw file):

}

#[tokio::test]

Why not add this to the already existing test "get_storage_at"?

[codecov]

Codecov Report

Merging #1000 (771e9ce) into main (3183bf8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1000   +/-   ##
=======================================
  Coverage   75.27%   75.27%           
=======================================
  Files          71       71           
  Lines        6054     6054           
  Branches     6054     6054           
=======================================
  Hits         4557     4557           
  Misses        689      689           
  Partials      808      808           

Files Changed	Coverage Δ
crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs	87.95% <100.00%> (ø)
crates/papyrus_gateway/src/v0_4_0/api/api_impl.rs	82.17% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[DvirYo-starkware]

Reviewed 1 of 5 files at r3, all commit messages.
Reviewable status: 1 of 5 files reviewed, 7 unresolved discussions (waiting on @OmriEshhar1 and @Yael-Starkware)

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 59 at r3 (raw file):

//TODO(yael): implement address 0x1 as a const function in starknet_api.
lazy_static! {

Consider putting this in the function get_storage_at because this constant is only used there.

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 140 at r3 (raw file):

        let res = state_reader
            .get_storage_at(state, &contract_address, &key)
            .map_err(internal_server_error);

Why don't you return the error if it exists? like in line 148.

---

crates/papyrus_gateway/src/v0_3_0/api/test.rs line 1107 at r3 (raw file):

    ));

    // Ask for storage at address 1 - the block hash table contract address

Consider putting this with the successful scenarios.

[Yael-Starkware]

Reviewable status: 1 of 5 files reviewed, 7 unresolved discussions (waiting on @DvirYo-starkware and @OmriEshhar1)

---

-- commits line 2 at r2:

Previously, DvirYo-starkware wrote…

Add () after the" fix" and consider changing "address != 1" to "address != 0x1"

Done.

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 57 at r2 (raw file):

Previously, DvirYo-starkware wrote…

Instead of creating the ContractAddress from this string every time, create it only one time. Try to create this using a const function or using the lazy_static macro.

Done.

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 137 at r2 (raw file):

Previously, DvirYo-starkware wrote…

Document what is special in this contract.
In addition, consider using only one if statement with two conditions.

Done.

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 59 at r3 (raw file):

Previously, DvirYo-starkware wrote…

Consider putting this in the function get_storage_at because this constant is only used there.

it is also being used in the test.

---

crates/papyrus_gateway/src/v0_3_0/api/test.rs line 1116 at r2 (raw file):

Previously, DvirYo-starkware wrote…

Why not add this to the already existing test "get_storage_at"?

Done.

[DvirYo-starkware]

Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Yael-Starkware)

[DvirYo-starkware]

Reviewed 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Yael-Starkware)

[Yael-Starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @DvirYo-starkware)

a discussion (no related file):

Previously, DvirYo-starkware wrote…

This is irrelevant to the execution.

Done

---

---

crates/papyrus_gateway/src/v0_3_0/api/api_impl.rs line 140 at r3 (raw file):

Previously, DvirYo-starkware wrote…

Why don't you return the error if it exists? like in line 148.

Done.

[OmriEshhar1]

Reviewed 1 of 5 files at r3, 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @DvirYo-starkware)