apollo_consensus_orchestrator: use the config manager client to update consensus context config

[guy-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[guy-starkware]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• apollo_consensus_orchestrator: use the config manager client to update consensus context config #11015 👈 (View in Graphite)
• apollo_consensus_orchestrator: add helper func to test sending proposal to validate #11163
• apollo_consensus_orchestrator: fix test util to be able to use number_of_times>1 #11162
• apollo_consensus_orchestrator: split consensus config into static and dynamic configs #10963
• main-v0.14.1-committer

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

[github-actions]

Artifacts upload workflows:

• Started at 2025-12-23T13:05:37Z
• Started at 2025-12-25T13:06:22Z
• Started at 2025-12-25T13:06:48Z
• Started at 2025-12-28T09:43:03Z
• Started at 2025-12-28T11:24:25Z
• Started at 2025-12-29T08:19:24Z
• Started at 2025-12-29T11:57:56Z
• Started at 2025-12-30T08:35:07Z
• Started at 2026-01-06T12:09:12Z
• Started at 2026-01-06T12:52:32Z
• Started at 2026-01-06T12:52:33Z
• Started at 2026-01-06T13:15:33Z
• Started at 2026-01-08T10:59:18Z
• Started at 2026-01-08T11:08:24Z
• Started at 2026-01-08T11:19:11Z
• Started at 2026-01-08T15:10:49Z
• Started at 2026-01-08T15:10:48Z

[ShahakShama]

@ShahakShama reviewed 13 files and all commit messages, and made 6 comments.
Reviewable status: 13 of 14 files reviewed, 6 unresolved discussions (waiting on @guy-starkware and @matanl-starkware).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1050 at r2 (raw file):

    // required for decision reached flow
    deps.state_sync_client.expect_add_new_block().times(2).returning(|_| Ok(()));
    // We never wrote block 0.

Rewrite this to: mock sync to not learn anything in this test

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1056 at r2 (raw file):

    let mut context = deps.build_context();

    // Validate block number 0.

Redundant comment IMO

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1067 at r3 (raw file):

    assert_eq!(proposal_commitment.0, STATE_DIFF_COMMITMENT.0.0);

    // Decision reached

Same here

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1082 at r3 (raw file):

    let mut modified_block_info = block_info(BlockNumber(1));
    modified_block_info.l2_gas_price_fri = GasPrice(ODDLY_SPECIFIC_L2_GAS_PRICE);

Why do you send the price you're overriding to? I thought the idea was to test that we override correctly

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1099 at r3 (raw file):

        ..Default::default()
    };
    let mut config_manager_client = MockConfigManagerClient::new();

Try to extract some of the common code to a utility function in cases where it makes sense

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1109 at r3 (raw file):

    modified_block_info.l1_data_gas_price_wei = GasPrice(ODDLY_SPECIFIC_L1_DATA_GAS_PRICE);

    context.set_height_and_round(BlockNumber(1), 1).await;

Consider splitting this test to two tests: one for advance to next height and one for advance to next round

[guy-starkware]

@guy-starkware made 5 comments and resolved 1 discussion.
Reviewable status: 13 of 14 files reviewed, 5 unresolved discussions (waiting on @matanl-starkware and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1056 at r2 (raw file):

Previously, ShahakShama wrote…

Redundant comment IMO

Not really. We have validation and building, and rounds 0 and 1 at different points in the test. This helps you know which part you are starting.

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1067 at r3 (raw file):

Previously, ShahakShama wrote…

Same here

Yeah, I agree on this one :)

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1082 at r3 (raw file):

Previously, ShahakShama wrote…

Why do you send the price you're overriding to? I thought the idea was to test that we override correctly

I have two options:

1. match the incoming block info to the expected (overridden) value, which makes the validation pass.
2. leave the regular block info and expect a failure to validate.
   I think (1) is more specific, since I don't need a way to check "why did it fail to validate".

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1099 at r3 (raw file):

Previously, ShahakShama wrote…

Try to extract some of the common code to a utility function in cases where it makes sense

Done.

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1109 at r3 (raw file):

Previously, ShahakShama wrote…

Consider splitting this test to two tests: one for advance to next height and one for advance to next round

Seems like a lot of setup for a couple of very similar (short) cases. I also like that we mix the proposing with validating. I think it is less of a headache to read that last case than to read a whole new test (with similar setup, or the setup is split off into a helper).

[dafnamatsry]

@dafnamatsry reviewed 10 files and all commit messages, and made 2 comments.
Reviewable status: 12 of 14 files reviewed, 7 unresolved discussions (waiting on @guy-starkware, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 719 at r4 (raw file):

            self.config.dynamic_config =
                config_manager_client.get_context_dynamic_config().await.unwrap();
        }

