Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new FuelGasPriceProvider #1889

Draft
wants to merge 56 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
378dae8
Introduce type with dummy test
MitchTurner May 8, 2024
cbebddc
Replace test with historical gas price test
MitchTurner May 8, 2024
bb24903
Get test passing
MitchTurner May 8, 2024
fb58594
Refactor
MitchTurner May 8, 2024
dc193d5
Add more logic to when it looks up historical prices, add tests
MitchTurner May 8, 2024
4af3946
Update CHANGELOG
MitchTurner May 9, 2024
398884e
Remove extra traits
MitchTurner May 9, 2024
96f20d9
Rethink ports
MitchTurner May 9, 2024
86f49a7
Write test to show that increase is called in non-profitable case
MitchTurner May 9, 2024
672f6cb
Add algo abstraction
MitchTurner May 10, 2024
2e80dec
Remove index trait
MitchTurner May 10, 2024
1134fd5
Add test for capped price
MitchTurner May 10, 2024
a818b9b
Add updating logic
MitchTurner May 10, 2024
edf1cb3
WIP add Result to functions
MitchTurner May 10, 2024
a389a13
Add errors to all the failure modes
MitchTurner May 10, 2024
19a41ac
Start writing real algo
MitchTurner May 10, 2024
0f8edce
Introduce real `BlockFullness`
MitchTurner May 11, 2024
613c6bb
Switch `BlockFullness` to ints
MitchTurner May 11, 2024
4b141b5
Add some naive tests to algo impl with tests
MitchTurner May 11, 2024
c3bae38
Merge branch 'master' into feature/fuel-gas-price-provider
MitchTurner May 13, 2024
b693a2a
WIP add benches
MitchTurner May 13, 2024
e9bf624
Add WIP algo testing to its own crate for compilation reasons
MitchTurner May 13, 2024
ada5897
Beef up graph
MitchTurner May 13, 2024
473a1a1
Add missed stuff
MitchTurner May 13, 2024
8e5224d
Fix some maths
MitchTurner May 13, 2024
883c695
Cleanup a little
MitchTurner May 13, 2024
b92ba83
Add tuned p value
MitchTurner May 13, 2024
ca4568d
Add p value
MitchTurner May 14, 2024
a8cf8e1
Introduce "more realistic" noise to cost signal
MitchTurner May 14, 2024
c51e15e
Simplify algo to ignore block fullness and add d value
MitchTurner May 14, 2024
9119cc0
Tune some
MitchTurner May 14, 2024
509d64a
Add cap to gas-price change
MitchTurner May 15, 2024
0cfb486
Make a little more stable
MitchTurner May 15, 2024
1526069
Add new chart for execution vs da vs total, implement naive exec calc
MitchTurner May 24, 2024
cc7f5e8
Refactor to split da and exec gas prices and have storage for both
MitchTurner May 25, 2024
6df0036
Use algo in adapter
MitchTurner May 27, 2024
f4941b1
Remove benchmarking stuff
MitchTurner May 27, 2024
9cb29c7
Merge branch 'master' into feature/fuel-gas-price-provider
MitchTurner May 27, 2024
a7eff14
Remove buggy/unneeded while loop
MitchTurner May 27, 2024
0c3b0e0
Have gas price provider return result instead of option
MitchTurner May 27, 2024
6349835
Merge branch 'master' into feature/fuel-gas-price-provider
MitchTurner May 27, 2024
c03caf1
Fix typo
MitchTurner May 27, 2024
b22ab44
Lint toml file
MitchTurner May 27, 2024
1b70458
Reformat
MitchTurner May 27, 2024
a0f0a74
Clean up
MitchTurner May 27, 2024
ba03b02
Use old da_gas_price if not caught up
MitchTurner May 27, 2024
fd3fb69
Fix bug in when da gas prices are recalculated
MitchTurner May 28, 2024
102bdc9
Make da price more stepwise to account for fewer blocks on da layer
MitchTurner May 28, 2024
80835e6
Increase max change for da gas price because of lower frequency, fix …
MitchTurner May 28, 2024
8ca7fe6
Merge remote-tracking branch 'origin' into feature/fuel-gas-price-pro…
MitchTurner May 28, 2024
29a5025
Add some rough concept of determining a good p and d value
MitchTurner May 28, 2024
d45d246
Appease Clippy-sama
MitchTurner May 28, 2024
daa2c68
Merge branch 'master' into feature/fuel-gas-price-provider
MitchTurner May 29, 2024
f5986aa
Remove file, use anyhow error
MitchTurner May 31, 2024
7746674
Fix bug that was not caught by tests :/
MitchTurner Jun 1, 2024
d5f4509
Fix name
MitchTurner Jun 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Added

