feat(starknet_os): os_logger: implement ResourceCounter::new

[dorimedini-starkware]

No description provided.

[dorimedini-starkware]

• feat(starknet_os): os_logger: implement OsLogger::exit_tx #5284
• feat(starknet_os): os_logger: implement OsLogger::enter_tx #5283
• feat(starknet_os): os_logger: implement OsLogger::exit_syscall #5282
• feat(starknet_os): os_logger: implement OsLogger::enter_syscall #5281
• feat(starknet_os): os_logger: implement ResourceCounter::sub_counter #5280
• feat(starknet_os): os_logger: implement ResourceCounter::new #5279 👈 (View in Graphite)
• feat(starknet_os): os_logger: implement TransactionTrace #5278
• feat(starknet_os): os_logger: extract logic to ResourceFinalizer trait #5277
• feat(starknet_os): os_logger: implement SyscallTrace #5276
• main

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

[reviewable-StarkWare]

This change is 

[nimrod-starkware]

Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

---

crates/starknet_os/src/hint_processor/os_logger.rs line 197 at r1 (raw file):

            ap_tracking,
            os_program,
        )?;

WDYT of passing the cairo struct and the field name instead of the selected flag? do you hit clippy::too_many_args?

Code quote:

        // The `BuiltinPointers` struct has two fields: selectable and non-selectable builtins.
        Self::insert_builtins(true, &mut builtin_ptrs_dict, ids_data, vm, ap_tracking, os_program)?;
        Self::insert_builtins(
            false,
            &mut builtin_ptrs_dict,
            ids_data,
            vm,
            ap_tracking,
            os_program,
        )?;

---

crates/starknet_os/src/hint_processor/os_logger.rs line 216 at r1 (raw file):

        };

        // We want all pointers except `segment_arena` and `sha256`.

move it next to excluded_builtins definition please

---

crates/starknet_os/src/hint_processor/os_logger.rs line 228 at r1 (raw file):

            if member_name == "segment_arena" || member_name == "sha256" {
                continue;
            }

Suggestion:

        // We want all pointers except `segment_arena` and `sha256`
        let excluded_builtins = ["segment_arena", "sha256"];
        for member_name in inner_members.keys() {
            if excluded_builtins.contains(member_name) {
                continue;
            }

---

crates/starknet_os/src/hint_processor/os_logger.rs line 241 at r1 (raw file):

            builtin_ptrs_dict.insert(
                BuiltinName::from_str(member_name)
                    .ok_or(OsLoggerError::UnknownBuiltin(member_name.clone()))?,

Suggestion:

.ok_or_else(|| OsLoggerError::UnknownBuiltin(member_name.clone()))?

[dorimedini-starkware]

Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @meship-starkware, @nimrod-starkware, and @Yoni-Starkware)

---

crates/starknet_os/src/hint_processor/os_logger.rs line 197 at r1 (raw file):

Previously, nimrod-starkware wrote…

WDYT of passing the cairo struct and the field name instead of the selected flag? do you hit clippy::too_many_args?

Done.

---

crates/starknet_os/src/hint_processor/os_logger.rs line 216 at r1 (raw file):

Previously, nimrod-starkware wrote…

move it next to excluded_builtins definition please

Done.

---

crates/starknet_os/src/hint_processor/os_logger.rs line 228 at r1 (raw file):

            if member_name == "segment_arena" || member_name == "sha256" {
                continue;
            }

Done.

---

crates/starknet_os/src/hint_processor/os_logger.rs line 241 at r1 (raw file):

            builtin_ptrs_dict.insert(
                BuiltinName::from_str(member_name)
                    .ok_or(OsLoggerError::UnknownBuiltin(member_name.clone()))?,

this is to only clone on error? nice

[nimrod-starkware]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @meship-starkware, and @Yoni-Starkware)

---

crates/starknet_os/src/hint_processor/os_logger.rs line 241 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this is to only clone on error? nice

yes, ok_or always evaluates. so it's better to use ok_or_else for heavier computations

[dorimedini-starkware]

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

---

crates/starknet_os/src/hint_processor/os_logger.rs line 241 at r1 (raw file):

Previously, nimrod-starkware wrote…

yes, ok_or always evaluates. so it's better to use ok_or_else for heavier computations

Done.

[nimrod-starkware]

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)

[nimrod-starkware]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)