refactor: move VersionedConstants macro, errors from Blockifier to StarknetAPI

[ayeletstarkware]

Refactoring the general versioned constants logic to make it reusable for Consensus in the next PR.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2025-01-08T15:49:09Z
• Started at 2025-01-08T15:51:57Z
• Started at 2025-01-09T16:00:46Z
• Started at 2025-01-12T11:25:12Z
• Started at 2025-01-13T15:09:06Z
• Started at 2025-01-14T12:41:51Z

[ayeletstarkware]

• refactor(consensus): use versioned constants in fee market, consensus context #3228
• feat(consensus): add versioned constants to Consensus #3223
• refactor: move VersionedConstants macro, errors from Blockifier to StarknetAPI #3195 👈 (View in Graphite)
• main

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

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.888 ms 35.374 ms 35.948 ms]
change: [+1.9400% +3.4033% +5.0744%] (p = 0.00 < 0.05)
Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
3 (3.00%) high mild
7 (7.00%) high severe

[dorimedini-starkware]

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ayeletstarkware)

---

crates/starknet_api/src/versioned_constants_logic.rs line 17 at r1 (raw file):

    #[error("Invalid Starknet version: {0}")]
    InvalidStarknetVersion(StarknetVersion),
}

you are planning on refactoring the macro into a more generic "X_constants" generating macro, right?
why not add the error type as an argument to the macro?
I guess you could even generate the error enum automatically.
Also, you can use paste! { $struct_name Error } to avoid an extra arg.

My issue is: you shouldn't be mentioning "versioned constants" in starknet-api; in any case I'd want the error enum definition to move back to blockifier crate

Code quote:

#[derive(Debug, Error)]
pub enum VersionedConstantsError {
    #[error(transparent)]
    IoError(#[from] io::Error),
    #[error("JSON file cannot be serialized into VersionedConstants: {0}")]
    ParseError(#[from] serde_json::Error),
    #[error("Invalid version: {version:?}")]
    InvalidVersion { version: String },
    #[error("Invalid Starknet version: {0}")]
    InvalidStarknetVersion(StarknetVersion),
}

---

crates/starknet_api/src/versioned_constants_logic.rs line 33 at r1 (raw file):

        use starknet_api::versioned_constants_logic::{
            VersionedConstantsError, VersionedConstantsResult,
        };

please don't use uses in macros: if you need the types, I would fully-qualify them

Code quote:

        use starknet_infra_utils::compile_time_cargo_manifest_dir;
        use paste::paste;
        use std::fs;
        use std::path::PathBuf;
        use std::sync::LazyLock;
        use starknet_api::block::StarknetVersion;
        use starknet_api::versioned_constants_logic::{
            VersionedConstantsError, VersionedConstantsResult,
        };

[dorimedini-starkware]

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

---

a discussion (no related file):
please add yoni/noa as additional reviewer

[ayeletstarkware]

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

---

a discussion (no related file):

Previously, dorimedini-starkware wrote…

please add yoni/noa as additional reviewer

Done.

---

crates/starknet_api/src/versioned_constants_logic.rs line 17 at r1 (raw file):

Previously, dorimedini-starkware wrote…

you are planning on refactoring the macro into a more generic "X_constants" generating macro, right?
why not add the error type as an argument to the macro?
I guess you could even generate the error enum automatically.
Also, you can use paste! { $struct_name Error } to avoid an extra arg.

My issue is: you shouldn't be mentioning "versioned constants" in starknet-api; in any case I'd want the error enum definition to move back to blockifier crate

It's not a generic X_constants macro. It's for versioned constants in Blockifier and Consensus, each with different constants.

Why pass the error type as an argument? The errors are currently related to parsing and versioning. If different structs need unique errors in the future, we can refactor.

Can you explain how to generate the error enum automatically and how paste! { $struct_name Error } would be used?

---

crates/starknet_api/src/versioned_constants_logic.rs line 33 at r1 (raw file):

Previously, dorimedini-starkware wrote…

please don't use uses in macros: if you need the types, I would fully-qualify them

Done. can you explain why not use use in macros? Fully qualifying types feels much longer.

[dorimedini-starkware]

Reviewable status: 4 of 7 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @noaov1)

---

crates/starknet_api/src/versioned_constants_logic.rs line 17 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

It's not a generic X_constants macro. It's for versioned constants in Blockifier and Consensus, each with different constants.

Why pass the error type as an argument? The errors are currently related to parsing and versioning. If different structs need unique errors in the future, we can refactor.

Can you explain how to generate the error enum automatically and how paste! { $struct_name Error } would be used?

VersionedConstants is the name of the blockifier constants; I guess consensus constants will have a different name. Thus, I don't want the VersionedConstantsError enum defined in starknet-api - it should be defined in the blockifier crate.
Since the error enum will have the same variants in the consensus context, you could do this in the macro implementation:

#[derive(Debug, Error)]
pub enum paste! { $struct_name Error } {
    #[error(transparent)]
    IoError(#[from] io::Error),
    #[error("JSON file cannot be serialized into {}: {0}", stringify!($struct_name))]
    ParseError(#[from] serde_json::Error),
    #[error("Invalid version: {version:?}")]
    InvalidVersion { version: String },
    #[error("Invalid Starknet version: {0}")]
    InvalidStarknetVersion(StarknetVersion),
}

if that doesn't work, you can add the error enum name as input to the macro

---

crates/starknet_api/src/versioned_constants_logic.rs line 33 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

Done. can you explain why not use use in macros? Fully qualifying types feels much longer.

because when you use the macro you implicitly use objects; i.e. invoking the macro imports objects into the scope that you may not want

[ayeletstarkware]

Reviewable status: 1 of 7 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

crates/starknet_api/src/versioned_constants_logic.rs line 17 at r1 (raw file):

Previously, dorimedini-starkware wrote…

VersionedConstants is the name of the blockifier constants; I guess consensus constants will have a different name. Thus, I don't want the VersionedConstantsError enum defined in starknet-api - it should be defined in the blockifier crate.
Since the error enum will have the same variants in the consensus context, you could do this in the macro implementation:

#[derive(Debug, Error)]
pub enum paste! { $struct_name Error } {
    #[error(transparent)]
    IoError(#[from] io::Error),
    #[error("JSON file cannot be serialized into {}: {0}", stringify!($struct_name))]
    ParseError(#[from] serde_json::Error),
    #[error("Invalid version: {version:?}")]
    InvalidVersion { version: String },
    #[error("Invalid Starknet version: {0}")]
    InvalidStarknetVersion(StarknetVersion),
}

if that doesn't work, you can add the error enum name as input to the macro

didn't work. passed it as an argument to the macro.

[dorimedini-starkware]

Reviewed 1 of 3 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @noaov1)

---

crates/blockifier/src/versioned_constants.rs line 1102 at r3 (raw file):

}

pub type VersionedConstantsResult<T> = Result<T, VersionedConstantsError>;

why move this back, and not create this error enum in the macro?

Code quote:

#[derive(Debug, Error)]
pub enum VersionedConstantsError {
    #[error(transparent)]
    IoError(#[from] io::Error),
    #[error("JSON file cannot be serialized into VersionedConstants: {0}")]
    ParseError(#[from] serde_json::Error),
    #[error("Invalid version: {version:?}")]
    InvalidVersion { version: String },
    #[error("Invalid Starknet version: {0}")]
    InvalidStarknetVersion(StarknetVersion),
}

pub type VersionedConstantsResult<T> = Result<T, VersionedConstantsError>;

[ayeletstarkware]

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

---

crates/blockifier/src/versioned_constants.rs line 1102 at r3 (raw file):

Previously, dorimedini-starkware wrote…

why move this back, and not create this error enum in the macro?

The only error from this enum used in the macro is InvalidStarknetVersion, so passing it as an argument makes sense.

[dorimedini-starkware]

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

[alonh5]

Reviewed 1 of 6 files at r1, 1 of 3 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)

[dorimedini-starkware]

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