chore: create struct for gateway compiler

[ArniStarkware]

This change is 

[ArniStarkware]

• chore: create struct for gateway compiler #383 : 2 dependent PRs (#391 , #392 ) 👈
• 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

Attention: Patch coverage is 80.76923% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.33%. Comparing base (528d350) to head (542d4c3).

Files	Patch %	Lines
crates/gateway/src/compilation.rs	81.81%	5 Missing and 3 partials ⚠️
crates/gateway/src/config.rs	80.00%	1 Missing ⚠️
crates/gateway/src/gateway.rs	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #383      +/-   ##
==========================================
+ Coverage   83.28%   83.33%   +0.04%     
==========================================
  Files          37       37              
  Lines        1723     1734      +11     
  Branches     1723     1734      +11     
==========================================
+ Hits         1435     1445      +10     
- Misses        211      212       +1     
  Partials       77       77              

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

[yair-starkware]

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

---

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

    pub stateless_tx_validator_config: StatelessTransactionValidatorConfig,
    pub stateful_tx_validator_config: StatefulTransactionValidatorConfig,
    pub gateway_compiler_config: GatewayCompilerConfig,

Consider renaming to just compiler_config

Code quote:

gateway_

---

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

    let optional_class_info = match &tx {
        RPCTransaction::Declare(declare_tx) => {
            let gateway_compiler = GatewayCompiler { config: Default::default() };

GatewayCompiler::default()

Code quote:

GatewayCompiler { config: Default::default() };

---

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

    let optional_class_info = match &external_tx {
        RPCTransaction::Declare(declare_tx) => Some(
            GatewayCompiler { config: Default::default() }

GatewayCompiler::default()

Code quote:

GatewayCompiler { config: Default::default() }

---

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

            GatewayCompiler { config: Default::default() }
                .compile_contract_class(declare_tx)
                .unwrap(),

Shouldn't this return a result instead of unwrapping?
If not, I would still use expect instead of unwrap

Code quote:

.unwrap(),

[ArniStarkware]

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

---

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

Previously, yair-starkware (Yair) wrote…

Consider renaming to just compiler_config

I like the name you suggested.
Done.

---

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

Previously, yair-starkware (Yair) wrote…

GatewayCompiler::default()

It is odd. I see this struct as corresponding to StatelessTransactionValidator/ StatefulTransactionValidator.

These structs do not implement Default.
Very soon, this config is going to be non-empty. For actual cases - it is going to be loaded from the node-config. For tests it should be manually set.

---

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

Previously, yair-starkware (Yair) wrote…

GatewayCompiler::default()

As discussed in the previous point.

---

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

Previously, yair-starkware (Yair) wrote…

Shouldn't this return a result instead of unwrapping?
If not, I would still use expect instead of unwrap

This is a test util, so there is no need to be type-safe here.

Indeed, we might want to be more careful and rename this function accordingly.

[yair-starkware]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dafnamatsry and @Yael-Starkware)

[ArniStarkware]

Reviewable status: 1 of 9 files reviewed, all discussions resolved (waiting on @dafnamatsry, @Yael-Starkware, and @yair-starkware)

---

crates/gateway/src/compilation.rs line 25 at r4 (raw file):

    #[allow(dead_code)]
    pub config: GatewayCompilerConfig,
}

Maybe we want to give this file a better name?
Maybe we want to rename the struct?

Code quote:

pub struct GatewayCompiler {
    #[allow(dead_code)]
    pub config: GatewayCompilerConfig,
}

[dafnamatsry]

Reviewed 1 of 9 files at r1, 6 of 7 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware, @Yael-Starkware, and @yair-starkware)

---

crates/gateway/src/compilation.rs line 95 at r4 (raw file):

// or unsupported so that the compiler would alert us on new builtins.
fn supported_builtins() -> &'static Vec<String> {
    static SUPPORTED_BUILTINS: OnceLock<Vec<String>> = OnceLock::new();

Wasn't it defined with lazy static? Why did it changed?

---

crates/gateway/src/compilation_config.rs line 1 at r4 (raw file):

use std::collections::BTreeMap;

The rest of the config structs are in the config.rs right?
Lets be consistent.

---

crates/gateway/src/compilation_test.rs line 24 at r4 (raw file):

    let declare_tx = RPCDeclareTransaction::V3(tx);

    let result = GatewayCompiler { config: Default::default() }.compile_contract_class(&declare_tx);

Can you define it with a fixture?

Code quote:

GatewayCompiler { config: Default::default() }

[ArniStarkware]

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

---

crates/gateway/src/compilation.rs line 95 at r4 (raw file):

Previously, dafnamatsry wrote…

Wasn't it defined with lazy static? Why did it changed?

It was changed Per Gilad's request.
See PR: #353.
Lazy static is deprecated.

---

crates/gateway/src/compilation_test.rs line 24 at r4 (raw file):

Previously, dafnamatsry wrote…

Can you define it with a fixture?

Done.

---

crates/gateway/src/compilation_config.rs line 1 at r4 (raw file):

Previously, dafnamatsry wrote…

The rest of the config structs are in the config.rs right?
Lets be consistent.

removed.

[dafnamatsry]

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware and @yair-starkware)

---

crates/gateway/src/compilation.rs line 95 at r4 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

It was changed Per Gilad's request.
See PR: #353.
Lazy static is deprecated.

Thanks!

[ArniStarkware]

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware and @yair-starkware)

[ArniStarkware]

Merge activity

• Jul 14, 7:12 AM EDT: @ArniStarkware started a stack merge that includes this pull request via Graphite.
• Jul 14, 7:12 AM EDT: @ArniStarkware merged this pull request with Graphite.