Skip to content

Commit

Permalink
chore(gas_price_service_v1): strictly ensure last_recorded_height is …
Browse files Browse the repository at this point in the history
…set, to avoid initial poll of da source
  • Loading branch information
rymnc committed Dec 30, 2024
1 parent c29dae7 commit 0b912e1
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ use std::ops::Deref;

#[async_trait::async_trait]
pub trait BlockCommitterApi: Send + Sync {
/// Used on first run to get the latest costs and seqno
async fn get_latest_costs(&self) -> DaBlockCostsResult<Option<RawDaBlockCosts>>;
/// Used to get the costs for a specific seqno
async fn get_costs_by_l2_block_number(
&self,
Expand Down Expand Up @@ -103,7 +101,10 @@ where
.get_costs_by_l2_block_number(*next_height.deref())
.await?
}
None => self.client.get_latest_costs().await?.into_iter().collect(),
None => {
// prevent calling the latest endpoint
return Err(anyhow!("last_recorded_height is None"));
}
};

tracing::info!("raw_da_block_costs: {:?}", raw_da_block_costs);
Expand Down Expand Up @@ -158,23 +159,6 @@ impl BlockCommitterApi for BlockCommitterHttpApi {
Ok(vec![])
}
}

async fn get_latest_costs(&self) -> DaBlockCostsResult<Option<RawDaBlockCosts>> {
// Latest: http://localhost:8080/v1/costs?variant=latest&limit=5
if let Some(url) = &self.url {
tracing::info!("getting latest costs");
let formatted_url = format!("{url}/v1/costs?variant=latest&limit=1");
tracing::info!("Formatted URL: {:?}", formatted_url);
let response = self.client.get(formatted_url).send().await?;
tracing::info!("response: {:?}", response);
let raw_da_block_costs = response.json::<Vec<RawDaBlockCosts>>().await?;
tracing::info!("Parsed: {:?}", raw_da_block_costs);
// only take the first element, since we are only looking for the most recent
Ok(raw_da_block_costs.first().cloned())
} else {
Ok(None)
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -290,56 +274,6 @@ mod test_block_committer_http_api {
// then
assert_eq!(actual, expected);
}

#[test]
fn get_latest_costs__when_url_is_none__then_returns_none() {
let rt = tokio::runtime::Runtime::new().unwrap();

// given
let block_committer = BlockCommitterHttpApi::new(None);

// when
let actual =
rt.block_on(async { block_committer.get_latest_costs().await.unwrap() });

// then
assert_eq!(actual, None);
}

#[test]
fn get_latest_costs__when_url_is_some__then_returns_expected_costs() {
let rt = tokio::runtime::Runtime::new().unwrap();
let mut mock = FakeServer::new();
let url = mock.url();

// given
let block_committer = BlockCommitterHttpApi::new(Some(url));
let not_expected = RawDaBlockCosts {
id: 1,
start_height: 1,
end_height: 10,
da_block_height: 1u64.into(),
cost: 1,
size: 1,
};
mock.add_response(not_expected);
let expected = RawDaBlockCosts {
id: 2,
start_height: 11,
end_height: 20,
da_block_height: 2u64.into(),
cost: 2,
size: 2,
};
mock.add_response(expected.clone());

// when
let actual =
rt.block_on(async { block_committer.get_latest_costs().await.unwrap() });

// then
assert_eq!(actual, Some(expected));
}
}
#[cfg(any(test, feature = "test-helpers"))]
pub mod fake_server {
Expand Down Expand Up @@ -471,9 +405,6 @@ mod tests {

#[async_trait::async_trait]
impl BlockCommitterApi for MockBlockCommitterApi {
async fn get_latest_costs(&self) -> DaBlockCostsResult<Option<RawDaBlockCosts>> {
Ok(self.value.clone())
}
async fn get_costs_by_l2_block_number(
&self,
l2_block_number: u32,
Expand Down Expand Up @@ -504,19 +435,18 @@ mod tests {
}

#[tokio::test]
async fn request_da_block_cost__when_last_value_is_none__then_get_latest_costs_is_called(
) {
async fn request_da_block_cost__when_last_value_is_none__then_error_is_thrown() {
// given
let da_block_costs = test_da_block_costs();
let expected = vec![(&da_block_costs).into()];
let mock_api = MockBlockCommitterApi::new(Some(da_block_costs));
let mut block_committer = BlockCommitterDaBlockCosts::new(mock_api, None);

// when
let actual = block_committer.request_da_block_costs().await.unwrap();
let actual = block_committer.request_da_block_costs().await;

// then
assert_eq!(actual, expected);
assert!(actual.is_err());
}

#[tokio::test]
Expand Down
7 changes: 7 additions & 0 deletions crates/services/gas_price_service/src/v1/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ where
storage_tx
.set_recorded_height(recorded_height)
.map_err(|err| anyhow!(err))?;
} else {
// we default to the l2 block height
// this is done so that we poll the da with a specific height
// to avoid initial loss
storage_tx
.set_recorded_height(height.into())
.map_err(|err| anyhow!(err))?;
}

let fee_in_wei = u128::from(block_fees).saturating_mul(1_000_000_000);
Expand Down

0 comments on commit 0b912e1

Please sign in to comment.