Enforce the block size limit #2195

rafal-ch · 2024-09-13T13:37:40Z

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.

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

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests

Before requesting review

I have reviewed the code myself

…nd `enum MaybeCheckedTransaction`

…onsensus_parameter

crates/services/upgradable-executor/wasm-executor/src/ext.rs

…k_size_consensus_parameter

…the_block_size_limit

…block_size_limit

xgreenx · 2024-09-20T14:21:14Z

Cargo.lock

@@ -1252,9 +1252,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b"

 [[package]]
 name = "bytes"
-version = "1.7.1"
+version = "1.7.2"


Usually, we run cargo update in a separate PR or during release. Could you revert this changes(unless you need them), please?=)

Changes to Cargo.lock reverted in 9bdea6d

crates/services/txpool/src/transaction_selector.rs

xgreenx · 2024-09-20T16:40:17Z

crates/services/txpool/src/transaction_selector.rs

+
+            let maybe_new_used_block_gas_space =
+                used_block_gas_space.checked_add(tx_block_gas_space);
+            let maybe_new_used_block_size_space =
+                used_block_size_space.checked_add(tx_block_size_space);


I think we can use saturating_add isntead

You're right. Updated in 36ab508

xgreenx · 2024-09-20T16:46:58Z

tests/tests/tx.rs

+            assert!(status.is_ok())
+        }
+
+        let _ = tokio::time::sleep(Duration::from_millis(10)).await;


Why do we need to wait? You can use the srv.shared_state.txpool.insert function if you want to be sure that the transaction is inserted. But client.submit(tx).await should work as well

It's a leftover from my initial test attempts. Removed in 15a2cf7.

cc: @MitchTurner - you may want to have a look at this.

xgreenx · 2024-09-20T16:47:44Z

tests/tests/tx.rs

+        assert!(status.is_ok())
+    }
+
+    let _ = tokio::time::sleep(Duration::from_millis(10)).await;


The same here, do we need to wait if you use manual block production?

See: #2195 (comment)

…mit' into 2133_enforce_the_block_size_limit

acerone85

Had a look at the code and the tests.

