Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(katana): gas oracle skeleton #2643

Merged
merged 6 commits into from
Nov 6, 2024
Merged

feat(katana): gas oracle skeleton #2643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
879b921
implement gas oracle skeleton
kariy Nov 6, 2024
9eba04d
fix test
kariy Nov 6, 2024
bdd1884
add genesis test
kariy Nov 6, 2024
e990069
update block env to use the updated values for gas prices
kariy Nov 6, 2024
e7fb1c4
update test db due to genesis change in the cli
kariy Nov 6, 2024
e338d47
udpate test db after mergin main
kariy Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bin/katana/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ url.workspace = true

[dev-dependencies]
assert_matches.workspace = true
starknet.workspace = true

[features]
default = [ "jemalloc", "slot" ]
Expand Down
153 changes: 125 additions & 28 deletions bin/katana/src/cli/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@ use anyhow::{Context, Result};
use clap::{Args, Parser};
use console::Style;
use dojo_utils::parse::parse_socket_address;
use katana_core::constants::{
DEFAULT_ETH_L1_GAS_PRICE, DEFAULT_SEQUENCER_ADDRESS, DEFAULT_STRK_L1_GAS_PRICE,
};
use katana_core::constants::DEFAULT_SEQUENCER_ADDRESS;
use katana_core::service::messaging::MessagingConfig;
use katana_node::config::db::DbConfig;
use katana_node::config::dev::DevConfig;
use katana_node::config::dev::{DevConfig, FixedL1GasPriceConfig};
use katana_node::config::execution::{
ExecutionConfig, DEFAULT_INVOCATION_MAX_STEPS, DEFAULT_VALIDATION_MAX_STEPS,
};
Expand All @@ -34,7 +32,7 @@ use katana_node::config::rpc::{
ApiKind, RpcConfig, DEFAULT_RPC_ADDR, DEFAULT_RPC_MAX_CONNECTIONS, DEFAULT_RPC_PORT,
};
use katana_node::config::{Config, SequencingConfig};
use katana_primitives::block::BlockHashOrNumber;
use katana_primitives::block::{BlockHashOrNumber, GasPrices};
use katana_primitives::chain::ChainId;
use katana_primitives::chain_spec::{self, ChainSpec};
use katana_primitives::class::ClassHash;
Expand Down Expand Up @@ -189,25 +187,31 @@ pub struct EnvironmentOptions {

#[arg(long)]
#[arg(help = "The maximum number of steps available for the account validation logic.")]
#[arg(default_value_t = DEFAULT_VALIDATION_MAX_STEPS)]
pub validate_max_steps: u32,
pub validate_max_steps: Option<u32>,

#[arg(long)]
#[arg(help = "The maximum number of steps available for the account execution logic.")]
#[arg(default_value_t = DEFAULT_INVOCATION_MAX_STEPS)]
pub invoke_max_steps: u32,
pub invoke_max_steps: Option<u32>,

#[arg(long = "eth-gas-price")]
#[arg(conflicts_with = "genesis")]
#[arg(long = "l1-eth-gas-price", value_name = "WEI")]
#[arg(help = "The L1 ETH gas price. (denominated in wei)")]
#[arg(default_value_t = DEFAULT_ETH_L1_GAS_PRICE)]
pub l1_eth_gas_price: u128,
#[arg(requires = "l1_strk_gas_price")]
pub l1_eth_gas_price: Option<u128>,

#[arg(long = "strk-gas-price")]
#[arg(conflicts_with = "genesis")]
#[arg(long = "l1-strk-gas-price", value_name = "FRI")]
#[arg(requires = "l1_eth_data_gas_price")]
#[arg(help = "The L1 STRK gas price. (denominated in fri)")]
#[arg(default_value_t = DEFAULT_STRK_L1_GAS_PRICE)]
pub l1_strk_gas_price: u128,
pub l1_strk_gas_price: Option<u128>,

#[arg(long = "l1-eth-data-gas-price", value_name = "WEI")]
#[arg(requires = "l1_strk_data_gas_price")]
#[arg(help = "The L1 ETH gas price. (denominated in wei)")]
pub l1_eth_data_gas_price: Option<u128>,

#[arg(long = "l1-strk-data-gas-price", value_name = "FRI")]
#[arg(requires = "l1_eth_gas_price")]
#[arg(help = "The L1 STRK gas prick. (denominated in fri)")]
pub l1_strk_data_gas_price: Option<u128>,
kariy marked this conversation as resolved.
Show resolved Hide resolved
kariy marked this conversation as resolved.
Show resolved Hide resolved
}

