Add Gas Price Updater Service #1938

MitchTurner · 2024-06-05T20:46:23Z

Closes: #1956

This is a subtask of #1624

In this PR we add the generic service that will post an algorithm for the providers to use. Not bothering with a real algorithm, that will be implemented later. For now, just show that the provider can get the value generated by the service.

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
Swhttps://github.com/FuelLabs/fuel-core/issues/1956ay compiler
Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
Someone else?

Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>

xgreenx · 2024-06-11T08:08:28Z

CHANGELOG.md

@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [#1888](https://github.com/FuelLabs/fuel-core/pull/1888): Upgraded `fuel-vm` to `0.51.0`. See [release](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.51.0) for more information.

 ### Added
+- [#1889](https://github.com/FuelLabs/fuel-core/pull/1889): Add new `FuelGasPriceProvider` that receives the gas price algorithm from a `GasPriceService`


We need to move this into unreleased section

xgreenx · 2024-06-11T08:17:12Z

crates/services/gas_price_service/src/lib.rs

+        Self(Arc::new(RwLock::new(algo)))
+    }
+
+    pub async fn read(&self) -> RwLockReadGuard<A> {


It is a bad idea to return RwLockReadGuard from the service. This means anyone outside the service can lock it. Could you add two separate methods like execute and estimate?

Yeah. You're right. I don't like it just for code cleanliness either.

To add an execute and estimate I'll have to constrain A somehow with a new trait.

The other approach would be just make A Clone, and clone it out of the lock.

I think it is okay to add constrains for the algorithm=) Gas provider expects that algorithm will provide next gas price=D

xgreenx · 2024-06-11T08:23:26Z

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

+    A: GasPriceEstimate + Send + Sync,
+{
+    async fn worst_case_gas_price(&self, height: BlockHeight) -> u64 {
+        self.algorithm.read().await.estimate(height)


The estimate naming is too vague. It would be nice to include the idea that it is a prediction of the price change for some period.

I changed it to match the provider: worst_case_gas_price.

xgreenx · 2024-06-11T08:24:11Z

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

+where
+    A: GasPriceAlgorithm + Send + Sync,
+{
+    async fn execute_algorithm(&self, block_bytes: u64) -> u64 {


[nit] next_gas_price sounds more clear to me=)

xgreenx · 2024-06-11T08:25:28Z

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

+pub struct BlockFullness {
+    used: u64,
+    capacity: u64,
+}
+
+impl BlockFullness {
+    pub fn new(used: u64, capacity: u64) -> Self {
+        Self { used, capacity }
+    }
+
+    pub fn used(&self) -> u64 {
+        self.used
+    }
+
+    pub fn capacity(&self) -> u64 {
+        self.capacity
+    }
+}


It seems is not used anywhere

crates/services/gas_price_service/src/lib.rs

xgreenx · 2024-06-11T08:34:17Z

crates/services/gas_price_service/src/lib.rs

+    /// The algorithm that can be used in the next block
+    next_block_algorithm: SharedGasPriceAlgo<A>,
+    /// The code that is run to update your specific algorithm
+    update_algorithm: U,


For me, it looks like the updater and algorithm should be one entity since only the algorithm knows how it works inside and what data it requires.

But we can leave it for now as it is and see how it will look like with a nonstatic algorithm.

I'm leaning to this approach so that the consumer of the algo doesn't need to have all the generics for the updater to read the values.

Could also use dyn but that's gross :)

xgreenx · 2024-06-11T08:35:16Z

crates/services/gas_price_service/src/lib.rs

+        let (watch_sender, watch_receiver) = tokio::sync::watch::channel(State::Started);
+        let watcher = StateWatcher::from(watch_receiver);


You can re-use helper function

xgreenx · 2024-06-11T08:38:02Z

crates/services/producer/src/block_producer.rs

+        let gas_price = if let Some(inner) = gas_price {
+            inner
+        } else {
+            self.gas_price_provider.gas_price(max_block_bytes).await?


It is better to move all calculations here=) Could you move the duplicated code into a small function that we could reuse in both places, please?=)

Plus, maybe for dry_run, we could just use the previous gas price without.

xgreenx · 2024-06-11T08:38:31Z

crates/services/txpool/src/ports.rs

+    async fn gas_price(
+        &self,
+        block_bytes: u64,
+    ) -> fuel_core_types::services::txpool::Result<GasPrice>;


Suggested change

) -> fuel_core_types::services::txpool::Result<GasPrice>;

) -> TxPoolResult<GasPrice>;

xgreenx

LGTM!=)

