Integrate with V1 algo for tests #2469

MitchTurner · 2024-11-30T21:36:24Z

Linked Issues/PRs

Description

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests
The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

Rust SDK
Sway compiler
Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
Someone else?

…-of-order

…data_tests.rs Co-authored-by: Rafał Chabowski <[email protected]>

…-of-order

…-be-received-out-of-order

…-of-order

…to feature/init-task-for-v1-gas-price-service

…-of-order

rymnc · 2025-01-01T11:47:21Z

bin/fuel-core/src/cli/run.rs


    /// The minimum allowed gas price
    #[arg(long = "min-gas-price", default_value = "0", env)]
    pub min_gas_price: u64,

    /// The percentage threshold for gas price increase
    #[arg(long = "gas-price-threshold-percent", default_value = "50", env)]
-    pub gas_price_threshold_percent: u64,
+    pub gas_price_threshold_percent: u8,


this is a breaking change

crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs

rymnc · 2025-01-01T11:48:39Z

crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs

@@ -30,20 +30,27 @@ mod tests;
 #[derive(Debug)]
 /// Receives the next gas price algorithm via a shared `BlockGasPriceAlgo` instance
 pub struct FuelGasPriceProvider<A> {
+    // // Scale the gas price down by this factor, for e.g. Wei to Gwei
+    // scaling_factor: u64,
    algorithm: SharedGasPriceAlgo<A>,
 }

 impl<A> Clone for FuelGasPriceProvider<A> {


do we need this impl if we just restrict A: Clone ?

We don't really care if A: Clone though, this is more precise.

rymnc

Added changes in-line that need to be made before merging. there are a few questions as well.

crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs

crates/fuel-core/src/service/sub_services.rs

rymnc · 2025-01-01T11:50:55Z

crates/fuel-core/src/service/sub_services.rs

-        config.clone().into(),
+    tracing::debug!("da_committer_url: {:?}", config.da_committer_url);
+    let committer_api = BlockCommitterHttpApi::new(config.da_committer_url.clone());
+    let da_source = BlockCommitterDaBlockCosts::new(committer_api, None);


if we are using None here as the polling interval and not taking in the value from config we should omit it and just use the default in the da source.

I was able to remove the Option, but its not for poll interval, it's for latest_recorded_height, which we now set internally.

crates/fuel-core/src/service/sub_services.rs

tests/tests/gas_price.rs

rymnc · 2025-01-01T12:07:50Z

tests/tests/gas_price.rs

+        driver.kill().await
+    });
+
+    // // rollback 50 blocks


@MitchTurner should this be uncommented?

tests/tests/gas_price.rs

…-service

## Linked Issues/PRs  ## Description  In the case we are syncing from an existing network, it is possible for the L2 blocks to sync slower than the DA costs, in which case you will try to apply DA costs to an algorithm that is missing the corresponding unrecorded L2 blocks. This PR - introduces a `latest_l2_height` to the `DaSourceService` which will filter out any DA bundles that include L2 blocks after the current height - moves the recorded height concept into the `DaSourceService` where it probably should have been to begin with ## Checklist - [ ] Breaking changes are clearly marked as such in the PR description and changelog - [ ] New behavior is reflected in tests - [ ] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [ ] I have reviewed the code myself - [ ] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: Aaryamann Challani <[email protected]>

## Linked Issues/PRs  - #2469 ## Description  - Adds a data retrieval service that fetches L2 and L1 cost data for gas price v1 simulations ## Checklist - [x] Breaking changes are clearly marked as such in the PR description and changelog - [x] New behavior is reflected in tests - [x] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [x] I have reviewed the code myself - [x] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: rymnc <[email protected]> Co-authored-by: Mitchell Turner <[email protected]>

