Skip to content

Commit

Permalink
Fix bug with gas price factor in V1 algorithm (#2201)
Browse files Browse the repository at this point in the history 
## Linked Issues/PRs
#2167

## Description
Before, the final `da_gas_price` was calculated by the block producer,
but its chosen value wasn't communicated back to the updater
immediately. Instead the value was implicitly calculated by the updater
when it received the L2 block information. This was bad because it
destroyed information with respect to the gas price factor.

A value of 1 could actually have been 110 or 120 in the updater, but
then divided by the factor and rounded back to 1. On update, this would
be interpreted as just 100, and the incremental changes would be lost.

Since we are no long calculating the `da_gas_price` at the time of block
production, we could store the _actual_ `da_gas_price` value in the
updater and avoid the lost information.

Many tests existed to cover the previous behavior they were reconfigured
to match the newer, simpler behavior.

## Checklist
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself
  • Loading branch information
@MitchTurner
MitchTurner authored Sep 17, 2024
1 parent a4dfa7c commit 83e0f96
Show file tree
Hide file tree
Showing 7 changed files with 410 additions and 375 deletions.
Binary file modified bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm
View file
Binary file not shown.
10 changes: 5 additions & 5 deletions crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ impl Simulator {
let updater = AlgorithmUpdaterV1 {
min_exec_gas_price: 10,
min_da_gas_price: 10,
// Change to adjust where the gas price starts on block 0
// Change to adjust where the exec gas price starts on block 0
new_scaled_exec_price: 10 * gas_price_factor,
// Change to adjust where the gas price starts on block 0
last_da_gas_price: 100,
// Change to adjust where the da gas price starts on block 0
new_scaled_da_gas_price: 100,
gas_price_factor: NonZeroU64::new(gas_price_factor).unwrap(),
l2_block_height: 0,
// Choose the ideal fullness percentage for the L2 block
Expand Down Expand Up @@ -114,7 +114,8 @@ impl Simulator {
for (index, ((fullness, bytes), da_block)) in blocks {
let height = index as u32 + 1;
exec_gas_prices.push(updater.new_scaled_exec_price);
let gas_price = updater.algorithm().calculate(max_block_bytes);
da_gas_prices.push(updater.new_scaled_da_gas_price);
let gas_price = updater.algorithm().calculate();
gas_prices.push(gas_price);
updater
.update_l2_block_data(
Expand All @@ -125,7 +126,6 @@ impl Simulator {
gas_price,
)
.unwrap();
da_gas_prices.push(updater.last_da_gas_price);
pessimistic_costs
.push(max_block_bytes as u128 * updater.latest_da_cost_per_byte);
actual_reward_totals.push(updater.total_da_rewards_excess);
Expand Down
266 changes: 130 additions & 136 deletions crates/fuel-gas-price-algorithm/src/v1.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
cmp::max,
num::NonZeroU64,
ops::Div,
};

#[cfg(test)]
Expand All @@ -18,14 +19,37 @@ pub enum Error {
FailedTooIncludeL2BlockData(String),
}

#[derive(Debug, Clone, PartialEq)]
pub struct AlgorithmV1 {
/// the combined Execution and DA gas prices
combined_gas_price: u64,
}

impl AlgorithmV1 {
pub fn calculate(&self) -> u64 {
self.combined_gas_price
}
}

/// The state of the algorithm used to update the gas price algorithm for each block
///
/// Because there will always be a delay between blocks submitted to the L2 chain and the blocks
/// being recorded on the DA chain, the updater needs to make "projections" about the cost of
/// recording any given block to the DA chain. This is done by tracking the cost per byte of recording
/// for the most recent blocks, and using the known bytes of the unrecorded blocks to estimate
/// the cost for that block. Every time the DA recording is updated, the projections are recalculated.
///
/// This projection will inevitably lead to error in the gas price calculation. Special care should be taken
/// to account for the worst case scenario when calculating the parameters of the algorithm.
///
/// An algorithm for calculating the gas price for the next block
///
/// The algorithm breaks up the gas price into two components:
/// - The execution gas price, which is used to cover the cost of executing the next block as well
/// as moderating the congestion of the network by increasing the price when traffic is high.
/// - The data availability (DA) gas price, which is used to cover the cost of recording the block on the DA chain
///
/// The execution gas price is calculated eagerly based on the fullness of the last received l2 block. Each
/// The execution gas price is calculated based on the fullness of the last received l2 block. Each
/// block has a capacity threshold, and if the block is above this threshold, the gas price is increased. If
/// it is below the threshold, the gas price is decreased.
/// The gas price can only change by a fixed amount each block.
Expand All @@ -44,96 +68,10 @@ pub enum Error {
/// The DA portion also uses a moving average of the profits over the last `avg_window` blocks
/// instead of the actual profit. Setting the `avg_window` to 1 will effectively disable the
/// moving average.
#[derive(Debug, Clone, PartialEq)]
pub struct AlgorithmV1 {
/// The lowest the algorithm allows the gas price to go
min_da_gas_price: u64,
/// The gas price for to cover the execution of the next block
new_exec_price: u64,
/// The gas price for the DA portion of the last block. This can be used to calculate
last_da_price: u64,
/// The maximum percentage that the DA portion of the gas price can change in a single block.
/// Using `u16` because it can go above 100% and possibly over 255%
max_change_percent: u16,
/// The latest known cost per byte for recording blocks on the DA chain
latest_da_cost_per_byte: u128,
/// The cumulative reward from the DA portion of the gas price
total_rewards: u128,
/// The cumulative cost of recording L2 blocks on the DA chain as of the last recorded block
total_costs: u128,
/// The P component of the PID control for the DA gas price
da_p_factor: i64,
/// The D component of the PID control for the DA gas price
da_d_factor: i64,
/// The average profit over the last `avg_window` blocks
last_profit: i128,
/// the previous profit
second_to_last_profit: i128,
}

impl AlgorithmV1 {
pub fn calculate(&self, _block_bytes: u64) -> u64 {
let p = self.p();
let d = self.d();
let da_change = self.change(p, d);

self.assemble_price(da_change)
}

fn p(&self) -> i128 {
let upcast_p: i128 = self.da_p_factor.into();
let checked_p = self.last_profit.checked_div(upcast_p);
// If the profit is positive, we want to decrease the gas price
checked_p.unwrap_or(0).saturating_mul(-1)
}

fn d(&self) -> i128 {
let upcast_d: i128 = self.da_d_factor.into();
let slope = self.last_profit.saturating_sub(self.second_to_last_profit);
let checked_d = slope.checked_div(upcast_d);
// if the slope is positive, we want to decrease the gas price
checked_d.unwrap_or(0).saturating_mul(-1)
}

fn change(&self, p: i128, d: i128) -> i128 {
let pd_change = p.saturating_add(d);
let upcast_percent = self.max_change_percent.into();
let max_change = self
.last_da_price
.saturating_mul(upcast_percent)
.saturating_div(100)
.into();
let clamped_change = pd_change.abs().min(max_change);
pd_change.signum().saturating_mul(clamped_change)
}

fn assemble_price(&self, change: i128) -> u64 {
let upcast_last_da_price: i128 = self.last_da_price.into();
let new_price_oversized = upcast_last_da_price.saturating_add(change);

let maybe_new_da_gas_price: u64 = new_price_oversized
.try_into()
.unwrap_or(if change.is_positive() { u64::MAX } else { 0 });

let new_da_gas_price = max(self.min_da_gas_price, maybe_new_da_gas_price);
self.new_exec_price.saturating_add(new_da_gas_price)
}
}

/// The state of the algorithm used to update the gas price algorithm for each block
///
/// Because there will always be a delay between blocks submitted to the L2 chain and the blocks
/// being recorded on the DA chain, the updater needs to make "projections" about the cost of
/// recording any given block to the DA chain. This is done by tracking the cost per byte of recording
/// for the most recent blocks, and using the known bytes of the unrecorded blocks to estimate
/// the cost for that block. Every time the DA recording is updated, the projections are recalculated.
///
/// This projection will inevitably lead to error in the gas price calculation. Special care should be taken
/// to account for the worst case scenario when calculating the parameters of the algorithm.
#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)]
pub struct AlgorithmUpdaterV1 {
// Execution
/// The gas price to cover the execution of the next block
/// The gas price (scaled by the `gas_price_factor`) to cover the execution of the next block
pub new_scaled_exec_price: u64,
/// The lowest the algorithm allows the exec gas price to go
pub min_exec_gas_price: u64,
Expand All @@ -149,7 +87,11 @@ pub struct AlgorithmUpdaterV1 {
// DA
/// The gas price for the DA portion of the last block. This can be used to calculate
/// the DA portion of the next block
pub last_da_gas_price: u64,
// pub last_da_gas_price: u64,

/// The gas price (scaled by the `gas_price_factor`) to cover the DA commitment of the next block
pub new_scaled_da_gas_price: u64,

/// Scale factor for the gas price.
pub gas_price_factor: NonZeroU64,
/// The lowest the algorithm allows the da gas price to go
Expand Down Expand Up @@ -240,7 +182,7 @@ impl AlgorithmUpdaterV1 {
used: u64,
capacity: NonZeroU64,
block_bytes: u64,
gas_price: u64,
_gas_price: u64,
) -> Result<(), Error> {
let expected = self.l2_block_height.saturating_add(1);
if height != expected {
Expand All @@ -250,66 +192,124 @@ impl AlgorithmUpdaterV1 {
})
} else {
self.l2_block_height = height;
// `gas_price_factor` will never be zero
#[allow(clippy::arithmetic_side_effects)]
let last_exec_price = self
.new_scaled_exec_price
.saturating_div(self.gas_price_factor.into());
// TODO: fix this nonsense when we fix the types https://github.com/FuelLabs/fuel-core/issues/2147
let projected_total_da_cost = i128::try_from(self.projected_total_da_cost)
.map_err(|_| {
Error::FailedTooIncludeL2BlockData(format!(
"Converting {:?} to an i64 from u256",
self.projected_total_da_cost
))
})?;

// rewards
let block_da_reward = used.saturating_mul(self.descaled_da_price());
self.total_da_rewards_excess = self
.total_da_rewards_excess
.saturating_add(block_da_reward.into());
let rewards = self.total_da_rewards_excess.try_into().unwrap_or(i128::MAX);
let last_profit = rewards.saturating_sub(projected_total_da_cost);
self.update_last_profit(last_profit);

// costs
let block_projected_da_cost =
(block_bytes as u128).saturating_mul(self.latest_da_cost_per_byte);
self.projected_total_da_cost = self
.projected_total_da_cost
.saturating_add(block_projected_da_cost);
// implicitly deduce what our da gas price was for the l2 block
self.last_da_gas_price = gas_price.saturating_sub(last_exec_price);
let projected_total_da_cost = self.projected_cost_as_i128();
let last_profit = rewards.saturating_sub(projected_total_da_cost);
self.update_last_profit(last_profit);

// gas prices
self.update_exec_gas_price(used, capacity);
let block_da_reward = used.saturating_mul(self.last_da_gas_price);
self.total_da_rewards_excess = self
.total_da_rewards_excess
.saturating_add(block_da_reward.into());
self.update_da_gas_price();
Ok(())
}
}

// We are assuming that the difference between u128::MAX and i128::MAX is negligible
fn projected_cost_as_i128(&self) -> i128 {
i128::try_from(self.projected_total_da_cost).unwrap_or(i128::MAX)
}

fn update_last_profit(&mut self, new_profit: i128) {
self.second_to_last_profit = self.last_profit;
self.last_profit = new_profit;
}

fn update_exec_gas_price(&mut self, used: u64, capacity: NonZeroU64) {
let threshold = *self.l2_block_fullness_threshold_percent as u64;
let mut exec_gas_price = self.new_scaled_exec_price;
let mut scaled_exec_gas_price = self.new_scaled_exec_price;
let fullness_percent = used
.saturating_mul(100)
.checked_div(capacity.into())
.unwrap_or(threshold);

match fullness_percent.cmp(&threshold) {
std::cmp::Ordering::Greater => {
let change_amount = self.change_amount(exec_gas_price);
exec_gas_price = exec_gas_price.saturating_add(change_amount);
let change_amount = self.exec_change(scaled_exec_gas_price);
scaled_exec_gas_price =
scaled_exec_gas_price.saturating_add(change_amount);
}
std::cmp::Ordering::Less => {
let change_amount = self.change_amount(exec_gas_price);
exec_gas_price = exec_gas_price.saturating_sub(change_amount);
let change_amount = self.exec_change(scaled_exec_gas_price);
scaled_exec_gas_price =
scaled_exec_gas_price.saturating_sub(change_amount);
}
std::cmp::Ordering::Equal => {}
}
self.new_scaled_exec_price = max(self.min_exec_gas_price, exec_gas_price);
self.new_scaled_exec_price =
max(self.min_scaled_exec_gas_price(), scaled_exec_gas_price);
}

fn change_amount(&self, principle: u64) -> u64 {
fn min_scaled_exec_gas_price(&self) -> u64 {
self.min_exec_gas_price
.saturating_mul(self.gas_price_factor.into())
}

fn update_da_gas_price(&mut self) {
let p = self.p();
let d = self.d();
let da_change = self.da_change(p, d);
let maybe_new_scaled_da_gas_price = i128::from(self.new_scaled_da_gas_price)
.checked_add(da_change)
.and_then(|x| u64::try_from(x).ok())
.unwrap_or_else(|| {
if da_change.is_positive() {
u64::MAX
} else {
0u64
}
});
self.new_scaled_da_gas_price = max(
self.min_scaled_da_gas_price(),
maybe_new_scaled_da_gas_price,
);
}

fn min_scaled_da_gas_price(&self) -> u64 {
self.min_da_gas_price
.saturating_mul(self.gas_price_factor.into())
}

fn p(&self) -> i128 {
let upcast_p = i128::from(self.da_p_component);
let checked_p = self.last_profit.checked_div(upcast_p);
// If the profit is positive, we want to decrease the gas price
checked_p.unwrap_or(0).saturating_mul(-1)
}

fn d(&self) -> i128 {
let upcast_d = i128::from(self.da_d_component);
let slope = self.last_profit.saturating_sub(self.second_to_last_profit);
let checked_d = slope.checked_div(upcast_d);
// if the slope is positive, we want to decrease the gas price
checked_d.unwrap_or(0).saturating_mul(-1)
}

fn da_change(&self, p: i128, d: i128) -> i128 {
let pd_change = p.saturating_add(d);
let upcast_percent = self.max_da_gas_price_change_percent.into();
let max_change = self
.new_scaled_da_gas_price
.saturating_mul(upcast_percent)
.saturating_div(100)
.into();
let clamped_change = pd_change.abs().min(max_change);
pd_change.signum().saturating_mul(clamped_change)
}

fn exec_change(&self, principle: u64) -> u64 {
principle
.saturating_mul(self.exec_gas_price_change_percent as u64)
.saturating_div(100)
Expand Down Expand Up @@ -361,25 +361,19 @@ impl AlgorithmUpdaterV1 {
.saturating_add(projection_portion);
}

fn descaled_exec_price(&self) -> u64 {
self.new_scaled_exec_price.div(self.gas_price_factor)
}

fn descaled_da_price(&self) -> u64 {
self.new_scaled_da_gas_price.div(self.gas_price_factor)
}

pub fn algorithm(&self) -> AlgorithmV1 {
AlgorithmV1 {
min_da_gas_price: self.min_da_gas_price,
#[allow(clippy::arithmetic_side_effects)]
// `gas_price_factor` will never be zero
new_exec_price: self
.new_scaled_exec_price
.saturating_div(self.gas_price_factor.into()),
last_da_price: self.last_da_gas_price,
max_change_percent: self.max_da_gas_price_change_percent,

latest_da_cost_per_byte: self.latest_da_cost_per_byte,
total_rewards: self.total_da_rewards_excess,
total_costs: self.projected_total_da_cost,
last_profit: self.last_profit,
second_to_last_profit: self.second_to_last_profit,
da_p_factor: self.da_p_component,
da_d_factor: self.da_d_component,
}
let combined_gas_price = self
.descaled_exec_price()
.saturating_add(self.descaled_da_price());
AlgorithmV1 { combined_gas_price }
}

// We only need to track the difference between the rewards and costs after we have true DA data
Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-gas-price-algorithm/src/v1/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl UpdaterBuilder {
AlgorithmUpdaterV1 {
min_exec_gas_price: self.min_exec_gas_price,
new_scaled_exec_price: self.starting_exec_gas_price,
last_da_gas_price: self.starting_da_gas_price,
new_scaled_da_gas_price: self.starting_da_gas_price,
exec_gas_price_change_percent: self.exec_gas_price_change_percent,
max_da_gas_price_change_percent: self.max_change_percent,

Expand Down
Loading

0 comments on commit 83e0f96

Please sign in to comment.