Acquire `da_finalization_period` from the command line #2240

rafal-ch · 2024-09-23T13:15:47Z

Linked Issues

Description

This PR extends the set of accepted command line parameters. The user is now able to provide the DA update time via the new --da-finalization-period argument.

It also fixes the issue with unrecorded_blocks collection which was not saturated with blocks.

Checklist

New behavior is reflected in tests

Before requesting review

I have reviewed the code myself

crates/fuel-gas-price-algorithm/gas-price-analysis/src/main.rs

MitchTurner · 2024-09-24T15:12:15Z

crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs

        fullness_and_bytes: &Vec<(u64, u64)>,
    ) -> Vec<Option<Vec<RecordedBlock>>> {
+        let nones = std::iter::repeat(None).take(da_finalization_rate);


Suggested change

let nones = std::iter::repeat(None).take(da_finalization_rate);

let l2_blocks_with_no_da_blocks = std::iter::repeat(None).take(da_finalization_rate);

Updated in ff0db6e

MitchTurner · 2024-09-24T15:12:42Z

crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs

@@ -207,7 +213,7 @@ impl Simulator {
                    }
                },
            );
-        da_blocks
+        nones.chain(da_blocks).collect()


Suggested change

nones.chain(da_blocks).collect()

l2_blocks_with_no_da_blocks.chain(da_blocks).collect()

Updated in ff0db6e

Co-authored-by: Mitchell Turner <[email protected]>

xgreenx · 2024-09-25T10:14:26Z

crates/fuel-gas-price-algorithm/src/v1.rs

+
+            // metadata
+            self.unrecorded_blocks.push(BlockBytes {
+                height,
+                block_bytes,
+            });


I don't see how it is related to the change in the gas price analyser

True, it's not related. It was something I discovered when working on this issue and decided to add a fix in the same PR.

Even though the PR description mentions this change, I should probably issue a separate ticket and create a different PR.

Would you like me to clean-up the situation for this case or you think we can merge as is?

If @MitchTurner is okay of merging it in this PR, I'm also okay=)

xgreenx · 2024-09-25T10:20:09Z

crates/fuel-gas-price-algorithm/src/v1.rs

+
+            // metadata
+            self.unrecorded_blocks.push(BlockBytes {
+                height,
+                block_bytes,
+            });


If @MitchTurner is okay of merging it in this PR, I'm also okay=)

@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 2 commits September 23, 2024 15:08

Acquire update_period from the command line

540b7fe

Add comment for --update_period param

cbdbce0

rafal-ch added the no changelog Skip the CI check of the changelog modification label Sep 23, 2024

rafal-ch self-assigned this Sep 23, 2024

Use da_finalization_period instead of update_period

3e3362a

rafal-ch changed the title ~~Acquire update_period from the command line~~ Acquire da_finalization_period from the command line Sep 23, 2024

rafal-ch added 2 commits September 24, 2024 14:51

Introduce a delay in da blocks

6394b23

Update unrecorded_blocks collection

0c24aa8

rafal-ch marked this pull request as ready for review September 24, 2024 13:58

rafal-ch requested review from xgreenx, Dentosal and MitchTurner as code owners September 24, 2024 13:58

rafal-ch mentioned this pull request Sep 24, 2024

gas price analysis: Investigate spikes in "Actual profit" when calculating with small da_finalization_period #2247

Open

Merge branch 'master' into 2218_parametrized_da_finalization_times

c272525

MitchTurner reviewed Sep 24, 2024

View reviewed changes

rafal-ch and others added 2 commits September 25, 2024 09:51

Update comment for Arg::da_finalization_period

a508273

Co-authored-by: Mitchell Turner <[email protected]>

Rename nones to l2_blocks_with_no_da_blocks

ff0db6e

MitchTurner approved these changes Sep 25, 2024

View reviewed changes

rafal-ch requested a review from a team September 25, 2024 10:08

xgreenx reviewed Sep 25, 2024

View reviewed changes

xgreenx approved these changes Sep 25, 2024

View reviewed changes

rafal-ch enabled auto-merge (squash) September 25, 2024 10:40

Merge branch 'master' into 2218_parametrized_da_finalization_times

c39d350

rafal-ch merged commit 4c5722e into master Sep 25, 2024
33 of 49 checks passed

rafal-ch deleted the 2218_parametrized_da_finalization_times branch September 25, 2024 10:56

MitchTurner mentioned this pull request Sep 27, 2024

Zeroing of our reward and costs doesn't work with Simulation #2264

Closed

xgreenx mentioned this pull request Oct 5, 2024

Release v0.37.0 #2302

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Acquire `da_finalization_period` from the command line #2240

Acquire `da_finalization_period` from the command line #2240

rafal-ch commented Sep 23, 2024 •

edited

Loading

MitchTurner Sep 24, 2024

rafal-ch Sep 25, 2024

MitchTurner Sep 24, 2024

rafal-ch Sep 25, 2024

xgreenx Sep 25, 2024

rafal-ch Sep 25, 2024

xgreenx Sep 25, 2024

xgreenx Sep 25, 2024

	let nones = std::iter::repeat(None).take(da_finalization_rate);
	let l2_blocks_with_no_da_blocks = std::iter::repeat(None).take(da_finalization_rate);

	nones.chain(da_blocks).collect()
	l2_blocks_with_no_da_blocks.chain(da_blocks).collect()

Acquire da_finalization_period from the command line #2240

Acquire da_finalization_period from the command line #2240

Conversation

rafal-ch commented Sep 23, 2024 • edited Loading

Linked Issues

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

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

Acquire `da_finalization_period` from the command line #2240

Acquire `da_finalization_period` from the command line #2240

rafal-ch commented Sep 23, 2024 •

edited

Loading