apollo_consensus_orchestrator: add SNIP-35 fee_proposals sliding window

[sirandreww-starkware]

No description provided.

[sirandreww-starkware]

• apollo_consensus_orchestrator: add SNIP-35 integration TODO doc #13982
• apollo_consensus_orchestrator: tmp dirty tests for SNIP-35 (NOT FOR MERGE) #13857
• apollo_l1_gas_price,apollo_l1_gas_price_config: move exchange_rate_decimals const to config #13881
• apollo_consensus_orchestrator: add SNIP-35 proposer-validator symmetry tests #13856
• apollo_consensus_orchestrator: add SNIP-35 module edge case tests #13855
• apollo_consensus_orchestrator: add SNIP-35 e2e test #13821
• apollo_consensus_manager: wire STRK/USD oracle into consensus manager #13820
• apollo_consensus_orchestrator: add SNIP-35 fee_proposal validation #13819
• apollo_consensus_orchestrator: include fee_proposal_fri in proposal commitment hash #13949
• apollo_starknet_client: read fee_proposal_fri from feeder gateway #13986
• apollo_consensus_orchestrator: wire SNIP-35 fee_proposal into cende blob #13985
• apollo_consensus_orchestrator: wire SNIP-35 fee_proposal into build_proposal #13818
• apollo_consensus_orchestrator,apollo_consensus_manager: move SNIP-35 bootstrap to pre-consensus #14061
• apollo_consensus_orchestrator: add fee_actual floor to EIP-1559 gas price calculation #13824
• apollo_consensus_orchestrator: add SNIP-35 fee_proposals sliding window #13817 👈 (View in Graphite)
• apollo_consensus_orchestrator,apollo_consensus_manager: add strk_exchange_rate_oracle field to deps #13823
• apollo_l1_gas_price_types,apollo_l1_gas_price: rename trait method eth_to_fri_rate to fetch_rate #13987
• apollo_dashboard: add SNIP-35 row #13954
• apollo_consensus_orchestrator: add SNIP-35 metrics #13816
• apollo_consensus_orchestrator: add SNIP-35 module tests #13815
• apollo_consensus_orchestrator: add SNIP-35 module #13854
• apollo_storage: add fee_proposal_fri migrator and bump blocks version to 6.1 #13936 : 2 other dependent PRs (#13814 , #13934 )
• apollo_protobuf: add fee_proposal_fri to sync proto SignedBlockHeader #13957
• starknet_api: add fee_proposal_fri to BlockHeaderWithoutHash #13822
• apollo_protobuf: add fee_proposal_fri to ProposalInit #13812 : 2 other dependent PRs (#13811 , #13935 )
• main

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

[reviewable-StarkWare]

This change is 

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2b0cc53. Configure here.}

[ShahakShama]

@ShahakShama reviewed 5 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on sirandreww-starkware).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 940 at r3 (raw file):

                        Err(e) => {
                            // Stop backfilling on the first error (likely `BlockNotFound` for
                            // heights before the chain was SNIP-35-enabled). The remaining window

if a height is before SNIP-35 you should get Ok(block with None fee proposal), not Err

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 944 at r3 (raw file):

                            // are committed, falling back to `l2_gas_price`.
                            warn!(
                                "SNIP-35 backfill stopped at block {h}: {e:?}. Window has {} / \

stopped -> failed

---

crates/apollo_consensus_orchestrator/src/test_utils.rs line 328 at r3 (raw file):

    }

    /// Default get_block returns NotFound for all blocks. Used by SNIP-35 backfill on startup.

If you want to simulate real life you should return a regular block here with none proposal fee

---

crates/apollo_consensus_orchestrator/src/snip35/mod.rs line 27 at r3 (raw file):

pub(crate) const PPT_DENOMINATOR: u128 = 1000;

/// Number of fee_proposal values used to compute fee_actual (SNIP-35).

Add TODO to consider moving this to versioned constants

[ShahakShama]

@matanl-starkware should also review this

@ShahakShama made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on sirandreww-starkware).

[matanl-starkware]

@matanl-starkware made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on sirandreww-starkware).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 238 at r3 (raw file):

    l1_da_mode: L1DataAvailabilityMode,
    previous_proposal_init: Option<PreviousProposalInitInfo>,
    /// SNIP-35: sliding window of recent fee_proposal values (size from config).

At the moment, it's a const, and Shahak suggested making it a VC.
Either way - not config.

