TMP - DO NOT MERGE

[dorimedini-starkware]

This change is 

[giladchase]

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

---

crates/blockifier/src/test_utils.rs line 337 at r1 (raw file):

    pub fn create_for_account_testing() -> BlockContext {
        let vm_resource_fee_cost = HashMap::from([

Should this be ResourcesMapping?

Code quote:

HashMap::from(

---

crates/blockifier/src/execution/contract_class.rs line 204 at r1 (raw file):

        let program = Program::new(
            builtins,
            Felt252::prime().to_str_radix(16),

Why was this removed?

Code quote:

      Felt252::prime().to_str_radix(16),

---

crates/blockifier/src/fee/fee_test.rs line 22 at r1 (raw file):

        (String::from(BITWISE_BUILTIN_NAME), 1),
        (String::from(POSEIDON_BUILTIN_NAME), 1),
    ]))

Perhaps add an impl for ResourcesMapping::from_iter([("n_steps",1800),..), something like

impl<'a> FromIterator<(&'a str, usize)> for ResourcesMapping {
    fn from_iter<I: IntoIterator<Item = (&'a str, usize)>>(iter: I) -> Self {
        Self(iter.into_iter().map(|(key, value)| (key.to_string(), value)).collect())
    }
}

Code quote:

    ResourcesMapping(HashMap::from([
        (String::from("n_steps"), 1800),
        (String::from(HASH_BUILTIN_NAME), 10),
        (String::from(RANGE_CHECK_BUILTIN_NAME), 24),
        (String::from(SIGNATURE_BUILTIN_NAME), 1),
        (String::from(BITWISE_BUILTIN_NAME), 1),
        (String::from(POSEIDON_BUILTIN_NAME), 1),
    ]))

---

crates/native_blockifier/src/py_transaction.rs line 123 at r1 (raw file):

        .collect();

    Ok(cairo_resource_fee_weights)

THANK U

Code quote:

}

fn process_cairo_resource_fee_weights(
    general_config: &PyAny,
) -> Result<HashMap<String, f64>, NativeBlockifierError> {
    let cairo_resource_fee_weights: HashMap<String, f64> =
        py_attr(general_config, "cairo_resource_fee_weights")?;

    // Remove the suffix "_builtin" from the keys, if exists.
    // FIXME: This should be fixed in python though...
    let cairo_resource_fee_weights = cairo_resource_fee_weights
        .into_iter()
        .map(|(k, v)| (k.trim_end_matches("_builtin").to_string(), v))
        .collect();

    Ok(cairo_resource_fee_weights)

[giladchase]

Reviewable status: 8 of 13 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware)

---

crates/blockifier/src/test_utils.rs line 7 at r1 (raw file):

use cairo_vm::vm::runners::builtin_runner::{
    BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, OUTPUT_BUILTIN_NAME,
    POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME,

Should these be in constants.rs? (I'm not sure...)

Code quote:

    BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, OUTPUT_BUILTIN_NAME,
    POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME,

---

crates/blockifier/src/test_utils.rs line 338 at r1 (raw file):

    pub fn create_for_account_testing() -> BlockContext {
        let vm_resource_fee_cost = HashMap::from([
            (String::from("n_steps"), 1_f64),

I think we have N_STEPS_RESOURCE in constants.rs

Code quote:

"n_steps"

[dorimedini-starkware]

Reviewable status: 8 of 13 files reviewed, 2 unresolved discussions (waiting on @giladchase)

---

crates/blockifier/src/test_utils.rs line 7 at r1 (raw file):

Previously, giladchase wrote…

Should these be in constants.rs? (I'm not sure...)

These are imported from LC. I can ask them to move these but not in the blockifier repo...

---

crates/blockifier/src/test_utils.rs line 337 at r1 (raw file):

Previously, giladchase wrote…

Should this be ResourcesMapping?

No; ResourcesMapping is a HashMap<String, usize>, not a HashMap<String, f64>

---

crates/blockifier/src/test_utils.rs line 338 at r1 (raw file):

Previously, giladchase wrote…

I think we have N_STEPS_RESOURCE in constants.rs

Done.

---

crates/blockifier/src/execution/contract_class.rs line 204 at r1 (raw file):

Previously, giladchase wrote…

Why was this removed?

from here, don't know why; maybe only default prime is used.
Should I escalate question to LC?

---

crates/blockifier/src/fee/fee_test.rs line 22 at r1 (raw file):

Previously, giladchase wrote…

Perhaps add an impl for ResourcesMapping::from_iter([("n_steps",1800),..), something like

impl<'a> FromIterator<(&'a str, usize)> for ResourcesMapping {
    fn from_iter<I: IntoIterator<Item = (&'a str, usize)>>(iter: I) -> Self {
        Self(iter.into_iter().map(|(key, value)| (key.to_string(), value)).collect())
    }
}

1. I think it's a bit excessive to add this just to drop HashMap::from
2. No guarantee that ResourcesMapping will always be a tuple struct with a single mapping

WDYT?

---

crates/native_blockifier/src/py_transaction.rs line 123 at r1 (raw file):

Previously, giladchase wrote…

THANK U

[giladchase]

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

---

crates/blockifier/src/execution/contract_class.rs line 204 at r1 (raw file):

Previously, dorimedini-starkware wrote…

from here, don't know why; maybe only default prime is used.
Should I escalate question to LC?

Mmm looks like boilerplate, if it is indeed the case then not worth it I think

---

crates/blockifier/src/fee/fee_test.rs line 22 at r1 (raw file):

Previously, dorimedini-starkware wrote…

1. I think it's a bit excessive to add this just to drop HashMap::from
2. No guarantee that ResourcesMapping will always be a tuple struct with a single mapping

WDYT?

Fair enough :)

---

crates/native_blockifier/src/py_transaction.rs line 123 at r1 (raw file):

Previously, dorimedini-starkware wrote…

😆

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

a discussion (no related file):

---