Skip to content

Commit 54bd7e1

Browse files
ShahakShamaclaude
andcommitted
apollo_consensus_orchestrator,apollo_consensus_manager: route STRK/USD via L1GasPriceProvider
Goal: PR 5 of 5 in the stack moving the STRK/USD oracle into L1GasPriceProvider. This is the behavior flip: `compute_snip35_fee_proposal` now asks the gas-price provider client for the STRK/USD rate instead of holding its own `ExchangeRateOracleClientTrait` object. With this in, the orchestrator no longer has direct oracle plumbing — every external rate goes through the existing `SharedL1GasPriceClient` seam. Change summary: - `SequencerConsensusContext::compute_snip35_fee_proposal` calls `self.deps.l1_gas_price_provider.get_strk_to_usd_rate(timestamp)`. The previous `Option<Arc<dyn ExchangeRateOracleClientTrait>>` outer match collapses into a plain `Result` match; the "freeze fee_proposal" fallback is preserved on `Err` and zero-rate. - `SequencerConsensusContextDeps::strk_to_usd_oracle` removed. - `TestDeps::strk_to_usd_oracle` removed and the `From` impl simplified. - `consensus_manager.rs` drops the now-invalid `strk_to_usd_oracle: None` field initializer. - Unused imports cleaned up: `ExchangeRateOracleClientTrait` and `MockExchangeRateOracleClientTrait` from orchestrator; `debug` from `tracing` (only used by the dead `None` branch). - `setup_default_gas_price_provider` registers a catch-all `expect_get_strk_to_usd_rate` returning `DEFAULT_STRK_TO_USD_RATE` so existing build_proposal tests keep working without per-test expectations. - The two SNIP-35 fee-proposal tests reorder their per-test mock registration to run BEFORE `setup_default_expectations`, since mockall matches expectations in oldest-first order — same pattern the codebase already uses for state_sync_client.expect_get_block. - `OracleBehavior::NotConfigured` variant removed; the `no_oracle_freezes_at_fee_actual` test case removed (redundant with `oracle_err_freezes_at_fee_actual` — both freeze at fee_actual now that there is no on/off switch). Decision points: - Original 6-PR plan included a "cleanup" PR removing the oracle wiring from `apollo_consensus_manager`. That PR collapsed into this one because this branch forks from BEFORE the wiring was added, so the cleanup is one line and shares the same compile-unit constraint. - The `setup_default_gas_price_provider` catch-all is needed because every build_proposal test now hits `compute_snip35_fee_proposal`, which always queries the provider. The alternative (per-test expectations) would have churned ~9 build_proposal call sites. - `DEFAULT_STRK_TO_USD_RATE = 3e17` was picked because the SNIP-35 test cases treat it as in-bounds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6a5c326 commit 54bd7e1

4 files changed

Lines changed: 42 additions & 59 deletions

File tree

