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.

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

refactor(katana-rpc): remove some components dependency from starknet rpc handler #2759

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 4 additions & 13 deletions crates/katana/node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
use katana_executor::{ExecutionFlags, ExecutorFactory};
use katana_pipeline::stage::Sequencing;
use katana_pool::ordering::FiFo;
use katana_pool::validation::stateful::TxValidator;
use katana_pool::TxPool;
use katana_primitives::block::GasPrices;
use katana_primitives::env::{CfgEnv, FeeTokenAddressses};
Expand Down Expand Up @@ -119,7 +118,6 @@
let pool = self.pool.clone();
let backend = self.backend.clone();
let block_producer = self.block_producer.clone();
let validator = self.block_producer.validator().clone();

// --- build and run sequencing task

Expand All @@ -138,7 +136,7 @@
.name("Sequencing")
.spawn(sequencing.into_future());

let node_components = (pool, backend, block_producer, validator, self.forked_client.take());
let node_components = (pool, backend, block_producer, self.forked_client.take());

Check warning on line 139 in crates/katana/node/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/node/src/lib.rs#L139

Added line #L139 was not covered by tests
let rpc = spawn(node_components, self.config.rpc.clone()).await?;

Ok(LaunchedNode { node: self, rpc })
Expand Down Expand Up @@ -250,16 +248,10 @@

// Moved from `katana_rpc` crate
pub async fn spawn<EF: ExecutorFactory>(
node_components: (
TxPool,
Arc<Backend<EF>>,
BlockProducer<EF>,
TxValidator,
Option<ForkedClient>,
),
node_components: (TxPool, Arc<Backend<EF>>, BlockProducer<EF>, Option<ForkedClient>),
config: RpcConfig,
) -> Result<RpcServer> {
let (pool, backend, block_producer, validator, forked_client) = node_components;
let (pool, backend, block_producer, forked_client) = node_components;

let mut methods = RpcModule::new(());
methods.register_method("health", |_, _| Ok(serde_json::json!({ "health": true })))?;
Expand All @@ -272,12 +264,11 @@
backend.clone(),
pool.clone(),
block_producer.clone(),
validator,
client,
cfg,
)
} else {
StarknetApi::new(backend.clone(), pool.clone(), block_producer.clone(), validator, cfg)
StarknetApi::new(backend.clone(), pool.clone(), Some(block_producer.clone()), cfg)
};

methods.merge(StarknetApiServer::into_rpc(server.clone()))?;
Expand Down
7 changes: 7 additions & 0 deletions crates/katana/rpc/rpc/src/starknet/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[derive(Debug, Clone)]
pub struct StarknetApiConfig {
/// The max chunk size that can be served from the `getEvents` method.
///
/// If `None`, the maximum chunk size is bounded by [`u64::MAX`].
pub max_event_page_size: Option<u64>,
}
86 changes: 49 additions & 37 deletions crates/katana/rpc/rpc/src/starknet/mod.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
//! Server implementation for the Starknet JSON-RPC API.

pub mod forking;
mod read;
mod trace;
mod write;

use std::sync::Arc;

