Skip to content

Commit

Permalink
Enforce the block size limit (#2195)
Browse files Browse the repository at this point in the history
Closes #2133

## Linked Issues/PRs
Built on top of: #2188

## Description
This PR adds the handling and enforcement of then new consensus
parameter added [here](#2188).

Changes summary:
- `TransactionsSource` and `transaction_selector` use the new
`block_transaction_size_limit` parameter.
- `ExecutionData` contains `used_size` in addition to `used_gas`
- Introduces integration tests covering the expected behavior
- E2E tests affected by the new limit are updated

## Checklist
- [X] Breaking changes are clearly marked as such in the PR description
and changelog
- [X] New behavior is reflected in tests

### Before requesting review
- [X] I have reviewed the code myself

---------

Co-authored-by: green <xgreenx9999@gmail.com>
  • Loading branch information
rafal-ch and xgreenx authored Sep 27, 2024
1 parent 7b8713c commit 4a55b7d
Show file tree
Hide file tree
Showing 13 changed files with 404 additions and 91 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased]

### Added
- [2195](https://github.com/FuelLabs/fuel-core/pull/2195): Added enforcement of the limit on the size of the L2 transactions per block according to the `block_transaction_size_limit` parameter.
- [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.
Expand Down
6 changes: 6 additions & 0 deletions bin/e2e-test-client/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ fn dev_config() -> Config {
.consensus_parameters
.set_tx_params(tx_parameters);
chain_config.consensus_parameters.set_fee_params(fee_params);
if let Err(_e) = chain_config
.consensus_parameters
.set_block_transaction_size_limit(u64::MAX)
{
eprintln!("failed to set block transaction size limit");
}
chain_config.state_transition_bytecode =
fuel_core::upgradable_executor::WASM_BYTECODE.to_vec();
let reader = reader.with_chain_config(chain_config);
Expand Down
9 changes: 6 additions & 3 deletions crates/fuel-core/src/service/adapters/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@ use fuel_core_types::{
};

impl fuel_core_executor::ports::TransactionsSource for TransactionsSource {
// TODO: Use `size_limit` https://github.com/FuelLabs/fuel-core/issues/2133
fn next(
&self,
gas_limit: u64,
transactions_limit: u16,
_: u32,
block_transaction_size_limit: u32,
) -> Vec<MaybeCheckedTransaction> {
self.txpool
.select_transactions(gas_limit, transactions_limit)
.select_transactions(
gas_limit,
transactions_limit,
block_transaction_size_limit,
)
.into_iter()
.map(|tx| {
MaybeCheckedTransaction::CheckedTransaction(
Expand Down
41 changes: 33 additions & 8 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ impl TransactionsSource for OnceTransactionsSource {
pub struct ExecutionData {
coinbase: u64,
used_gas: u64,
used_size: u32,
tx_count: u16,
found_mint: bool,
message_ids: Vec<MessageId>,
Expand All @@ -224,6 +225,7 @@ impl ExecutionData {
ExecutionData {
coinbase: 0,
used_gas: 0,
used_size: 0,
tx_count: 0,
found_mint: false,
message_ids: Vec::new(),
Expand Down Expand Up @@ -296,6 +298,7 @@ where
skipped_transactions,
coinbase,
used_gas,
used_size,
..
} = execution_data;

Expand All @@ -306,8 +309,8 @@ where
let finalized_block_id = block.id();

debug!(
"Block {:#x} fees: {} gas: {}",
finalized_block_id, coinbase, used_gas
"Block {:#x} fees: {} gas: {} tx_size: {}",
finalized_block_id, coinbase, used_gas, used_size
);

let result = ExecutionResult {
Expand All @@ -331,6 +334,7 @@ where
let ExecutionData {
coinbase,
used_gas,
used_size,
tx_status,
events,
changes,
Expand All @@ -340,8 +344,8 @@ where
let finalized_block_id = block.id();

debug!(
"Block {:#x} fees: {} gas: {}",
finalized_block_id, coinbase, used_gas
"Block {:#x} fees: {} gas: {} tx_size: {}",
finalized_block_id, coinbase, used_gas, used_size
);

let result = ValidationResult { tx_status, events };
Expand Down Expand Up @@ -575,10 +579,15 @@ where
..
} = components;
let block_gas_limit = self.consensus_params.block_gas_limit();
let block_transaction_size_limit = self
.consensus_params
.block_transaction_size_limit()
.try_into()
.unwrap_or(u32::MAX);

let mut remaining_gas_limit = block_gas_limit.saturating_sub(data.used_gas);
// TODO: Handle `remaining_size` https://github.com/FuelLabs/fuel-core/issues/2133
let remaining_size = u32::MAX;
let mut remaining_block_transaction_size_limit =
block_transaction_size_limit.saturating_sub(data.used_size);

// We allow at most u16::MAX transactions in a block, including the mint transaction.
// When processing l2 transactions, we must take into account transactions from the l1
Expand All @@ -587,7 +596,11 @@ where
let mut remaining_tx_count = MAX_TX_COUNT.saturating_sub(data.tx_count);

let mut regular_tx_iter = l2_tx_source
.next(remaining_gas_limit, remaining_tx_count, remaining_size)
.next(
remaining_gas_limit,
remaining_tx_count,
remaining_block_transaction_size_limit,
)
.into_iter()
.take(remaining_tx_count as usize)
.peekable();
Expand All @@ -614,11 +627,17 @@ where
}
}
remaining_gas_limit = block_gas_limit.saturating_sub(data.used_gas);
remaining_block_transaction_size_limit =
block_transaction_size_limit.saturating_sub(data.used_size);
remaining_tx_count = MAX_TX_COUNT.saturating_sub(data.tx_count);
}

regular_tx_iter = l2_tx_source
.next(remaining_gas_limit, remaining_tx_count, remaining_size)
.next(
remaining_gas_limit,
remaining_tx_count,
remaining_block_transaction_size_limit,
)
.into_iter()
.take(remaining_tx_count as usize)
.peekable();
Expand Down Expand Up @@ -1413,6 +1432,8 @@ where
tx_id: TxId,
) -> ExecutorResult<()> {
let (used_gas, tx_fee) = self.total_fee_paid(tx, &receipts, gas_price)?;
let used_size = tx.metered_bytes_size().try_into().unwrap_or(u32::MAX);

execution_data.coinbase = execution_data
.coinbase
.checked_add(tx_fee)
Expand All @@ -1421,6 +1442,10 @@ where
.used_gas
.checked_add(used_gas)
.ok_or(ExecutorError::GasOverflow)?;
execution_data.used_size = execution_data
.used_size
.checked_add(used_size)
.ok_or(ExecutorError::TxSizeOverflow)?;

if !reverted {
execution_data
Expand Down
4 changes: 2 additions & 2 deletions crates/services/executor/src/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,14 @@ impl TransactionExt for MaybeCheckedTransaction {
}

pub trait TransactionsSource {
/// Returns the next batch of transactions to satisfy the `gas_limit`.
/// Returns the next batch of transactions to satisfy the `gas_limit` and `block_transaction_size_limit`.
/// The returned batch has at most `tx_count_limit` transactions, none
/// of which has a size in bytes greater than `size_limit`.
fn next(
&self,
gas_limit: u64,
tx_count_limit: u16,
size_limit: u32,
block_transaction_size_limit: u32,
) -> Vec<MaybeCheckedTransaction>;
}

Expand Down
10 changes: 5 additions & 5 deletions crates/services/txpool/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ use fuel_core_types::{
txpool::{
ArcPoolTx,
InsertionResult,
PoolTransaction,
TransactionStatus,
},
},
Expand Down Expand Up @@ -396,13 +395,14 @@ impl<P2P, ViewProvider, WasmChecker, GasPriceProvider, ConsensusProvider, MP>
&self,
max_gas: u64,
transactions_limit: u16,
block_transaction_size_limit: u32,
) -> Vec<ArcPoolTx> {
let mut guard = self.txpool.lock();
let txs = guard.includable();
let sorted_txs: Vec<Arc<PoolTransaction>> = select_transactions(txs, max_gas)
.into_iter()
.take(transactions_limit as usize)
.collect();
let sorted_txs: Vec<_> =
select_transactions(txs, max_gas, block_transaction_size_limit)
.take(transactions_limit as usize)
.collect();

for tx in sorted_txs.iter() {
guard.remove_committed_tx(&tx.id());
Expand Down
Loading

0 comments on commit 4a55b7d

Please sign in to comment.