…atility (#2541) ## Linked Issues/PRs  - #2469 ## Description  Adds a new config param `max_da_gas_price`, which `new_scaled_da_gas_price` depends upon while mutating it. ## Checklist - [ ] Breaking changes are clearly marked as such in the PR description and changelog - [ ] New behavior is reflected in tests - [ ] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [ ] I have reviewed the code myself - [ ] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: Brandon Kite <[email protected]> Co-authored-by: Mitchell Turner <[email protected]>

MitchTurner and others added 30 commits October 30, 2024 17:26

Remove height parameter, change tests

448d8c2

Modify to take vec instead of range

c5866a3

Add tests to make sure it is sane

c54f098

Rename variable

fcf4477

Fix analyzer

159d5b4

Rename some params

ba080ff

Use slice instead of vec

d2b3c28

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

128a9ba

…-of-order

Cleanup code a bit

5f19932

Update crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_…

922f3e0

…data_tests.rs Co-authored-by: Rafał Chabowski <[email protected]>

Pick nit

d7e9bdd

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

12c517e

…-of-order

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

f8dca06

…-of-order

Optimize projected calc by caching running total

2bb86fe

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

60771e9

…-of-order

Fix analyzer

03da963

Simplify metadata

ea14e6f

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

b615b33

…-of-order

Merge remote-tracking branch 'origin' into feature/allow-DA-blocks-to…

cfd1a71

…-be-received-out-of-order

Resolve logical conflicts from merge

d983891

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

4c2b844

…-of-order

WIP

1836656

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

48fe978

…-of-order

Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…

18fefb7

…to feature/init-task-for-v1-gas-price-service

Get compiling

b15a93f

Start adding tests

279191c

Get existing tests passing

63b0266

WIP

477c731

Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…

b578a8e

…-of-order

Ignore entire batches with blocks not in the unrecorded_blocks

7ca55c7

rymnc force-pushed the chore/add-tests-for-v1-gas-service branch 3 times, most recently from 8d219d2 to ff8243c Compare December 25, 2024 18:37

test: produce blocks so that the metadata is upgraded

eb202cf

rymnc force-pushed the chore/add-tests-for-v1-gas-service branch from ff8243c to eb202cf Compare December 25, 2024 18:55

Add tests for cost recording

c29dae7

rymnc reviewed Jan 1, 2025

View reviewed changes

crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs Outdated Show resolved Hide resolved

rymnc reviewed Jan 1, 2025

View reviewed changes

crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs Outdated Show resolved Hide resolved

rymnc reviewed Jan 1, 2025

View reviewed changes

MitchTurner and others added 8 commits January 6, 2025 11:14

Merge remote-tracking branch 'origin' into chore/add-tests-for-v1-gas…

2469fb4

…-service

Merge branch 'master' into chore/add-tests-for-v1-gas-service

2217670

Fix test

b380e99

Cleanup mess from debugging

bc983cb

Merge branch 'master' into chore/add-tests-for-v1-gas-service

4ed9ec1

Remove unused imports, reconstitute test

4a21da4

Lint toml file

a7eaa32

This was referenced Jan 8, 2025

Create new simulation using the actual gas price service #2538

Draft

Gas price simulation data retriever #2533

Merged

chore(gas_price_service_v1): strictly ensure last_recorded_height is set, to avoid initial poll of da source #2523

Open

rymnc mentioned this pull request Jan 8, 2025

feat(gas_price_service_v1): include max_da_gas_price to control volatility #2541

Merged

9 tasks

rymnc force-pushed the chore/add-tests-for-v1-gas-service branch from dd287e8 to 31fd51f Compare January 9, 2025 04:38

Merge branch 'master' into chore/add-tests-for-v1-gas-service

8d12301

rymnc mentioned this pull request Jan 9, 2025

fix(nextest): override timeout for slow ci tests #2542

Closed

9 tasks

rymnc and others added 2 commits January 9, 2025 14:20

fix: default da poll interval to be higher than 10ms

6ea6a5c

rymnc added the release label Jan 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate with V1 algo for tests #2469

Integrate with V1 algo for tests #2469

MitchTurner commented Nov 30, 2024 •

edited

Loading

rymnc Jan 1, 2025

rymnc Jan 1, 2025

MitchTurner Jan 6, 2025

rymnc left a comment

rymnc Jan 1, 2025

MitchTurner Jan 6, 2025

rymnc Jan 1, 2025

Integrate with V1 algo for tests #2469

Are you sure you want to change the base?

Integrate with V1 algo for tests #2469

Conversation

MitchTurner commented Nov 30, 2024 • edited Loading

Linked Issues/PRs

Description

Checklist

Before requesting review

After merging, notify other teams

rymnc Jan 1, 2025

Choose a reason for hiding this comment

rymnc Jan 1, 2025

Choose a reason for hiding this comment

MitchTurner Jan 6, 2025

Choose a reason for hiding this comment

rymnc left a comment

Choose a reason for hiding this comment

rymnc Jan 1, 2025

Choose a reason for hiding this comment

MitchTurner Jan 6, 2025

Choose a reason for hiding this comment

rymnc Jan 1, 2025

Choose a reason for hiding this comment

MitchTurner commented Nov 30, 2024 •

edited

Loading