fix(blockifier_reexecution): bug in get storage at

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

• fix(blockifier_reexecution): bug in get storage at #1698 👈
• build(blockifier_reexecution): convert declare starknet api to declare blockifier #1658
• test(blockifier_reexecution): add support for declare and deploy account tx with rpc #1604
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @AvivYossef-starkware and the rest of your teammates on Graphite

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.90%. Comparing base (e3165c4) to head (25ae07d).
Report is 193 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1698      +/-   ##
==========================================
+ Coverage   40.10%   41.90%   +1.79%     
==========================================
  Files          26      197     +171     
  Lines        1895    23140   +21245     
  Branches     1895    23140   +21245     
==========================================
+ Hits          760     9696    +8936     
- Misses       1100    12988   +11888     
- Partials       35      456     +421     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dorimedini-starkware]

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

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

                Ok(Felt::default())
            }
            Err(e) => panic!("Unexpected error in get_storage_at: {:?}", e),

this isn't a bug in rpc state reader?

Code quote:

            Ok(value) => Ok(value),
            // If the contract address is not found, The blockifier expect to get the default value
            // of felt.
            Err(e) if e.to_string().contains("Contract address not found for request") => {
                Ok(Felt::default())
            }
            Err(e) => panic!("Unexpected error in get_storage_at: {:?}", e),

[AvivYossef-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this isn't a bug in rpc state reader?

My guess is that the gateway group is using papyrus. So they just receive 0 instead of an error when doing this request

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

My guess is that the gateway group is using papyrus. So they just receive 0 instead of an error when doing this request

who - other than us - is using the RPC reader, then?

[AvivYossef-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

who - other than us - is using the RPC reader, then?

The gateway group

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

The gateway group

no, you said the gateway group is using papyrus state reader. not the rpc reader...

1. if the GW group are using the rpc reader, why do they allow the error behavior instead of returning zero like papyrus?
2. if they are not using the rpc reader, who is?

[AvivYossef-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

no, you said the gateway group is using papyrus state reader. not the rpc reader...

1. if the GW group are using the rpc reader, why do they allow the error behavior instead of returning zero like papyrus?
2. if they are not using the rpc reader, who is?

The gateway group uses the RPC reader. Papyrus is also an RPC server, so they tested their RPC reader with Papyrus while we use the link to other RPC server

[dorimedini-starkware]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

The gateway group uses the RPC reader. Papyrus is also an RPC server, so they tested their RPC reader with Papyrus while we use the link to other RPC server

so the remote RPC returns an error where the papyrus reader returns zero?

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

so the remote RPC returns an error where the papyrus reader returns zero?

the papyrus server does not behave like the RPC server we use?

[AvivYossef-starkware]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

the papyrus server does not behave like the RPC server we use?

That is what I thought at the beginning, but after first looking at the papyrus_rpc crate, I think that the papyrus RPC server returns an Error as well. I have a meeting with Yael tomorrow to understand it

[dorimedini-starkware]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

That is what I thought at the beginning, but after first looking at the papyrus_rpc crate, I think that the papyrus RPC server returns an Error as well. I have a meeting with Yael tomorrow to understand it

OK, keeping this blocked for now, because it changes the behavior of the client - I would like to understand why (if) we need it

[AvivYossef-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 59 at r1 (raw file):

Previously, dorimedini-starkware wrote…

OK, keeping this blocked for now, because it changes the behavior of the client - I would like to understand why (if) we need it

It changes the behavior of our wrapper; I didn't touch the gateway`s code

[dorimedini-starkware]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 75 at r5 (raw file):

            Err(e) if e.to_string().contains("Contract address not found for request") => {
                Ok(Felt::default())
            }

Suggestion:

            // TODO: Once the RPC state reader bug is fixed (CONTRACT_NOT_FOUND error should cause the
            //   reader to return the default felt), remove this `match` arm.
            Err(e) if e.to_string().contains("Contract address not found for request") => {
                Ok(Felt::default())
            }