crates/services/txpool/src/ports.rs

crates/services/producer/src/block_producer.rs

@Dentosal

## Version v0.29.0 ### Added - [#1889](#1889): Add new `FuelGasPriceProvider` that receives the gas price algorithm from a `GasPriceService` ### Changed - [#1942](#1942): Sequential relayer's commits. - [#1952](#1952): Change tip sorting to ratio between tip and max gas sorting in txpool - [#1960](#1960): Update fuel-vm to v0.53.0. ### Fixed - [#1950](#1950): Fix cursor `BlockHeight` encoding in `SortedTXCursor` ## What's Changed * Fix code coverage compilation and tests by @Dentosal in #1943 * Weekly `cargo update` by @github-actions in #1949 * Fix cursor block height decoding in SortedTXCursor by @AurelienFT in #1950 * Sequential relayer's commits by @xgreenx in #1942 * Add Gas Price Updater Service by @MitchTurner in #1938 * Change tip sorting to ratio between tip and max gas sorting in txpool by @AurelienFT in #1952 * deps(client): update eventsource-client to fix CVE(s) by @Br1ght0ne in #1954 * Update fuel-vm to v0.53.0 by @Dentosal in #1960 ## New Contributors * @AurelienFT made their first contribution in #1950 **Full Changelog**: v0.28.0...v0.29.0

@Dentosal

## Version v0.29.0 ### Added - [#1889](FuelLabs/fuel-core#1889): Add new `FuelGasPriceProvider` that receives the gas price algorithm from a `GasPriceService` ### Changed - [#1942](FuelLabs/fuel-core#1942): Sequential relayer's commits. - [#1952](FuelLabs/fuel-core#1952): Change tip sorting to ratio between tip and max gas sorting in txpool - [#1960](FuelLabs/fuel-core#1960): Update fuel-vm to v0.53.0. ### Fixed - [#1950](FuelLabs/fuel-core#1950): Fix cursor `BlockHeight` encoding in `SortedTXCursor` ## What's Changed * Fix code coverage compilation and tests by @Dentosal in FuelLabs/fuel-core#1943 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#1949 * Fix cursor block height decoding in SortedTXCursor by @AurelienFT in FuelLabs/fuel-core#1950 * Sequential relayer's commits by @xgreenx in FuelLabs/fuel-core#1942 * Add Gas Price Updater Service by @MitchTurner in FuelLabs/fuel-core#1938 * Change tip sorting to ratio between tip and max gas sorting in txpool by @AurelienFT in FuelLabs/fuel-core#1952 * deps(client): update eventsource-client to fix CVE(s) by @Br1ght0ne in FuelLabs/fuel-core#1954 * Update fuel-vm to v0.53.0 by @Dentosal in FuelLabs/fuel-core#1960 ## New Contributors * @AurelienFT made their first contribution in FuelLabs/fuel-core#1950 **Full Changelog**: FuelLabs/fuel-core@v0.28.0...v0.29.0

MitchTurner added 30 commits May 8, 2024 14:22

Introduce type with dummy test

378dae8

Replace test with historical gas price test

cbebddc

Get test passing

bb24903

Refactor

fb58594

Add more logic to when it looks up historical prices, add tests

dc193d5

Update CHANGELOG

4af3946

Remove extra traits

398884e

Rethink ports

96f20d9

Write test to show that increase is called in non-profitable case

86f49a7

Add algo abstraction

672f6cb

Remove index trait

2e80dec

Add test for capped price

1134fd5

Add updating logic

a818b9b

WIP add Result to functions

edf1cb3

Add errors to all the failure modes

a389a13

Start writing real algo

