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

Modify block producer to take into account the total gas used by the L1 transactions #1785

Merged
merged 36 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2112149
Redesign producer relayer trait to allow info about cost, add failing…
MitchTurner Mar 29, 2024
e71e858
Add passing test for limiting da inclusion by gas limit
MitchTurner Mar 29, 2024
46939ae
Update CHANGELOG
MitchTurner Mar 29, 2024
4d11df6
Fix CHANGELOG formatting
MitchTurner Mar 29, 2024
a3890f8
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Mar 29, 2024
9175f7b
Implement event lookup for real adapter
MitchTurner Mar 29, 2024
5cb72c4
Make function always available
MitchTurner Mar 29, 2024
2f283a4
Fix addtition
MitchTurner Mar 29, 2024
c1a025f
Fix tests
MitchTurner Mar 29, 2024
fbb85fe
Fix relayer feature stuff
MitchTurner Mar 29, 2024
3402243
Fix more relayer feature issues
MitchTurner Mar 29, 2024
9997cb6
More of same
MitchTurner Mar 29, 2024
37edfc0
Empty
MitchTurner Mar 29, 2024
069738f
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Mar 29, 2024
73dd733
Refactor tests a bit
MitchTurner Mar 29, 2024
3415fa8
Remove unused imput
MitchTurner Mar 29, 2024
6c53316
Appease Clippy-sama
MitchTurner Mar 29, 2024
0492ff0
Sum with saturating
MitchTurner Mar 30, 2024
fe75066
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Mar 30, 2024
f271235
Revert changes to indentation
MitchTurner Apr 1, 2024
8ddec16
Split up relayer methods for producer
MitchTurner Apr 1, 2024
eda5023
Update relayer impl for producer
MitchTurner Apr 1, 2024
ce832df
Fix for non relayer feature
MitchTurner Apr 1, 2024
ec80451
Remove commented code
MitchTurner Apr 1, 2024
7f4cbc7
Move sync test to trait impl
MitchTurner Apr 1, 2024
321a39e
Fix relayer feature thing
MitchTurner Apr 1, 2024
7c0ef76
Appease Clippy-sama
MitchTurner Apr 1, 2024
4577a3a
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Apr 1, 2024
650549a
Fix typing ambiguity
MitchTurner Apr 1, 2024
dabc8d8
Remove commented code
MitchTurner Apr 1, 2024
e7dbb65
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Apr 1, 2024
8b59aea
Merge branch 'master' into take-into-account-gas-when-including-L1-txs
MitchTurner Apr 2, 2024
e08ba99
Add test to address bug in DA height selection
MitchTurner Apr 3, 2024
1021022
Merge branch 'take-into-account-gas-when-including-L1-txs' of github.…
MitchTurner Apr 3, 2024
c628f99
Appease Clippy-sama
MitchTurner Apr 3, 2024
4980eba
Add test to throw error if no new da block found
MitchTurner Apr 3, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Description of the upcoming release here.

#### Breaking