- [#1889](https://github.com/FuelLabs/fuel-core/pull/1889): Add new `FuelGasPriceProvider`

### Changed

- [#1886](https://github.com/FuelLabs/fuel-core/pull/1886): Use ref to `Block` in validation code
Expand Down
Binary file not shown.
Binary file modified bin/fuel-core/chainspec/testnet/state_transition_bytecode.wasm
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this file, it is not used anymore=)

Binary file not shown.
2 changes: 2 additions & 0 deletions crates/fuel-core/src/service/adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ pub mod relayer;
pub mod sync;
pub mod txpool;

pub mod fuel_gas_price_provider;

#[derive(Debug, Clone)]
pub struct ConsensusParametersProvider {
shared_state: consensus_parameters_provider::SharedState,
Expand Down
188 changes: 188 additions & 0 deletions crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
use crate::service::adapters::fuel_gas_price_provider::ports::GasPriceAlgorithm;
use fuel_core_producer::block_producer::gas_price::{
GasPriceParams,
GasPriceProvider,
};
use fuel_core_types::fuel_types::BlockHeight;
use ports::{
DARecordingCostHistory,
FuelBlockHistory,
};
use std::cell::RefCell;

pub mod ports;

pub mod algorithm_adapter;

use ports::{
Error,
Result,
};

#[cfg(test)]
mod tests;

/// Gives the gas price for a given block height, and calculates the gas price if not yet committed.
pub struct FuelGasPriceProvider<FB, DA, A> {
profitablility_totals: ProfitablilityTotals,

// adapters
block_history: FB,
da_recording_cost_history: DA,
algorithm: A,
}

impl<FB, DA, A> FuelGasPriceProvider<FB, DA, A> {
pub fn new(block_history: FB, da_recording_cost_history: DA, algorithm: A) -> Self {
Self {
profitablility_totals: ProfitablilityTotals::default(),
block_history,
da_recording_cost_history,
algorithm,
}
}
}

struct ProfitablilityTotals {
totaled_block_height: RefCell<BlockHeight>,
total_reward: RefCell<u64>,
total_cost: RefCell<u64>,
}

impl Default for ProfitablilityTotals {
fn default() -> Self {
Self {
totaled_block_height: RefCell::new(0.into()),
total_reward: RefCell::new(0),
total_cost: RefCell::new(0),
}
}
}

impl ProfitablilityTotals {
#[allow(dead_code)]
pub fn new(block_height: BlockHeight, reward: u64, cost: u64) -> Self {
Self {
totaled_block_height: RefCell::new(block_height),
total_reward: RefCell::new(reward),
total_cost: RefCell::new(cost),
}
}

fn update(&self, block_height: BlockHeight, reward: u64, cost: u64) {
let mut totaled_block_height = self.totaled_block_height.borrow_mut();
let mut total_reward = self.total_reward.borrow_mut();
let mut total_cost = self.total_cost.borrow_mut();
while *totaled_block_height < block_height {
*totaled_block_height = block_height;
*total_reward += reward;
*total_cost += cost;
}
}
}

impl<FB, DA, A> FuelGasPriceProvider<FB, DA, A>
where
FB: FuelBlockHistory,
DA: DARecordingCostHistory,
A: GasPriceAlgorithm,
{
fn inner_gas_price(&self, requested_block_height: BlockHeight) -> Result<u64> {
let latest_block = self
.block_history
.latest_height()
.map_err(Error::UnableToGetLatestBlockHeight)?;
if latest_block > requested_block_height {
self.block_history
.gas_price(requested_block_height)
.map_err(Error::UnableToGetGasPrice)?
.ok_or(Error::GasPriceNotFoundForBlockHeight(
requested_block_height,
))
} else if Self::asking_for_next_block(latest_block, requested_block_height) {
self.calculate_new_gas_price(latest_block)
} else {
Err(Error::RequestedBlockHeightTooHigh {
requested: requested_block_height,
latest: latest_block,
})
}
}

fn asking_for_next_block(
latest_block: BlockHeight,
block_height: BlockHeight,
) -> bool {
*latest_block + 1 == *block_height
}

fn calculate_new_gas_price(&self, latest_block_height: BlockHeight) -> Result<u64> {
self.update_totals(latest_block_height)?;
let previous_gas_price = self
.block_history
.gas_price(latest_block_height)
.map_err(Error::UnableToGetGasPrice)?
.ok_or(Error::GasPriceNotFoundForBlockHeight(latest_block_height))?;
let block_fullness = self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previous block's fullness should be part of the GasProviderService. On each block imported from the block importer, the service will update its internal fullness variables and dump new values into the database.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean GasProviderService. Right now the producer, the txpool, and graphql each get a copy of the impl GasPriceProvider for StaticGasPrice.

That's how I imagine it will still work, but just with this new FuelGasPriceProvider instead of StaticGasPrice. I'm assuming that all the GasPriceProvider needs is a BlockHeight and then it has access to all the DBs it needs to assemble the gas price. I don't know why it needs to store block_fullness when it can just look up the block's data in its DB.

.block_history
.block_fullness(latest_block_height)
.map_err(Error::UnableToGetBlockFullness)?
.ok_or(Error::BlockFullnessNotFoundForBlockHeight(
latest_block_height,
))?;
let new_gas_price_candidate = self.algorithm.calculate_gas_price(
previous_gas_price,
self.total_reward(),
self.total_cost(),
block_fullness,
);
let new_gas_price = core::cmp::min(
new_gas_price_candidate,
self.algorithm.maximum_next_gas_price(previous_gas_price),
);
MitchTurner marked this conversation as resolved.
Show resolved Hide resolved
Ok(new_gas_price)
}

fn total_reward(&self) -> u64 {
*self.profitablility_totals.total_reward.borrow()
}

fn total_cost(&self) -> u64 {
*self.profitablility_totals.total_cost.borrow()
}

fn totaled_block_height(&self) -> BlockHeight {
*self.profitablility_totals.totaled_block_height.borrow()
}

fn update_totals(&self, latest_block_height: BlockHeight) -> Result<()> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to take into account the fact that DA recording has some lag to it. Possibly 12-24 blocks? How does that effect the gas_price calculation? The current design doesn't take that into account.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented a solution for this. The DA gas price is only updated when the da recorded height changes. Might need more testing.

while self.totaled_block_height() < latest_block_height {
let block_height = (*self.totaled_block_height() + 1).into();
let reward = self
.block_history
.production_reward(block_height)
.map_err(Error::UnableToGetProductionReward)?
.ok_or(Error::ProductionRewardNotFoundForBlockHeight(block_height))?;
let cost = self
.da_recording_cost_history
.recording_cost(block_height)
.map_err(Error::UnableToGetRecordingCost)?
.ok_or(Error::RecordingCostNotFoundForBlockHeight(block_height))?;
self.profitablility_totals
.update(block_height, reward, cost);
}
Ok(())
}
}

impl<FB, DA, A> GasPriceProvider for FuelGasPriceProvider<FB, DA, A>
where
FB: FuelBlockHistory,
DA: DARecordingCostHistory,
A: GasPriceAlgorithm,
{
fn gas_price(&self, params: GasPriceParams) -> Option<u64> {
// TODO: handle error
self.inner_gas_price(params.block_height()).ok()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
use crate::service::adapters::fuel_gas_price_provider::ports::{
BlockFullness,
GasPriceAlgorithm,
};

// TODO: Don't use floats 🤢
pub struct FuelGasPriceAlgorithm {
_target_profitability: f32,
_max_price_change_percentage: f32,
}

impl GasPriceAlgorithm for FuelGasPriceAlgorithm {
fn calculate_gas_price(
&self,
previous_gas_price: u64,
total_production_reward: u64,
total_da_recording_cost: u64,
block_fullness: BlockFullness,
) -> u64 {
if total_da_recording_cost > total_production_reward {
previous_gas_price + 1
} else {
if block_fullness.used() > block_fullness.capacity() / 2 {
previous_gas_price + 1
} else {
previous_gas_price - 1
}
}
}

fn maximum_next_gas_price(&self, _previous_gas_price: u64) -> u64 {
todo!()
}
}

#[cfg(test)]
mod tests {
#![allow(non_snake_case)]
use super::*;

#[test]
fn calculate_gas_price__above_50_percent_increases_gas_price() {
// given
let old_gas_price = 444;
let total_production_reward = 100;
let total_da_recording_cost = total_production_reward;
// 60% full
let block_fullness = BlockFullness::new(60, 100);
let algo = FuelGasPriceAlgorithm {
_target_profitability: 1.,
_max_price_change_percentage: f32::MAX,
};
// when
let new_gas_price = algo.calculate_gas_price(
old_gas_price,
total_production_reward,
total_da_recording_cost,
block_fullness,
);

// then
assert!(new_gas_price > old_gas_price);
}

#[test]
fn calculate_gas_price__below_50_but_not_profitable_increase_gas_price() {
// given
let old_gas_price = 444;
let total_production_reward = 100;
let total_da_recording_cost = total_production_reward + 1;
// 40% full
let block_fullness = BlockFullness::new(40, 100);
let algo = FuelGasPriceAlgorithm {
_target_profitability: 1.,
_max_price_change_percentage: f32::MAX,
};

// when
let new_gas_price = algo.calculate_gas_price(
old_gas_price,
total_production_reward,
total_da_recording_cost,
block_fullness,
);

// then
assert!(new_gas_price > old_gas_price);
}

#[test]
fn calculate_gas_price__below_50_and_profitable_decrease_gas_price() {
// given
let old_gas_price = 444;
let total_production_reward = 100;
let total_da_recording_cost = total_production_reward - 1;
// 40% full
let block_fullness = BlockFullness::new(40, 100);
let algo = FuelGasPriceAlgorithm {
_target_profitability: 1.,
_max_price_change_percentage: f32::MAX,
};

// when
let new_gas_price = algo.calculate_gas_price(
old_gas_price,
total_production_reward,
total_da_recording_cost,
block_fullness,
);

// then
assert!(new_gas_price < old_gas_price);
}
}