chore(blockifier): derive serde for transaction_execution_info

[aner-starkware]

This change is 

[dorimedini-starkware]

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

a discussion (no related file):
why?
what's the use case?

---

[aner-starkware]

Previously, dorimedini-starkware wrote…

why?
what's the use case?

🤷, it's the assignment from github issues...

[dorimedini-starkware]

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

a discussion (no related file):

Previously, aner-starkware wrote…

🤷, it's the assignment from github issues...

ah
ok then, please gate it behind a feature, so we verify we don't use this internally

---

[meship-starkware]

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

a discussion (no related file):
In all open issues PRs, Ariel asked us to add @tdelabro

---

[aner-starkware]

Previously, dorimedini-starkware wrote…

ah
ok then, please gate it behind a feature, so we verify we don't use this internally

What does that mean?

[aner-starkware]

Previously, meship-starkware (Meshi Peled) wrote…

In all open issues PRs, Ariel asked us to add @tdelabro

Thanks.

[dorimedini-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aner-starkware, @noaov1, and @tdelabro)

a discussion (no related file):

Previously, aner-starkware wrote…

What does that mean?

1. add a feature (call it, say, transaction_serde in the features section of the blockifier's cargo.toml
2. use cfg_attr to conditionally derive (De)Serialize if the feature is active (see cfg_attr examples in the repo)

---

[tdelabro]

@aner-starkware you can take a look to widely used crate like: https://github.com/rust-num/num-bigint/blob/master/Cargo.toml

The Cargo.toml has a serde feature, and the serde crate is flaged as an optional dependency. Read carefully this section in order to understand how to specify those things correctly: https://doc.rust-lang.org/cargo/reference/features.html#optional-dependencies

Then instead of having #[derive(Serialize, Deserialize)] in your code you have instead

#[derive(Copy, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serizalize, serde::Deserialize)]
pub struct MyStruct {}

[TzahiTaub]

Reviewed 5 of 5 files at r1.
Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @aner-starkware, @dorimedini-starkware, @noaov1, and @tdelabro)

---

crates/blockifier/Cargo.toml line 15 at r2 (raw file):

concurrency = []
jemalloc = ["dep:tikv-jemallocator"]
transaction_serde = []

AFAIU, no need for explicit dependencies as serde is a non-optional dependency already.

[dorimedini-starkware]

Reviewed 1 of 7 files at r2, 6 of 6 files at r3, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @aner-starkware, @noaov1, and @tdelabro)

---

crates/blockifier/src/execution/call_info.rs line 32 at r3 (raw file):

#[cfg_attr(test, derive(Clone))]
#[cfg_attr(feature = "transaction_serde", derive(Deserialize))]
#[derive(Debug, Default, Eq, PartialEq, Serialize)]

this is cleaner IMO; WDYT?
this way you only need to name the feature once

Suggestion:

use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use serde::Serialize;
use starknet_api::core::{ClassHash, ContractAddress, EthAddress, PatriciaKey};
use starknet_api::state::StorageKey;
use starknet_api::transaction::{EventContent, L2ToL1Payload};
use starknet_api::{felt, patricia_key};
use starknet_types_core::felt::Felt;

use crate::execution::entry_point::CallEntryPoint;
use crate::fee::gas_usage::get_message_segment_length;
use crate::state::cached_state::StorageEntry;

#[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))]
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)]
pub struct Retdata(pub Vec<Felt>);

#[macro_export]
macro_rules! retdata {
    ( $( $x:expr ),* ) => {
        Retdata(vec![$($x),*])
    };
}

#[cfg_attr(test, derive(Clone))]
#[cfg_attr(feature = "transaction_serde", derive(Deserialize))]
#[derive(Debug, Default, Eq, PartialEq, Serialize)]

---

crates/blockifier/src/execution/entry_point.rs line 8 at r3 (raw file):

use num_traits::{Inv, Zero};
#[cfg(feature = "transaction_serde")]
use serde::Deserialize;

see above

Code quote:

#[cfg(feature = "transaction_serde")]
use serde::Deserialize;

---

crates/blockifier/src/fee/actual_cost.rs line 3 at r3 (raw file):

use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
#[cfg(feature = "transaction_serde")]
use serde::{Deserialize, Serialize};

see above

Code quote:

#[cfg(feature = "transaction_serde")]
use serde::{Deserialize, Serialize};

---

crates/blockifier/src/state/cached_state.rs line 7 at r3 (raw file):

use indexmap::IndexMap;
#[cfg(feature = "transaction_serde")]
use serde::{Deserialize, Serialize};

see above

Code quote:

#[cfg(feature = "transaction_serde")]
use serde::{Deserialize, Serialize};

---

crates/blockifier/src/transaction/objects.rs line 7 at r3 (raw file):

use num_traits::Pow;
#[cfg(feature = "transaction_serde")]
use serde::Deserialize;

see above

Code quote:

#[cfg(feature = "transaction_serde")]
use serde::Deserialize;

[TzahiTaub]

Reviewable status: 2 of 7 files reviewed, 8 unresolved discussions (waiting on @aner-starkware, @dorimedini-starkware, @noaov1, and @tdelabro)

---

crates/blockifier/src/execution/call_info.rs line 32 at r3 (raw file):

Previously, dorimedini-starkware wrote…

this is cleaner IMO; WDYT?
this way you only need to name the feature once

Agree

[dorimedini-starkware]

Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.59%. Comparing base (d80da28) to head (194066d).
Report is 201 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   76.51%   76.59%   +0.07%     
==========================================
  Files         329      348      +19     
  Lines       34931    36771    +1840     
  Branches    34931    36771    +1840     
==========================================
+ Hits        26729    28165    +1436     
- Misses       5909     6283     +374     
- Partials     2293     2323      +30     

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

[dorimedini-starkware]

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

[TzahiTaub]

Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @noaov1)

[TzahiTaub]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)

[dorimedini-starkware]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)