feat: use starknet_api builtin for name trait

[ArniStarkware]

This change is 

The TODO is addressed here: starkware-libs/starknet-api#288

[ArniStarkware]

• chore: use builtins iter to define os_order_iter #351
• feat: use starknet_api builtin for name trait #319 👈
• chore: code to compilation file from gateway #344 : 1 other dependent PR (#297 )
• main

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

Join @ArniStarkware and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.58%. Comparing base (26060ef) to head (dba37cb).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   82.38%   82.58%   +0.20%     
==========================================
  Files          33       33              
  Lines        1612     1631      +19     
  Branches     1612     1631      +19     
==========================================
+ Hits         1328     1347      +19     
  Misses        219      219              
  Partials       65       65              

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

[yair-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @Yael-Starkware)

---

crates/gateway/src/gateway.rs line 187 at r1 (raw file):

// TODO(Arni): Add to a config.
fn get_supported_builtins() -> Vec<String> {

Please use https://docs.rs/once_cell/latest/once_cell/sync/struct.Lazy.html

---

crates/gateway/src/gateway.rs line 187 at r1 (raw file):

// TODO(Arni): Add to a config.
fn get_supported_builtins() -> Vec<String> {

Consider adding is_supported method to Builtin together with using https://docs.rs/strum/latest/strum/derive.EnumIter.html

This way whenever a new builtin is created it won't be forgotten in this function

[Yael-Starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware)

---

crates/gateway/src/utils.rs line 164 at r1 (raw file):

    true
}

I think this whole code should move the starknet-api , it is relevant for all repos (mempool and blockifier for sure).

[ArniStarkware]

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I think this whole code should move the starknet-api , it is relevant for all repos (mempool and blockifier for sure).

I agree :)
Here it is in Starknet API: https://reviewable.io/reviews/starkware-libs/starknet-api/288#- (in main-mempool branch).

[ArniStarkware]

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

---

crates/gateway/src/gateway.rs line 187 at r1 (raw file):

Previously, yair-starkware (Yair) wrote…

Please use https://docs.rs/once_cell/latest/once_cell/sync/struct.Lazy.html

Done.

---

crates/gateway/src/gateway.rs line 187 at r1 (raw file):

Previously, yair-starkware (Yair) wrote…

Consider adding is_supported method to Builtin together with using https://docs.rs/strum/latest/strum/derive.EnumIter.html

This way whenever a new builtin is created it won't be forgotten in this function

Done. See related Starknet API PR: https://reviewable.io/reviews/starkware-libs/starknet-api/290#-

[ArniStarkware]

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

---

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I agree :)
Here it is in Starknet API: https://reviewable.io/reviews/starkware-libs/starknet-api/288#- (in main-mempool branch).

Done*.

---

crates/gateway/src/utils.rs line 195 at r3 (raw file):

        }
    }
}

The name method is taken from starknet-api (The code supporting it is already merged).

Code quote:

impl NameExt for Builtin {
    fn name(&self) -> &'static str {
        match self {
            Builtin::RangeCheck => RANGE_CHACK_BUILTIN_NAME,
            Builtin::Pedersen => PEDERSEN_BUILTIN_NAME,
            Builtin::Poseidon => POSEIDON_BUILTIN_NAME,
            Builtin::EcOp => EC_OP_BUILTIN_NAME,
            Builtin::Ecdsa => ECDSA_BUILTIN_NAME,
            Builtin::Bitwise => BITWISE_BUILTIN_NAME,
            Builtin::Keccak => KECCAK_BUILTIN_NAME,
            Builtin::SegmentArena => SEGMENT_ARENA_BUILTIN_NAME,
        }
    }
}

[ArniStarkware]

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done*.

See another related PR in SN-API: starkware-libs/starknet-api#290

[Yael-Starkware]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

No - They are a property of the gateway.

is_supported_builtin - is a property of the gateway, which is going to be used to create the replacement of the static variable - a config variable.

lets add a todo to consider moving it to starknet-api.
I think it is a property of the whole system and should be in a central place

[ArniStarkware]

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

---

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

lets add a todo to consider moving it to starknet-api.
I think it is a property of the whole system and should be in a central place

Done.

[ArniStarkware]

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

---

crates/gateway/src/utils.rs line 164 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

See todo: TODO(Arni): Consider moving into Starknet-api.

[Yael-Starkware]

Reviewed 2 of 3 files at r7, 1 of 2 files at r8, all commit messages.
Reviewable status: 3 of 6 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/gateway/src/compilation.rs line 66 at r8 (raw file):

// TODO(Arni): Consider moving into Starknet-api.
// List of supported builtins.
// This is an explicit function so that it is explicitly desiced which builtins are supported.

decides?

Code quote:

desiced

---

crates/gateway/src/compilation.rs line 68 at r8 (raw file):

// This is an explicit function so that it is explicitly desiced which builtins are supported.
// If new builtins are added, they should be added here.
fn is_supported_builtin(builtin: &Builtin) -> bool {

Suggestion:

is_os_supported_builtin

[ArniStarkware]

Reviewable status: 2 of 6 files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware and @yair-starkware)

---

crates/gateway/src/compilation.rs line 66 at r8 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

decides?

Done

---

crates/gateway/src/compilation.rs line 68 at r8 (raw file):

// This is an explicit function so that it is explicitly desiced which builtins are supported.
// If new builtins are added, they should be added here.
fn is_supported_builtin(builtin: &Builtin) -> bool {

Done

[Yael-Starkware]

Reviewable status: 2 of 6 files reviewed, 1 unresolved discussion (waiting on @yair-starkware)

[yair-starkware]

Reviewed 2 of 4 files at r3, 1 of 2 files at r8, all commit messages.
Reviewable status: 3 of 6 files reviewed, all discussions resolved (waiting on @Yael-Starkware)

[yair-starkware]

Reviewed 2 of 3 files at r7, 1 of 1 files at r9.
Reviewable status: 4 of 6 files reviewed, all discussions resolved (waiting on @Yael-Starkware)