Use bytes from unrecorded_blocks rather from the block from DA
#2252
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,12 +66,6 @@ pub struct AlgorithmUpdaterV0 { | |
| pub l2_block_fullness_threshold_percent: u64, | ||
| } | ||
|
|
||
| impl AlgorithmUpdaterV0 { | ||
| pub fn new( | ||
| new_exec_price: u64, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| use crate::utils::cumulative_percentage_change; | ||
| use std::{ | ||
| cmp::max, | ||
| collections::BTreeMap, | ||
| num::NonZeroU64, | ||
| ops::{ | ||
| Div, | ||
| Range, | ||
| }, | ||
| }; | ||
|
|
||
| #[cfg(test)] | ||
| mod tests; | ||
|
|
||
|
|
@@ -16,9 +19,11 @@ pub enum Error { | |
| #[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), | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq)] | ||
|
|
@@ -94,6 +99,8 @@ impl AlgorithmV1 { | |
| /// 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. | ||
| type Height = u32; | ||
| type Bytes = u64; | ||
| #[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] | ||
| pub struct AlgorithmUpdaterV1 { | ||
| // Execution | ||
|
|
@@ -144,8 +151,9 @@ pub struct AlgorithmUpdaterV1 { | |
| pub second_to_last_profit: i128, | ||
| /// The latest known cost per byte for recording blocks on the DA chain | ||
| pub latest_da_cost_per_byte: u128, | ||
|
|
||
| /// The unrecorded blocks that are used to calculate the projected cost of recording blocks | ||
| pub unrecorded_blocks: BTreeMap<Height, Bytes>, | ||
| } | ||
|
|
||
| /// A value that represents a value between 0 and 100. Higher values are clamped to 100 | ||
|
|
@@ -176,29 +184,17 @@ impl core::ops::Deref for ClampedPercentage { | |
| } | ||
| } | ||
|
|
||
| impl AlgorithmUpdaterV1 { | ||
| pub fn update_da_record_data( | ||
| &mut self, | ||
| height_range: Range<u32>, | ||
| range_cost: u128, | ||
| ) -> Result<(), Error> { | ||
| if !height_range.is_empty() { | ||
| self.da_block_update(height_range, range_cost)?; | ||
| self.recalculate_projected_cost(); | ||
| // self.normalize_rewards_and_costs(); | ||
|
There was a problem hiding this comment. Is this commented-out on purpose? There was a problem hiding this comment. Yes. I added the TODO with the related issue. |
||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
|
|
@@ -208,7 +204,7 @@ impl AlgorithmUpdaterV1 { | |
| used: u64, | ||
| capacity: NonZeroU64, | ||
| block_bytes: u64, | ||
| fee_wei: u128, | ||
| ) -> Result<(), Error> { | ||
| let expected = self.l2_block_height.saturating_add(1); | ||
| if height != expected { | ||
|
|
@@ -226,6 +222,8 @@ impl AlgorithmUpdaterV1 { | |
| // costs | ||
| self.update_projected_cost(block_bytes); | ||
| let projected_total_da_cost = self.clamped_projected_cost_as_i128(); | ||
|
|
||
| // profit | ||
| let last_profit = rewards.saturating_sub(projected_total_da_cost); | ||
| self.update_last_profit(last_profit); | ||
|
|
||
|
|
@@ -234,19 +232,15 @@ impl AlgorithmUpdaterV1 { | |
| self.update_da_gas_price(); | ||
|
|
||
| // metadata | ||
| self.unrecorded_blocks.insert(height, block_bytes); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| fn update_rewards(&mut self, fee_wei: u128) { | ||
| let block_da_reward = self.da_portion_of_fee(fee_wei); | ||
| self.total_da_rewards_excess = | ||
| self.total_da_rewards_excess.saturating_add(block_da_reward); | ||
| } | ||
|
|
||
| fn update_projected_cost(&mut self, block_bytes: u64) { | ||
|
|
@@ -258,12 +252,11 @@ impl AlgorithmUpdaterV1 { | |
| } | ||
|
|
||
| // Take the `fee_wei` and return the portion of the fee that should be used for paying DA costs | ||
| fn da_portion_of_fee(&self, fee_wei: u128) -> u128 { | ||
| // fee_wei * (da_price / (exec_price + da_price)) | ||
| let numerator = fee_wei.saturating_mul(self.descaled_da_price() as u128); | ||
| let denominator = (self.descaled_exec_price() as u128) | ||
| .saturating_add(self.descaled_da_price() as u128); | ||
| if denominator == 0 { | ||
| 0 | ||
| } else { | ||
|
|
@@ -374,43 +367,60 @@ impl AlgorithmUpdaterV1 { | |
|
|
||
| fn da_block_update( | ||
| &mut self, | ||
| height_range: Range<u32>, | ||
| 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_block_cost = self | ||
| .latest_known_total_da_cost_excess | ||
| .saturating_add(range_cost); | ||
| self.latest_known_total_da_cost_excess = new_block_cost; | ||
| self.latest_da_cost_per_byte = new_cost_per_byte; | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| fn drain_l2_block_bytes_for_range( | ||
|
There was a problem hiding this comment. So, we can consider improving performance here still. There was a problem hiding this comment. Switched to |
||
| &mut self, | ||
| height_range: Range<u32>, | ||
| ) -> Result<u128, Error> { | ||
| let mut total: u128 = 0; | ||
| for expected_height in height_range { | ||
| let (actual_height, bytes) = self | ||
| .unrecorded_blocks | ||
| .pop_first() | ||
rafal-ch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .ok_or(Error::L2BlockExpectedNotFound(expected_height))?; | ||
| if actual_height != expected_height { | ||
| return Err(Error::L2BlockExpectedNotFound(expected_height)); | ||
| } | ||
| total = total.saturating_add(bytes as u128); | ||
| } | ||
| 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)| { | ||
| (bytes as u128).saturating_mul(self.latest_da_cost_per_byte) | ||
| }) | ||
| .sum(); | ||
| self.projected_total_da_cost = self | ||
|
|
@@ -436,9 +446,12 @@ impl AlgorithmUpdaterV1 { | |
| } | ||
| } | ||
|
|
||
| // TODO: This breaks our simulation now that we are using an extended finalization period. We | ||
| // can either keep this function and fix the simulation, or we might decide to remove it. | ||
| // https://github.com/FuelLabs/fuel-core/issues/2264 | ||
| // We only need to track the difference between the rewards and costs after we have true DA data | ||
| // Normalize, or zero out the lower value and subtract it from the higher value | ||
| fn _normalize_rewards_and_costs(&mut self) { | ||
| let (excess, projected_cost_excess) = | ||
| if self.total_da_rewards_excess > self.latest_known_total_da_cost_excess { | ||
| ( | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this type seems complicated enough that a named type should make it more readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.