crates/apollo_consensus_manager/src/consensus_manager.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ impl ConsensusManager {
308308
outbound_proposal_sender: outbound_internal_sender,
309309
vote_broadcast_client: votes_broadcast_channels.broadcast_topic_client.clone(),
310310
config_manager_client: Some(Arc::clone(&config_manager_client)),
311-
strk_to_usd_oracle: None,
312311
},
313312
)
314313
}

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use apollo_config::behavior_mode::BehaviorMode;
2323
use apollo_config_manager_types::communication::SharedConfigManagerClient;
2424
use apollo_consensus::types::{ConsensusContext, ConsensusError, ProposalCommitment, Round};
2525
use apollo_consensus_orchestrator_config::config::ContextConfig;
26-
use apollo_l1_gas_price_types::{ExchangeRateOracleClientTrait, L1GasPriceProviderClient};
26+
use apollo_l1_gas_price_types::L1GasPriceProviderClient;
2727
use apollo_network::network_manager::{BroadcastTopicClient, BroadcastTopicClientTrait};
2828
use apollo_protobuf::consensus::{
2929
BuildParam,
@@ -75,7 +75,7 @@ use tokio::task::JoinHandle;
7575
use tokio::time::sleep;
7676
use tokio_util::sync::CancellationToken;
7777
use tokio_util::task::AbortOnDropHandle;
78-
use tracing::{debug, error, error_span, info, instrument, trace, warn, Instrument};
78+
use tracing::{error, error_span, info, instrument, trace, warn, Instrument};
7979

8080
use crate::build_proposal::{build_proposal, BuildProposalError, ProposalBuildArguments};
8181
use crate::cende::{
@@ -272,10 +272,6 @@ pub struct SequencerConsensusContextDeps {
272272
// Used to broadcast votes to other consensus nodes.
273273
pub vote_broadcast_client: BroadcastTopicClient<Vote>,
274274
pub config_manager_client: Option<SharedConfigManagerClient>,
275-
/// STRK/USD oracle for SNIP-35 dynamic gas pricing. The underlying trait is generic; this
276-
/// field's name labels the rate semantics. The instance must be configured with STRK/USD
277-
/// URLs at construction time. None if disabled.
278-
pub strk_to_usd_oracle: Option<Arc<dyn ExchangeRateOracleClientTrait>>,
279275
}
280276

281277
#[derive(thiserror::Error, PartialEq, Debug)]
@@ -458,24 +454,19 @@ impl SequencerConsensusContext {
458454
};
459455
SNIP35_FEE_ACTUAL.set_lossy(fee_actual.0);
460456

461-
let fee_target = match &self.deps.strk_to_usd_oracle {
462-
Some(oracle) => match oracle.fetch_rate(timestamp).await {
463-
Ok(rate) => {
464-
SNIP35_STRK_USD_RATE.set_lossy(rate);
465-
let target = compute_fee_target(TARGET_ATTO_USD_PER_L2_GAS, rate);
466-
match target {
467-
Some(t) => SNIP35_FEE_TARGET.set_lossy(t.0),
468-
None => warn!("STRK/USD oracle returned zero rate, freezing fee_proposal"),
469-
}
470-
target
471-
}
472-
Err(e) => {
473-
warn!("STRK/USD oracle error: {e:?}, freezing fee_proposal");
474-
None
457+
let fee_target = match self.deps.l1_gas_price_provider.get_strk_to_usd_rate(timestamp).await
458+
{
459+
Ok(rate) => {
460+
SNIP35_STRK_USD_RATE.set_lossy(rate);
461+
let target = compute_fee_target(TARGET_ATTO_USD_PER_L2_GAS, rate);
462+
match target {
463+
Some(t) => SNIP35_FEE_TARGET.set_lossy(t.0),
464+
None => warn!("STRK/USD oracle returned zero rate, freezing fee_proposal"),
475465
}
476-
},
477-
None => {
478-
debug!("No STRK/USD oracle configured, freezing fee_proposal");
466+
target
467+
}
468+
Err(e) => {
469+
warn!("STRK/USD oracle error: {e:?}, freezing fee_proposal");
479470
None
480471
}
481472
};

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@ use apollo_l1_gas_price_types::errors::{
2727
L1GasPriceClientError,
2828
L1GasPriceProviderError,
2929
};
30-
use apollo_l1_gas_price_types::{
31-
MockExchangeRateOracleClientTrait,
32-
MockL1GasPriceProviderClient,
33-
PriceInfo,
34-
};
30+
use apollo_l1_gas_price_types::{MockL1GasPriceProviderClient, PriceInfo};
3531
use apollo_protobuf::consensus::{
3632
BuildParam,
3733
CommitmentParts,
@@ -1619,28 +1615,20 @@ async fn test_initialize_fee_proposals_window(
16191615

16201616
#[derive(Clone)]
16211617
enum OracleBehavior {
1622-
/// No oracle is configured (`strk_to_usd_oracle = None`).
1623-
NotConfigured,
1624-
/// Oracle is configured and `fetch_rate` returns `Ok(rate)`.
1618+
/// Provider returns `Ok(rate)` for the STRK/USD query.
16251619
Ok(u128),
1626-
/// Oracle is configured and `fetch_rate` returns `Err(_)`.
1620+
/// Provider returns `Err(_)` for the STRK/USD query.
16271621
Err,
16281622
}
16291623

16301624
// fee_actual = 10 gwei => margin bounds [9_980_039_920, 10_020_000_000].
16311625
#[rstest]
16321626
#[case::no_fee_actual_freezes_at_l2_gas_price(
16331627
None,
1634-
OracleBehavior::NotConfigured,
1628+
OracleBehavior::Ok(0),
16351629
GasPrice(7_000_000_000),
16361630
GasPrice(7_000_000_000)
16371631
)]
1638-
#[case::no_oracle_freezes_at_fee_actual(
1639-
Some(GasPrice(10_000_000_000)),
1640-
OracleBehavior::NotConfigured,
1641-
GasPrice(7_000_000_000),
1642-
GasPrice(10_000_000_000)
1643-
)]
16441632
#[case::oracle_zero_rate_freezes_at_fee_actual(
16451633
Some(GasPrice(10_000_000_000)),
16461634
OracleBehavior::Ok(0),
@@ -1679,22 +1667,23 @@ async fn test_compute_snip35_fee_proposal(
16791667
#[case] expected_fee_proposal: GasPrice,
16801668
) {
16811669
let (mut deps, _network) = create_test_and_network_deps();
1682-
deps.setup_default_expectations();
1683-
deps.strk_to_usd_oracle = match oracle_behavior {
1684-
OracleBehavior::NotConfigured => None,
1670+
// Register the specific strk_to_usd_rate expectation BEFORE setup_default_expectations so
1671+
// the catch-all default installed there does not shadow it (mockall matches oldest first).
1672+
match oracle_behavior {
16851673
OracleBehavior::Ok(rate) => {
1686-
let mut mock = MockExchangeRateOracleClientTrait::new();
1687-
mock.expect_fetch_rate().returning(move |_| Ok(rate));
1688-
Some(Arc::new(mock))
1674+
deps.l1_gas_price_provider
1675+
.expect_get_strk_to_usd_rate()
1676+
.returning(move |_| Ok(rate));
16891677
}
16901678
OracleBehavior::Err => {
1691-
let mut mock = MockExchangeRateOracleClientTrait::new();
1692-
mock.expect_fetch_rate().returning(|_| {
1693-
Err(ExchangeRateOracleClientError::RequestError("test".to_string()))
1679+
deps.l1_gas_price_provider.expect_get_strk_to_usd_rate().returning(|_| {
1680+
Err(L1GasPriceClientError::ExchangeRateOracleClientError(
1681+
ExchangeRateOracleClientError::RequestError("test".to_string()),
1682+
))
16941683
});
1695-
Some(Arc::new(mock))
16961684
}
1697-
};
1685+
}
1686+
deps.setup_default_expectations();
16981687

16991688
let mut context = deps.build_context();
17001689
context.l2_gas_price = l2_gas_price;
@@ -1713,16 +1702,17 @@ async fn test_compute_snip35_fee_proposal_converges_to_oracle_target() {
17131702
];
17141703

17151704
let (mut deps, _network) = create_test_and_network_deps();
1716-
deps.setup_default_expectations();
1717-
let mut mock = MockExchangeRateOracleClientTrait::new();
1705+
// Register per-phase strk_to_usd_rate expectations BEFORE setup_default_expectations so they
1706+
// are not shadowed by the catch-all default (mockall matches oldest first).
17181707
let mut seq = mockall::Sequence::new();
17191708
for &(rate, _, n_blocks) in &phases {
1720-
mock.expect_fetch_rate()
1709+
deps.l1_gas_price_provider
1710+
.expect_get_strk_to_usd_rate()
17211711
.times(usize::try_from(n_blocks).unwrap())
17221712
.in_sequence(&mut seq)
17231713
.returning(move |_| Ok(rate));
17241714
}
1725-
deps.strk_to_usd_oracle = Some(Arc::new(mock));
1715+
deps.setup_default_expectations();
17261716
let mut context = deps.build_context();
17271717

17281718
// Bootstrap the window with 75 gwei (the $0.04 target).

crates/apollo_consensus_orchestrator/src/test_utils.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ pub(crate) const CHAIN_ID: ChainId = ChainId::Mainnet;
9393
// values here.
9494
pub(crate) const ETH_TO_FRI_RATE: u128 = 2 * u128::pow(10, 18);
9595

96+
// A STRK/USD rate that yields an in-bounds SNIP-35 fee target in proposal-building tests
97+
// where the specific value doesn't matter.
98+
pub(crate) const DEFAULT_STRK_TO_USD_RATE: u128 = 300_000_000_000_000_000;
99+
96100
pub(crate) static TX_BATCH: LazyLock<Vec<ConsensusTransaction>> =
97101
LazyLock::new(|| (0..3).map(generate_invoke_tx).collect());
98102

@@ -129,8 +133,6 @@ pub(crate) struct TestDeps {
129133
pub clock: Arc<dyn Clock>,
130134
pub outbound_proposal_sender: mpsc::Sender<(HeightAndRound, mpsc::Receiver<ProposalPart>)>,
131135
pub vote_broadcast_client: BroadcastTopicClient<Vote>,
132-
pub strk_to_usd_oracle:
133-
Option<Arc<dyn apollo_l1_gas_price_types::ExchangeRateOracleClientTrait>>,
134136
}
135137

136138
impl From<TestDeps> for SequencerConsensusContextDeps {
@@ -145,7 +147,6 @@ impl From<TestDeps> for SequencerConsensusContextDeps {
145147
outbound_proposal_sender: deps.outbound_proposal_sender,
146148
vote_broadcast_client: deps.vote_broadcast_client,
147149
config_manager_client: None,
148-
strk_to_usd_oracle: deps.strk_to_usd_oracle,
149150
}
150151
}
151152
}
@@ -325,6 +326,9 @@ impl TestDeps {
325326
blob_fee: GasPrice(TEMP_ETH_BLOB_GAS_FEE_IN_WEI),
326327
}));
327328
self.l1_gas_price_provider.expect_get_rate().return_const(Ok(ETH_TO_FRI_RATE));
329+
self.l1_gas_price_provider
330+
.expect_get_strk_to_usd_rate()
331+
.return_const(Ok(DEFAULT_STRK_TO_USD_RATE));
328332
}
329333

330334
fn setup_default_state_sync_get_block(&mut self) {
@@ -381,7 +385,6 @@ pub(crate) fn create_test_and_network_deps() -> (TestDeps, NetworkDependencies)
381385
clock,
382386
outbound_proposal_sender,
383387
vote_broadcast_client: votes_topic_client,
384-
strk_to_usd_oracle: None,
385388
};
386389

387390
let network_deps =

0 commit comments

Comments
 (0)