- [1785](https://github.com/FuelLabs/fuel-core/pull/1785): Producer will only include DA height if it has enough gas to include the associate forced transactions.
- [#1771](https://github.com/FuelLabs/fuel-core/pull/1771): Contract 'states' and 'balances' brought back into `ContractConfig`. Parquet now writes a file per table.
- [1779](https://github.com/FuelLabs/fuel-core/pull/1779): Modify Relayer service to order Events from L1 by block index
- [#1783](https://github.com/FuelLabs/fuel-core/pull/1783): The PR upgrade `fuel-vm` to `0.48.0` release. Because of some breaking changes, we also adapted our codebase to follow them:
Expand Down
19 changes: 11 additions & 8 deletions bin/fuel-core/src/cli/run.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(unused_variables)]

use crate::{
cli::{
default_db_path,
Expand Down Expand Up @@ -86,18 +87,18 @@ pub struct Command {

/// The maximum database cache size in bytes.
#[arg(
long = "max-database-cache-size",
default_value_t = DEFAULT_DATABASE_CACHE_SIZE,
env
long = "max-database-cache-size",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you revert back indent changes in this file, please?=)

default_value_t = DEFAULT_DATABASE_CACHE_SIZE,
env
)]
pub max_database_cache_size: usize,

#[clap(
name = "DB_PATH",
long = "db-path",
value_parser,
default_value = default_db_path().into_os_string(),
env
name = "DB_PATH",
long = "db-path",
value_parser,
default_value = default_db_path().into_os_string(),
env
)]
pub database_path: PathBuf,

Expand Down Expand Up @@ -316,6 +317,7 @@ impl Command {
tx_blacklist_messages,
tx_blacklist_contracts,
);
let block_gas_limit = chain_config.consensus_parameters.block_gas_limit();

let config = Config {
addr,
Expand All @@ -342,6 +344,7 @@ impl Command {
utxo_validation,
coinbase_recipient,
metrics,
block_gas_limit,
},
static_gas_price: min_gas_price,
block_importer,
Expand Down
5 changes: 3 additions & 2 deletions crates/fuel-core/src/service/adapters/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,20 @@ impl fuel_core_executor::ports::RelayerPort for Database<Relayer> {
}
}

fn get_events(&self, _da_height: &DaBlockHeight) -> anyhow::Result<Vec<Event>> {
fn get_events(&self, da_height: &DaBlockHeight) -> anyhow::Result<Vec<Event>> {
#[cfg(feature = "relayer")]
{
use fuel_core_storage::StorageAsRef;
let events = self
.storage::<fuel_core_relayer::storage::EventsHistory>()
.get(_da_height)?
.get(da_height)?
.map(|cow| cow.into_owned())
.unwrap_or_default();
Ok(events)
}
#[cfg(not(feature = "relayer"))]
{
let _ = da_height;
Ok(vec![])
}
}
Expand Down
40 changes: 31 additions & 9 deletions crates/fuel-core/src/service/adapters/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use fuel_core_types::{
ConsensusParametersVersion,
StateTransitionBytecodeVersion,
},
primitives,
primitives::DaBlockHeight,
},
fuel_tx,
fuel_tx::Transaction,
Expand Down Expand Up @@ -123,31 +123,53 @@ impl fuel_core_producer::ports::DryRunner for ExecutorAdapter {

#[async_trait::async_trait]
impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter {
async fn wait_for_at_least(
async fn get_latest_da_blocks_with_costs(
&self,
height: &primitives::DaBlockHeight,
) -> anyhow::Result<primitives::DaBlockHeight> {
starting_from: &DaBlockHeight,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice, maybe, to move this logic to block producer.

In this case, the block producer will have control over the number of iterations. Because we had a long delay/finalization and we have 100 blocks with a maxed gas limit, you will do 100 iterations while you need only one.

We could use something like:

#[async_trait::async_trait]
pub trait Relayer: Send + Sync {
    /// Wait for the relayer to reach at least this height and return the
    /// latest height (which is guaranteed to be >= height).
    async fn wait_for_at_least(
        &self,
        height: &DaBlockHeight,
    ) -> anyhow::Result<DaBlockHeight>;

    /// Returns the gas cost of events at the given height.
    async fn gas_cost_at_height(
        &self,
        height: &DaBlockHeight,
    ) -> anyhow::Result<Word>;
}

) -> anyhow::Result<Vec<(DaBlockHeight, u64)>> {
#[cfg(feature = "relayer")]
{
if let Some(sync) = self.relayer_synced.as_ref() {
sync.await_at_least_synced(height).await?;
sync.get_finalized_da_height()
sync.await_at_least_synced(starting_from).await?;
let highest = sync.get_finalized_da_height()?;
(starting_from.0..=highest.0)
.map(|height| get_gas_cost_for_height(height, sync))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that the current usage of the Genesis da block height will lead to iterations from 0..5M.

It may not be a huge problem in the long term, but it may slow down the start of the fuel core for local development.

.collect()
} else {
Ok(0u64.into())
Ok(Vec::new())
}
}
#[cfg(not(feature = "relayer"))]
{
anyhow::ensure!(
**height == 0,
**starting_from == 0,
"Cannot have a da height above zero without a relayer"
);
// If the relayer is not enabled, then all blocks are zero.
Ok(0u64.into())
Ok(Vec::new())
}
}
}

#[cfg(feature = "relayer")]
fn get_gas_cost_for_height(
height: u64,
sync: &fuel_core_relayer::SharedState<
Database<crate::database::database_description::relayer::Relayer>,
>,
) -> anyhow::Result<(DaBlockHeight, u64)> {
let da_height = DaBlockHeight(height);
let cost = sync
.database()
.storage::<fuel_core_relayer::storage::EventsHistory>()
.get(&da_height)?
.map(|cow| cow.into_owned())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to call into_owned here. It is the place where we want to use references if possible.

.unwrap_or_default()
.iter()
.fold(0u64, |acc, event| acc.saturating_add(event.cost()));
Ok((da_height, cost))
}

impl fuel_core_producer::ports::BlockProducerDatabase for Database {
fn get_block(&self, height: &BlockHeight) -> StorageResult<Cow<CompressedBlock>> {
self.storage::<FuelBlocks>()
Expand Down
35 changes: 25 additions & 10 deletions crates/services/producer/src/block_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ where
block_time: Tai64,
) -> anyhow::Result<PartialBlockHeader> {
let mut block_header = self._new_header(height, block_time)?;
let new_da_height = self.select_new_da_height(block_header.da_height).await?;
let previous_da_height = block_header.da_height;
let new_da_height = self.select_new_da_height(previous_da_height).await?;

block_header.application.da_height = new_da_height;

Expand All @@ -275,17 +276,31 @@ where
&self,
previous_da_height: DaBlockHeight,
) -> anyhow::Result<DaBlockHeight> {
let best_height = self.relayer.wait_for_at_least(&previous_da_height).await?;
if best_height < previous_da_height {
// If this happens, it could mean a block was erroneously imported
// without waiting for our relayer's da_height to catch up to imported da_height.
return Err(Error::InvalidDaFinalizationState {
best: best_height,
previous_block: previous_da_height,
let gas_limit = self.config.block_gas_limit;
let list = self
.relayer
.get_latest_da_blocks_with_costs(&previous_da_height)
.await?;
let mut new_best = previous_da_height;
let mut total_cost: u64 = 0;
for (da_height, cost) in list.iter() {
if da_height < &previous_da_height {
return Err(Error::InvalidDaFinalizationState {
best: *da_height,
previous_block: previous_da_height,
}
.into());
}
if da_height > &new_best {
total_cost = total_cost.saturating_add(*cost);
if total_cost > gas_limit {
break;
} else {
new_best = *da_height;
}
}
.into())
}
Ok(best_height)
Ok(new_best)
}

fn _new_header(
Expand Down
Loading
Loading