#[cfg(feature = "slot")]
Expand Down Expand Up @@ -320,6 +324,8 @@ impl NodeArgs {

if let Some(genesis) = self.starknet.genesis.clone() {
chain_spec.genesis = genesis;
} else {
chain_spec.genesis.sequencer_address = *DEFAULT_SEQUENCER_ADDRESS;
}

// generate dev accounts
Expand All @@ -329,9 +335,6 @@ impl NodeArgs {
.generate();

chain_spec.genesis.extend_allocations(accounts.into_iter().map(|(k, v)| (k, v.into())));
chain_spec.genesis.sequencer_address = *DEFAULT_SEQUENCER_ADDRESS;
chain_spec.genesis.gas_prices.eth = self.starknet.environment.l1_eth_gas_price;
chain_spec.genesis.gas_prices.strk = self.starknet.environment.l1_strk_gas_price;

#[cfg(feature = "slot")]
if self.slot.controller {
Expand All @@ -342,16 +345,42 @@ impl NodeArgs {
}

fn dev_config(&self) -> DevConfig {
let fixed_gas_prices = if self.starknet.environment.l1_eth_gas_price.is_some() {
// It is safe to unwrap all of these here because the CLI parser ensures if one is set,
// all must be set.

let eth_gas_price = self.starknet.environment.l1_eth_gas_price.unwrap();
let strk_gas_price = self.starknet.environment.l1_strk_gas_price.unwrap();
let eth_data_gas_price = self.starknet.environment.l1_eth_data_gas_price.unwrap();
let strk_data_gas_price = self.starknet.environment.l1_strk_data_gas_price.unwrap();

let gas_price = GasPrices { eth: eth_gas_price, strk: strk_gas_price };
let data_gas_price = GasPrices { eth: eth_data_gas_price, strk: strk_data_gas_price };

Some(FixedL1GasPriceConfig { gas_price, data_gas_price })
} else {
None
};

DevConfig {
fixed_gas_prices,
fee: !self.starknet.disable_fee,
account_validation: !self.starknet.disable_validate,
}
}

fn execution_config(&self) -> ExecutionConfig {
ExecutionConfig {
invocation_max_steps: self.starknet.environment.invoke_max_steps,
validation_max_steps: self.starknet.environment.validate_max_steps,
invocation_max_steps: self
.starknet
.environment
.invoke_max_steps
.unwrap_or(DEFAULT_INVOCATION_MAX_STEPS),
validation_max_steps: self
.starknet
.environment
.validate_max_steps
.unwrap_or(DEFAULT_VALIDATION_MAX_STEPS),
..Default::default()
}
}
Expand Down Expand Up @@ -487,6 +516,9 @@ PREFUNDED ACCOUNTS

#[cfg(test)]
mod test {
use assert_matches::assert_matches;
use katana_primitives::{address, felt};

use super::*;

#[test]
Expand All @@ -501,8 +533,6 @@ mod test {
assert_eq!(config.execution.validation_max_steps, DEFAULT_VALIDATION_MAX_STEPS);
assert_eq!(config.db.dir, None);
assert_eq!(config.chain.id, ChainId::parse("KATANA").unwrap());
assert_eq!(config.chain.genesis.gas_prices.eth, DEFAULT_ETH_L1_GAS_PRICE);
assert_eq!(config.chain.genesis.gas_prices.strk, DEFAULT_STRK_L1_GAS_PRICE);
assert_eq!(config.chain.genesis.sequencer_address, *DEFAULT_SEQUENCER_ADDRESS);
}

Expand All @@ -520,10 +550,40 @@ mod test {
"100",
"--db-dir",
"/path/to/db",
"--eth-gas-price",
]);
let config = args.config().unwrap();

assert!(!config.dev.fee);
assert!(!config.dev.account_validation);
assert_eq!(config.execution.invocation_max_steps, 200);
assert_eq!(config.execution.validation_max_steps, 100);
assert_eq!(config.db.dir, Some(PathBuf::from("/path/to/db")));
assert_eq!(config.chain.id, ChainId::GOERLI);
assert_eq!(config.chain.genesis.sequencer_address, *DEFAULT_SEQUENCER_ADDRESS);
}

#[test]
fn custom_fixed_gas_prices() {
let args = NodeArgs::parse_from([
"katana",
"--disable-fee",
"--disable-validate",
"--chain-id",
"SN_GOERLI",
"--invoke-max-steps",
"200",
"--validate-max-steps",
"100",
"--db-dir",
"/path/to/db",
"--l1-eth-gas-price",
"10",
"--strk-gas-price",
"--l1-strk-gas-price",
"20",
"--l1-eth-data-gas-price",
"1",
"--l1-strk-data-gas-price",
"2",
]);
let config = args.config().unwrap();

Expand All @@ -533,7 +593,44 @@ mod test {
assert_eq!(config.execution.validation_max_steps, 100);
assert_eq!(config.db.dir, Some(PathBuf::from("/path/to/db")));
assert_eq!(config.chain.id, ChainId::GOERLI);
assert_eq!(config.chain.genesis.gas_prices.eth, 10);
assert_eq!(config.chain.genesis.gas_prices.strk, 20);
assert_matches!(config.dev.fixed_gas_prices, Some(prices) => {
assert_eq!(prices.gas_price.eth, 10);
assert_eq!(prices.gas_price.strk, 20);
assert_eq!(prices.data_gas_price.eth, 1);
assert_eq!(prices.data_gas_price.strk, 2);
})
}

#[test]
fn genesis_with_fixed_gas_prices() {
let config = NodeArgs::parse_from([
"katana",
"--genesis",
"./tests/test-data/genesis.json",
"--l1-eth-gas-price",
"100",
"--l1-strk-gas-price",
"200",
"--l1-eth-data-gas-price",
"111",
"--l1-strk-data-gas-price",
"222",
])
.config()
.unwrap();

assert_eq!(config.chain.genesis.number, 0);
assert_eq!(config.chain.genesis.parent_hash, felt!("0x999"));
assert_eq!(config.chain.genesis.timestamp, 5123512314);
assert_eq!(config.chain.genesis.state_root, felt!("0x99"));
assert_eq!(config.chain.genesis.sequencer_address, address!("0x100"));
assert_eq!(config.chain.genesis.gas_prices.eth, 9999);
assert_eq!(config.chain.genesis.gas_prices.strk, 8888);
assert_matches!(config.dev.fixed_gas_prices, Some(prices) => {
assert_eq!(prices.gas_price.eth, 100);
assert_eq!(prices.gas_price.strk, 200);
assert_eq!(prices.data_gas_price.eth, 111);
assert_eq!(prices.data_gas_price.strk, 222);
})
}
}
62 changes: 22 additions & 40 deletions bin/katana/tests/test-data/genesis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,24 @@
{
"number": 0,
"parentHash": "0x999",
"timestamp": 5123512314,
"stateRoot": "0x99",
"sequencerAddress": "0x100",
"gasPrices": {
"ETH": 1111,
"STRK": 2222
},
"feeToken": {
"address": "0x55",
"name": "ETHER",
"symbol": "ETH",
"decimals": 18,
"storage": {
"0x111": "0x1",
"0x222": "0x2"
}
},
"universalDeployer": {
"address": "0x041a78e741e5af2fec34b695679bc6891742439f7afb8484ecd7766661ad02bf",
"storage": {
"0x10": "0x100"
}
},
"accounts": {
"0x66efb28ac62686966ae85095ff3a772e014e7fbf56d4c5f6fac5606d4dde23a": {
"publicKey": "0x1",
"balance": "0xD3C21BCECCEDA1000000",
"nonce": "0x1",
"storage": {
"0x1": "0x1",
"0x2": "0x2"
}
}
},
"contracts": {
},
"classes": [
]
"number": 0,
"parentHash": "0x999",
"timestamp": 5123512314,
"stateRoot": "0x99",
"sequencerAddress": "0x100",
"gasPrices": {
"ETH": 9999,
"STRK": 8888
},
"accounts": {
"0x66efb28ac62686966ae85095ff3a772e014e7fbf56d4c5f6fac5606d4dde23a": {
"publicKey": "0x1",
"balance": "0xD3C21BCECCEDA1000000",
"nonce": "0x1",
"storage": {
"0x1": "0x1",
"0x2": "0x2"
}
}
},
"contracts": {},
"classes": []
}
2 changes: 1 addition & 1 deletion crates/dojo/test-utils/src/sequencer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl TestSequencer {
}

pub fn get_default_test_config(sequencing: SequencingConfig) -> Config {
let dev = DevConfig { fee: false, account_validation: true };
let dev = DevConfig { fee: false, account_validation: true, fixed_gas_prices: None };
let mut chain = ChainSpec { id: ChainId::SEPOLIA, ..Default::default() };
chain.genesis.sequencer_address = *DEFAULT_SEQUENCER_ADDRESS;

Expand Down
25 changes: 25 additions & 0 deletions crates/katana/core/src/backend/gas_oracle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use katana_primitives::block::GasPrices;

// TODO: implement a proper gas oracle function - sample the l1 gas and data gas prices
// currently this just return the hardcoded value set from the cli or if not set, the default value.
#[derive(Debug)]
pub struct L1GasOracle {
gas_prices: GasPrices,
data_gas_prices: GasPrices,
}

impl L1GasOracle {
pub fn fixed(gas_prices: GasPrices, data_gas_prices: GasPrices) -> Self {
Self { gas_prices, data_gas_prices }
}

/// Returns the current gas prices.
pub fn current_gas_prices(&self) -> GasPrices {
self.gas_prices.clone()
}

/// Returns the current data gas prices.
pub fn current_data_gas_prices(&self) -> GasPrices {
self.data_gas_prices.clone()
}
Comment on lines +16 to +24
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider optimizing getter methods to avoid unnecessary cloning.

The current implementation clones the gas prices on every call. Consider returning references instead:

-    pub fn current_gas_prices(&self) -> GasPrices {
-        self.gas_prices.clone()
+    pub fn current_gas_prices(&self) -> &GasPrices {
+        &self.gas_prices
     }

-    pub fn current_data_gas_prices(&self) -> GasPrices {
-        self.data_gas_prices.clone()
+    pub fn current_data_gas_prices(&self) -> &GasPrices {
+        &self.data_gas_prices
     }

This change would improve performance by avoiding unnecessary cloning, especially if these methods are called frequently during block updates.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Returns the current gas prices.
pub fn current_gas_prices(&self) -> GasPrices {
self.gas_prices.clone()
}
/// Returns the current data gas prices.
pub fn current_data_gas_prices(&self) -> GasPrices {
self.data_gas_prices.clone()
}
/// Returns the current gas prices.
pub fn current_gas_prices(&self) -> &GasPrices {
&self.gas_prices
}
/// Returns the current data gas prices.
pub fn current_data_gas_prices(&self) -> &GasPrices {
&self.data_gas_prices
}

}
13 changes: 13 additions & 0 deletions crates/katana/core/src/backend/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use gas_oracle::L1GasOracle;
use katana_executor::{ExecutionOutput, ExecutionResult, ExecutorFactory};
use katana_primitives::block::{
FinalityStatus, Header, PartialHeader, SealedBlock, SealedBlockWithStatus,
Expand All @@ -20,6 +21,7 @@ use starknet_types_core::hash::{self, StarkHash};
use tracing::info;

pub mod contract;
pub mod gas_oracle;
pub mod storage;

use self::storage::Blockchain;
Expand All @@ -38,6 +40,8 @@ pub struct Backend<EF: ExecutorFactory> {
pub block_context_generator: RwLock<BlockContextGenerator>,

pub executor_factory: Arc<EF>,

pub gas_oracle: L1GasOracle,
}

impl<EF: ExecutorFactory> Backend<EF> {
Expand Down Expand Up @@ -104,6 +108,15 @@ impl<EF: ExecutorFactory> Backend<EF> {

block_env.number += 1;
block_env.timestamp = timestamp;

// update the gas prices
self.update_block_gas_prices(block_env);
}

/// Updates the gas prices in the block environment.
pub fn update_block_gas_prices(&self, block_env: &mut BlockEnv) {
block_env.l1_gas_prices = self.gas_oracle.current_gas_prices();
block_env.l1_data_gas_prices = self.gas_oracle.current_data_gas_prices();
}

pub fn mine_empty_block(
Expand Down
Loading
Loading