diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a784749090..4dcd90f58b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - [2131](https://github.com/FuelLabs/fuel-core/pull/2131): Add flow in TxPool in order to ask to newly connected peers to share their transaction pool - [2182](https://github.com/FuelLabs/fuel-core/pull/2151): Limit number of transactions that can be fetched via TxSource::next +- [2189](https://github.com/FuelLabs/fuel-core/pull/2151): Select next DA height to never include more than u16::MAX -1 transactions from L1. + ### Changed diff --git a/crates/fuel-core/src/service/adapters/producer.rs b/crates/fuel-core/src/service/adapters/producer.rs index 0449de9e841..d7ac7590106 100644 --- a/crates/fuel-core/src/service/adapters/producer.rs +++ b/crates/fuel-core/src/service/adapters/producer.rs @@ -18,7 +18,10 @@ use fuel_core_producer::{ ConsensusParametersProvider as ConsensusParametersProviderTrait, GasPriceProvider, }, - ports::TxPool, + ports::{ + RelayerBlockInfo, + TxPool, + }, }; use fuel_core_storage::{ iter::{ @@ -139,13 +142,19 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter { } } - async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result { + async fn get_cost_and_transactions_number_for_block( + &self, + height: &DaBlockHeight, + ) -> anyhow::Result { #[cfg(feature = "relayer")] { if let Some(sync) = self.relayer_synced.as_ref() { - get_gas_cost_for_height(**height, sync) + get_gas_cost_and_transactions_number_for_height(**height, sync) } else { - Ok(0) + Ok(RelayerBlockInfo { + gas_cost: 0, + tx_count: 0, + }) } } #[cfg(not(feature = "relayer"))] @@ -155,29 +164,41 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter { "Cannot have a da height above zero without a relayer" ); // If the relayer is not enabled, then all blocks are zero. - Ok(0) + Ok(RelayerBlockInfo { + gas_cost: 0, + tx_count: 0, + }) } } } #[cfg(feature = "relayer")] -fn get_gas_cost_for_height( +fn get_gas_cost_and_transactions_number_for_height( height: u64, sync: &fuel_core_relayer::SharedState< crate::database::Database< crate::database::database_description::relayer::Relayer, >, >, -) -> anyhow::Result { +) -> anyhow::Result { let da_height = DaBlockHeight(height); - let cost = sync + let (gas_cost, tx_count) = sync .database() .storage::() .get(&da_height)? .unwrap_or_default() .iter() - .fold(0u64, |acc, event| acc.saturating_add(event.cost())); - Ok(cost) + .fold((0u64, 0u64), |(gas_cost, tx_count), event| { + let gas_cost = gas_cost.saturating_add(event.cost()); + let tx_count = match event { + fuel_core_types::services::relayer::Event::Message(_) => tx_count, + fuel_core_types::services::relayer::Event::Transaction(_) => { + tx_count.saturating_add(1) + } + }; + (gas_cost, tx_count) + }); + Ok(RelayerBlockInfo { gas_cost, tx_count }) } impl fuel_core_producer::ports::BlockProducerDatabase for OnChainIterableKeyValueView { diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index c6da9767bd3..253f845d422 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -3,8 +3,11 @@ use crate::{ ConsensusParametersProvider, GasPriceProvider as GasPriceProviderConstraint, }, - ports, - ports::BlockProducerDatabase, + ports::{ + self, + BlockProducerDatabase, + RelayerBlockInfo, + }, Config, }; use anyhow::{ @@ -361,8 +364,10 @@ where .consensus_parameters_provider .consensus_params_at_version(&block_header.consensus_parameters_version)? .block_gas_limit(); + // We have a hard limit of u16::MAX transactions per block, including the final mint transactions. + // Therefore we choose the `new_da_height` to never include more than u16::MAX - 1 transactions in a block. let new_da_height = self - .select_new_da_height(gas_limit, previous_da_height) + .select_new_da_height(gas_limit, previous_da_height, u16::MAX - 1) .await?; block_header.application.da_height = new_da_height; @@ -384,9 +389,12 @@ where &self, gas_limit: u64, previous_da_height: DaBlockHeight, + transactions_limit: u16, ) -> anyhow::Result { let mut new_best = previous_da_height; let mut total_cost: u64 = 0; + let transactions_limit: u64 = transactions_limit as u64; + let mut total_transactions: u64 = 0; let highest = self .relayer .wait_for_at_least_height(&previous_da_height) @@ -405,17 +413,19 @@ where let next_da_height = previous_da_height.saturating_add(1); for height in next_da_height..=highest.0 { - let cost = self + let RelayerBlockInfo { gas_cost, tx_count } = self .relayer - .get_cost_for_block(&DaBlockHeight(height)) + .get_cost_and_transactions_number_for_block(&DaBlockHeight(height)) .await?; - total_cost = total_cost.saturating_add(cost); - if total_cost > gas_limit { + total_cost = total_cost.saturating_add(gas_cost); + total_transactions = total_transactions.saturating_add(tx_count); + if total_cost > gas_limit || total_transactions > transactions_limit { break; - } else { - new_best = DaBlockHeight(height); } + + new_best = DaBlockHeight(height); } + if new_best == previous_da_height { Err(anyhow!(NO_NEW_DA_HEIGHT_FOUND)) } else { diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index b0ab390d37b..8a532361ede 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -285,7 +285,7 @@ mod produce_and_execute_block_txpool { let da_height = DaBlockHeight(100u64); let prev_height = 1u32.into(); let ctx = TestContextBuilder::new() - .with_latest_block_height(da_height) + .with_latest_da_block_height_from_relayer(da_height) .with_prev_da_height(da_height) .with_prev_height(prev_height) .build(); @@ -311,7 +311,7 @@ mod produce_and_execute_block_txpool { let prev_da_height = DaBlockHeight(100u64); let prev_height = 1u32.into(); let ctx = TestContextBuilder::new() - .with_latest_block_height(prev_da_height - 1u64.into()) + .with_latest_da_block_height_from_relayer(prev_da_height - 1u64.into()) .with_prev_da_height(prev_da_height) .with_prev_height(prev_height) .build(); @@ -360,7 +360,7 @@ mod produce_and_execute_block_txpool { .map(|(height, gas_cost)| (DaBlockHeight(height), gas_cost)); let ctx = TestContextBuilder::new() - .with_latest_block_height((prev_da_height + 4u64).into()) + .with_latest_da_block_height_from_relayer((prev_da_height + 4u64).into()) .with_latest_blocks_with_gas_costs(latest_blocks_with_gas_costs) .with_prev_da_height(prev_da_height.into()) .with_block_gas_limit(block_gas_limit) @@ -390,6 +390,52 @@ mod produce_and_execute_block_txpool { assert_eq!(expected, actual); } + #[tokio::test] + async fn will_only_advance_da_height_if_enough_transactions_remaining() { + // given + let prev_da_height = 100; + let prev_height = 1u32.into(); + // 0 + 15_000 + 15_000 + 15_000 + 21_000 = 66_000 > 65_535 + let latest_blocks_with_transaction_numbers = vec![ + (prev_da_height, 0u64), + (prev_da_height + 1, 15_000), + (prev_da_height + 2, 15_000), + (prev_da_height + 3, 15_000), + (prev_da_height + 4, 21_000), + ] + .into_iter() + .map(|(height, gas_cost)| (DaBlockHeight(height), gas_cost)); + + let ctx = TestContextBuilder::new() + .with_latest_da_block_height_from_relayer((prev_da_height + 4u64).into()) + .with_latest_blocks_with_transactions(latest_blocks_with_transaction_numbers) + .with_prev_da_height(prev_da_height.into()) + .with_prev_height(prev_height) + .build(); + + let producer = ctx.producer(); + let next_height = prev_height + .succ() + .expect("The block height should be valid"); + + // when + let res = producer + .produce_and_execute_block_txpool(next_height, Tai64::now()) + .await + .unwrap(); + + // then + let expected = prev_da_height + 3; + let actual: u64 = res + .into_result() + .block + .header() + .application() + .da_height + .into(); + assert_eq!(expected, actual); + } + #[tokio::test] async fn if_each_block_is_full_then_only_advance_one_at_a_time() { // given @@ -407,7 +453,7 @@ mod produce_and_execute_block_txpool { .map(|(height, gas_cost)| (DaBlockHeight(height), gas_cost)); let ctx = TestContextBuilder::new() - .with_latest_block_height((prev_da_height + 4u64).into()) + .with_latest_da_block_height_from_relayer((prev_da_height + 4u64).into()) .with_latest_blocks_with_gas_costs(latest_blocks_with_gas_costs) .with_prev_da_height(prev_da_height.into()) .with_block_gas_limit(block_gas_limit) @@ -455,7 +501,7 @@ mod produce_and_execute_block_txpool { .map(|(height, gas_cost)| (DaBlockHeight(height), gas_cost)); let ctx = TestContextBuilder::new() - .with_latest_block_height((prev_da_height + 1u64).into()) + .with_latest_da_block_height_from_relayer((prev_da_height + 1u64).into()) .with_latest_blocks_with_gas_costs(latest_blocks_with_gas_costs) .with_prev_da_height(prev_da_height.into()) .with_block_gas_limit(block_gas_limit) @@ -898,7 +944,7 @@ impl TestContext { struct TestContextBuilder { latest_block_height: DaBlockHeight, - blocks_with_gas_costs: HashMap, + blocks_with_gas_costs_and_transactions_number: HashMap, prev_da_height: DaBlockHeight, block_gas_limit: Option, prev_height: BlockHeight, @@ -909,7 +955,7 @@ impl TestContextBuilder { fn new() -> Self { Self { latest_block_height: 0u64.into(), - blocks_with_gas_costs: HashMap::new(), + blocks_with_gas_costs_and_transactions_number: HashMap::new(), prev_da_height: 1u64.into(), block_gas_limit: None, prev_height: 0u32.into(), @@ -917,17 +963,55 @@ impl TestContextBuilder { } } - fn with_latest_block_height(mut self, latest_block_height: DaBlockHeight) -> Self { + fn with_latest_da_block_height_from_relayer( + mut self, + latest_block_height: DaBlockHeight, + ) -> Self { self.latest_block_height = latest_block_height; self } + fn with_latest_blocks_with_gas_costs_and_transactions_number( + mut self, + latest_blocks_with_gas_costs_and_transactions: impl Iterator< + Item = (DaBlockHeight, (u64, u64)), + >, + ) -> Self { + self.blocks_with_gas_costs_and_transactions_number + .extend(latest_blocks_with_gas_costs_and_transactions); + self + } + + // Helper function that can be used in tests where transaction numbers in a da block are irrelevant fn with_latest_blocks_with_gas_costs( mut self, latest_blocks_with_gas_costs: impl Iterator, ) -> Self { - self.blocks_with_gas_costs - .extend(latest_blocks_with_gas_costs); + let latest_blocks_with_gas_costs_and_transactions_number = + latest_blocks_with_gas_costs + .into_iter() + .map(|(da_block_height, gas_costs)| (da_block_height, (gas_costs, 0))); + // Assigning `self` necessary to avoid the compiler complaining about the mutability of `self` + self = self.with_latest_blocks_with_gas_costs_and_transactions_number( + latest_blocks_with_gas_costs_and_transactions_number, + ); + self + } + + // Helper function that can be used in tests where gas costs in a da block are irrelevant + fn with_latest_blocks_with_transactions( + mut self, + latest_blocks_with_transactions: impl Iterator, + ) -> Self { + let latest_blocks_with_gas_costs_and_transactions_number = + latest_blocks_with_transactions.into_iter().map( + |(da_block_height, transactions)| (da_block_height, (0, transactions)), + ); + + // Assigning `self` necessary to avoid the compiler complaining about the mutability of `self` + self = self.with_latest_blocks_with_gas_costs_and_transactions_number( + latest_blocks_with_gas_costs_and_transactions_number, + ); self } @@ -982,7 +1066,9 @@ impl TestContextBuilder { let mock_relayer = MockRelayer { latest_block_height: self.latest_block_height, - latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(), + latest_da_blocks_with_costs_and_transactions_number: self + .blocks_with_gas_costs_and_transactions_number + .clone(), ..Default::default() }; @@ -1004,7 +1090,9 @@ impl TestContextBuilder { let mock_relayer = MockRelayer { latest_block_height: self.latest_block_height, - latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(), + latest_da_blocks_with_costs_and_transactions_number: self + .blocks_with_gas_costs_and_transactions_number + .clone(), ..Default::default() }; diff --git a/crates/services/producer/src/mocks.rs b/crates/services/producer/src/mocks.rs index 912796185b0..b2c21f0971e 100644 --- a/crates/services/producer/src/mocks.rs +++ b/crates/services/producer/src/mocks.rs @@ -3,6 +3,7 @@ use crate::ports::{ BlockProducerDatabase, DryRunner, Relayer, + RelayerBlockInfo, TxPool, }; use fuel_core_storage::{ @@ -59,7 +60,8 @@ use std::{ pub struct MockRelayer { pub block_production_key: Address, pub latest_block_height: DaBlockHeight, - pub latest_da_blocks_with_costs: HashMap, + pub latest_da_blocks_with_costs_and_transactions_number: + HashMap, } #[async_trait::async_trait] @@ -72,13 +74,16 @@ impl Relayer for MockRelayer { Ok(highest) } - async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result { - let cost = self - .latest_da_blocks_with_costs + async fn get_cost_and_transactions_number_for_block( + &self, + height: &DaBlockHeight, + ) -> anyhow::Result { + let (gas_cost, tx_count) = self + .latest_da_blocks_with_costs_and_transactions_number .get(height) .cloned() .unwrap_or_default(); - Ok(cost) + Ok(RelayerBlockInfo { gas_cost, tx_count }) } } diff --git a/crates/services/producer/src/ports.rs b/crates/services/producer/src/ports.rs index a50b2ebe33c..af11d1648e9 100644 --- a/crates/services/producer/src/ports.rs +++ b/crates/services/producer/src/ports.rs @@ -62,6 +62,11 @@ pub trait TxPool: Send + Sync { ) -> Self::TxSource; } +pub struct RelayerBlockInfo { + pub gas_cost: u64, + pub tx_count: u64, +} + #[async_trait::async_trait] pub trait Relayer: Send + Sync { /// Wait for the relayer to reach at least this height and return the latest height. @@ -71,7 +76,10 @@ pub trait Relayer: Send + Sync { ) -> anyhow::Result; /// Get the total Forced Transaction gas cost for the block at the given height. - async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result; + async fn get_cost_and_transactions_number_for_block( + &self, + height: &DaBlockHeight, + ) -> anyhow::Result; } pub trait BlockProducer: Send + Sync {