It looks like the implementation for OnceTransactionsSource is still ignoring the block_txs_size parameter (

fuel-core/crates/services/executor/src/executor.rs

Line 192 in 79ca0d0

impl TransactionsSource for OnceTransactionsSource {

), but this could be done as a follow up.

LGTM.

rafal-ch · 2024-09-25T13:47:52Z

It looks like the implementation for OnceTransactionsSource is still ignoring the block_txs_size parameter

Summoning @xgreenx for a comment.

According to my current understanding, we don't necessarily need the support for block_txs_size in the OnceTransactionsSource.

…block_size_limit

…mit' into 2133_enforce_the_block_size_limit

xgreenx · 2024-09-25T15:11:12Z

According to my current understanding, we don't necessarily need the support for block_txs_size in the OnceTransactionsSource.

Yes, OnceTransactionsSource shouldn't event exist in production, it is just a hack for dry run=)

xgreenx · 2024-09-25T15:13:07Z

crates/services/executor/src/executor.rs

@@ -194,7 +194,7 @@ impl TransactionsSource for OnceTransactionsSource {
        &self,
        _: u64,
        transactions_limit: u16,
-        _: u32,


I missed that, but the block size should be u32.

Updated in e748837

xgreenx · 2024-09-25T15:13:49Z

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: u64,


It is a breaking change

Updated in e748837

…mit' into 2133_enforce_the_block_size_limit

@rymnc

## Version v0.37.0 ### Added - [1609](#1609): Add DA compression support. Compressed blocks are stored in the offchain database when blocks are produced, and can be fetched using the GraphQL API. - [2290](#2290): Added a new CLI argument `--graphql-max-directives`. The default value is `10`. - [2195](#2195): Added enforcement of the limit on the size of the L2 transactions per block according to the `block_transaction_size_limit` parameter. - [2131](#2131): Add flow in TxPool in order to ask to newly connected peers to share their transaction pool - [2182](#2151): Limit number of transactions that can be fetched via TxSource::next - [2189](#2151): Select next DA height to never include more than u16::MAX -1 transactions from L1. - [2162](#2162): Pool structure with dependencies, etc.. for the next transaction pool module. Also adds insertion/verification process in PoolV2 and tests refactoring - [2265](#2265): Integrate Block Committer API for DA Block Costs. - [2280](#2280): Allow comma separated relayer addresses in cli - [2299](#2299): Support blobs in the predicates. - [2300](#2300): Added new function to `fuel-core-client` for checking whether a blob exists. ### Changed #### Breaking - [2299](#2299): Anyone who wants to participate in the transaction broadcasting via p2p must upgrade to support new predicates on the TxPool level. - [2299](#2299): Upgraded `fuel-vm` to `0.58.0`. More information in the [release](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.58.0). - [2276](#2276): Changed how complexity for blocks is calculated. The default complexity now is 80_000. All queries that somehow touch the block header now are more expensive. - [2290](#2290): Added a new GraphQL limit on number of `directives`. The default value is `10`. - [2206](#2206): Use timestamp of last block when dry running transactions. - [2153](#2153): Updated default gas costs for the local testnet configuration to match `fuel-core 0.35.0`. ## What's Changed * fix: use core-test.fuellabs.net for dnsaddr resolution by @rymnc in #2214 * Removed state transition bytecode from the local testnet by @xgreenx in #2215 * Send whole transaction pool upon subscription to gossip by @AurelienFT in #2131 * Update default gas costs based on 0.35.0 benchmarks by @xgreenx in #2153 * feat: Use timestamp of last block when dry running transactions by @netrome in #2206 * fix(dnsaddr_resolution): use fqdn separator to prevent suffixing by dns resolvers by @rymnc in #2222 * TransactionSource: specify maximum number of transactions to be fetched by @acerone85 in #2182 * Implement worst case scenario for price algorithm v1 by @rafal-ch in #2219 * chore(gas_price_service): define port for L2 data by @rymnc in #2224 * Block producer selects da height to never exceed u64::MAX - 1 transactions from L1 by @acerone85 in #2189 * Weekly `cargo update` by @github-actions in #2236 * Use fees to calculate DA reward and avoid issues with Gwei/Wei conversions by @MitchTurner in #2229 * Protect against passing `i128::MIN` to `abs()` which causes overflow by @rafal-ch in #2241 * Acquire `da_finalization_period` from the command line by @rafal-ch in #2240 * Executor: test Tx_count limit with incorrect tx source by @acerone85 in #2242 * Minor updates to docs + a few typos fixed by @rafal-ch in #2250 * chore(gas_price_service): move algorithm_updater to fuel-core-gas-price-service by @rymnc in #2246 * Use single heavy input in the `transaction_throughput.rs` benchmarks by @xgreenx in #2205 * Enforce the block size limit by @rafal-ch in #2195 * feat: build ARM and AMD in parallel by @mchristopher in #2130 * Weekly `cargo update` by @github-actions in #2268 * chore(gas_price_service): split into v0 and v1 and squash FuelGasPriceUpdater type into GasPriceService by @rymnc in #2256 * feat(gas_price_service): update block committer da source with established contract by @rymnc in #2265 * Use bytes from `unrecorded_blocks` rather from the block from DA by @MitchTurner in #2252 * TxPool v2 General architecture by @AurelienFT in #2162 * Add value delimiter and tests args by @AurelienFT in #2280 * fix(da_block_costs): remove Arc<Mutex<>> on shared_state and expose channel by @rymnc in #2278 * fix(combined_database): syncing auxiliary databases on startup with custom behaviour by @rymnc in #2272 * fix: Manually encode Authorization header for eventsource_client by @Br1ght0ne in #2284 * Address `async-graphql` vulnerability by @MitchTurner in #2290 * Update the WASM compatibility tests for `0.36` release by @rafal-ch in #2271 * DA compression by @Dentosal in #1609 * Use different port for every version compatibility test by @rafal-ch in #2301 * Fix block query complexity by @xgreenx in #2297 * Support blobs in predicates by @Voxelot in #2299 **Full Changelog**: v0.36.0...v0.37.0

rafal-ch added 14 commits September 11, 2024 17:51

Add support for ConsensusParametersVersion::V2

fa1288b

Update CHANGELOG.md

0c65bbe

Remove the temporary comment

25432d2

Fix imports in tests

0a0b6c2

#[allow(clippy::large_enum_variant)] for enum FakeProducedBlock a…

3068d2f

…nd `enum MaybeCheckedTransaction`

Update expected GraphQL schema

0656491

Update state_transition_bytecode.wasm

a1ea188

Thread the block_transaction_size_limit parameter across the system

345a03f

Add used_size to ExecutionData

256dfb7

Remove stray comment

960745c

Transaction selector respects block size limit

ecb57d8

select_transactions() does not need to collect()

a75d865

Split test cases using rstest

6e2bf19

Merge remote-tracking branch 'upstream/master' into 2133_block_size_c…

7c14352

…onsensus_parameter

rafal-ch commented Sep 13, 2024

View reviewed changes

crates/services/upgradable-executor/wasm-executor/src/ext.rs Outdated Show resolved Hide resolved

xgreenx and others added 11 commits September 15, 2024 11:00

Applying several breaking changes to the WASM interface from backlog

c002c0c

Make CI happy

2cfcaa5

Merge branch 'master' into feature/wasm-interface-breaking-changes

af91864

Remove timeout error because bytecode is not compiled

8032b7a

Use always compiled WASM bytecode for local testnet

94851b8

Merge branch 'feature/wasm-interface-breaking-changes' into 2133_bloc…

b7d8da6

…k_size_consensus_parameter

Merge branch '2133_block_size_consensus_parameter' into 2133_enforce_…

96b4c7c

…the_block_size_limit

Update patch for fuel-vm to master branch

c85879b

Merge branch '2133_block_size_consensus_parameter' into 2133_enforce_…

309d1af

…the_block_size_limit

Add integration test verifying the block size limit enforcement

03004ac

Add more integration tests for block transaction size limit

7851fb1

rafal-ch mentioned this pull request Sep 17, 2024

Add support for ConsensusParametersVersion::V2 #2188

Merged

9 tasks

Base automatically changed from 2133_block_size_consensus_parameter to feature/wasm-interface-breaking-changes September 17, 2024 13:58

Base automatically changed from feature/wasm-interface-breaking-changes to master September 17, 2024 14:50

Merge remote-tracking branch 'upstream/master' into 2133_enforce_the_…

445a51c

…block_size_limit

xgreenx reviewed Sep 20, 2024

View reviewed changes

acerone85 mentioned this pull request Sep 20, 2024

Introduce Limiter abstraction for Executor parameters #2234

Closed

rafal-ch added 4 commits September 21, 2024 16:14

Revert changes to Cargo.lock

9bdea6d

Simplify code of select_transactions() by using saturating_add()

36ab508

Remove superfluous calls to tokio::time::sleep in tests

15a2cf7

Merge remote-tracking branch 'upstream/2133_enforce_the_block_size_li…

26fd531

…mit' into 2133_enforce_the_block_size_limit

rafal-ch mentioned this pull request Sep 21, 2024

Improve transaction selector by adding early exit when block is already filled #2235

Closed

Merge branch 'master' into 2133_enforce_the_block_size_limit

251c01d

xgreenx previously approved these changes Sep 24, 2024

View reviewed changes

rafal-ch requested a review from a team September 25, 2024 09:55

Merge branch 'master' into 2133_enforce_the_block_size_limit

be29818

acerone85 previously approved these changes Sep 25, 2024

View reviewed changes

rafal-ch added 2 commits September 25, 2024 15:56

Merge remote-tracking branch 'upstream/master' into 2133_enforce_the_…

92da3b0

…block_size_limit

Merge remote-tracking branch 'upstream/2133_enforce_the_block_size_li…

70e9178

…mit' into 2133_enforce_the_block_size_limit

rafal-ch dismissed stale reviews from acerone85 and xgreenx via 70e9178 September 25, 2024 13:56

Merge branch 'master' into 2133_enforce_the_block_size_limit

ef22a3b

xgreenx requested changes Sep 25, 2024

View reviewed changes

rafal-ch and others added 3 commits September 26, 2024 16:21

Change block_transaction_size_limit to be u32 instead of u64

e748837

Merge remote-tracking branch 'upstream/2133_enforce_the_block_size_li…

0872751

…mit' into 2133_enforce_the_block_size_limit

Merge branch 'master' into 2133_enforce_the_block_size_limit

c226aee

xgreenx approved these changes Sep 26, 2024

View reviewed changes

acerone85 approved these changes Sep 27, 2024

View reviewed changes

xgreenx merged commit 4a55b7d into master Sep 27, 2024
36 checks passed

xgreenx deleted the 2133_enforce_the_block_size_limit branch September 27, 2024 08:43

xgreenx mentioned this pull request Oct 5, 2024

Release v0.37.0 #2302

Merged

rafal-ch mentioned this pull request Oct 29, 2024

Versioned Storage for Modifications History #2233

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enforce the block size limit #2195

Enforce the block size limit #2195

rafal-ch commented Sep 13, 2024 •

edited

Loading

xgreenx Sep 20, 2024

rafal-ch Sep 21, 2024

xgreenx Sep 20, 2024

rafal-ch Sep 21, 2024 •

edited

Loading

xgreenx Sep 20, 2024

rafal-ch Sep 21, 2024

xgreenx Sep 20, 2024

rafal-ch Sep 21, 2024

acerone85 left a comment

rafal-ch commented Sep 25, 2024

xgreenx commented Sep 25, 2024

xgreenx Sep 25, 2024

rafal-ch Sep 26, 2024

xgreenx Sep 25, 2024

rafal-ch Sep 26, 2024

Enforce the block size limit #2195

Enforce the block size limit #2195

Conversation

rafal-ch commented Sep 13, 2024 • edited Loading

Linked Issues/PRs

Description

Checklist

Before requesting review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafal-ch Sep 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acerone85 left a comment

Choose a reason for hiding this comment

rafal-ch commented Sep 25, 2024

xgreenx commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafal-ch commented Sep 13, 2024 •

edited

Loading

rafal-ch Sep 21, 2024 •

edited

Loading