From 4a55b7de69c235679637b59b7f24e18ffb905892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= <88321181+rafal-ch@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:43:02 +0200 Subject: [PATCH] Enforce the block size limit (#2195) Closes #2133 ## Linked Issues/PRs Built on top of: https://github.com/FuelLabs/fuel-core/pull/2188 ## Description This PR adds the handling and enforcement of then new consensus parameter added [here](https://github.com/FuelLabs/fuel-core/pull/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 --- CHANGELOG.md | 1 + .../tests/integration_tests.rs | 6 + .../src/service/adapters/executor.rs | 9 +- crates/services/executor/src/executor.rs | 41 ++- crates/services/executor/src/ports.rs | 4 +- crates/services/txpool/src/service.rs | 10 +- .../txpool/src/transaction_selector.rs | 245 +++++++++++++----- .../upgradable-executor/src/instance.rs | 15 +- .../wasm-executor/src/ext.rs | 13 +- .../wasm-executor/src/tx_source.rs | 4 +- crates/types/src/services/executor.rs | 2 + tests/tests/blocks.rs | 3 + tests/tests/tx.rs | 142 +++++++++- 13 files changed, 404 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcd90f58b8..c39d22022c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/bin/e2e-test-client/tests/integration_tests.rs b/bin/e2e-test-client/tests/integration_tests.rs index b325ee32763..794d54d2e95 100644 --- a/bin/e2e-test-client/tests/integration_tests.rs +++ b/bin/e2e-test-client/tests/integration_tests.rs @@ -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); diff --git a/crates/fuel-core/src/service/adapters/executor.rs b/crates/fuel-core/src/service/adapters/executor.rs index 9218965aee3..05d4400a1f4 100644 --- a/crates/fuel-core/src/service/adapters/executor.rs +++ b/crates/fuel-core/src/service/adapters/executor.rs @@ -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 { self.txpool - .select_transactions(gas_limit, transactions_limit) + .select_transactions( + gas_limit, + transactions_limit, + block_transaction_size_limit, + ) .into_iter() .map(|tx| { MaybeCheckedTransaction::CheckedTransaction( diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 7985753e06b..23017477481 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -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, @@ -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(), @@ -296,6 +298,7 @@ where skipped_transactions, coinbase, used_gas, + used_size, .. } = execution_data; @@ -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 { @@ -331,6 +334,7 @@ where let ExecutionData { coinbase, used_gas, + used_size, tx_status, events, changes, @@ -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 }; @@ -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 @@ -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(); @@ -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(); @@ -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) @@ -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 diff --git a/crates/services/executor/src/ports.rs b/crates/services/executor/src/ports.rs index 2ed7dba00f3..36f89bcbfdb 100644 --- a/crates/services/executor/src/ports.rs +++ b/crates/services/executor/src/ports.rs @@ -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; } diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs index d4899542bca..6bef4b021bc 100644 --- a/crates/services/txpool/src/service.rs +++ b/crates/services/txpool/src/service.rs @@ -41,7 +41,6 @@ use fuel_core_types::{ txpool::{ ArcPoolTx, InsertionResult, - PoolTransaction, TransactionStatus, }, }, @@ -396,13 +395,14 @@ impl &self, max_gas: u64, transactions_limit: u16, + block_transaction_size_limit: u32, ) -> Vec { let mut guard = self.txpool.lock(); let txs = guard.includable(); - let sorted_txs: Vec> = 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()); diff --git a/crates/services/txpool/src/transaction_selector.rs b/crates/services/txpool/src/transaction_selector.rs index 80158172714..6b3bec6f6aa 100644 --- a/crates/services/txpool/src/transaction_selector.rs +++ b/crates/services/txpool/src/transaction_selector.rs @@ -6,37 +6,46 @@ use fuel_core_types::{ // transaction selection could use a plugin based approach in the // future for block producers to customize block building (e.g. alternative priorities besides gas fees) -// Expects sorted by gas price transactions, highest first +// Expects sorted by tip, highest first pub fn select_transactions( includable_txs: impl Iterator, max_gas: u64, -) -> Vec { - // Select all txs that fit into the block, preferring ones with higher gas price. - // + block_transaction_size_limit: u32, +) -> impl Iterator { // Future improvements to this algorithm may take into account the parallel nature of // transactions to maximize throughput. - let mut used_block_space: Word = 0; + + let mut used_block_gas_space: Word = 0; + let mut used_block_size_space: u32 = 0; + // The type of the index for the transaction is `u16`, so we need to // limit it to `MAX` value minus 1(because of the `Mint` transaction). let takes_txs = u16::MAX - 1; - // Pick as many transactions as we can fit into the block (greedy) + // Pick as many transactions as we can fit into the block (greedy), + // respecting both gas and size limit includable_txs - .filter(|tx| { - let tx_block_space = tx.max_gas(); - if let Some(new_used_space) = used_block_space.checked_add(tx_block_space) { - if new_used_space <= max_gas { - used_block_space = new_used_space; - true - } else { - false - } + .filter(move |tx| { + let tx_block_gas_space = tx.max_gas(); + let tx_block_size_space = + tx.metered_bytes_size().try_into().unwrap_or(u32::MAX); + + let new_used_block_gas_space = + used_block_gas_space.saturating_add(tx_block_gas_space); + let new_used_block_size_space = + used_block_size_space.saturating_add(tx_block_size_space); + + if new_used_block_gas_space <= max_gas + && new_used_block_size_space <= block_transaction_size_limit + { + used_block_gas_space = new_used_block_gas_space; + used_block_size_space = new_used_block_size_space; + true } else { false } }) .take(takes_txs as usize) - .collect() } #[cfg(test)] @@ -49,6 +58,7 @@ mod tests { RegId, }, fuel_crypto::rand::{ + rngs::ThreadRng, thread_rng, Rng, }, @@ -69,6 +79,9 @@ mod tests { use super::*; + const UNLIMITED_SIZE: u32 = u32::MAX; + const UNLIMITED_GAS: u64 = u64::MAX; + #[derive(Debug, Clone, Copy, PartialEq)] struct TxGas { pub tip: u64, @@ -77,9 +90,28 @@ mod tests { /// A test helper that generates set of txs with given gas prices and limits and runs /// `select_transactions` against that, returning the list of selected gas price, limit pairs - fn make_txs_and_select(txs: &[TxGas], block_gas_limit: Word) -> Vec { + fn make_txs_and_select( + txs: &[TxGas], + block_gas_limit: Word, + block_transaction_size_limit: u32, + ) -> Vec { let mut rng = thread_rng(); + let txs = make_txs(txs, &mut rng); + select_transactions( + txs.into_iter(), + block_gas_limit, + block_transaction_size_limit, + ) + .map(|tx| TxGas { + limit: tx.script_gas_limit().unwrap_or_default(), + tip: tx.tip(), + }) + .collect() + } + + /// A test helper that generates set of txs with given gas prices and limits. + fn make_txs(txs: &[TxGas], rng: &mut ThreadRng) -> Vec> { let fee_params = FeeParameters::default() .with_gas_per_byte(0) .with_gas_price_factor(1); @@ -87,85 +119,96 @@ mod tests { let mut txs = txs .iter() .map(|tx_gas| { - let script = TransactionBuilder::script( + let mut script_builder = TransactionBuilder::script( vec![op::ret(RegId::ONE)].into_iter().collect(), vec![], - ) - .tip(tx_gas.tip) - .script_gas_limit(tx_gas.limit) - .add_unsigned_coin_input( - SecretKey::random(&mut rng), - rng.gen(), - 1_000_000, - Default::default(), - Default::default(), - ) - .add_output(Output::Change { - to: Default::default(), - amount: 0, - asset_id: Default::default(), - }) - .with_fee_params(fee_params) - .with_gas_costs(GasCosts::free()) + ); + + let script = script_builder + .tip(tx_gas.tip) + .script_gas_limit(tx_gas.limit) + .add_unsigned_coin_input( + SecretKey::random(rng), + rng.gen(), + 1_000_000, + Default::default(), + Default::default(), + ) + .add_output(Output::Change { + to: Default::default(), + amount: 0, + asset_id: Default::default(), + }) + .with_fee_params(fee_params) + .with_gas_costs(GasCosts::free()); + + // Ensure there is some randomness in the transaction size + let witnesses_count = rng.gen_range(0..=50); + for _ in 0..=witnesses_count { + script.add_witness( + std::iter::repeat(0) + .take(rng.gen_range(0..=100)) + .collect_vec() + .into(), + ); + } + // The block producer assumes transactions are already checked // so it doesn't need to compute valid sigs for tests - .finalize_checked_basic(Default::default()); - - PoolTransaction::Script(script, ConsensusParametersVersion::MIN) + PoolTransaction::Script( + script.finalize_checked_basic(Default::default()), + ConsensusParametersVersion::MIN, + ) }) .map(Arc::new) .collect::>(); txs.sort_by_key(|a| core::cmp::Reverse(a.tip())); - - select_transactions(txs.into_iter(), block_gas_limit) - .into_iter() - .map(|tx| TxGas { - limit: tx.script_gas_limit().unwrap_or_default(), - tip: tx.tip(), - }) - .collect() + txs } #[test] fn selector_works_with_empty_input() { - let selected = make_txs_and_select(&[], 1_000_000); + let selected = make_txs_and_select(&[], 1_000_000, 1_000_000); assert!(selected.is_empty()); } #[rstest::rstest] #[test] - #[case(999, vec![])] - #[case(1000, vec![TxGas { tip: 5, limit: 1000 }])] - #[case(2500, vec![TxGas { tip: 5, limit: 1000 }, TxGas { tip: 2, limit: 1000 }])] - #[case(4000, vec![ + #[case(999, UNLIMITED_SIZE, vec![])] + #[case(1000, UNLIMITED_SIZE, vec![TxGas { tip: 5, limit: 1000 }])] + #[case(2500, UNLIMITED_SIZE, vec![TxGas { tip: 5, limit: 1000 }, TxGas { tip: 2, limit: 1000 }])] + #[case(4000, UNLIMITED_SIZE, vec![ TxGas { tip: 5, limit: 1000 }, TxGas { tip: 4, limit: 3000 } ])] - #[case(5000, vec![ + #[case(5000, UNLIMITED_SIZE, vec![ TxGas { tip: 5, limit: 1000 }, TxGas { tip: 4, limit: 3000 }, TxGas { tip: 2, limit: 1000 }]) ] - #[case(6_000, vec![ + #[case(6_000, UNLIMITED_SIZE, vec![ TxGas { tip: 5, limit: 1000 }, TxGas { tip: 4, limit: 3000 }, TxGas { tip: 3, limit: 2000 } ])] - #[case(7_000, vec![ + #[case(7_000, UNLIMITED_SIZE, vec![ TxGas { tip: 5, limit: 1000 }, TxGas { tip: 4, limit: 3000 }, TxGas { tip: 3, limit: 2000 }, TxGas { tip: 2, limit: 1000 } ])] - #[case(8_000, vec![ + #[case(8_000, UNLIMITED_SIZE, vec![ TxGas { tip: 5, limit: 1000 }, TxGas { tip: 4, limit: 3000 }, TxGas { tip: 3, limit: 2000 }, TxGas { tip: 2, limit: 1000 }, TxGas { tip: 1, limit: 1000 } ])] - fn selector_prefers_highest_gas_txs_and_sorts( - #[case] selection_limit: u64, + + // TODO[RC]: Does it actually prefer highest gas? + fn selector_prefers_highest_gas_txs( + #[case] selection_gas_limit: u64, + #[case] selection_size_limit: u32, #[case] expected_txs: Vec, ) { #[rustfmt::skip] @@ -177,15 +220,16 @@ mod tests { TxGas { tip: 2, limit: 1000 }, ]; - let selected = make_txs_and_select(&original, selection_limit); + let selected = + make_txs_and_select(&original, selection_gas_limit, selection_size_limit); assert_eq!( expected_txs, selected, - "Wrong txs selected for max_gas: {selection_limit}" + "Wrong txs selected for max_gas: {selection_gas_limit}" ); } #[test] - fn selector_doesnt_exceed_max_gas_per_block() { + fn selector_does_not_exceed_max_gas_per_block() { #[rustfmt::skip] let original = [ TxGas { tip: 3, limit: 2000 }, @@ -198,11 +242,90 @@ mod tests { for k in 0..original.len() { for perm in original.into_iter().permutations(k) { for gas_limit in [999, 1000, 2000, 2500, 3000, 5000, 6000, 10_000] { - let selected = make_txs_and_select(perm.as_slice(), gas_limit); + let selected = + make_txs_and_select(perm.as_slice(), gas_limit, UNLIMITED_SIZE); let total_gas: Word = selected.iter().map(|g| g.limit).sum(); assert!(total_gas <= gas_limit); } } } } + + #[rstest::fixture] + pub fn block_transaction_size_limit_fixture() -> (Vec>, u32) { + const TOTAL_TXN_COUNT: usize = 1000; + + let mut rng = thread_rng(); + + // Create `TOTAL_TXN_COUNT` transactions with random tip. + let txs = make_txs( + std::iter::repeat_with(|| TxGas { + tip: rng.gen(), + limit: 1, + }) + .take(TOTAL_TXN_COUNT) + .collect::>() + .as_slice(), + &mut rng, + ); + let total_txs_size = txs + .iter() + .map(|tx| tx.metered_bytes_size().try_into().unwrap_or(u32::MAX)) + .sum(); + + (txs, total_txs_size) + } + + #[rstest::rstest] + fn unlimited_size_request_returns_all_transactions( + block_transaction_size_limit_fixture: (Vec>, u32), + ) { + let (txs, total_size) = block_transaction_size_limit_fixture; + + let selected = + select_transactions(txs.into_iter(), UNLIMITED_GAS, UNLIMITED_SIZE); + let selected_size: u32 = selected + .map(|tx| tx.metered_bytes_size().try_into().unwrap_or(u32::MAX)) + .sum(); + assert_eq!(selected_size, total_size); + } + + #[rstest::rstest] + fn selection_respects_specified_size( + block_transaction_size_limit_fixture: (Vec>, u32), + ) { + let (txs, total_size) = block_transaction_size_limit_fixture; + + let mut rng = thread_rng(); + for selection_size_limit in + std::iter::repeat_with(|| rng.gen_range(1..=total_size)).take(100) + { + let selected = select_transactions( + txs.clone().into_iter(), + UNLIMITED_GAS, + selection_size_limit, + ); + let selected_size: u32 = selected + .map(|tx| tx.metered_bytes_size().try_into().unwrap_or(u32::MAX)) + .sum(); + assert!(selected_size <= selection_size_limit); + } + } + + #[rstest::rstest] + fn selection_can_exactly_fit_the_requested_size( + block_transaction_size_limit_fixture: (Vec>, u32), + ) { + let (txs, _) = block_transaction_size_limit_fixture; + + let smallest_txn_size = txs + .iter() + .map(|tx| tx.metered_bytes_size().try_into().unwrap_or(u32::MAX)) + .sorted() + .next() + .expect("txs should not be empty"); + let selected = + select_transactions(txs.into_iter(), UNLIMITED_GAS, smallest_txn_size); + assert_eq!(selected.collect::>().len(), 1); + } } diff --git a/crates/services/upgradable-executor/src/instance.rs b/crates/services/upgradable-executor/src/instance.rs index 1dfe0e2f04a..6c2983b73fd 100644 --- a/crates/services/upgradable-executor/src/instance.rs +++ b/crates/services/upgradable-executor/src/instance.rs @@ -64,7 +64,7 @@ trait CallerHelper { source: Arc, gas_limit: u64, tx_number_limit: u16, - size_limit: u32, + block_transaction_size_limit: u32, ) -> anyhow::Result where Source: TransactionsSource; @@ -84,13 +84,13 @@ impl<'a> CallerHelper for Caller<'a, ExecutionState> { source: Arc, gas_limit: u64, tx_number_limit: u16, - size_limit: u32, + block_transaction_size_limit: u32, ) -> anyhow::Result where Source: TransactionsSource, { let txs: Vec<_> = source - .next(gas_limit, tx_number_limit, size_limit) + .next(gas_limit, tx_number_limit, block_transaction_size_limit) .into_iter() .map(|tx| match tx { MaybeCheckedTransaction::CheckedTransaction(checked, _) => { @@ -254,7 +254,7 @@ impl Instance { let closure = move |mut caller: Caller<'_, ExecutionState>, gas_limit: u64, tx_number_limit: u32, - size_limit: u32| + block_transaction_size_limit: u32| -> anyhow::Result { let Some(source) = source.clone() else { return Ok(0); @@ -264,7 +264,12 @@ impl Instance { anyhow::anyhow!("The number of transactions is more than `u16::MAX`: {e}") })?; - caller.peek_next_txs_bytes(source, gas_limit, tx_number_limit, size_limit) + caller.peek_next_txs_bytes( + source, + gas_limit, + tx_number_limit, + block_transaction_size_limit, + ) }; Func::wrap(&mut self.store, closure) diff --git a/crates/services/upgradable-executor/wasm-executor/src/ext.rs b/crates/services/upgradable-executor/wasm-executor/src/ext.rs index 99a5bbdfeee..a29da96958c 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/ext.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/ext.rs @@ -84,7 +84,7 @@ mod host { pub(crate) fn peek_next_txs_size( gas_limit: u64, tx_count_limit: u32, - size_limit: u32, + block_transaction_size_limit: u32, ) -> u32; } @@ -160,10 +160,15 @@ pub fn input(size: usize) -> anyhow::Result { pub fn next_transactions( gas_limit: u64, tx_count_limit: u16, - size_limit: u32, + block_transaction_size_limit: u32, ) -> anyhow::Result> { - let next_size = - unsafe { host::peek_next_txs_size(gas_limit, tx_count_limit as u32, size_limit) }; + let next_size = unsafe { + host::peek_next_txs_size( + gas_limit, + tx_count_limit as u32, + block_transaction_size_limit, + ) + }; if next_size == 0 { return Ok(Vec::new()); diff --git a/crates/services/upgradable-executor/wasm-executor/src/tx_source.rs b/crates/services/upgradable-executor/wasm-executor/src/tx_source.rs index 1cbdf64bde9..db9d8e8635b 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/tx_source.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/tx_source.rs @@ -17,9 +17,9 @@ impl TransactionsSource for WasmTxSource { &self, gas_limit: u64, tx_count_limit: u16, - size_limit: u32, + block_transaction_size_limit: u32, ) -> Vec { - ext::next_transactions(gas_limit, tx_count_limit, size_limit) + ext::next_transactions(gas_limit, tx_count_limit, block_transaction_size_limit) .expect("Failed to get next transactions") } } diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 4e2826eb2f0..abe0fb395bc 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -266,6 +266,8 @@ pub enum Error { FeeOverflow, #[display(fmt = "The computed gas caused an integer overflow")] GasOverflow, + #[display(fmt = "The computed transaction size caused an integer overflow")] + TxSizeOverflow, #[display(fmt = "The block is missing `Mint` transaction.")] MintMissing, #[display(fmt = "Found the second entry of the `Mint` transaction in the block.")] diff --git a/tests/tests/blocks.rs b/tests/tests/blocks.rs index 9b5411cd038..8449cf95ebb 100644 --- a/tests/tests/blocks.rs +++ b/tests/tests/blocks.rs @@ -445,6 +445,9 @@ mod full_block { let chain_config = local_node_config.snapshot_reader.chain_config().clone(); let mut consensus_parameters = chain_config.consensus_parameters; consensus_parameters.set_block_gas_limit(u64::MAX); + consensus_parameters + .set_block_transaction_size_limit(u64::MAX) + .expect("should be able to set the limit"); let snapshot_reader = local_node_config.snapshot_reader.with_chain_config( fuel_core::chain_config::ChainConfig { consensus_parameters, diff --git a/tests/tests/tx.rs b/tests/tests/tx.rs index 80838379eb9..58acbaf0dfe 100644 --- a/tests/tests/tx.rs +++ b/tests/tests/tx.rs @@ -1,5 +1,9 @@ use crate::helpers::TestContext; use fuel_core::{ + chain_config::{ + ChainConfig, + StateConfig, + }, schema::tx::receipt::all_receipts, service::{ Config, @@ -19,10 +23,14 @@ use fuel_core_poa::{ Trigger, }; use fuel_core_types::{ - fuel_asm::*, + fuel_asm::{ + op, + RegId, + }, fuel_crypto::SecretKey, fuel_tx::{ field::ReceiptsRoot, + Chargeable, *, }, fuel_types::ChainId, @@ -190,6 +198,138 @@ async fn dry_run_above_block_gas_limit() { } } +fn arb_large_script_tx( + max_fee_limit: Word, + size: usize, + rng: &mut R, +) -> Transaction { + let mut script: Vec<_> = std::iter::repeat(op::noop()).take(size).collect(); + script.push(op::ret(RegId::ONE)); + let script_bytes = script.iter().flat_map(|op| op.to_bytes()).collect(); + let mut builder = TransactionBuilder::script(script_bytes, vec![]); + let asset_id = *builder.get_params().base_asset_id(); + builder + .max_fee_limit(max_fee_limit) + .script_gas_limit(22430) + .add_unsigned_coin_input( + SecretKey::random(rng), + rng.gen(), + u32::MAX as u64, + asset_id, + Default::default(), + ) + .finalize() + .into() +} + +fn config_with_size_limit(block_transaction_size_limit: u32) -> Config { + let mut chain_config = ChainConfig::default(); + chain_config + .consensus_parameters + .set_block_transaction_size_limit(block_transaction_size_limit as u64) + .expect("should be able to set the limit"); + let state_config = StateConfig::default(); + + let mut config = Config::local_node_with_configs(chain_config, state_config); + config.block_production = Trigger::Never; + config +} + +#[tokio::test] +async fn transaction_selector_can_saturate_block_according_to_block_transaction_size_limit( +) { + let mut rng = rand::rngs::StdRng::from_entropy(); + + // Create 5 transactions of increasing sizes. + let arb_tx_count = 5; + let transactions: Vec<(_, _)> = (0..arb_tx_count) + .map(|i| { + let script_bytes_count = 10_000 + (i * 100); + let tx = + arb_large_script_tx(189028 + i as Word, script_bytes_count, &mut rng); + let size = tx + .as_script() + .expect("script tx expected") + .metered_bytes_size() as u32; + (tx, size) + }) + .collect(); + + // Run 5 cases. Each one will allow one more transaction to be included due to size. + for n in 1..=arb_tx_count { + // Calculate proper size limit for 'n' transactions + let block_transaction_size_limit: u32 = + transactions.iter().take(n).map(|(_, size)| size).sum(); + let config = config_with_size_limit(block_transaction_size_limit); + let srv = FuelService::new_node(config).await.unwrap(); + let client = FuelClient::from(srv.bound_address); + + // Submit all transactions + for (tx, _) in &transactions { + let status = client.submit(tx).await; + assert!(status.is_ok()) + } + + // Produce a block. + let block_height = client.produce_blocks(1, None).await.unwrap(); + + // Assert that only 'n' first transactions (+1 for mint) were included. + let block = client.block_by_height(block_height).await.unwrap().unwrap(); + assert_eq!(block.transactions.len(), n + 1); + let expected_ids: Vec<_> = transactions + .iter() + .take(n) + .map(|(tx, _)| tx.id(&ChainId::default())) + .collect(); + let actual_ids: Vec<_> = block.transactions.into_iter().take(n).collect(); + assert_eq!(expected_ids, actual_ids); + } +} + +#[tokio::test] +async fn transaction_selector_can_select_a_transaction_that_fits_the_block_size_limit() { + let mut rng = rand::rngs::StdRng::from_entropy(); + + // Create 5 transactions of decreasing sizes. + let arb_tx_count = 5; + let transactions: Vec<(_, _)> = (0..arb_tx_count) + .map(|i| { + let script_bytes_count = 10_000 + (i * 100); + let tx = + arb_large_script_tx(189028 + i as Word, script_bytes_count, &mut rng); + let size = tx + .as_script() + .expect("script tx expected") + .metered_bytes_size() as u32; + (tx, size) + }) + .rev() + .collect(); + + let (smallest_tx, smallest_size) = transactions.last().unwrap(); + + // Allow only the smallest transaction to fit. + let config = config_with_size_limit(*smallest_size); + let srv = FuelService::new_node(config).await.unwrap(); + let client = FuelClient::from(srv.bound_address); + + // Submit all transactions + for (tx, _) in &transactions { + let status = client.submit(tx).await; + assert!(status.is_ok()) + } + + // Produce a block. + let block_height = client.produce_blocks(1, None).await.unwrap(); + + // Assert that only the smallest transaction (+ mint) was included. + let block = client.block_by_height(block_height).await.unwrap().unwrap(); + assert_eq!(block.transactions.len(), 2); + let expected_id = smallest_tx.id(&ChainId::default()); + let actual_id = block.transactions.first().unwrap(); + assert_eq!(&expected_id, actual_id); +} + #[tokio::test] async fn submit() { let srv = FuelService::new_node(Config::local_node()).await.unwrap();