Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(gas_price_service): minor refactors to make the code more readable #2636

Merged
merged 2 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ pub mod storage;

impl<Storage> SetMetadataStorage for Storage
where
Storage: Send + Sync,
Storage: Modifiable,
Storage: Send + Sync + Modifiable,
for<'a> StorageTransaction<&'a mut Storage>:
StorageMutate<GasPriceMetadata, Error = StorageError>,
{
Expand Down Expand Up @@ -95,7 +94,7 @@ where
.map_err(|err| GasPriceError::CouldNotFetchMetadata {
source_error: err.into(),
})?;
Ok(metadata.map(|inner| inner.into_owned()))
Ok(metadata.map(std::borrow::Cow::into_owned))
}
}

Expand All @@ -117,9 +116,13 @@ where

impl<Storage> GasPriceServiceAtomicStorage for Storage
where
Storage: 'static,
Storage: GetMetadataStorage + GetLatestRecordedHeight,
Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable + Send + Sync,
Storage: GetMetadataStorage
+ GetLatestRecordedHeight
+ KeyValueInspect<Column = GasPriceColumn>
+ Modifiable
+ Send
+ Sync
+ 'static,
{
type Transaction<'a> = StorageTransaction<&'a mut Storage> where Self: 'a;

Expand All @@ -138,8 +141,7 @@ where

impl<Storage> SetLatestRecordedHeight for Storage
where
Storage: Send + Sync,
Storage: StorageMutate<RecordedHeights, Error = StorageError>,
Storage: Send + Sync + StorageMutate<RecordedHeights, Error = StorageError>,
{
fn set_recorded_height(
&mut self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ pub enum UpdaterMetadata {
impl UpdaterMetadata {
pub fn l2_block_height(&self) -> BlockHeight {
match self {
UpdaterMetadata::V0(v0) => v0.l2_block_height.into(),
UpdaterMetadata::V1(v1) => v1.l2_block_height.into(),
Self::V0(v0) => v0.l2_block_height.into(),
Self::V1(v1) => v1.l2_block_height.into(),
}
}

pub fn v1(&self) -> Option<&V1Metadata> {
match self {
UpdaterMetadata::V1(v1) => Some(v1),
Self::V1(v1) => Some(v1),
_ => None,
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/services/gas_price_service/src/v0/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ where
.ok_or_else(|| anyhow!("Block gas capacity must be non-zero"))
}

async fn set_metadata(&mut self) -> anyhow::Result<()> {
fn set_metadata(&mut self) -> anyhow::Result<()> {
let metadata: UpdaterMetadata = self.algorithm_updater.clone().into();
self.metadata_storage
.set_metadata(&metadata)
Expand All @@ -92,7 +92,7 @@ where
self.algorithm_updater
.update_l2_block_data(height, gas_used, capacity)?;

self.set_metadata().await?;
self.set_metadata()?;
Ok(())
}

Expand All @@ -102,7 +102,7 @@ where
) -> anyhow::Result<()> {
match l2_block {
BlockInfo::GenesisBlock => {
self.set_metadata().await?;
self.set_metadata()?;
}
BlockInfo::Block {
height,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,9 @@ where
{
let metadata = metadata_storage
.get_metadata(&metadata_height.into())?
.ok_or(anyhow::anyhow!(
"Expected metadata to exist for height: {metadata_height}"
))?;
.ok_or_else(|| {
anyhow::anyhow!("Expected metadata to exist for height: {metadata_height}")
})?;

let mut algo_updater = if let UpdaterMetadata::V0(metadata) = metadata {
Ok(AlgorithmUpdaterV0::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl From<&RawDaBlockCosts> for DaBlockCosts {
id: bundle_id,
..
} = *raw_da_block_costs;
DaBlockCosts {
Self {
bundle_id,
// construct a vec of l2 blocks from the start_height to the end_height
l2_blocks: start_height..=end_height,
Expand Down Expand Up @@ -90,10 +90,12 @@ where
&mut self,
last_recorded_height: &BlockHeight,
) -> DaBlockCostsResult<Vec<DaBlockCosts>> {
let next_height = last_recorded_height.succ().ok_or(anyhow!(
"Failed to increment the last recorded height: {:?}",
last_recorded_height
))?;
let next_height = last_recorded_height.succ().ok_or_else(|| {
anyhow!(
"Failed to increment the last recorded height: {:?}",
last_recorded_height
)
})?;

let raw_da_block_costs: Vec<_> = self
.client
Expand Down Expand Up @@ -309,15 +311,14 @@ pub mod fake_server {
let guard = shared_responses.lock().unwrap();
let most_recent = guard
.iter()
.fold(None, |acc, x| match acc {
None => Some(x),
Some(acc) => {
.fold(None, |acc, x| {
acc.map_or(Some(x), |acc: &RawDaBlockCosts| {
if x.end_height > acc.end_height {
Some(x)
} else {
Some(acc)
}
}
})
})
.cloned();
let response: Vec<RawDaBlockCosts> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ where
tracing::debug!("Received block costs: {:?}", da_block_costs_res);
let da_block_costs = da_block_costs_res?;
let filtered_block_costs = self
.filter_costs_that_have_values_greater_than_l2_block_height(da_block_costs)?;
.filter_costs_that_have_values_greater_than_l2_block_height(da_block_costs);
tracing::debug!(
"the latest l2 height is: {:?}",
self.latest_l2_height.load(Ordering::Acquire)
Expand All @@ -130,13 +130,12 @@ where
fn filter_costs_that_have_values_greater_than_l2_block_height(
&self,
da_block_costs: Vec<DaBlockCosts>,
) -> Result<impl Iterator<Item = DaBlockCosts>> {
) -> impl Iterator<Item = DaBlockCosts> {
let latest_l2_height = self.latest_l2_height.load(Ordering::Acquire);
let iter = da_block_costs.into_iter().filter(move |da_block_costs| {
da_block_costs.into_iter().filter(move |da_block_costs| {
let end = *da_block_costs.l2_blocks.end();
end < latest_l2_height
});
Ok(iter)
})
}

#[cfg(test)]
Expand Down
5 changes: 2 additions & 3 deletions crates/services/gas_price_service/src/v1/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,19 +174,18 @@ where
tracing::debug!("Updating gas price algorithm");
self.apply_block_info_to_gas_algorithm(block).await?;

self.notify_da_source_service_l2_block(block)?;
self.notify_da_source_service_l2_block(block);
Ok(())
}

fn notify_da_source_service_l2_block(&self, block: BlockInfo) -> anyhow::Result<()> {
fn notify_da_source_service_l2_block(&self, block: BlockInfo) {
tracing::debug!("Notifying the Da source service of the latest L2 block");
match block {
BlockInfo::GenesisBlock => {}
BlockInfo::Block { height, .. } => {
self.latest_l2_block.store(height, Ordering::Release);
}
}
Ok(())
}
}

Expand Down
36 changes: 16 additions & 20 deletions crates/services/gas_price_service/src/v1/uninitialized_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,8 @@ where
.unwrap_or(genesis_block_height)
.into();

let gas_price_metadata_height = gas_metadata_height
.map(|x| x.into())
.unwrap_or(latest_block_height);
let gas_price_metadata_height =
gas_metadata_height.map_or(latest_block_height, std::convert::Into::into);

let (algo_updater, shared_algo) = initialize_algorithm(
&config,
Expand Down Expand Up @@ -186,11 +185,8 @@ where
.into();

let maybe_metadata_height = self.gas_metadata_height;
let metadata_height = if let Some(metadata_height) = maybe_metadata_height {
metadata_height.into()
} else {
latest_block_height
};
let metadata_height =
maybe_metadata_height.map_or(latest_block_height, std::convert::Into::into);

let l2_block_source = FuelL2BlockSource::new(
self.genesis_block_height,
Expand All @@ -200,19 +196,19 @@ where

let starting_recorded_height = match self.gas_price_db.get_recorded_height()? {
Some(height) => height,
None => match self.config.starting_recorded_height {
Some(height) => {
tracing::debug!(
"Using provided starting recorded height: {:?}",
height
);
height
}
None => {
None => self.config.starting_recorded_height.map_or_else(
|| {
tracing::debug!("No starting recorded height provided, defaulting to latest block height");
BlockHeight::from(latest_block_height)
}
},
},
|height| {
tracing::debug!(
"Using provided starting recorded height: {:?}",
height
);
height
},
),
};

let poll_duration = self.config.da_poll_interval;
Expand Down Expand Up @@ -295,7 +291,7 @@ where
state_watcher: &StateWatcher,
_params: Self::TaskParams,
) -> anyhow::Result<Self::Task> {
UninitializedTask::init(self, state_watcher).await
Self::init(self, state_watcher).await
}
}

Expand Down
Loading