Skip to content

Commit

Permalink
chore(starknet_l1_provider): use client (#2649)
Browse files Browse the repository at this point in the history
- in batcher: replace their dummy l1 provider client with the real
  thing. The only effective change in behavior is that `validate` now
  returns an enum instead of a bool.
- changes in sequencer_node are boilerplate, did nothing different from
  other similar nodes.
- Added a field in the l1 config, currently unused, in order to make the
  config test pass which seems to require at least one field.

Co-Authored-By: Gilad Chase <gilad@starkware.com>
  • Loading branch information
giladchase and Gilad Chase authored Dec 17, 2024
1 parent 42c5799 commit 995a89a
Show file tree
Hide file tree
Showing 17 changed files with 223 additions and 53 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

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

55 changes: 55 additions & 0 deletions config/sequencer/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,56 @@
"privacy": "Public",
"value": "0.0.0.0:8080"
},
"components.l1_provider.execution_mode": {
"description": "The component execution mode.",
"privacy": "Public",
"value": "LocalExecutionWithRemoteDisabled"
},
"components.l1_provider.local_server_config.#is_none": {
"description": "Flag for an optional field.",
"privacy": "TemporaryValue",
"value": false
},
"components.l1_provider.local_server_config.channel_buffer_size": {
"description": "The communication channel buffer size.",
"privacy": "Public",
"value": 32
},
"components.l1_provider.remote_client_config.#is_none": {
"description": "Flag for an optional field.",
"privacy": "TemporaryValue",
"value": true
},
"components.l1_provider.remote_client_config.idle_connections": {
"description": "The maximum number of idle connections to keep alive.",
"privacy": "Public",
"value": 18446744073709551615
},
"components.l1_provider.remote_client_config.idle_timeout": {
"description": "The duration in seconds to keep an idle connection open before closing.",
"privacy": "Public",
"value": 90
},
"components.l1_provider.remote_client_config.retries": {
"description": "The max number of retries for sending a message.",
"privacy": "Public",
"value": 3
},
"components.l1_provider.remote_client_config.socket": {
"description": "The remote component server socket.",
"privacy": "Public",
"value": "0.0.0.0:8080"
},
"components.l1_provider.remote_server_config.#is_none": {
"description": "Flag for an optional field.",
"privacy": "TemporaryValue",
"value": true
},
"components.l1_provider.remote_server_config.socket": {
"description": "The remote component server socket.",
"privacy": "Public",
"value": "0.0.0.0:8080"
},
"components.mempool.execution_mode": {
"description": "The component execution mode.",
"privacy": "Public",
Expand Down Expand Up @@ -784,6 +834,11 @@
"privacy": "Public",
"value": 8080
},
"l1_provider_config._poll_interval": {
"description": "Interval in milliseconds between each scraping attempt of L1.",
"privacy": "Public",
"value": 100
},
"mempool_p2p_config.network_buffer_size": {
"description": "Network buffer size.",
"privacy": "Public",
Expand Down
2 changes: 2 additions & 0 deletions crates/starknet_batcher/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ papyrus_storage.workspace = true
serde.workspace = true
starknet_api.workspace = true
starknet_batcher_types.workspace = true
starknet_l1_provider_types.workspace = true
starknet_mempool_types.workspace = true
starknet_sequencer_infra.workspace = true
starknet_state_sync_types.workspace = true
Expand All @@ -36,4 +37,5 @@ mockall.workspace = true
rstest.workspace = true
starknet-types-core.workspace = true
starknet_api = { workspace = true, features = ["testing"] }
starknet_l1_provider_types = { workspace = true, features = ["testing"] }
starknet_mempool_types = { workspace = true, features = ["testing"] }
23 changes: 13 additions & 10 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use starknet_batcher_types::batcher_types::{
ValidateBlockInput,
};
use starknet_batcher_types::errors::BatcherError;
use starknet_l1_provider_types::SharedL1ProviderClient;
use starknet_mempool_types::communication::SharedMempoolClient;
use starknet_mempool_types::mempool_types::CommitBlockArgs;
use starknet_sequencer_infra::component_definitions::ComponentStarter;
Expand All @@ -42,11 +43,7 @@ use crate::block_builder::{
};
use crate::config::BatcherConfig;
use crate::proposal_manager::{GenerateProposalError, ProposalManager, ProposalManagerTrait};
use crate::transaction_provider::{
DummyL1ProviderClient,
ProposeTransactionProvider,
ValidateTransactionProvider,
};
use crate::transaction_provider::{ProposeTransactionProvider, ValidateTransactionProvider};
use crate::utils::{
deadline_as_instant,
proposal_status_from,
Expand All @@ -62,6 +59,7 @@ pub struct Batcher {
pub config: BatcherConfig,
pub storage_reader: Arc<dyn BatcherStorageReaderTrait>,
pub storage_writer: Box<dyn BatcherStorageWriterTrait>,
pub l1_provider_client: SharedL1ProviderClient,
pub mempool_client: SharedMempoolClient,

active_height: Option<BlockNumber>,
Expand All @@ -77,6 +75,7 @@ impl Batcher {
config: BatcherConfig,
storage_reader: Arc<dyn BatcherStorageReaderTrait>,
storage_writer: Box<dyn BatcherStorageWriterTrait>,
l1_provider_client: SharedL1ProviderClient,
mempool_client: SharedMempoolClient,
block_builder_factory: Box<dyn BlockBuilderFactoryTrait>,
proposal_manager: Box<dyn ProposalManagerTrait>,
Expand All @@ -85,6 +84,7 @@ impl Batcher {
config: config.clone(),
storage_reader,
storage_writer,
l1_provider_client,
mempool_client,
active_height: None,
block_builder_factory,
Expand Down Expand Up @@ -136,8 +136,7 @@ impl Batcher {

let tx_provider = ProposeTransactionProvider::new(
self.mempool_client.clone(),
// TODO: use a real L1 provider client.
Arc::new(DummyL1ProviderClient),
self.l1_provider_client.clone(),
self.config.max_l1_handler_txs_per_block_proposal,
);

Expand Down Expand Up @@ -186,8 +185,7 @@ impl Batcher {

let tx_provider = ValidateTransactionProvider {
tx_receiver: input_tx_receiver,
// TODO: use a real L1 provider client.
l1_provider_client: Arc::new(DummyL1ProviderClient),
l1_provider_client: self.l1_provider_client.clone(),
};

let (block_builder, abort_signal_sender) = self
Expand Down Expand Up @@ -429,7 +427,11 @@ impl Batcher {
}
}

pub fn create_batcher(config: BatcherConfig, mempool_client: SharedMempoolClient) -> Batcher {
pub fn create_batcher(
config: BatcherConfig,
mempool_client: SharedMempoolClient,
l1_provider_client: SharedL1ProviderClient,
) -> Batcher {
let (storage_reader, storage_writer) = papyrus_storage::open_storage(config.storage.clone())
.expect("Failed to open batcher's storage");

Expand All @@ -445,6 +447,7 @@ pub fn create_batcher(config: BatcherConfig, mempool_client: SharedMempoolClient
config,
storage_reader,
storage_writer,
l1_provider_client,
mempool_client,
block_builder_factory,
proposal_manager,
Expand Down
4 changes: 4 additions & 0 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use starknet_batcher_types::batcher_types::{
ValidateBlockInput,
};
use starknet_batcher_types::errors::BatcherError;
use starknet_l1_provider_types::MockL1ProviderClient;
use starknet_mempool_types::communication::MockMempoolClient;
use starknet_mempool_types::mempool_types::CommitBlockArgs;
use starknet_state_sync_types::state_sync_types::SyncBlock;
Expand Down Expand Up @@ -105,6 +106,7 @@ struct MockDependencies {
storage_reader: MockBatcherStorageReaderTrait,
storage_writer: MockBatcherStorageWriterTrait,
mempool_client: MockMempoolClient,
l1_provider_client: MockL1ProviderClient,
proposal_manager: MockProposalManagerTraitWrapper,
block_builder_factory: MockBlockBuilderFactoryTrait,
}
Expand All @@ -116,6 +118,7 @@ impl Default for MockDependencies {
Self {
storage_reader,
storage_writer: MockBatcherStorageWriterTrait::new(),
l1_provider_client: MockL1ProviderClient::new(),
mempool_client: MockMempoolClient::new(),
proposal_manager: MockProposalManagerTraitWrapper::new(),
block_builder_factory: MockBlockBuilderFactoryTrait::new(),
Expand All @@ -128,6 +131,7 @@ fn create_batcher(mock_dependencies: MockDependencies) -> Batcher {
BatcherConfig { outstream_content_buffer_size: STREAMING_CHUNK_SIZE, ..Default::default() },
Arc::new(mock_dependencies.storage_reader),
Box::new(mock_dependencies.storage_writer),
Arc::new(mock_dependencies.l1_provider_client),
Arc::new(mock_dependencies.mempool_client),
Box::new(mock_dependencies.block_builder_factory),
Box::new(mock_dependencies.proposal_manager),
Expand Down
52 changes: 20 additions & 32 deletions crates/starknet_batcher/src/transaction_provider.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::cmp::min;
use std::sync::Arc;
use std::vec;

use async_trait::async_trait;
#[cfg(test)]
use mockall::automock;
use starknet_api::executable_transaction::{L1HandlerTransaction, Transaction};
use starknet_api::executable_transaction::Transaction;
use starknet_api::transaction::TransactionHash;
use starknet_l1_provider_types::errors::L1ProviderClientError;
use starknet_l1_provider_types::{SharedL1ProviderClient, ValidationStatus as L1ValidationStatus};
use starknet_mempool_types::communication::{MempoolClientError, SharedMempoolClient};
use thiserror::Error;
use tracing::warn;

type TransactionProviderResult<T> = Result<T, TransactionProviderError>;

Expand All @@ -19,6 +19,8 @@ pub enum TransactionProviderError {
MempoolError(#[from] MempoolClientError),
#[error("L1Handler transaction validation failed for tx with hash {0}.")]
L1HandlerTransactionValidationFailed(TransactionHash),
#[error(transparent)]
L1ProviderError(#[from] L1ProviderClientError),
}

#[derive(Debug, PartialEq)]
Expand Down Expand Up @@ -64,8 +66,17 @@ impl ProposeTransactionProvider {
}
}

fn get_l1_handler_txs(&mut self, n_txs: usize) -> Vec<Transaction> {
self.l1_provider_client.get_txs(n_txs).into_iter().map(Transaction::L1Handler).collect()
async fn get_l1_handler_txs(
&mut self,
n_txs: usize,
) -> TransactionProviderResult<Vec<Transaction>> {
Ok(self
.l1_provider_client
.get_txs(n_txs)
.await?
.into_iter()
.map(Transaction::L1Handler)
.collect())
}

async fn get_mempool_txs(
Expand All @@ -90,7 +101,7 @@ impl TransactionProvider for ProposeTransactionProvider {
if self.phase == TxProviderPhase::L1 {
let n_l1handler_txs_to_get =
min(self.max_l1_handler_txs_per_block - self.n_l1handler_txs_so_far, n_txs);
let mut l1handler_txs = self.get_l1_handler_txs(n_l1handler_txs_to_get);
let mut l1handler_txs = self.get_l1_handler_txs(n_l1handler_txs_to_get).await?;
self.n_l1handler_txs_so_far += l1handler_txs.len();

// Determine whether we need to switch to mempool phase.
Expand Down Expand Up @@ -131,7 +142,9 @@ impl TransactionProvider for ValidateTransactionProvider {
}
for tx in &buffer {
if let Transaction::L1Handler(tx) = tx {
if !self.l1_provider_client.validate(tx) {
let l1_validation_status = self.l1_provider_client.validate(tx.tx_hash).await?;
if l1_validation_status != L1ValidationStatus::Validated {
// TODO: add the validation status into the error.
return Err(TransactionProviderError::L1HandlerTransactionValidationFailed(
tx.tx_hash,
));
Expand All @@ -141,28 +154,3 @@ impl TransactionProvider for ValidateTransactionProvider {
Ok(NextTxs::Txs(buffer))
}
}

// TODO: Remove L1Provider code when the communication module of l1_provider is added.
#[cfg_attr(test, automock)]
#[async_trait]
pub trait L1ProviderClient: Send + Sync {
fn get_txs(&self, n_txs: usize) -> Vec<L1HandlerTransaction>;
fn validate(&self, tx: &L1HandlerTransaction) -> bool;
}

pub type SharedL1ProviderClient = Arc<dyn L1ProviderClient>;

pub struct DummyL1ProviderClient;

#[async_trait]
impl L1ProviderClient for DummyL1ProviderClient {
fn get_txs(&self, _n_txs: usize) -> Vec<L1HandlerTransaction> {
warn!("Dummy L1 provider client is used, no L1 transactions are provided.");
vec![]
}

fn validate(&self, _tx: &L1HandlerTransaction) -> bool {
warn!("Dummy L1 provider client is used, tx is not really validated.");
true
}
}
15 changes: 8 additions & 7 deletions crates/starknet_batcher/src/transaction_provider_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use rstest::{fixture, rstest};
use starknet_api::executable_transaction::{L1HandlerTransaction, Transaction};
use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
use starknet_api::tx_hash;
use starknet_l1_provider_types::{MockL1ProviderClient, ValidationStatus as L1ValidationStatus};
use starknet_mempool_types::communication::MockMempoolClient;

use crate::transaction_provider::{
MockL1ProviderClient,
NextTxs,
ProposeTransactionProvider,
TransactionProvider,
Expand All @@ -33,7 +33,7 @@ impl MockDependencies {
self.l1_provider_client
.expect_get_txs()
.with(eq(n_to_request))
.returning(move |_| vec![L1HandlerTransaction::default(); n_to_return]);
.returning(move |_| Ok(vec![L1HandlerTransaction::default(); n_to_return]));
}

fn expect_get_mempool_txs(&mut self, n_to_request: usize) {
Expand All @@ -42,11 +42,11 @@ impl MockDependencies {
});
}

fn expect_validate_l1handler(&mut self, tx: L1HandlerTransaction, result: bool) {
fn expect_validate_l1handler(&mut self, tx: L1HandlerTransaction, result: L1ValidationStatus) {
self.l1_provider_client
.expect_validate()
.withf(move |tx_arg| tx_arg == &tx)
.returning(move |_| result);
.withf(move |tx_arg| tx_arg == &tx.tx_hash)
.returning(move |_| Ok(result));
}

async fn simulate_input_txs(&mut self, txs: Vec<Transaction>) {
Expand Down Expand Up @@ -163,7 +163,7 @@ async fn no_more_l1_handler(mut mock_dependencies: MockDependencies) {
#[tokio::test]
async fn validate_flow(mut mock_dependencies: MockDependencies) {
let test_tx = test_l1handler_tx();
mock_dependencies.expect_validate_l1handler(test_tx.clone(), true);
mock_dependencies.expect_validate_l1handler(test_tx.clone(), L1ValidationStatus::Validated);
mock_dependencies
.simulate_input_txs(vec![
Transaction::L1Handler(test_tx),
Expand All @@ -183,7 +183,8 @@ async fn validate_flow(mut mock_dependencies: MockDependencies) {
#[tokio::test]
async fn validate_fails(mut mock_dependencies: MockDependencies) {
let test_tx = test_l1handler_tx();
mock_dependencies.expect_validate_l1handler(test_tx.clone(), false);
mock_dependencies
.expect_validate_l1handler(test_tx.clone(), L1ValidationStatus::AlreadyIncludedOnL2);
mock_dependencies
.simulate_input_txs(vec![
Transaction::L1Handler(test_tx),
Expand Down
1 change: 1 addition & 0 deletions crates/starknet_integration_tests/src/config_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ pub async fn get_http_only_component_config(gateway_socket: SocketAddr) -> Compo
mempool: get_disabled_component_config(),
mempool_p2p: get_disabled_component_config(),
state_sync: get_disabled_component_config(),
l1_provider: get_disabled_component_config(),
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/starknet_l1_provider/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ testing = []
async-trait.workspace = true
indexmap.workspace = true
papyrus_base_layer.workspace = true
papyrus_config.workspace = true
serde.workspace = true
starknet_api.workspace = true
starknet_l1_provider_types.workspace = true
Expand Down
Loading

0 comments on commit 995a89a

Please sign in to comment.