Skip to content

Commit

Permalink
refactor: remove Option<BlockHeight> and use new enum where applica…
Browse files Browse the repository at this point in the history
…ble (#2033)

closes #2005 

I replaced `Option<BlockHeight>` with `BlockHeightQuery` where
applicable. IIUC there are `Option<BlockHeight>` which should remain.

## Checklist
- [X] Breaking changes are clearly marked as such in the PR description
and changelog
- [X] New behavior is reflected in tests
- [X] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [X] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
  • Loading branch information
matt-user authored Nov 26, 2024
1 parent 38e844d commit 000fcb1
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 43 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2376](https://github.com/FuelLabs/fuel-core/pull/2376): Add a way to fetch transactions in P2P without specifying a peer.
- [2327](https://github.com/FuelLabs/fuel-core/pull/2327): Add more services tests and more checks of the pool. Also add an high level documentation for users of the pool and contributors.
- [2416](https://github.com/FuelLabs/fuel-core/issues/2416): Define the `GasPriceServiceV1` task.

- [2033](https://github.com/FuelLabs/fuel-core/pull/2033): Remove `Option<BlockHeight>` in favor of `BlockHeightQuery` where applicable.

### Fixed
- [2366](https://github.com/FuelLabs/fuel-core/pull/2366): The `importer_gas_price_for_block` metric is properly collected.
Expand Down
12 changes: 11 additions & 1 deletion crates/fuel-core/src/graphql_api/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ pub mod worker {
relayed_transactions::RelayedTransactionStatuses,
},
};
use derive_more::Display;
use fuel_core_services::stream::BoxStream;
use fuel_core_storage::{
Error as StorageError,
Expand Down Expand Up @@ -317,6 +318,15 @@ pub mod worker {
fn transaction(&mut self) -> Self::Transaction<'_>;
}

/// Represents either the Genesis Block or a block at a specific height
#[derive(Copy, Clone, Debug, Display, PartialEq, Eq, Hash, Ord, PartialOrd)]
pub enum BlockAt {
/// Block at a specific height
Specific(BlockHeight),
/// Genesis block
Genesis,
}

pub trait OffChainDatabaseTransaction:
StorageMutate<OwnedMessageIds, Error = StorageError>
+ StorageMutate<OwnedCoins, Error = StorageError>
Expand Down Expand Up @@ -369,7 +379,7 @@ pub mod worker {
/// Return the import result at the given height.
fn block_event_at_height(
&self,
height: Option<BlockHeight>,
height: BlockAt,
) -> anyhow::Result<SharedImportResult>;
}

Expand Down
10 changes: 9 additions & 1 deletion crates/fuel-core/src/graphql_api/worker_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use crate::{
fuel_core_graphql_api::{
ports::{
self,
worker::OffChainDatabaseTransaction,
worker::{
BlockAt,
OffChainDatabaseTransaction,
},
},
storage::{
blocks::FuelBlockIdsToHeights,
Expand Down Expand Up @@ -537,6 +540,11 @@ where
let next_block_height =
off_chain_height.map(|height| BlockHeight::new(height.saturating_add(1)));

let next_block_height = match next_block_height {
Some(block_height) => BlockAt::Specific(block_height),
None => BlockAt::Genesis,
};

let import_result =
import_result_provider.block_event_at_height(next_block_height)?;

Expand Down
6 changes: 4 additions & 2 deletions crates/fuel-core/src/schema/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ use fuel_core_types::{
header::BlockHeader,
},
fuel_tx::TxId,
fuel_types,
fuel_types::BlockHeight,
fuel_types::{
self,
BlockHeight,
},
};
use futures::{
Stream,
Expand Down
3 changes: 2 additions & 1 deletion crates/fuel-core/src/service/adapters/graphql_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
database::OnChainIterableKeyValueView,
fuel_core_graphql_api::ports::{
worker,
worker::BlockAt,
BlockProducerPort,
ConsensusProvider,
DatabaseMessageProof,
Expand Down Expand Up @@ -218,7 +219,7 @@ impl worker::BlockImporter for GraphQLBlockImporter {

fn block_event_at_height(
&self,
height: Option<BlockHeight>,
height: BlockAt,
) -> anyhow::Result<SharedImportResult> {
self.import_result_provider_adapter.result_at_height(height)
}
Expand Down
76 changes: 39 additions & 37 deletions crates/fuel-core/src/service/adapters/import_result_provider.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
use crate::{
database::Database,
fuel_core_graphql_api::ports::worker::BlockAt,
service::adapters::ExecutorAdapter,
};
use fuel_core_importer::ports::Validator;
use fuel_core_storage::{
not_found,
transactional::AtomicView,
};
use fuel_core_types::{
fuel_types::BlockHeight,
services::{
block_importer::{
ImportResult,
SharedImportResult,
},
executor::ValidationResult,
use fuel_core_types::services::{
block_importer::{
ImportResult,
SharedImportResult,
},
executor::ValidationResult,
};
use std::sync::Arc;

Expand All @@ -37,38 +35,42 @@ impl ImportResultProvider {
impl ImportResultProvider {
pub fn result_at_height(
&self,
height: Option<BlockHeight>,
height: BlockAt,
) -> anyhow::Result<SharedImportResult> {
if let Some(height) = height {
let sealed_block = self
.on_chain_database
.latest_view()?
.get_sealed_block_by_height(&height)?
.ok_or(not_found!("SealedBlock"))?;
match height {
BlockAt::Specific(height) => {
let sealed_block = self
.on_chain_database
.latest_view()?
.get_sealed_block_by_height(&height)?
.ok_or(not_found!("SealedBlock"))?;

let ValidationResult { tx_status, events } = self
.executor_adapter
.validate(&sealed_block.entity)?
.into_result();
let result = ImportResult::new_from_local(sealed_block, tx_status, events);
Ok(Arc::new(result))
} else {
let genesis_height = self
.on_chain_database
.latest_view()?
.genesis_height()?
.ok_or(not_found!("Genesis height"))?;
let sealed_block = self
.on_chain_database
.latest_view()?
.get_sealed_block_by_height(&genesis_height)?
.ok_or(not_found!("SealedBlock"))?;
let ValidationResult { tx_status, events } = self
.executor_adapter
.validate(&sealed_block.entity)?
.into_result();
let result =
ImportResult::new_from_local(sealed_block, tx_status, events);
Ok(Arc::new(result))
}
BlockAt::Genesis => {
let genesis_height = self
.on_chain_database
.latest_view()?
.genesis_height()?
.ok_or(not_found!("Genesis height"))?;
let sealed_block = self
.on_chain_database
.latest_view()?
.get_sealed_block_by_height(&genesis_height)?
.ok_or(not_found!("SealedBlock"))?;

Ok(Arc::new(ImportResult::new_from_local(
sealed_block,
vec![],
vec![],
)))
Ok(Arc::new(ImportResult::new_from_local(
sealed_block,
vec![],
vec![],
)))
}
}
}
}

0 comments on commit 000fcb1

Please sign in to comment.