19a41ac

Introduce real BlockFullness

0f8edce

Switch BlockFullness to ints

613c6bb

Add some naive tests to algo impl with tests

4b141b5

Merge branch 'master' into feature/fuel-gas-price-provider

c3bae38

WIP add benches

b693a2a

Add WIP algo testing to its own crate for compilation reasons

e9bf624

Beef up graph

ada5897

Add missed stuff

473a1a1

Fix some maths

8e5224d

Cleanup a little

883c695

Add tuned p value

b92ba83

Add p value

ca4568d

Introduce "more realistic" noise to cost signal

a8cf8e1

Simplify algo to ignore block fullness and add d value

c51e15e

xgreenx assigned MitchTurner Jun 10, 2024

MitchTurner and others added 5 commits June 10, 2024 14:07

Merge branch 'master' into feature/fuel-gas-price-service

a455bd1

Update crates/services/gas_price_service/Cargo.toml

9f9d7ba

Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>

Add some docs and better errors

dac6d29

Fmt

a4037ea

Merge branch 'master' into feature/fuel-gas-price-service

9438dfc

MitchTurner requested a review from a team June 11, 2024 02:14

Dentosal previously approved these changes Jun 11, 2024

View reviewed changes

xgreenx requested changes Jun 11, 2024

View reviewed changes

Add changes from review suggestions

4470ab7

MitchTurner dismissed Dentosal’s stale review via 4470ab7 June 11, 2024 17:08

MitchTurner added 2 commits June 11, 2024 10:33

Add constraint to shared algo for cleaner access

2d0932a

Merge branch 'master' into feature/fuel-gas-price-service

46c1969

MitchTurner mentioned this pull request Jun 11, 2024

Implement our gas price algorithm for our GasPriceService #1955

Closed

MitchTurner added 3 commits June 11, 2024 10:39

Fmt

d4150e9

Rename a couple functions

1e1848b

Fmt

843c480

MitchTurner requested a review from xgreenx June 11, 2024 17:49

MitchTurner added 2 commits June 11, 2024 10:55

Fix compilation, appease Clippy-sama

e8399f5

More cleanup

8ad3260

xgreenx reviewed Jun 11, 2024

View reviewed changes

crates/services/txpool/src/ports.rs Outdated Show resolved Hide resolved

crates/services/producer/src/block_producer.rs Outdated Show resolved Hide resolved

xgreenx added 2 commits June 11, 2024 21:27

Update crates/services/producer/src/block_producer.rs

ab475e5

Update crates/services/txpool/src/ports.rs

421dc45

xgreenx approved these changes Jun 11, 2024

View reviewed changes

MitchTurner enabled auto-merge (squash) June 11, 2024 19:32

MitchTurner merged commit 6d31acb into master Jun 11, 2024
29 checks passed

MitchTurner deleted the feature/fuel-gas-price-service branch June 11, 2024 19:39

xgreenx mentioned this pull request Jun 14, 2024

Release v0.29.0 #1963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Gas Price Updater Service #1938

Add Gas Price Updater Service #1938

MitchTurner commented Jun 5, 2024 •

edited

Loading

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

MitchTurner Jun 11, 2024

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

MitchTurner Jun 11, 2024

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

MitchTurner Jun 11, 2024 •

edited

Loading

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

xgreenx Jun 11, 2024

xgreenx left a comment

		let (watch_sender, watch_receiver) = tokio::sync::watch::channel(State::Started);
		let watcher = StateWatcher::from(watch_receiver);

	) -> fuel_core_types::services::txpool::Result<GasPrice>;
	) -> TxPoolResult<GasPrice>;

Add Gas Price Updater Service #1938

Add Gas Price Updater Service #1938

Conversation

MitchTurner commented Jun 5, 2024 • edited Loading

Checklist

Before requesting review

After merging, notify other teams

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

Choose a reason for hiding this comment

MitchTurner Jun 11, 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

xgreenx left a comment

Choose a reason for hiding this comment

MitchTurner commented Jun 5, 2024 •

edited

Loading

MitchTurner Jun 11, 2024 •

edited

Loading