Code quote:

(size from config)

[sirandreww-starkware]

@sirandreww-starkware made 5 comments.
Reviewable status: 2 of 5 files reviewed, 4 unresolved discussions (waiting on ShahakShama).

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 238 at r3 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

At the moment, it's a const, and Shahak suggested making it a VC.
Either way - not config.

Done +-

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 940 at r3 (raw file):

Previously, ShahakShama wrote…

if a height is before SNIP-35 you should get Ok(block with None fee proposal), not Err

Done.

---

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 944 at r3 (raw file):

Previously, ShahakShama wrote…

stopped -> failed

Done.

---

crates/apollo_consensus_orchestrator/src/test_utils.rs line 328 at r3 (raw file):

Previously, ShahakShama wrote…

If you want to simulate real life you should return a regular block here with none proposal fee

Done

---

crates/apollo_consensus_orchestrator/src/snip35/mod.rs line 27 at r3 (raw file):

Previously, ShahakShama wrote…

Add TODO to consider moving this to versioned constants

Done.

[ShahakShama]

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

---

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

        self.state_sync_client
            .expect_get_block()
            .returning(|_| Ok(apollo_state_sync_types::state_sync_types::SyncBlock::default()));

Add use statement, and set SyncBlock's fee proposal field to None explicitly

[matanl-starkware]

@matanl-starkware reviewed 5 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on sirandreww-starkware).

---

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

                CONSENSUS_L2_GAS_PRICE.set_lossy(gas_price_u64);
            }
            // SNIP-35: on first height, backfill the window so fee_actual is available immediately.

Consider extracting this entire block into a function to avoid this (very unlikely) chunk of code path from the main flow.

Code quote:

// SNIP-35: on first height, backfill the window so fee_actual is available immediately.

---

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

                let start = height.0.saturating_sub(window_size);
                for h in start..height.0 {
                    match self.deps.state_sync_client.get_block(BlockNumber(h)).await {

Are we sure all the relevant blocks are available for state_sync at this stage (up to N-1)?

Code quote:

self.deps.state_sync_client.get_block

---

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

                        Err(e) => {
                            // `Err` means the block could not be retrieved (e.g., earlier than
                            // chain start); pre-V0_14_3 blocks return `Ok` with `None` above.

"e.g. earlier than chain start" - I'm not sure this particular example is relevant.

Code quote:

                            // `Err` means the block could not be retrieved (e.g., earlier than
                            // chain start); pre-V0_14_3 blocks return `Ok` with `None` above.

[matanl-starkware]

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

[sirandreww-starkware]

@sirandreww-starkware made 4 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ShahakShama).

---

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

Previously, matanl-starkware (Matan Lior) wrote…

Consider extracting this entire block into a function to avoid this (very unlikely) chunk of code path from the main flow.

Done.

---

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

Previously, matanl-starkware (Matan Lior) wrote…

Are we sure all the relevant blocks are available for state_sync at this stage (up to N-1)?

Good question, looking into it.
Actually, let me try something:
@cursor[bot] can you check this? keep in mind that all nodes must have the same exact window?

---

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

Previously, matanl-starkware (Matan Lior) wrote…

"e.g. earlier than chain start" - I'm not sure this particular example is relevant.

Done.

---

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

Previously, ShahakShama wrote…

Add use statement, and set SyncBlock's fee proposal field to None explicitly

Done

[ShahakShama]

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

---

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

    // consensus enters `set_height_and_round`, since blocking inside this call to retry is unsafe
    // (peers may already be voting at this height). Track and fix in a follow-up PR.
    async fn backfill_fee_proposals_window(&mut self, height: BlockNumber) {

Consider bootstrap instead of backfill

---

crates/apollo_consensus_orchestrator/src/test_utils.rs line 334 at r6 (raw file):

    fn setup_default_state_sync_get_block(&mut self) {
        self.state_sync_client.expect_get_block().returning(|_| {
            let mut sync_block = SyncBlock::default();

Consider

let sync_block = SyncBlock { block_header_without_hash: BlockHeaderWithoutHash { fee_proposal_fri, ..default::Default() }, ..default::Default() };

I'm debating myself what's cleaner so fill free to reject my proposal

[sirandreww-starkware]

@sirandreww-starkware reviewed 5 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on sirandreww-starkware).

---

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

Previously, ShahakShama wrote…

Consider bootstrap instead of backfill

Done