chore: add create client macro #1874

nadin-Starkware · 2024-11-07T14:11:33Z

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

reviewable-StarkWare · 2024-11-07T14:11:39Z

This change is

codecov · 2024-11-07T14:29:52Z

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 5.50%. Comparing base (e3165c4) to head (8a6d6c1).
Report is 279 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/sequencer_node/src/clients.rs	0.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #1874       +/-   ##
==========================================
- Coverage   40.10%   5.50%   -34.60%     
==========================================
  Files          26     261      +235     
  Lines        1895   30326    +28431     
  Branches     1895   30326    +28431     
==========================================
+ Hits          760    1670      +910     
- Misses       1100   28601    +27501     
- Partials       35      55       +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Itay-Tsabary-Starkware

Reviewable status: 0 of 1 files reviewed, 8 unresolved discussions (waiting on @nadin-Starkware)

crates/sequencer_node/src/clients.rs line 44 at r1 (raw file):

/// This macro simplifies the creation of a client by evaluating the execution mode
/// and conditionally constructing the client only if the mode is enabled (either
/// LocalExecutionWithRemoteDisabled or LocalExecutionWithRemoteEnabled).

Let's make this more concise: wydt about

A macro for creating a component client, determined by the component's execution mode. Returns a local client if the component is run locally, otherwise `None`.

Later when adding the remote client we can modify this doc to detail the remote client option as well.

Code quote:

/// A macro to create a shared client based on the component's execution mode.
///
/// This macro simplifies the creation of a client by evaluating the execution mode
/// and conditionally constructing the client only if the mode is enabled (either
/// LocalExecutionWithRemoteDisabled or LocalExecutionWithRemoteEnabled).

crates/sequencer_node/src/clients.rs line 49 at r1 (raw file):

///
/// * $execution_mode - A reference to the execution mode to evaluate, expected to be of type
///   ComponentExecutionMode.

A reference to the component's execution mode, i.e., type &ComponentExecutionMode

Code quote:

/// * $execution_mode - A reference to the execution mode to evaluate, expected to be of type
///   ComponentExecutionMode.

crates/sequencer_node/src/clients.rs line 50 at r1 (raw file):

/// * $execution_mode - A reference to the execution mode to evaluate, expected to be of type
///   ComponentExecutionMode.
/// * $client_type - The type of the client to create, such as LocalBatcherClient.

The client type to create, e.g., LocalBatcherClient. The client type should have a function $client_type::new(tx : $channel_expr).

Code quote:

The type of the client to create, such as LocalBatcherClient.

crates/sequencer_node/src/clients.rs line 51 at r1 (raw file):

///   ComponentExecutionMode.
/// * $client_type - The type of the client to create, such as LocalBatcherClient.
/// * $channel_expr - An expression to retrieve the channel required for the client, e.g.,

Sender side for the client.

Code quote:

An expression to retrieve the channel required for the client

crates/sequencer_node/src/clients.rs line 52 at r1 (raw file):

/// * $client_type - The type of the client to create, such as LocalBatcherClient.
/// * $channel_expr - An expression to retrieve the channel required for the client, e.g.,
///   channels.take_batcher_tx().

Let's make this the 2nd arguement (i.e., replace the order with client type), so we can better describe what client type is.

Code quote:

/// * $channel_expr - An expression to retrieve the channel required for the client, e.g.,
///   channels.take_batcher_tx().

crates/sequencer_node/src/clients.rs line 57 at r1 (raw file):

///
/// An `Option<Arc<dyn ClientType>>` containing the client if the execution mode is enabled,
/// or None if the execution mode is Disabled.

Be more explicit regarding the enabled option, as Disabled is an actual enum variant whereas enabled isn't.

Code quote:

/// An `Option<Arc<dyn ClientType>>` containing the client if the execution mode is enabled,
/// or None if the execution mode is Disabled.

crates/sequencer_node/src/clients.rs line 74 at r1 (raw file):

///     None => println!("Client not created because the execution mode is disabled."),
/// }
/// ```

Can you make this example compile/pass?
I.e., drop the ignore

Code quote:

/// ```rust,ignore
/// // Assuming ComponentExecutionMode and channels are defined, and LocalBatcherClient
/// // has a new method that accepts a channel.
/// let batcher_client: Option<SharedBatcherClient> = create_client!(
///     &config.components.batcher.execution_mode,
///     LocalBatcherClient,
///     channels.take_batcher_tx()
/// );
///
/// match batcher_client {
///     Some(client) => println!("Client created: {:?}", client),
///     None => println!("Client not created because the execution mode is disabled."),
/// }
/// ```

crates/sequencer_node/src/clients.rs line 91 at r1 (raw file):

    channels: &mut SequencerNodeCommunication,
) -> SequencerNodeClients {
    let batcher_client: Option<SharedBatcherClient> = create_client!(

Are these type annotations required?

Code quote:

Option<SharedBatcherClient>

Itay-Tsabary-Starkware

Reviewable status: 0 of 1 files reviewed, 8 unresolved discussions (waiting on @nadin-Starkware)

crates/sequencer_node/src/clients.rs line 110 at r1 (raw file):

        &config.components.mempool_p2p.execution_mode,
        LocalMempoolP2pPropagatorClient,
        channels.take_mempool_p2p_propagator_tx()

Not for this pr:

When'll have the remote clients, they will also need the channel_tx, so we'd have to switch from using take to clone. FYI.

Code quote:

channels.take_mempool_p2p_propagator_tx()

nadin-Starkware

Reviewable status: 0 of 1 files reviewed, 8 unresolved discussions (waiting on @Itay-Tsabary-Starkware)

crates/sequencer_node/src/clients.rs line 44 at r1 (raw file):