use forking::ForkedClient;
use katana_core::backend::Backend;
use katana_core::service::block_producer::{BlockProducer, BlockProducerMode, PendingExecutor};
use katana_executor::{ExecutionResult, ExecutorFactory};
use katana_pool::validation::stateful::TxValidator;
use katana_pool::{TransactionPool, TxPool};
use katana_primitives::block::{
BlockHash, BlockHashOrNumber, BlockIdOrTag, BlockNumber, BlockTag, FinalityStatus,
Expand Down Expand Up @@ -51,75 +44,85 @@
use crate::utils;
use crate::utils::events::{Cursor, EventBlockId};

pub type StarknetApiResult<T> = Result<T, StarknetApiError>;
mod config;
pub mod forking;
mod read;
mod trace;
mod write;

#[allow(missing_debug_implementations)]
pub struct StarknetApi<EF: ExecutorFactory> {
inner: Arc<Inner<EF>>,
}
pub use config::StarknetApiConfig;
use forking::ForkedClient;

#[derive(Debug, Clone)]
pub struct StarknetApiConfig {
pub max_event_page_size: Option<u64>,
}
type StarknetApiResult<T> = Result<T, StarknetApiError>;

impl<EF: ExecutorFactory> Clone for StarknetApi<EF> {
fn clone(&self) -> Self {
Self { inner: Arc::clone(&self.inner) }
}
/// Handler for the Starknet JSON-RPC server.
///
/// This struct implements all the JSON-RPC traits required to serve the Starknet API (ie,
/// [read](katana_rpc_api::starknet::StarknetApi),
/// [write](katana_rpc_api::starknet::StarknetWriteApi), and
/// [trace](katana_rpc_api::starknet::StarknetTraceApi) APIs.
#[allow(missing_debug_implementations)]
pub struct StarknetApi<EF>
where
EF: ExecutorFactory,
{
inner: Arc<StarknetApiInner<EF>>,

Check warning on line 69 in crates/katana/rpc/rpc/src/starknet/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/starknet/mod.rs#L67-L69

Added lines #L67 - L69 were not covered by tests
}

struct Inner<EF: ExecutorFactory> {
validator: TxValidator,
struct StarknetApiInner<EF>
where
EF: ExecutorFactory,
{
pool: TxPool,
backend: Arc<Backend<EF>>,
block_producer: BlockProducer<EF>,
blocking_task_pool: BlockingTaskPool,
forked_client: Option<ForkedClient>,
blocking_task_pool: BlockingTaskPool,
block_producer: Option<BlockProducer<EF>>,
Comment on lines +79 to +80
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sensei, ensure proper handling of None for block_producer.

Since block_producer is now an Option, please verify that all usages of block_producer in StarknetApiInner safely handle the None case to prevent potential None dereferences and runtime errors.

config: StarknetApiConfig,
}

impl<EF: ExecutorFactory> StarknetApi<EF> {
impl<EF> StarknetApi<EF>
where
EF: ExecutorFactory,
{

Check warning on line 87 in crates/katana/rpc/rpc/src/starknet/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/starknet/mod.rs#L84-L87

Added lines #L84 - L87 were not covered by tests
pub fn new(
backend: Arc<Backend<EF>>,
pool: TxPool,
block_producer: BlockProducer<EF>,
validator: TxValidator,
block_producer: Option<BlockProducer<EF>>,
config: StarknetApiConfig,
) -> Self {
Self::new_inner(backend, pool, block_producer, validator, None, config)
Self::new_inner(backend, pool, block_producer, None, config)

Check warning on line 94 in crates/katana/rpc/rpc/src/starknet/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/starknet/mod.rs#L94

Added line #L94 was not covered by tests
}

pub fn new_forked(
backend: Arc<Backend<EF>>,
pool: TxPool,
block_producer: BlockProducer<EF>,
validator: TxValidator,
forked_client: ForkedClient,
config: StarknetApiConfig,
) -> Self {
Self::new_inner(backend, pool, block_producer, validator, Some(forked_client), config)
Self::new_inner(backend, pool, Some(block_producer), Some(forked_client), config)
}

fn new_inner(
backend: Arc<Backend<EF>>,
pool: TxPool,
block_producer: BlockProducer<EF>,
validator: TxValidator,
block_producer: Option<BlockProducer<EF>>,
forked_client: Option<ForkedClient>,
config: StarknetApiConfig,
) -> Self {
let blocking_task_pool =
BlockingTaskPool::new().expect("failed to create blocking task pool");
let inner = Inner {

let inner = StarknetApiInner {
pool,
backend,
block_producer,
blocking_task_pool,
validator,
forked_client,
config,
};

Self { inner: Arc::new(inner) }
}

Expand Down Expand Up @@ -184,10 +187,10 @@

/// Returns the pending state if the sequencer is running in _interval_ mode. Otherwise `None`.
fn pending_executor(&self) -> Option<PendingExecutor> {
match &*self.inner.block_producer.producer.read() {
self.inner.block_producer.as_ref().and_then(|bp| match &*bp.producer.read() {
BlockProducerMode::Instant(_) => None,
BlockProducerMode::Interval(producer) => Some(producer.executor()),
}
})
Comment on lines +190 to +193
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential None dereference in pending_executor method.

In the pending_executor method, when accessing self.inner.block_producer.as_ref(), please ensure that the scenario where block_producer is None is properly handled to avoid panics.

}

fn state(&self, block_id: &BlockIdOrTag) -> StarknetApiResult<Box<dyn StateProvider>> {
Expand Down Expand Up @@ -357,7 +360,7 @@
// TODO: this is a temporary solution, we should have a better way to handle this.
// perhaps a pending/pool state provider that implements all the state provider traits.
let result = if let BlockIdOrTag::Tag(BlockTag::Pending) = block_id {
this.inner.validator.pool_nonce(contract_address)?
this.inner.pool.validator().pool_nonce(contract_address)?

Check warning on line 363 in crates/katana/rpc/rpc/src/starknet/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/starknet/mod.rs#L363

Added line #L363 was not covered by tests
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Careful, sensei! validator() may no longer be available.

The call to this.inner.pool.validator() might be invalid since TxValidator has been removed from StarknetApi dependencies. Please update this call to reflect the new structure or provide an alternative means to access the required functionality.

} else {
let state = this.state(&block_id)?;
state.nonce(contract_address)?
Expand Down Expand Up @@ -1122,3 +1125,12 @@
Ok(id)
}
}

impl<EF> Clone for StarknetApi<EF>
where
EF: ExecutorFactory,
{
fn clone(&self) -> Self {
Self { inner: Arc::clone(&self.inner) }
}
}
Loading