From 448d8c22d5a425d86d48bc25e56dbac8c2f16db2 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 17:26:46 +0200 Subject: [PATCH 01/19] Remove height parameter, change tests --- crates/fuel-gas-price-algorithm/src/v1.rs | 57 ++++------ .../fuel-gas-price-algorithm/src/v1/tests.rs | 1 - .../v1/tests/update_da_record_data_tests.rs | 102 +++++++++++------- .../gas_price_service/src/v1/metadata.rs | 7 -- 4 files changed, 84 insertions(+), 83 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 9a9da066fee..e45e6dcae8f 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -16,14 +16,12 @@ mod tests; pub enum Error { #[error("Skipped L2 block update: expected {expected:?}, got {got:?}")] SkippedL2Block { expected: u32, got: u32 }, - #[error("Skipped DA block update: expected {expected:?}, got {got:?}")] - SkippedDABlock { expected: u32, got: u32 }, #[error("Could not calculate cost per byte: {bytes:?} bytes, {cost:?} cost")] CouldNotCalculateCostPerByte { bytes: u128, cost: u128 }, #[error("Failed to include L2 block data: {0}")] FailedTooIncludeL2BlockData(String), - #[error("L2 block expected but not found in unrecorded blocks: {0}")] - L2BlockExpectedNotFound(u32), + #[error("L2 block expected but not found in unrecorded blocks: {height:?}")] + L2BlockExpectedNotFound { height: u32 }, } // TODO: separate exec gas price and DA gas price into newtypes for clarity @@ -131,8 +129,6 @@ pub struct AlgorithmUpdaterV1 { pub max_da_gas_price_change_percent: u16, /// The cumulative reward from the DA portion of the gas price pub total_da_rewards_excess: u128, - /// The height of the last L2 block recorded on the DA chain - pub da_recorded_block_height: u32, /// The cumulative cost of recording L2 blocks on the DA chain as of the last recorded block pub latest_known_total_da_cost_excess: u128, /// The predicted cost of recording L2 blocks on the DA chain as of the last L2 block @@ -517,30 +513,19 @@ impl AlgorithmUpdaterV1 { height_range: Range, range_cost: u128, ) -> Result<(), Error> { - let expected = self.da_recorded_block_height.saturating_add(1); - let first = height_range.start; - if first != expected { - Err(Error::SkippedDABlock { - expected, - got: first, - }) - } else { - let last = height_range.end.saturating_sub(1); - let range_bytes = self.drain_l2_block_bytes_for_range(height_range)?; - let new_cost_per_byte: u128 = range_cost.checked_div(range_bytes).ok_or( - Error::CouldNotCalculateCostPerByte { - bytes: range_bytes, - cost: range_cost, - }, - )?; - self.da_recorded_block_height = last; - let new_da_block_cost = self - .latest_known_total_da_cost_excess - .saturating_add(range_cost); - self.latest_known_total_da_cost_excess = new_da_block_cost; - self.latest_da_cost_per_byte = new_cost_per_byte; - Ok(()) - } + let range_bytes = self.drain_l2_block_bytes_for_range(height_range)?; + let new_cost_per_byte: u128 = range_cost.checked_div(range_bytes).ok_or( + Error::CouldNotCalculateCostPerByte { + bytes: range_bytes, + cost: range_cost, + }, + )?; + let new_da_block_cost = self + .latest_known_total_da_cost_excess + .saturating_add(range_cost); + self.latest_known_total_da_cost_excess = new_da_block_cost; + self.latest_da_cost_per_byte = new_cost_per_byte; + Ok(()) } fn drain_l2_block_bytes_for_range( @@ -549,13 +534,11 @@ impl AlgorithmUpdaterV1 { ) -> Result { let mut total: u128 = 0; for expected_height in height_range { - let (actual_height, bytes) = self - .unrecorded_blocks - .pop_first() - .ok_or(Error::L2BlockExpectedNotFound(expected_height))?; - if actual_height != expected_height { - return Err(Error::L2BlockExpectedNotFound(expected_height)); - } + let bytes = self.unrecorded_blocks.remove(&expected_height).ok_or( + Error::L2BlockExpectedNotFound { + height: expected_height, + }, + )?; total = total.saturating_add(bytes as u128); } Ok(total) diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests.rs index e7150378428..c7d8c2c47b0 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests.rs @@ -184,7 +184,6 @@ impl UpdaterBuilder { l2_block_fullness_threshold_percent: self.l2_block_capacity_threshold.into(), total_da_rewards_excess: self.total_rewards, - da_recorded_block_height: self.da_recorded_block_height, latest_da_cost_per_byte: self.da_cost_per_byte, projected_total_da_cost: self.project_total_cost, latest_known_total_da_cost_excess: self.latest_known_total_cost, diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index 123520b4e99..e4993e8f156 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -6,43 +6,72 @@ use crate::v1::{ Error, }; -#[test] -fn update_da_record_data__increases_block() { - // given - let da_recorded_block_height = 0; - let recorded_range = 1u32..3; - let recorded_cost = 200; - let unrecorded_blocks = vec![ - BlockBytes { - height: 1, - block_bytes: 1000, - }, - BlockBytes { - height: 2, - block_bytes: 2000, - }, - ]; - let mut updater = UpdaterBuilder::new() - .with_da_recorded_block_height(da_recorded_block_height) - .with_unrecorded_blocks(unrecorded_blocks) - .build(); +// #[test] +// fn update_da_record_data__increases_block() { +// // given +// let da_recorded_block_height = 0; +// let recorded_range = 1u32..3; +// let recorded_cost = 200; +// let unrecorded_blocks = vec![ +// BlockBytes { +// height: 1, +// block_bytes: 1000, +// }, +// BlockBytes { +// height: 2, +// block_bytes: 2000, +// }, +// ]; +// let mut updater = UpdaterBuilder::new() +// .with_da_recorded_block_height(da_recorded_block_height) +// .with_unrecorded_blocks(unrecorded_blocks) +// .build(); +// +// // when +// updater +// .update_da_record_data(recorded_range, recorded_cost) +// .unwrap(); +// +// // then +// let expected = 2; +// let actual = updater.da_recorded_block_height; +// assert_eq!(expected, actual); +// } - // when - updater - .update_da_record_data(recorded_range, recorded_cost) - .unwrap(); - - // then - let expected = 2; - let actual = updater.da_recorded_block_height; - assert_eq!(expected, actual); -} +// #[test] +// fn update_da_record_data__throws_error_if_out_of_order() { +// // given +// let da_recorded_block_height = 0; +// let bad_recorded_range = 2u32..4; +// let recorded_cost = 200; +// let unrecorded_blocks = vec![BlockBytes { +// height: 1, +// block_bytes: 1000, +// }]; +// let mut updater = UpdaterBuilder::new() +// .with_da_recorded_block_height(da_recorded_block_height) +// .with_unrecorded_blocks(unrecorded_blocks) +// .build(); +// +// // when +// let actual_error = updater +// .update_da_record_data(bad_recorded_range, recorded_cost) +// .unwrap_err(); +// +// // then +// let expected_error = Error::SkippedDABlock { +// expected: 1, +// got: 2, +// }; +// assert_eq!(actual_error, expected_error); +// } #[test] -fn update_da_record_data__throws_error_if_out_of_order() { +fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( +) { // given let da_recorded_block_height = 0; - let bad_recorded_range = 2u32..4; + let recorded_range = 1u32..3; let recorded_cost = 200; let unrecorded_blocks = vec![BlockBytes { height: 1, @@ -55,14 +84,11 @@ fn update_da_record_data__throws_error_if_out_of_order() { // when let actual_error = updater - .update_da_record_data(bad_recorded_range, recorded_cost) + .update_da_record_data(recorded_range, recorded_cost) .unwrap_err(); // then - let expected_error = Error::SkippedDABlock { - expected: 1, - got: 2, - }; + let expected_error = Error::L2BlockExpectedNotFound { height: 2 }; assert_eq!(actual_error, expected_error); } @@ -186,7 +212,7 @@ fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_mat .unwrap(); // then - assert_eq!(updater.l2_block_height, updater.da_recorded_block_height); + assert_eq!(updater.unrecorded_blocks.len(), 0); assert_eq!( updater.projected_total_da_cost, updater.latest_known_total_da_cost_excess diff --git a/crates/services/gas_price_service/src/v1/metadata.rs b/crates/services/gas_price_service/src/v1/metadata.rs index 422cdf42507..0ceae95a768 100644 --- a/crates/services/gas_price_service/src/v1/metadata.rs +++ b/crates/services/gas_price_service/src/v1/metadata.rs @@ -20,8 +20,6 @@ pub struct V1Metadata { pub gas_price_factor: NonZeroU64, /// The cumulative reward from the DA portion of the gas price pub total_da_rewards_excess: u128, - /// The height of the last L2 block recorded on the DA chain - pub da_recorded_block_height: u32, /// The cumulative cost of recording L2 blocks on the DA chain as of the last recorded block pub latest_known_total_da_cost_excess: u128, /// The predicted cost of recording L2 blocks on the DA chain as of the last L2 block @@ -53,9 +51,6 @@ impl V1Metadata { .saturating_mul(config.gas_price_factor.get()), gas_price_factor: config.gas_price_factor, total_da_rewards_excess: 0, - // TODO: Set to `None` after: - // https://github.com/FuelLabs/fuel-core/issues/2397 - da_recorded_block_height: 0, latest_known_total_da_cost_excess: 0, projected_total_da_cost: 0, last_profit: 0, @@ -91,7 +86,6 @@ impl From for V1Metadata { new_scaled_da_gas_price: updater.new_scaled_da_gas_price, gas_price_factor: updater.gas_price_factor, total_da_rewards_excess: updater.total_da_rewards_excess, - da_recorded_block_height: updater.da_recorded_block_height, latest_known_total_da_cost_excess: updater.latest_known_total_da_cost_excess, projected_total_da_cost: updater.projected_total_da_cost, last_profit: updater.last_profit, @@ -119,7 +113,6 @@ pub fn v1_algorithm_from_metadata( new_scaled_da_gas_price: metadata.new_scaled_da_gas_price, gas_price_factor: metadata.gas_price_factor, total_da_rewards_excess: metadata.total_da_rewards_excess, - da_recorded_block_height: metadata.da_recorded_block_height, latest_known_total_da_cost_excess: metadata.latest_known_total_da_cost_excess, projected_total_da_cost: metadata.projected_total_da_cost, last_profit: metadata.last_profit, From c5866a37c72453694185e52e6592d86befd41334 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 17:50:37 +0200 Subject: [PATCH 02/19] Modify to take vec instead of range --- crates/fuel-gas-price-algorithm/src/v1.rs | 17 ++--- .../v1/tests/update_da_record_data_tests.rs | 70 ++----------------- 2 files changed, 12 insertions(+), 75 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index e45e6dcae8f..bc824026c73 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -3,10 +3,7 @@ use std::{ cmp::max, collections::BTreeMap, num::NonZeroU64, - ops::{ - Div, - Range, - }, + ops::Div, }; #[cfg(test)] @@ -303,11 +300,11 @@ impl core::ops::Deref for ClampedPercentage { impl AlgorithmUpdaterV1 { pub fn update_da_record_data( &mut self, - height_range: Range, + heights: Vec, range_cost: u128, ) -> Result<(), Error> { - if !height_range.is_empty() { - self.da_block_update(height_range, range_cost)?; + if !heights.is_empty() { + self.da_block_update(heights, range_cost)?; self.recalculate_projected_cost(); } Ok(()) @@ -510,10 +507,10 @@ impl AlgorithmUpdaterV1 { fn da_block_update( &mut self, - height_range: Range, + heights: Vec, range_cost: u128, ) -> Result<(), Error> { - let range_bytes = self.drain_l2_block_bytes_for_range(height_range)?; + let range_bytes = self.drain_l2_block_bytes_for_range(heights)?; let new_cost_per_byte: u128 = range_cost.checked_div(range_bytes).ok_or( Error::CouldNotCalculateCostPerByte { bytes: range_bytes, @@ -530,7 +527,7 @@ impl AlgorithmUpdaterV1 { fn drain_l2_block_bytes_for_range( &mut self, - height_range: Range, + height_range: Vec, ) -> Result { let mut total: u128 = 0; for expected_height in height_range { diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index e4993e8f156..a18dab86fb3 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -6,72 +6,12 @@ use crate::v1::{ Error, }; -// #[test] -// fn update_da_record_data__increases_block() { -// // given -// let da_recorded_block_height = 0; -// let recorded_range = 1u32..3; -// let recorded_cost = 200; -// let unrecorded_blocks = vec![ -// BlockBytes { -// height: 1, -// block_bytes: 1000, -// }, -// BlockBytes { -// height: 2, -// block_bytes: 2000, -// }, -// ]; -// let mut updater = UpdaterBuilder::new() -// .with_da_recorded_block_height(da_recorded_block_height) -// .with_unrecorded_blocks(unrecorded_blocks) -// .build(); -// -// // when -// updater -// .update_da_record_data(recorded_range, recorded_cost) -// .unwrap(); -// -// // then -// let expected = 2; -// let actual = updater.da_recorded_block_height; -// assert_eq!(expected, actual); -// } - -// #[test] -// fn update_da_record_data__throws_error_if_out_of_order() { -// // given -// let da_recorded_block_height = 0; -// let bad_recorded_range = 2u32..4; -// let recorded_cost = 200; -// let unrecorded_blocks = vec![BlockBytes { -// height: 1, -// block_bytes: 1000, -// }]; -// let mut updater = UpdaterBuilder::new() -// .with_da_recorded_block_height(da_recorded_block_height) -// .with_unrecorded_blocks(unrecorded_blocks) -// .build(); -// -// // when -// let actual_error = updater -// .update_da_record_data(bad_recorded_range, recorded_cost) -// .unwrap_err(); -// -// // then -// let expected_error = Error::SkippedDABlock { -// expected: 1, -// got: 2, -// }; -// assert_eq!(actual_error, expected_error); -// } - #[test] fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( ) { // given let da_recorded_block_height = 0; - let recorded_range = 1u32..3; + let recorded_range = (1u32..3).collect(); let recorded_cost = 200; let unrecorded_blocks = vec![BlockBytes { height: 1, @@ -108,7 +48,7 @@ fn update_da_record_data__updates_cost_per_byte() { let new_cost_per_byte = 100; let recorded_cost = (block_bytes * new_cost_per_byte) as u128; - let recorded_range = 1u32..2; + let recorded_range = (1u32..2).collect(); // when updater .update_da_record_data(recorded_range, recorded_cost) @@ -151,7 +91,7 @@ fn update_da_record_data__updates_known_total_cost() { .with_unrecorded_blocks(unrecorded_blocks) .build(); - let recorded_range = 11u32..14; + let recorded_range = (11u32..14).collect(); let recorded_cost = 300; // when updater @@ -204,7 +144,7 @@ fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_mat let new_cost_per_byte = 100; let block_cost = block_bytes * new_cost_per_byte; - let recorded_range = 11u32..14; + let recorded_range = (11u32..14).collect(); let recorded_cost = block_cost * 3; // when updater @@ -279,7 +219,7 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range = *min..(max + 1); + let recorded_range = (*min..(max + 1)).collect(); // when updater From c54f09802168189dd0bc91128c0bbd3e046a5ec2 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 18:31:43 +0200 Subject: [PATCH 03/19] Add tests to make sure it is sane --- crates/fuel-gas-price-algorithm/src/v1.rs | 2 +- .../fuel-gas-price-algorithm/src/v1/tests.rs | 7 - .../v1/tests/update_da_record_data_tests.rs | 137 +++++++++++++++++- 3 files changed, 130 insertions(+), 16 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index bc824026c73..85b43defbbc 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -546,7 +546,7 @@ impl AlgorithmUpdaterV1 { let projection_portion: u128 = self .unrecorded_blocks .iter() - .map(|(_, &bytes)| (bytes as u128)) + .map(|(_, &bytes)| bytes as u128) .fold(0_u128, |acc, n| acc.saturating_add(n)) .saturating_mul(self.latest_da_cost_per_byte); self.projected_total_da_cost = self diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests.rs index c7d8c2c47b0..816c491cc4e 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests.rs @@ -35,7 +35,6 @@ pub struct UpdaterBuilder { l2_block_capacity_threshold: u8, total_rewards: u128, - da_recorded_block_height: u32, da_cost_per_byte: u128, project_total_cost: u128, latest_known_total_cost: u128, @@ -63,7 +62,6 @@ impl UpdaterBuilder { l2_block_capacity_threshold: 50, total_rewards: 0, - da_recorded_block_height: 0, da_cost_per_byte: 0, project_total_cost: 0, latest_known_total_cost: 0, @@ -133,11 +131,6 @@ impl UpdaterBuilder { self } - fn with_da_recorded_block_height(mut self, da_recorded_block_height: u32) -> Self { - self.da_recorded_block_height = da_recorded_block_height; - self - } - fn with_da_cost_per_byte(mut self, da_cost_per_byte: u128) -> Self { self.da_cost_per_byte = da_cost_per_byte; self diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index a18dab86fb3..c69919cfa4b 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -10,7 +10,6 @@ use crate::v1::{ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( ) { // given - let da_recorded_block_height = 0; let recorded_range = (1u32..3).collect(); let recorded_cost = 200; let unrecorded_blocks = vec![BlockBytes { @@ -18,7 +17,6 @@ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecord block_bytes: 1000, }]; let mut updater = UpdaterBuilder::new() - .with_da_recorded_block_height(da_recorded_block_height) .with_unrecorded_blocks(unrecorded_blocks) .build(); @@ -64,7 +62,6 @@ fn update_da_record_data__updates_cost_per_byte() { fn update_da_record_data__updates_known_total_cost() { // given let da_cost_per_byte = 20; - let da_recorded_block_height = 10; let l2_block_height = 15; let projected_total_cost = 2000; let known_total_cost = 1500; @@ -84,7 +81,6 @@ fn update_da_record_data__updates_known_total_cost() { ]; let mut updater = UpdaterBuilder::new() .with_da_cost_per_byte(da_cost_per_byte) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost) .with_known_total_cost(known_total_cost) @@ -108,7 +104,6 @@ fn update_da_record_data__updates_known_total_cost() { fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_match() { // given let da_cost_per_byte = 20; - let da_recorded_block_height = 10; let l2_block_height = 13; let known_total_cost = 1500; let unrecorded_blocks = vec![ @@ -133,7 +128,6 @@ fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_mat let projected_total_cost = known_total_cost + guessed_cost; let mut updater = UpdaterBuilder::new() .with_da_cost_per_byte(da_cost_per_byte as u128) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost as u128) .with_known_total_cost(known_total_cost as u128) @@ -164,7 +158,6 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g ) { // given let da_cost_per_byte = 20; - let da_recorded_block_height = 10; let l2_block_height = 15; let original_known_total_cost = 1500; let block_bytes = 1000; @@ -202,7 +195,6 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g let projected_total_cost = original_known_total_cost + guessed_cost; let mut updater = UpdaterBuilder::new() .with_da_cost_per_byte(da_cost_per_byte as u128) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost as u128) .with_known_total_cost(original_known_total_cost as u128) @@ -236,3 +228,132 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g let expected = new_known_total_cost + guessed_part; assert_eq!(actual, expected as u128); } + +#[test] +fn update_da_record_data__updates_known_total_cost_if_blocks_are_out_of_order() { + // given + let da_cost_per_byte = 20; + let block_bytes = 1000; + let unrecorded_blocks = vec![ + BlockBytes { + height: 1, + block_bytes, + }, + BlockBytes { + height: 2, + block_bytes, + }, + BlockBytes { + height: 3, + block_bytes, + }, + ]; + let old_known_total_cost = 500; + let old_projected_total_cost = + old_known_total_cost + (block_bytes as u128 * da_cost_per_byte * 3); + let old_da_cost_per_byte = 20; + let mut updater = UpdaterBuilder::new() + .with_da_cost_per_byte(da_cost_per_byte) + .with_unrecorded_blocks(unrecorded_blocks) + .with_da_cost_per_byte(old_da_cost_per_byte) + .with_known_total_cost(old_known_total_cost) + .with_projected_total_cost(old_projected_total_cost) + .build(); + let new_cost_per_byte = 100; + let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; + let recorded_range = vec![2, 3]; + + // when + updater + .update_da_record_data(recorded_range, recorded_cost) + .unwrap(); + + // and + let expected = updater.latest_known_total_da_cost_excess + + (block_bytes * new_cost_per_byte) as u128; + let actual = updater.projected_total_da_cost; + assert_eq!(actual, expected); +} +#[test] +fn update_da_record_data__updates_projected_total_cost_if_blocks_are_out_of_order() { + // given + let da_cost_per_byte = 20; + let block_bytes = 1000; + let unrecorded_blocks = vec![ + BlockBytes { + height: 1, + block_bytes, + }, + BlockBytes { + height: 2, + block_bytes, + }, + BlockBytes { + height: 3, + block_bytes, + }, + ]; + let old_known_total_cost = 500; + let old_projected_total_cost = + old_known_total_cost + (block_bytes as u128 * da_cost_per_byte * 3); + let old_da_cost_per_byte = 20; + let mut updater = UpdaterBuilder::new() + .with_da_cost_per_byte(da_cost_per_byte) + .with_unrecorded_blocks(unrecorded_blocks) + .with_da_cost_per_byte(old_da_cost_per_byte) + .with_known_total_cost(old_known_total_cost) + .with_projected_total_cost(old_projected_total_cost) + .build(); + let new_cost_per_byte = 100; + let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; + let recorded_range = vec![2, 3]; + + // when + updater + .update_da_record_data(recorded_range, recorded_cost) + .unwrap(); + + // then + let expected = updater.latest_known_total_da_cost_excess + + (block_bytes * new_cost_per_byte) as u128; + let actual = updater.projected_total_da_cost; + assert_eq!(actual, expected); +} + +#[test] +fn update_da_record_data__updates_unrecorded_blocks() { + // given + let da_cost_per_byte = 20; + let block_bytes = 1000; + let unrecorded_blocks = vec![ + BlockBytes { + height: 1, + block_bytes, + }, + BlockBytes { + height: 2, + block_bytes, + }, + BlockBytes { + height: 3, + block_bytes, + }, + ]; + let mut updater = UpdaterBuilder::new() + .with_da_cost_per_byte(da_cost_per_byte) + .with_unrecorded_blocks(unrecorded_blocks) + .build(); + let new_cost_per_byte = 100; + let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; + let recorded_range = vec![2, 3]; + + // when + updater + .update_da_record_data(recorded_range, recorded_cost) + .unwrap(); + + // then + let expected = vec![(1, block_bytes)]; + let actual: Vec<_> = updater.unrecorded_blocks.into_iter().collect(); + assert_eq!(actual, expected); +} From fcf44777e9c13573e9e7d0a345113089fac49131 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 18:36:07 +0200 Subject: [PATCH 04/19] Rename variable --- crates/fuel-gas-price-algorithm/src/v1.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 85b43defbbc..b4b690763cb 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -510,10 +510,10 @@ impl AlgorithmUpdaterV1 { heights: Vec, range_cost: u128, ) -> Result<(), Error> { - let range_bytes = self.drain_l2_block_bytes_for_range(heights)?; - let new_cost_per_byte: u128 = range_cost.checked_div(range_bytes).ok_or( + let recorded_bytes = self.drain_l2_block_bytes_for_range(heights)?; + let new_cost_per_byte: u128 = range_cost.checked_div(recorded_bytes).ok_or( Error::CouldNotCalculateCostPerByte { - bytes: range_bytes, + bytes: recorded_bytes, cost: range_cost, }, )?; From 159d5b4c68f325d27c16643a0ffc27d212c90a95 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 18:38:37 +0200 Subject: [PATCH 05/19] Fix analyzer --- .../gas-price-analysis/src/charts.rs | 6 +++--- .../gas-price-analysis/src/simulation.rs | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs index 6b4c82f09ff..3c8b1175b05 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs @@ -268,15 +268,15 @@ pub fn draw_profit( const PROJECTED_PROFIT_COLOR: RGBColor = RED; const PESSIMISTIC_BLOCK_COST_COLOR: RGBColor = BLUE; let actual_profit_gwei: Vec<_> = actual_profit - .into_iter() + .iter() .map(|x| x / ONE_GWEI as i128) .collect(); let projected_profit_gwei: Vec<_> = projected_profit - .into_iter() + .iter() .map(|x| x / ONE_GWEI as i128) .collect(); let pessimistic_block_costs_gwei: Vec<_> = pessimistic_block_costs - .into_iter() + .iter() .map(|x| x / ONE_GWEI as u128) .collect(); let min = *std::cmp::min( diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs index 590952207ba..efca39438c3 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs @@ -103,7 +103,6 @@ impl Simulator { // Increase to make the da price change faster max_da_gas_price_change_percent: 10, total_da_rewards_excess: 0, - da_recorded_block_height: 0, // Change to adjust the cost per byte of the DA on block 0 latest_da_cost_per_byte: 0, projected_total_da_cost: 0, @@ -162,9 +161,8 @@ impl Simulator { // Update DA blocks on the occasion there is one if let Some((range, cost)) = da_block { for height in range.to_owned() { - updater - .update_da_record_data(height..(height + 1), cost) - .unwrap(); + let block_heights = (height..(height) + 1).collect(); + updater.update_da_record_data(block_heights, cost).unwrap(); actual_costs.push(updater.latest_known_total_da_cost_excess) } } From ba080ff3989d7e5dc5b36894417048654ed2ca7c Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 18:41:35 +0200 Subject: [PATCH 06/19] Rename some params --- .../gas-price-analysis/src/charts.rs | 6 ++---- crates/fuel-gas-price-algorithm/src/v1.rs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs index 3c8b1175b05..c0ce77d261c 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs @@ -267,10 +267,8 @@ pub fn draw_profit( const ACTUAL_PROFIT_COLOR: RGBColor = BLACK; const PROJECTED_PROFIT_COLOR: RGBColor = RED; const PESSIMISTIC_BLOCK_COST_COLOR: RGBColor = BLUE; - let actual_profit_gwei: Vec<_> = actual_profit - .iter() - .map(|x| x / ONE_GWEI as i128) - .collect(); + let actual_profit_gwei: Vec<_> = + actual_profit.iter().map(|x| x / ONE_GWEI as i128).collect(); let projected_profit_gwei: Vec<_> = projected_profit .iter() .map(|x| x / ONE_GWEI as i128) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index b4b690763cb..60d2142eabd 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -301,10 +301,10 @@ impl AlgorithmUpdaterV1 { pub fn update_da_record_data( &mut self, heights: Vec, - range_cost: u128, + recording_cost: u128, ) -> Result<(), Error> { if !heights.is_empty() { - self.da_block_update(heights, range_cost)?; + self.da_block_update(heights, recording_cost)?; self.recalculate_projected_cost(); } Ok(()) @@ -508,18 +508,18 @@ impl AlgorithmUpdaterV1 { fn da_block_update( &mut self, heights: Vec, - range_cost: u128, + recording_cost: u128, ) -> Result<(), Error> { let recorded_bytes = self.drain_l2_block_bytes_for_range(heights)?; - let new_cost_per_byte: u128 = range_cost.checked_div(recorded_bytes).ok_or( + let new_cost_per_byte: u128 = recording_cost.checked_div(recorded_bytes).ok_or( Error::CouldNotCalculateCostPerByte { bytes: recorded_bytes, - cost: range_cost, + cost: recording_cost, }, )?; let new_da_block_cost = self .latest_known_total_da_cost_excess - .saturating_add(range_cost); + .saturating_add(recording_cost); self.latest_known_total_da_cost_excess = new_da_block_cost; self.latest_da_cost_per_byte = new_cost_per_byte; Ok(()) @@ -527,10 +527,10 @@ impl AlgorithmUpdaterV1 { fn drain_l2_block_bytes_for_range( &mut self, - height_range: Vec, + heights: Vec, ) -> Result { let mut total: u128 = 0; - for expected_height in height_range { + for expected_height in heights { let bytes = self.unrecorded_blocks.remove(&expected_height).ok_or( Error::L2BlockExpectedNotFound { height: expected_height, From d2b3c280c0b1abb2a0e7105172aebcbd63102d6c Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 30 Oct 2024 19:31:52 +0200 Subject: [PATCH 07/19] Use slice instead of vec --- .../gas-price-analysis/src/simulation.rs | 4 +-- crates/fuel-gas-price-algorithm/src/v1.rs | 13 +++----- .../v1/tests/update_da_record_data_tests.rs | 32 +++++++++---------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs index efca39438c3..22d994a7a68 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs @@ -161,8 +161,8 @@ impl Simulator { // Update DA blocks on the occasion there is one if let Some((range, cost)) = da_block { for height in range.to_owned() { - let block_heights = (height..(height) + 1).collect(); - updater.update_da_record_data(block_heights, cost).unwrap(); + let block_heights: Vec = (height..(height) + 1).collect(); + updater.update_da_record_data(&block_heights, cost).unwrap(); actual_costs.push(updater.latest_known_total_da_cost_excess) } } diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 60d2142eabd..56764e0ae1c 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -300,7 +300,7 @@ impl core::ops::Deref for ClampedPercentage { impl AlgorithmUpdaterV1 { pub fn update_da_record_data( &mut self, - heights: Vec, + heights: &[u32], recording_cost: u128, ) -> Result<(), Error> { if !heights.is_empty() { @@ -507,7 +507,7 @@ impl AlgorithmUpdaterV1 { fn da_block_update( &mut self, - heights: Vec, + heights: &[u32], recording_cost: u128, ) -> Result<(), Error> { let recorded_bytes = self.drain_l2_block_bytes_for_range(heights)?; @@ -525,15 +525,12 @@ impl AlgorithmUpdaterV1 { Ok(()) } - fn drain_l2_block_bytes_for_range( - &mut self, - heights: Vec, - ) -> Result { + fn drain_l2_block_bytes_for_range(&mut self, heights: &[u32]) -> Result { let mut total: u128 = 0; for expected_height in heights { - let bytes = self.unrecorded_blocks.remove(&expected_height).ok_or( + let bytes = self.unrecorded_blocks.remove(expected_height).ok_or( Error::L2BlockExpectedNotFound { - height: expected_height, + height: *expected_height, }, )?; total = total.saturating_add(bytes as u128); diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index c69919cfa4b..8ec3356fadb 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -10,7 +10,7 @@ use crate::v1::{ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( ) { // given - let recorded_range = (1u32..3).collect(); + let recorded_range: Vec = (1u32..3).collect(); let recorded_cost = 200; let unrecorded_blocks = vec![BlockBytes { height: 1, @@ -22,7 +22,7 @@ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecord // when let actual_error = updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap_err(); // then @@ -46,10 +46,10 @@ fn update_da_record_data__updates_cost_per_byte() { let new_cost_per_byte = 100; let recorded_cost = (block_bytes * new_cost_per_byte) as u128; - let recorded_range = (1u32..2).collect(); + let recorded_range: Vec = (1u32..2).collect(); // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // then @@ -87,11 +87,11 @@ fn update_da_record_data__updates_known_total_cost() { .with_unrecorded_blocks(unrecorded_blocks) .build(); - let recorded_range = (11u32..14).collect(); + let recorded_range: Vec = (11u32..14).collect(); let recorded_cost = 300; // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // then @@ -138,11 +138,11 @@ fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_mat let new_cost_per_byte = 100; let block_cost = block_bytes * new_cost_per_byte; - let recorded_range = (11u32..14).collect(); + let recorded_range: Vec = (11u32..14).collect(); let recorded_cost = block_cost * 3; // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // then @@ -211,11 +211,11 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range = (*min..(max + 1)).collect(); + let recorded_range: Vec = (*min..(max + 1)).collect(); // when updater - .update_da_record_data(recorded_range, recorded_cost as u128) + .update_da_record_data(&recorded_range, recorded_cost as u128) .unwrap(); // then @@ -261,11 +261,11 @@ fn update_da_record_data__updates_known_total_cost_if_blocks_are_out_of_order() .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range = vec![2, 3]; + let recorded_range: Vec = vec![2, 3]; // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // and @@ -306,11 +306,11 @@ fn update_da_record_data__updates_projected_total_cost_if_blocks_are_out_of_orde .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range = vec![2, 3]; + let recorded_range: Vec = vec![2, 3]; // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // then @@ -345,11 +345,11 @@ fn update_da_record_data__updates_unrecorded_blocks() { .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range = vec![2, 3]; + let recorded_range: Vec = vec![2, 3]; // when updater - .update_da_record_data(recorded_range, recorded_cost) + .update_da_record_data(&recorded_range, recorded_cost) .unwrap(); // then From 5f199323422386e91373282e131c9c69e760b111 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Thu, 31 Oct 2024 11:38:35 +0200 Subject: [PATCH 08/19] Cleanup code a bit --- .../gas-price-analysis/src/charts.rs | 14 ++++++-- crates/fuel-gas-price-algorithm/src/v1.rs | 9 +++-- .../v1/tests/update_da_record_data_tests.rs | 34 +++++++++---------- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs index c0ce77d261c..7b884357dfc 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/charts.rs @@ -257,6 +257,14 @@ pub fn draw_bytes_and_cost_per_block( Ok(()) } +fn one_gwei_i128() -> i128 { + i128::from(ONE_GWEI) +} + +fn one_gwei_u128() -> u128 { + u128::from(ONE_GWEI) +} + pub fn draw_profit( drawing_area: &DrawingArea, actual_profit: &[i128], @@ -268,14 +276,14 @@ pub fn draw_profit( const PROJECTED_PROFIT_COLOR: RGBColor = RED; const PESSIMISTIC_BLOCK_COST_COLOR: RGBColor = BLUE; let actual_profit_gwei: Vec<_> = - actual_profit.iter().map(|x| x / ONE_GWEI as i128).collect(); + actual_profit.iter().map(|x| x / one_gwei_i128()).collect(); let projected_profit_gwei: Vec<_> = projected_profit .iter() - .map(|x| x / ONE_GWEI as i128) + .map(|x| x / one_gwei_i128()) .collect(); let pessimistic_block_costs_gwei: Vec<_> = pessimistic_block_costs .iter() - .map(|x| x / ONE_GWEI as u128) + .map(|x| x / one_gwei_u128()) .collect(); let min = *std::cmp::min( actual_profit_gwei diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 56764e0ae1c..034898c984a 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -510,7 +510,7 @@ impl AlgorithmUpdaterV1 { heights: &[u32], recording_cost: u128, ) -> Result<(), Error> { - let recorded_bytes = self.drain_l2_block_bytes_for_range(heights)?; + let recorded_bytes = self.drain_l2_block_bytes_for_heights(heights)?; let new_cost_per_byte: u128 = recording_cost.checked_div(recorded_bytes).ok_or( Error::CouldNotCalculateCostPerByte { bytes: recorded_bytes, @@ -525,7 +525,10 @@ impl AlgorithmUpdaterV1 { Ok(()) } - fn drain_l2_block_bytes_for_range(&mut self, heights: &[u32]) -> Result { + fn drain_l2_block_bytes_for_heights( + &mut self, + heights: &[u32], + ) -> Result { let mut total: u128 = 0; for expected_height in heights { let bytes = self.unrecorded_blocks.remove(expected_height).ok_or( @@ -543,7 +546,7 @@ impl AlgorithmUpdaterV1 { let projection_portion: u128 = self .unrecorded_blocks .iter() - .map(|(_, &bytes)| bytes as u128) + .map(|(_, &bytes)| u128::from(bytes)) .fold(0_u128, |acc, n| acc.saturating_add(n)) .saturating_mul(self.latest_da_cost_per_byte); self.projected_total_da_cost = self diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index 8ec3356fadb..1da66b2903d 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -10,7 +10,7 @@ use crate::v1::{ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( ) { // given - let recorded_range: Vec = (1u32..3).collect(); + let recorded_heights: Vec = (1u32..3).collect(); let recorded_cost = 200; let unrecorded_blocks = vec![BlockBytes { height: 1, @@ -22,7 +22,7 @@ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecord // when let actual_error = updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap_err(); // then @@ -46,10 +46,10 @@ fn update_da_record_data__updates_cost_per_byte() { let new_cost_per_byte = 100; let recorded_cost = (block_bytes * new_cost_per_byte) as u128; - let recorded_range: Vec = (1u32..2).collect(); + let recorded_heights: Vec = (1u32..2).collect(); // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); // then @@ -87,11 +87,11 @@ fn update_da_record_data__updates_known_total_cost() { .with_unrecorded_blocks(unrecorded_blocks) .build(); - let recorded_range: Vec = (11u32..14).collect(); + let recorded_heights: Vec = (11u32..14).collect(); let recorded_cost = 300; // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); // then @@ -138,11 +138,11 @@ fn update_da_record_data__if_da_height_matches_l2_height_projected_and_known_mat let new_cost_per_byte = 100; let block_cost = block_bytes * new_cost_per_byte; - let recorded_range: Vec = (11u32..14).collect(); + let recorded_heights: Vec = (11u32..14).collect(); let recorded_cost = block_cost * 3; // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); // then @@ -211,11 +211,11 @@ fn update_da_record_data__da_block_updates_projected_total_cost_with_known_and_g }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range: Vec = (*min..(max + 1)).collect(); + let recorded_heights: Vec = (*min..(max + 1)).collect(); // when updater - .update_da_record_data(&recorded_range, recorded_cost as u128) + .update_da_record_data(&recorded_heights, recorded_cost as u128) .unwrap(); // then @@ -261,14 +261,14 @@ fn update_da_record_data__updates_known_total_cost_if_blocks_are_out_of_order() .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range: Vec = vec![2, 3]; + let recorded_heights: Vec = vec![3, 2]; // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); - // and + // then let expected = updater.latest_known_total_da_cost_excess + (block_bytes * new_cost_per_byte) as u128; let actual = updater.projected_total_da_cost; @@ -306,11 +306,11 @@ fn update_da_record_data__updates_projected_total_cost_if_blocks_are_out_of_orde .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range: Vec = vec![2, 3]; + let recorded_heights: Vec = vec![3, 2]; // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); // then @@ -345,11 +345,11 @@ fn update_da_record_data__updates_unrecorded_blocks() { .build(); let new_cost_per_byte = 100; let recorded_cost = 2 * (block_bytes * new_cost_per_byte) as u128; - let recorded_range: Vec = vec![2, 3]; + let recorded_heights: Vec = vec![3, 2]; // when updater - .update_da_record_data(&recorded_range, recorded_cost) + .update_da_record_data(&recorded_heights, recorded_cost) .unwrap(); // then From 922f3e00126b51766e344d8834a99accd5c198bf Mon Sep 17 00:00:00 2001 From: Mitchell Turner Date: Thu, 31 Oct 2024 12:31:22 +0200 Subject: [PATCH 09/19] Update crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rafał Chabowski <88321181+rafal-ch@users.noreply.github.com> --- .../src/v1/tests/update_da_record_data_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index 1da66b2903d..f826a092a34 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -274,6 +274,7 @@ fn update_da_record_data__updates_known_total_cost_if_blocks_are_out_of_order() let actual = updater.projected_total_da_cost; assert_eq!(actual, expected); } + #[test] fn update_da_record_data__updates_projected_total_cost_if_blocks_are_out_of_order() { // given From d7e9bddde76025618676674fe43704636f77c186 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Thu, 31 Oct 2024 12:32:59 +0200 Subject: [PATCH 10/19] Pick nit --- crates/fuel-gas-price-algorithm/src/v1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 034898c984a..8a4b98bb1af 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -17,7 +17,7 @@ pub enum Error { CouldNotCalculateCostPerByte { bytes: u128, cost: u128 }, #[error("Failed to include L2 block data: {0}")] FailedTooIncludeL2BlockData(String), - #[error("L2 block expected but not found in unrecorded blocks: {height:?}")] + #[error("L2 block expected but not found in unrecorded blocks: {height}")] L2BlockExpectedNotFound { height: u32 }, } From 2bb86fe406840c0cd2944ba0cb3356f6b790badf Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Fri, 1 Nov 2024 10:24:05 +0200 Subject: [PATCH 11/19] Optimize projected calc by caching running total --- crates/fuel-gas-price-algorithm/src/v1.rs | 14 ++++++++------ crates/fuel-gas-price-algorithm/src/v1/tests.rs | 4 ++++ .../src/v1/tests/update_l2_block_data_tests.rs | 15 +++++++++++---- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 8a4b98bb1af..bfc1433a6b5 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -145,6 +145,8 @@ pub struct AlgorithmUpdaterV1 { pub l2_activity: L2ActivityTracker, /// The unrecorded blocks that are used to calculate the projected cost of recording blocks pub unrecorded_blocks: BTreeMap, + /// Total unrecorded block bytes + pub unrecorded_blocks_bytes: u128, } /// The `L2ActivityTracker` tracks the chain activity to determine a safety mode for setting the DA price. @@ -348,6 +350,9 @@ impl AlgorithmUpdaterV1 { // metadata self.unrecorded_blocks.insert(height, block_bytes); + self.unrecorded_blocks_bytes = self + .unrecorded_blocks_bytes + .saturating_add(block_bytes as u128); Ok(()) } } @@ -538,16 +543,13 @@ impl AlgorithmUpdaterV1 { )?; total = total.saturating_add(bytes as u128); } + self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total); Ok(total) } fn recalculate_projected_cost(&mut self) { - // add the cost of the remaining blocks - let projection_portion: u128 = self - .unrecorded_blocks - .iter() - .map(|(_, &bytes)| u128::from(bytes)) - .fold(0_u128, |acc, n| acc.saturating_add(n)) + let projection_portion = self + .unrecorded_blocks_bytes .saturating_mul(self.latest_da_cost_per_byte); self.projected_total_da_cost = self .latest_known_total_da_cost_excess diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests.rs index 816c491cc4e..9e3dc879651 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests.rs @@ -193,6 +193,10 @@ impl UpdaterBuilder { .try_into() .expect("Should never be non-zero"), l2_activity: self.l2_activity, + unrecorded_blocks_bytes: self + .unrecorded_blocks + .iter() + .fold(0u128, |acc, b| acc + u128::from(b.block_bytes)), } } } diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_l2_block_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_l2_block_data_tests.rs index 5cb56bbde2d..8ce3b6b5d4e 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_l2_block_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_l2_block_data_tests.rs @@ -591,9 +591,10 @@ fn update_l2_block_data__retains_existing_blocks_and_adds_l2_block_to_unrecorded { // given let starting_block = 0; + let first_block_bytes = 1200; let preexisting_block = BlockBytes { height: 0, - block_bytes: 1000, + block_bytes: first_block_bytes, }; let mut updater = UpdaterBuilder::new() @@ -604,27 +605,33 @@ fn update_l2_block_data__retains_existing_blocks_and_adds_l2_block_to_unrecorded let height = 1; let used = 50; let capacity = 100.try_into().unwrap(); - let block_bytes = 1000; + let new_block_bytes = 1000; let new_gas_price = 100; // when updater - .update_l2_block_data(height, used, capacity, block_bytes, new_gas_price) + .update_l2_block_data(height, used, capacity, new_block_bytes, new_gas_price) .unwrap(); // then let block_bytes = BlockBytes { height, - block_bytes, + block_bytes: new_block_bytes, }; let contains_block_bytes = updater.unrecorded_blocks.contains_key(&block_bytes.height); assert!(contains_block_bytes); + // and let contains_preexisting_block_bytes = updater .unrecorded_blocks .contains_key(&preexisting_block.height); assert!(contains_preexisting_block_bytes); + + // and + let expected = first_block_bytes + new_block_bytes; + let actual = updater.unrecorded_blocks_bytes; + assert_eq!(expected as u128, actual); } fn capped_l2_activity_tracker() -> L2ActivityTracker { From 03da96395a3e72ada6d4d799e5ce7507d5a07ea0 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Fri, 1 Nov 2024 10:32:59 +0200 Subject: [PATCH 12/19] Fix analyzer --- .../gas-price-analysis/src/simulation.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs index 485da6b5ba2..92003cebb19 100644 --- a/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs +++ b/crates/fuel-gas-price-algorithm/gas-price-analysis/src/simulation.rs @@ -113,6 +113,7 @@ impl Simulator { last_profit: 0, second_to_last_profit: 0, l2_activity: always_normal_activity, + unrecorded_blocks_bytes: 0, } } From ea14e6f5dc2608c55c64cba6570b5253bbd264ad Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Fri, 1 Nov 2024 10:43:52 +0200 Subject: [PATCH 13/19] Simplify metadata --- .../gas_price_service/src/v1/metadata.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/services/gas_price_service/src/v1/metadata.rs b/crates/services/gas_price_service/src/v1/metadata.rs index 0ceae95a768..3fbdf24fadf 100644 --- a/crates/services/gas_price_service/src/v1/metadata.rs +++ b/crates/services/gas_price_service/src/v1/metadata.rs @@ -22,9 +22,6 @@ pub struct V1Metadata { pub total_da_rewards_excess: u128, /// The cumulative cost of recording L2 blocks on the DA chain as of the last recorded block pub latest_known_total_da_cost_excess: u128, - /// The predicted cost of recording L2 blocks on the DA chain as of the last L2 block - /// (This value is added on top of the `latest_known_total_da_cost` if the L2 height is higher) - pub projected_total_da_cost: u128, /// The last profit pub last_profit: i128, /// The profit before last @@ -52,7 +49,6 @@ impl V1Metadata { gas_price_factor: config.gas_price_factor, total_da_rewards_excess: 0, latest_known_total_da_cost_excess: 0, - projected_total_da_cost: 0, last_profit: 0, second_to_last_profit: 0, latest_da_cost_per_byte: 0, @@ -87,7 +83,6 @@ impl From for V1Metadata { gas_price_factor: updater.gas_price_factor, total_da_rewards_excess: updater.total_da_rewards_excess, latest_known_total_da_cost_excess: updater.latest_known_total_da_cost_excess, - projected_total_da_cost: updater.projected_total_da_cost, last_profit: updater.last_profit, second_to_last_profit: updater.second_to_last_profit, latest_da_cost_per_byte: updater.latest_da_cost_per_byte, @@ -106,6 +101,16 @@ pub fn v1_algorithm_from_metadata( config.decrease_range_size, config.block_activity_threshold.into(), ); + let unrecorded_blocks_bytes: u128 = metadata + .unrecorded_blocks + .iter() + .map(|(_, size)| u128::from(*size)) + .sum(); + let projected_portion = + unrecorded_blocks_bytes.saturating_mul(metadata.latest_da_cost_per_byte); + let projected_total_da_cost = metadata + .latest_known_total_da_cost_excess + .saturating_add(projected_portion); let unrecorded_blocks = metadata.unrecorded_blocks.into_iter().collect(); AlgorithmUpdaterV1 { new_scaled_exec_price: metadata.new_scaled_exec_price, @@ -114,7 +119,7 @@ pub fn v1_algorithm_from_metadata( gas_price_factor: metadata.gas_price_factor, total_da_rewards_excess: metadata.total_da_rewards_excess, latest_known_total_da_cost_excess: metadata.latest_known_total_da_cost_excess, - projected_total_da_cost: metadata.projected_total_da_cost, + projected_total_da_cost, last_profit: metadata.last_profit, second_to_last_profit: metadata.second_to_last_profit, latest_da_cost_per_byte: metadata.latest_da_cost_per_byte, @@ -129,5 +134,6 @@ pub fn v1_algorithm_from_metadata( da_p_component: config.da_p_component, da_d_component: config.da_d_component, unrecorded_blocks, + unrecorded_blocks_bytes, } } From d983891b99b6e51109d884f9c5cf73a9adacd124 Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Sun, 17 Nov 2024 15:58:23 +0700 Subject: [PATCH 14/19] Resolve logical conflicts from merge --- .../v1/tests/update_da_record_data_tests.rs | 18 +++++---------- .../src/v1/da_source_service.rs | 4 ++-- .../block_committer_costs.rs | 22 +++++++++++++------ .../gas_price_service/src/v1/metadata.rs | 14 +++++++++--- .../gas_price_service/src/v1/service.rs | 4 ++-- 5 files changed, 36 insertions(+), 26 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index 0032c278e12..c88da67601b 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -363,7 +363,6 @@ fn update_da_record_data__updates_unrecorded_blocks() { fn update_da_record_data__da_block_lowers_da_gas_price() { // given let da_cost_per_byte = 40; - let da_recorded_block_height = 10; let l2_block_height = 11; let original_known_total_cost = 150; let unrecorded_blocks = vec![BlockBytes { @@ -381,7 +380,6 @@ fn update_da_record_data__da_block_lowers_da_gas_price() { .with_da_cost_per_byte(da_cost_per_byte as u128) .with_da_p_component(da_p_component) .with_last_profit(10, 0) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost as u128) .with_known_total_cost(original_known_total_cost as u128) @@ -398,13 +396,13 @@ fn update_da_record_data__da_block_lowers_da_gas_price() { }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range = *min..(max + 1); + let recorded_range: Vec = (*min..(max + 1)).collect(); let old_da_gas_price = updater.new_scaled_da_gas_price; // when updater - .update_da_record_data(recorded_range, recorded_cost as u128) + .update_da_record_data(&recorded_range, recorded_cost as u128) .unwrap(); // then @@ -418,7 +416,6 @@ fn update_da_record_data__da_block_lowers_da_gas_price() { fn update_da_record_data__da_block_increases_da_gas_price() { // given let da_cost_per_byte = 40; - let da_recorded_block_height = 10; let l2_block_height = 11; let original_known_total_cost = 150; let unrecorded_blocks = vec![BlockBytes { @@ -436,7 +433,6 @@ fn update_da_record_data__da_block_increases_da_gas_price() { .with_da_cost_per_byte(da_cost_per_byte as u128) .with_da_p_component(da_p_component) .with_last_profit(-10, 0) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost as u128) .with_known_total_cost(original_known_total_cost as u128) @@ -453,13 +449,13 @@ fn update_da_record_data__da_block_increases_da_gas_price() { }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range = *min..(max + 1); + let recorded_range: Vec = (*min..(max + 1)).collect(); let old_da_gas_price = updater.new_scaled_da_gas_price; // when updater - .update_da_record_data(recorded_range, recorded_cost as u128) + .update_da_record_data(&recorded_range, recorded_cost as u128) .unwrap(); // then @@ -473,7 +469,6 @@ fn update_da_record_data__da_block_increases_da_gas_price() { fn update_da_record_data__da_block_will_not_change_da_gas_price() { // given let da_cost_per_byte = 40; - let da_recorded_block_height = 10; let l2_block_height = 11; let original_known_total_cost = 150; let unrecorded_blocks = vec![BlockBytes { @@ -491,7 +486,6 @@ fn update_da_record_data__da_block_will_not_change_da_gas_price() { .with_da_cost_per_byte(da_cost_per_byte as u128) .with_da_p_component(da_p_component) .with_last_profit(0, 0) - .with_da_recorded_block_height(da_recorded_block_height) .with_l2_block_height(l2_block_height) .with_projected_total_cost(projected_total_cost as u128) .with_known_total_cost(original_known_total_cost as u128) @@ -508,13 +502,13 @@ fn update_da_record_data__da_block_will_not_change_da_gas_price() { }); let min = recorded_heights.iter().min().unwrap(); let max = recorded_heights.iter().max().unwrap(); - let recorded_range = *min..(max + 1); + let recorded_range: Vec = (*min..(max + 1)).collect(); let old_da_gas_price = updater.new_scaled_da_gas_price; // when updater - .update_da_record_data(recorded_range, recorded_cost as u128) + .update_da_record_data(&recorded_range, recorded_cost as u128) .unwrap(); // then diff --git a/crates/services/gas_price_service/src/v1/da_source_service.rs b/crates/services/gas_price_service/src/v1/da_source_service.rs index 840345bff98..ff2c703b518 100644 --- a/crates/services/gas_price_service/src/v1/da_source_service.rs +++ b/crates/services/gas_price_service/src/v1/da_source_service.rs @@ -7,7 +7,7 @@ pub mod service; #[derive(Debug, Default, Clone, Eq, Hash, PartialEq)] pub struct DaBlockCosts { - pub l2_block_range: core::ops::Range, + pub l2_blocks: Vec, pub blob_size_bytes: u32, pub blob_cost_wei: u128, } @@ -30,7 +30,7 @@ mod tests { async fn run__when_da_block_cost_source_gives_value_shared_state_is_updated() { // given let expected_da_cost = DaBlockCosts { - l2_block_range: 0..10, + l2_blocks: (0..10).collect(), blob_size_bytes: 1024 * 128, blob_cost_wei: 2, }; diff --git a/crates/services/gas_price_service/src/v1/da_source_service/block_committer_costs.rs b/crates/services/gas_price_service/src/v1/da_source_service/block_committer_costs.rs index e0629cc0edd..111e911b855 100644 --- a/crates/services/gas_price_service/src/v1/da_source_service/block_committer_costs.rs +++ b/crates/services/gas_price_service/src/v1/da_source_service/block_committer_costs.rs @@ -42,7 +42,7 @@ pub struct RawDaBlockCosts { /// Sequence number (Monotonically increasing nonce) pub sequence_number: u32, /// The range of blocks that the costs apply to - pub blocks_range: core::ops::Range, + pub blocks_heights: Vec, /// The DA block height of the last transaction for the range of blocks pub da_block_height: DaBlockHeight, /// Rolling sum cost of posting blobs (wei) @@ -54,7 +54,11 @@ pub struct RawDaBlockCosts { impl From<&RawDaBlockCosts> for DaBlockCosts { fn from(raw_da_block_costs: &RawDaBlockCosts) -> Self { DaBlockCosts { - l2_block_range: raw_da_block_costs.blocks_range.clone(), + l2_blocks: raw_da_block_costs + .blocks_heights + .clone() + .into_iter() + .collect(), blob_size_bytes: raw_da_block_costs.total_size_bytes, blob_cost_wei: raw_da_block_costs.total_cost, } @@ -198,8 +202,11 @@ mod tests { let mut value = self.value.clone(); if let Some(value) = &mut value { value.sequence_number = seq_no; - value.blocks_range = - value.blocks_range.end * seq_no..value.blocks_range.end * seq_no + 10; + value.blocks_heights = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + .to_vec() + .iter() + .map(|x| x * seq_no) + .collect(); value.da_block_height = value.da_block_height + ((seq_no + 1) as u64).into(); value.total_cost += 1; @@ -218,7 +225,7 @@ mod tests { fn test_da_block_costs() -> RawDaBlockCosts { RawDaBlockCosts { sequence_number: 1, - blocks_range: 0..10, + blocks_heights: (0..10).collect(), da_block_height: 1u64.into(), total_cost: 1, total_size_bytes: 1, @@ -254,7 +261,7 @@ mod tests { let actual = block_committer.request_da_block_cost().await.unwrap(); // then - assert_ne!(da_block_costs.blocks_range, actual.l2_block_range); + assert_ne!(da_block_costs.blocks_heights, actual.l2_blocks); } #[tokio::test] @@ -293,7 +300,8 @@ mod tests { let mut value = self.value.clone(); if let Some(value) = &mut value { value.sequence_number = seq_no; - value.blocks_range = value.blocks_range.end..value.blocks_range.end + 10; + value.blocks_heights = + value.blocks_heights.iter().map(|x| x + seq_no).collect(); value.da_block_height = value.da_block_height + 1u64.into(); value.total_cost -= 1; value.total_size_bytes -= 1; diff --git a/crates/services/gas_price_service/src/v1/metadata.rs b/crates/services/gas_price_service/src/v1/metadata.rs index 3e9665cd7b5..68253981eec 100644 --- a/crates/services/gas_price_service/src/v1/metadata.rs +++ b/crates/services/gas_price_service/src/v1/metadata.rs @@ -3,7 +3,10 @@ use fuel_gas_price_algorithm::v1::{ AlgorithmUpdaterV1, L2ActivityTracker, }; -use std::num::NonZeroU64; +use std::{ + collections::BTreeMap, + num::NonZeroU64, +}; #[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] pub struct V1Metadata { @@ -82,14 +85,18 @@ impl From<&V1AlgorithmConfig> for AlgorithmUpdaterV1 { value.decrease_range_size, value.block_activity_threshold.into(), ); - let unrecorded_blocks = value.unrecorded_blocks.clone().into_iter().collect(); + let unrecorded_blocks: BTreeMap<_, _> = + value.unrecorded_blocks.clone().into_iter().collect(); + let unrecorded_blocks_bytes: u128 = unrecorded_blocks + .values() + .map(|size| u128::from(*size)) + .sum(); Self { new_scaled_exec_price: value.new_exec_gas_price, l2_block_height: 0, new_scaled_da_gas_price: value.min_da_gas_price, gas_price_factor: value.gas_price_factor, total_da_rewards_excess: 0, - da_recorded_block_height: 0, latest_known_total_da_cost_excess: 0, projected_total_da_cost: 0, last_profit: 0, @@ -106,6 +113,7 @@ impl From<&V1AlgorithmConfig> for AlgorithmUpdaterV1 { da_p_component: value.da_p_component, da_d_component: value.da_d_component, unrecorded_blocks, + unrecorded_blocks_bytes, } } } diff --git a/crates/services/gas_price_service/src/v1/service.rs b/crates/services/gas_price_service/src/v1/service.rs index 5ab56160334..21ff0ffe4e6 100644 --- a/crates/services/gas_price_service/src/v1/service.rs +++ b/crates/services/gas_price_service/src/v1/service.rs @@ -139,7 +139,7 @@ where da_block_costs: DaBlockCosts, ) -> anyhow::Result<()> { self.algorithm_updater.update_da_record_data( - da_block_costs.l2_block_range, + &da_block_costs.l2_blocks, da_block_costs.blob_cost_wei, )?; @@ -474,7 +474,7 @@ mod tests { let da_source = DaSourceService::new( DummyDaBlockCosts::new( Ok(DaBlockCosts { - l2_block_range: 1..2, + l2_blocks: (1..2).collect(), blob_cost_wei: 9000, blob_size_bytes: 3000, }), From 7ca55c7828bf92a83aa1ba46b0331ccbe6d0625d Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Tue, 19 Nov 2024 17:46:33 +0700 Subject: [PATCH 15/19] Ignore entire batches with blocks not in the unrecorded_blocks --- Cargo.lock | 1 + crates/fuel-gas-price-algorithm/Cargo.toml | 1 + crates/fuel-gas-price-algorithm/src/v1.rs | 58 +++++++++++-------- .../v1/tests/update_da_record_data_tests.rs | 20 +++---- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a3d5b110deb..a307d8f998b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3917,6 +3917,7 @@ dependencies = [ "rand", "serde", "thiserror 1.0.69", + "tracing", ] [[package]] diff --git a/crates/fuel-gas-price-algorithm/Cargo.toml b/crates/fuel-gas-price-algorithm/Cargo.toml index 662a8e5be08..24cb619585c 100644 --- a/crates/fuel-gas-price-algorithm/Cargo.toml +++ b/crates/fuel-gas-price-algorithm/Cargo.toml @@ -17,6 +17,7 @@ path = "src/lib.rs" [dependencies] serde = { workspace = true, features = ["derive"] } thiserror = { workspace = true } +tracing = { workspace = true } [dev-dependencies] proptest = { workspace = true } diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 28ef8a343a2..d9479521199 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -516,36 +516,46 @@ impl AlgorithmUpdaterV1 { heights: &[u32], recording_cost: u128, ) -> Result<(), Error> { - let recorded_bytes = self.drain_l2_block_bytes_for_heights(heights)?; - let new_cost_per_byte: u128 = recording_cost.checked_div(recorded_bytes).ok_or( - Error::CouldNotCalculateCostPerByte { - bytes: recorded_bytes, - cost: recording_cost, - }, - )?; - let new_da_block_cost = self - .latest_known_total_da_cost_excess - .saturating_add(recording_cost); - self.latest_known_total_da_cost_excess = new_da_block_cost; - self.latest_da_cost_per_byte = new_cost_per_byte; + let maybe_recorded_bytes = self.drain_l2_block_bytes_for_heights(heights); + if let Some(recorded_bytes) = maybe_recorded_bytes { + let new_cost_per_byte: u128 = recording_cost + .checked_div(recorded_bytes) + .ok_or(Error::CouldNotCalculateCostPerByte { + bytes: recorded_bytes, + cost: recording_cost, + })?; + let new_da_block_cost = self + .latest_known_total_da_cost_excess + .saturating_add(recording_cost); + self.latest_known_total_da_cost_excess = new_da_block_cost; + self.latest_da_cost_per_byte = new_cost_per_byte; + } Ok(()) } - fn drain_l2_block_bytes_for_heights( - &mut self, - heights: &[u32], - ) -> Result { + fn drain_l2_block_bytes_for_heights(&mut self, heights: &[u32]) -> Option { + let mut should_ignore_batch = false; let mut total: u128 = 0; for expected_height in heights { - let bytes = self.unrecorded_blocks.remove(expected_height).ok_or( - Error::L2BlockExpectedNotFound { - height: *expected_height, - }, - )?; - total = total.saturating_add(bytes as u128); + let maybe_bytes = self.unrecorded_blocks.remove(expected_height); + if let Some(bytes) = maybe_bytes { + total = total.saturating_add(bytes as u128); + } else { + tracing::error!( + "L2 block expected but not found in unrecorded blocks: {}. Ignoring batch of heights: {:?}", + expected_height, + heights, + ); + should_ignore_batch = true; + } + } + if should_ignore_batch { + None + } else { + self.unrecorded_blocks_bytes = + self.unrecorded_blocks_bytes.saturating_sub(total); + Some(total) } - self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total); - Ok(total) } fn recalculate_projected_cost(&mut self) { diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index c88da67601b..65a6ea54daf 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -1,13 +1,10 @@ -use crate::v1::{ - tests::{ - BlockBytes, - UpdaterBuilder, - }, - Error, +use crate::v1::tests::{ + BlockBytes, + UpdaterBuilder, }; #[test] -fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecorded_blocks( +fn update_da_record_data__ignores_batch_if_receives_a_block_missing_from_unrecorded_blocks( ) { // given let recorded_heights: Vec = (1u32..3).collect(); @@ -19,15 +16,16 @@ fn update_da_record_data__throws_error_if_receives_a_block_missing_from_unrecord let mut updater = UpdaterBuilder::new() .with_unrecorded_blocks(unrecorded_blocks) .build(); + let old = updater.algorithm(); // when - let actual_error = updater + updater .update_da_record_data(&recorded_heights, recorded_cost) - .unwrap_err(); + .unwrap(); // then - let expected_error = Error::L2BlockExpectedNotFound { height: 2 }; - assert_eq!(actual_error, expected_error); + let new = updater.algorithm(); + assert_eq!(new, old); } #[test] From 6ac41de9ef1fd499341eb99c325fb1d50b81188e Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Tue, 19 Nov 2024 17:49:59 +0700 Subject: [PATCH 16/19] Add comments to explain unexpected behavior --- crates/fuel-gas-price-algorithm/src/v1.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index d9479521199..e859b158af1 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -517,6 +517,8 @@ impl AlgorithmUpdaterV1 { recording_cost: u128, ) -> Result<(), Error> { let maybe_recorded_bytes = self.drain_l2_block_bytes_for_heights(heights); + // If we weren't able to get the bytes for all the heights, we can't update the cost per byte in a logical way. + // We will just accept the predicted cost as the real cost and move on with the algorithm if let Some(recorded_bytes) = maybe_recorded_bytes { let new_cost_per_byte: u128 = recording_cost .checked_div(recorded_bytes) @@ -533,6 +535,8 @@ impl AlgorithmUpdaterV1 { Ok(()) } + // Get the bytes for all specified heights, or get none of them. + // Always remove the blocks from the unrecorded blocks so they don't build up indefinitely fn drain_l2_block_bytes_for_heights(&mut self, heights: &[u32]) -> Option { let mut should_ignore_batch = false; let mut total: u128 = 0; From cc90cbd41f93bf79ed89e32a6d8ec6495332a1ef Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 20 Nov 2024 10:17:27 +0700 Subject: [PATCH 17/19] Fix tracing messages to be emitted at the right time --- crates/fuel-gas-price-algorithm/src/v1.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index e859b158af1..cd62e7288ed 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -531,6 +531,8 @@ impl AlgorithmUpdaterV1 { .saturating_add(recording_cost); self.latest_known_total_da_cost_excess = new_da_block_cost; self.latest_da_cost_per_byte = new_cost_per_byte; + } else { + tracing::error!("Ignoring batch of heights: {:?}", heights); } Ok(()) } @@ -546,9 +548,8 @@ impl AlgorithmUpdaterV1 { total = total.saturating_add(bytes as u128); } else { tracing::error!( - "L2 block expected but not found in unrecorded blocks: {}. Ignoring batch of heights: {:?}", + "L2 block expected but not found in unrecorded blocks: {}", expected_height, - heights, ); should_ignore_batch = true; } From 7ac01aa29d4315a5a64b85c363da214758e84e0a Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 20 Nov 2024 11:27:51 +0700 Subject: [PATCH 18/19] Fix logic to always remove recorded blocks, use predicted cost as best estimate --- crates/fuel-gas-price-algorithm/src/v1.rs | 77 +++++++++++++++-------- 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index cd62e7288ed..9d2536be94e 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -1,4 +1,10 @@ -use crate::utils::cumulative_percentage_change; +use crate::{ + utils::cumulative_percentage_change, + v1::BytesForHeights::{ + Complete, + Incomplete, + }, +}; use std::{ cmp::max, collections::BTreeMap, @@ -299,6 +305,13 @@ impl core::ops::Deref for ClampedPercentage { } } +enum BytesForHeights { + // Was able to find all the heights + Complete(u128), + // Was not able to find all the heights + Incomplete(u128), +} + impl AlgorithmUpdaterV1 { pub fn update_da_record_data( &mut self, @@ -516,31 +529,42 @@ impl AlgorithmUpdaterV1 { heights: &[u32], recording_cost: u128, ) -> Result<(), Error> { - let maybe_recorded_bytes = self.drain_l2_block_bytes_for_heights(heights); - // If we weren't able to get the bytes for all the heights, we can't update the cost per byte in a logical way. - // We will just accept the predicted cost as the real cost and move on with the algorithm - if let Some(recorded_bytes) = maybe_recorded_bytes { - let new_cost_per_byte: u128 = recording_cost - .checked_div(recorded_bytes) - .ok_or(Error::CouldNotCalculateCostPerByte { - bytes: recorded_bytes, - cost: recording_cost, - })?; - let new_da_block_cost = self - .latest_known_total_da_cost_excess - .saturating_add(recording_cost); - self.latest_known_total_da_cost_excess = new_da_block_cost; - self.latest_da_cost_per_byte = new_cost_per_byte; - } else { - tracing::error!("Ignoring batch of heights: {:?}", heights); - } + let bytes_for_heights = self.drain_l2_block_bytes_for_heights(heights); + match bytes_for_heights { + Complete(bytes) => { + let new_cost_per_byte: u128 = recording_cost.checked_div(bytes).ok_or( + Error::CouldNotCalculateCostPerByte { + bytes, + cost: recording_cost, + }, + )?; + let new_da_block_cost = self + .latest_known_total_da_cost_excess + .saturating_add(recording_cost); + self.latest_known_total_da_cost_excess = new_da_block_cost; + self.latest_da_cost_per_byte = new_cost_per_byte; + } + // If we weren't able to get the bytes for all the heights, we can't update the cost per byte in a logical way. + // We will just accept the predicted cost as the real cost and move on with the algorithm + // We can safely say that the cost didn't exceed the `recording_cost`, so we will take the + // minimum of the two costs + Incomplete(bytes) => { + let new_guess_recording_cost = + bytes.saturating_mul(self.latest_da_cost_per_byte); + let new_da_block_cost = self + .latest_known_total_da_cost_excess + .saturating_add(new_guess_recording_cost) + .min(recording_cost); + self.latest_known_total_da_cost_excess = new_da_block_cost; + } + }; Ok(()) } // Get the bytes for all specified heights, or get none of them. // Always remove the blocks from the unrecorded blocks so they don't build up indefinitely - fn drain_l2_block_bytes_for_heights(&mut self, heights: &[u32]) -> Option { - let mut should_ignore_batch = false; + fn drain_l2_block_bytes_for_heights(&mut self, heights: &[u32]) -> BytesForHeights { + let mut missed_some_heights = false; let mut total: u128 = 0; for expected_height in heights { let maybe_bytes = self.unrecorded_blocks.remove(expected_height); @@ -551,15 +575,14 @@ impl AlgorithmUpdaterV1 { "L2 block expected but not found in unrecorded blocks: {}", expected_height, ); - should_ignore_batch = true; + missed_some_heights = true; } } - if should_ignore_batch { - None + self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total); + if missed_some_heights { + Incomplete(total) } else { - self.unrecorded_blocks_bytes = - self.unrecorded_blocks_bytes.saturating_sub(total); - Some(total) + Complete(total) } } From 9f78fa6d4c94f6af9522e31f9e00d6c6d11af24c Mon Sep 17 00:00:00 2001 From: Mitch Turner Date: Wed, 20 Nov 2024 17:59:30 +0700 Subject: [PATCH 19/19] Add tests, fix bad bug --- crates/fuel-gas-price-algorithm/src/v1.rs | 8 ++-- .../v1/tests/update_da_record_data_tests.rs | 45 ++++++++++++++++++- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index 9d2536be94e..c97b874eb15 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -549,12 +549,12 @@ impl AlgorithmUpdaterV1 { // We can safely say that the cost didn't exceed the `recording_cost`, so we will take the // minimum of the two costs Incomplete(bytes) => { - let new_guess_recording_cost = - bytes.saturating_mul(self.latest_da_cost_per_byte); + let new_guess_recording_cost = bytes + .saturating_mul(self.latest_da_cost_per_byte) + .min(recording_cost); let new_da_block_cost = self .latest_known_total_da_cost_excess - .saturating_add(new_guess_recording_cost) - .min(recording_cost); + .saturating_add(new_guess_recording_cost); self.latest_known_total_da_cost_excess = new_da_block_cost; } }; diff --git a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs index 65a6ea54daf..6e584e7629d 100644 --- a/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs +++ b/crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_data_tests.rs @@ -4,17 +4,55 @@ use crate::v1::tests::{ }; #[test] -fn update_da_record_data__ignores_batch_if_receives_a_block_missing_from_unrecorded_blocks( +fn update_da_record_data__if_receives_batch_with_unknown_blocks_will_include_known_blocks_with_previous_cost( +) { + // given + let recorded_heights: Vec = (1u32..3).collect(); + let recorded_cost = 1_000_000; + let block_bytes = 1000; + let unrecorded_blocks = vec![BlockBytes { + height: 1, + block_bytes, + }]; + let cost_per_byte = 333; + let known_total_cost = 10_000; + let mut updater = UpdaterBuilder::new() + .with_unrecorded_blocks(unrecorded_blocks) + .with_da_cost_per_byte(cost_per_byte) + .with_known_total_cost(known_total_cost) + .build(); + let old = updater.algorithm(); + + // when + updater + .update_da_record_data(&recorded_heights, recorded_cost) + .unwrap(); + + // then + let new = updater.algorithm(); + assert_eq!(new, old); + let expected = known_total_cost + block_bytes as u128 * cost_per_byte; + let actual = updater.latest_known_total_da_cost_excess; + assert_eq!(expected, actual); +} + +#[test] +fn update_da_record_data__if_receives_batch_with_unknown_blocks_will_never_increase_cost_more_than_recorded_cost( ) { // given let recorded_heights: Vec = (1u32..3).collect(); let recorded_cost = 200; + let block_bytes = 1000; let unrecorded_blocks = vec![BlockBytes { height: 1, - block_bytes: 1000, + block_bytes, }]; + let cost_per_byte = 333; + let known_total_cost = 10_000; let mut updater = UpdaterBuilder::new() .with_unrecorded_blocks(unrecorded_blocks) + .with_da_cost_per_byte(cost_per_byte) + .with_known_total_cost(known_total_cost) .build(); let old = updater.algorithm(); @@ -26,6 +64,9 @@ fn update_da_record_data__ignores_batch_if_receives_a_block_missing_from_unrecor // then let new = updater.algorithm(); assert_eq!(new, old); + let expected = known_total_cost + recorded_cost; + let actual = updater.latest_known_total_da_cost_excess; + assert_eq!(expected, actual); } #[test]