Refactor/env utils #447
Refactor/env utils #447
Conversation
crates/utils/src/env_utils.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn get_env_car_optional_or_panic(key: &str) -> Option<String> { |
There was a problem hiding this comment.
| pub fn get_env_car_optional_or_panic(key: &str) -> Option<String> { | |
| pub fn get_env_var_optional_or_panic(key: &str) -> Option<String> { |
crates/utils/src/env_utils.rs
Outdated
There was a problem hiding this comment.
should we keep all if they aren't being used right now?
There was a problem hiding this comment.
Valid, we'll add them when we need them.
Removed for now.
crates/utils/src/lib.rs
Outdated
| @@ -0,0 +1 @@ | |||
| pub mod env_utils; | |||
There was a problem hiding this comment.
Do we need a full crate for this? cc @notlesh
There was a problem hiding this comment.
Agree, these are some pretty short utility functions, an entire utils crate seems like overkill to me as it would come with a lot of extra overhead.
There was a problem hiding this comment.
@heemankv I would suggest so. Or just inline the code and skip the functions part.
There was a problem hiding this comment.
Alright added, can you re-review @whichqua
| @@ -11,6 +12,12 @@ use starknet_api::{contract_address, felt, patricia_key}; | |||
|
|
|||
| use crate::utils::{felt_to_u128, FeltConversionError}; | |||
|
|
|||
| /// Fetches environment variable based on provided key. | |||
| /// If key is not found in env, it returns default value provided | |||
| pub fn get_env_var_or_default(key: &str, default: &str) -> String { | |||
There was a problem hiding this comment.
| pub fn get_env_var_or_default(key: &str, default: &str) -> String { | |
| #[inline] | |
| fn get_env_var_or_default(key: &str, default: &str) -> String { |
crates/rpc-replay/Cargo.toml
Outdated
| @@ -16,6 +16,7 @@ starknet_api = { workspace = true } | |||
| starknet-os-types = { workspace = true } | |||
| thiserror = { workspace = true } | |||
| tokio = { workspace = true } | |||
| serial_test = "3.2.0" | |||
There was a problem hiding this comment.
This is a dev dependency. +1 if we can avoid adding it
There was a problem hiding this comment.
I have now updated it to be a dev-dependency, I hope that solves the issue,
This is required to ensure
test_build_block_context_with_default_env_workstest_build_block_context_with_default_env_fails
works together properly.
WDYT ?
| ); | ||
| let eth_fee_token_address = get_env_var_or_default( | ||
| "SNOS_ETH_FEE_TOKEN_ADDRESS", | ||
| "0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7", |
There was a problem hiding this comment.
Consider creating constants for these, something like:
const DEFAULT_ETH_FEE_TOKEN_ADDRESS = ....
Cargo.toml
Outdated
There was a problem hiding this comment.
Are these changes needed?
There was a problem hiding this comment.
These are lint changes, will revert to keep as is.
notlesh
merged commit 3c95181
into
keep-starknet-strange:tmp/snos_devnet_zero_txs
Issue Number: N/A
Type
Description
strkandethtoken addresses found inrpc-replaycrate.Breaking changes?