This function can be called multiple times for the same round, in which case we don't want the dynamic config to change right?
I think you should move this part down to be after the check that the round has advanced.

Code quote:

        if let Some(config_manager_client) = self.deps.config_manager_client.clone() {
            self.config.dynamic_config =
                config_manager_client.get_context_dynamic_config().await.unwrap();
        }

---

crates/apollo_consensus_orchestrator/src/test_utils.rs line 149 at r4 (raw file):

        self.batcher
            .expect_start_height()
            .times(1)

Why? This is an important check.

Code quote:

.times(1)

[guy-starkware]

@guy-starkware made 2 comments.
Reviewable status: 4 of 14 files reviewed, 7 unresolved discussions (waiting on @dafnamatsry, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 719 at r4 (raw file):

Previously, dafnamatsry wrote…

This function can be called multiple times for the same round, in which case we don't want the dynamic config to change right?
I think you should move this part down to be after the check that the round has advanced.

I think this function is called when we start a new height and when we start a new round. These are the places I want to update my config.

If I move it down after the first return, I don't get an update when a new height starts.

---

crates/apollo_consensus_orchestrator/src/test_utils.rs line 149 at r4 (raw file):

Previously, dafnamatsry wrote…

Why? This is an important check.

Your comment caused me to dig deeper into how the consensus context tests set up their expectations. I did a big re-haul of the test setup in the previous PRs. I think it will be good if you have a look at them also.

This specific comment I think is no longer relevant...

[dafnamatsry]

@dafnamatsry made 1 comment.
Reviewable status: 4 of 14 files reviewed, 7 unresolved discussions (waiting on @guy-starkware, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 719 at r4 (raw file):

Previously, guy-starkware wrote…

I think this function is called when we start a new height and when we start a new round. These are the places I want to update my config.

If I move it down after the first return, I don't get an update when a new height starts.

It is called for sure multiple times for a round, so if you want to avoid updating the dynamic config in the middle of the round you must move this line down (maybe call it twice on height change & round change).

[guy-starkware]

@guy-starkware made 1 comment.
Reviewable status: 4 of 14 files reviewed, 7 unresolved discussions (waiting on @dafnamatsry, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 719 at r4 (raw file):

Previously, dafnamatsry wrote…

It is called for sure multiple times for a round, so if you want to avoid updating the dynamic config in the middle of the round you must move this line down (maybe call it twice on height change & round change).

Ok, now I see what you mean.
I added a helper function and called it twice, when updating height and round.

[dafnamatsry]

@dafnamatsry reviewed 4 files and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: 8 of 14 files reviewed, 6 unresolved discussions (waiting on @guy-starkware, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 873 at r6 (raw file):

        if let Some(config_manager_client) = self.deps.config_manager_client.clone() {
            self.config.dynamic_config =
                config_manager_client.get_context_dynamic_config().await.unwrap();

Just log the error and continue (sorry I didn't noticed it before)

Code quote:

.unwrap()

[guy-starkware]

@guy-starkware made 1 comment.
Reviewable status: 1 of 14 files reviewed, 6 unresolved discussions (waiting on @dafnamatsry, @matanl-starkware, and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 873 at r6 (raw file):

Previously, dafnamatsry wrote…

Just log the error and continue (sorry I didn't noticed it before)

Done.

[guy-starkware]

@guy-starkware reviewed 14 files and all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dafnamatsry, @matanl-starkware, and @ShahakShama).

[guy-starkware]

@guy-starkware resolved 2 discussions and dismissed @dafnamatsry from a discussion.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @matanl-starkware and @ShahakShama).

[ShahakShama]

@ShahakShama reviewed all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware and @matanl-starkware).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1082 at r3 (raw file):

Previously, guy-starkware wrote…

I have two options:

1. match the incoming block info to the expected (overridden) value, which makes the validation pass.
2. leave the regular block info and expect a failure to validate.
   I think (1) is more specific, since I don't need a way to check "why did it fail to validate".

If it doesn't make the test over-complicated, I prefer having both (first 2 then 1)
I'll leave this up to you

[guy-starkware]

@guy-starkware made 1 comment.
Reviewable status: 12 of 14 files reviewed, 1 unresolved discussion (waiting on @matanl-starkware and @ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs line 1082 at r3 (raw file):

Previously, ShahakShama wrote…

If it doesn't make the test over-complicated, I prefer having both (first 2 then 1)
I'll leave this up to you

Yeah, that makes sense.