apollo_node: add local committer server

[yoavGrs]

No description provided.

[reviewable-StarkWare]

This change is 

[yoavGrs]

• apollo_batcher: add revert task #11451 : 2 dependent PRs (#11496 , #11516 )
• apollo_batcher: define custom input task type #11595
• apollo_batcher: wait for revert result #11484
• apollo_batcher: perform commitment tasks #11442
• apollo_batcher: use mock commit block #11622
• apollo_node: add local committer server #11564 👈 (View in Graphite)
• main-v0.14.1-committer

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @amosStarkware, @dorimedini-starkware, and @yoavGrs).

---

crates/apollo_node/src/servers.rs line 324 at r1 (raw file):

    let committer_server = create_local_server!(
        REGULAR_LOCAL_SERVER,

committer works sequentially, right?

Code quote:

REGULAR_LOCAL_SERVER

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @amosStarkware, @dorimedini-starkware, and @yoavGrs).

---

crates/apollo_committer/src/communication.rs line 7 at r1 (raw file):

use starknet_committer::block_committer::commit::CommitBlockTrait;

use crate::committer::{ApolloCommitter, Committer, StorageConstructor};

What's the difference between ApolloCommitter and Committer?

Code quote:

ApolloCommitter, Committer

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @amosStarkware, @dorimedini-starkware, and @Itay-Tsabary-Starkware).

---

crates/apollo_committer/src/communication.rs line 7 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

What's the difference between ApolloCommitter and Committer?

Committer is a generic struct.
ApolloCommitter is the committer with the chosen types.

ApolloNode knows only ApolloCommitter

---

crates/apollo_node/src/servers.rs line 324 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

committer works sequentially, right?

Yap.

[dorimedini-starkware]

@dorimedini-starkware reviewed 4 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware, @Itay-Tsabary-Starkware, @rotem-starkware, and @yoavGrs).

---

crates/apollo_committer/src/metrics.rs line 14 at r1 (raw file):

define_infra_metrics!(committer);

pub fn register_metrics() {}

aren't there metrics we want registered? @rotem-starkware

Code quote:

pub fn register_metrics() {}

---

crates/apollo_node/src/servers.rs line 538 at r1 (raw file):

    let committer_server = create_remote_server!(
        &config.components.committer.execution_mode,
        || { clients.get_committer_local_client() },

not to be inconsistent but isn't this equivalent...? or does rust not like this due to the self argument?

Suggestion:

clients.get_committer_local_client,

---

crates/apollo_committer/src/committer.rs line 273 at r1 (raw file):

        default_component_start_fn::<Self>().await;
        register_metrics();
    }

the batcher starter checks the height exists; do we want a similar check that the commitment offset exists?

Code quote:

    async fn start(&mut self) {
        default_component_start_fn::<Self>().await;
        register_metrics();
    }

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware, @Itay-Tsabary-Starkware, and @rotem-starkware).

---

crates/apollo_committer/src/committer.rs line 273 at r1 (raw file):

Previously, dorimedini-starkware wrote…

the batcher starter checks the height exists; do we want a similar check that the commitment offset exists?

The batcher doesn't check; it reads the height for its metrics.
If the committer metrics need the offset, we will read it as well.

---

crates/apollo_node/src/servers.rs line 538 at r1 (raw file):

Previously, dorimedini-starkware wrote…

not to be inconsistent but isn't this equivalent...? or does rust not like this due to the self argument?

method, not a field

[dorimedini-starkware]

@dorimedini-starkware resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Itay-Tsabary-Starkware, @rotem-starkware, and @yoavGrs).

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rotem-starkware and @yoavGrs).

---

crates/apollo_node/src/servers.rs line 538 at r1 (raw file):

Previously, yoavGrs wrote…

method, not a field

iirc this is rust being dumb with its macros

[rotem-starkware]

@rotem-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware).

---

crates/apollo_committer/src/metrics.rs line 14 at r1 (raw file):

Previously, dorimedini-starkware wrote…

aren't there metrics we want registered? @rotem-starkware

we have in this PR #11154

[dorimedini-starkware]

@dorimedini-starkware resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs).

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs).