Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add unrecorded blocks abstraction to gas price algo #2468

Merged
merged 101 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
101 commits
Select commit Hold shift + click to select a range
448d8c2
Remove height parameter, change tests
MitchTurner Oct 30, 2024
c5866a3
Modify to take vec instead of range
MitchTurner Oct 30, 2024
c54f098
Add tests to make sure it is sane
MitchTurner Oct 30, 2024
fcf4477
Rename variable
MitchTurner Oct 30, 2024
159d5b4
Fix analyzer
MitchTurner Oct 30, 2024
ba080ff
Rename some params
MitchTurner Oct 30, 2024
d2b3c28
Use slice instead of vec
MitchTurner Oct 30, 2024
128a9ba
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Oct 30, 2024
5f19932
Cleanup code a bit
MitchTurner Oct 31, 2024
922f3e0
Update crates/fuel-gas-price-algorithm/src/v1/tests/update_da_record_…
MitchTurner Oct 31, 2024
d7e9bdd
Pick nit
MitchTurner Oct 31, 2024
12c517e
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Oct 31, 2024
f8dca06
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Oct 31, 2024
2bb86fe
Optimize projected calc by caching running total
MitchTurner Nov 1, 2024
60771e9
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 1, 2024
03da963
Fix analyzer
MitchTurner Nov 1, 2024
ea14e6f
Simplify metadata
MitchTurner Nov 1, 2024
b615b33
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 14, 2024
cfd1a71
Merge remote-tracking branch 'origin' into feature/allow-DA-blocks-to…
MitchTurner Nov 17, 2024
d983891
Resolve logical conflicts from merge
MitchTurner Nov 17, 2024
4c2b844
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 17, 2024
1836656
WIP
MitchTurner Nov 18, 2024
48fe978
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 18, 2024
18fefb7
Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…
MitchTurner Nov 19, 2024
b15a93f
Get compiling
MitchTurner Nov 19, 2024
279191c
Start adding tests
MitchTurner Nov 19, 2024
63b0266
Get existing tests passing
MitchTurner Nov 19, 2024
477c731
WIP
MitchTurner Nov 19, 2024
b578a8e
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 19, 2024
7ca55c7
Ignore entire batches with blocks not in the unrecorded_blocks
MitchTurner Nov 19, 2024
6ac41de
Add comments to explain unexpected behavior
MitchTurner Nov 19, 2024
f2af4aa
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 20, 2024
4e07415
Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…
MitchTurner Nov 20, 2024
cc90cbd
Fix tracing messages to be emitted at the right time
MitchTurner Nov 20, 2024
11df7ae
Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…
MitchTurner Nov 20, 2024
7ac01aa
Fix logic to always remove recorded blocks, use predicted cost as bes…
MitchTurner Nov 20, 2024
ca6e1e5
Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…
MitchTurner Nov 20, 2024
2592ad0
Update CHANGELOG
MitchTurner Nov 20, 2024
eb233ab
Remove some TODOs
MitchTurner Nov 20, 2024
d32b347
Remove other todo
MitchTurner Nov 20, 2024
9f78fa6
Add tests, fix bad bug
MitchTurner Nov 20, 2024
6b9969c
Merge branch 'feature/allow-DA-blocks-to-be-received-out-of-order' in…
MitchTurner Nov 20, 2024
fc8c427
Fix copy-paste errors
MitchTurner Nov 20, 2024
87dc547
Fix broken tests
MitchTurner Nov 20, 2024
008f5fb
Correct test, use trace instead of debug
MitchTurner Nov 27, 2024
a409e6e
Merge remote-tracking branch 'origin' into feature/init-task-for-v1-g…
MitchTurner Nov 29, 2024
24d4a5d
Introduce unrecorded blocks abstraction without implementation
MitchTurner Nov 29, 2024
9f6a649
Use fuel storage in impl
MitchTurner Nov 29, 2024
1261c4d
Implement adapter
MitchTurner Nov 29, 2024
58f749b
Add simple tests
MitchTurner Nov 29, 2024
f969e88
Fix test, update CHANGELOG
MitchTurner Nov 29, 2024
b2051ce
update comment
MitchTurner Nov 29, 2024
68a21de
Make generic arg consistent
MitchTurner Nov 29, 2024
9276cc9
Appease Clippy-sama
MitchTurner Nov 29, 2024
3679422
Remove estra import
MitchTurner Nov 30, 2024
2a06ec0
Merge branch 'master' into feature/init-task-for-v1-gas-price-service
MitchTurner Nov 30, 2024
6af9589
Restructure the algo methods to accomodate storage txs
MitchTurner Dec 3, 2024
b9d6571
Update crates/services/gas_price_service/src/v0/service.rs
MitchTurner Dec 3, 2024
770998e
WIP
MitchTurner Dec 3, 2024
0c4640e
Simplify generic constraints
MitchTurner Dec 3, 2024
309d2cb
Merge branch 'feature/init-task-for-v1-gas-price-service', remote-tra…
MitchTurner Dec 3, 2024
5d5d48b
WIP adding transactionable storage
MitchTurner Dec 3, 2024
3c0a0ff
WIP fixing lifetime issues
MitchTurner Dec 4, 2024
36bbb4d
Incremental improvements to compiler errors
MitchTurner Dec 4, 2024
1447d90
Fix compilation errors with lifetimes
MitchTurner Dec 4, 2024
755f7b0
Add passing test
MitchTurner Dec 4, 2024
05ddee6
Fix another test
MitchTurner Dec 4, 2024
d9bbe36
Fix another test
MitchTurner Dec 4, 2024
b70b2eb
Fix remaining test
MitchTurner Dec 4, 2024
f443289
Merge branch 'master' into feature/init-task-for-v1-gas-price-service
MitchTurner Dec 4, 2024
921b6e2
Merge branch 'feature/init-task-for-v1-gas-price-service' into chore/…
MitchTurner Dec 4, 2024
8009b07
Remove commented code
MitchTurner Dec 4, 2024
0bb7e7a
More cleanup
MitchTurner Dec 4, 2024
ed9dab4
Remove unused dep
MitchTurner Dec 4, 2024
6c5df4c
Update CHANGELOG
MitchTurner Dec 4, 2024
a6caf6c
Add test for updating sequence number
MitchTurner Dec 4, 2024
5dcfcb7
Add test for reading sequence number
MitchTurner Dec 4, 2024
a587b8a
Separate traits out into get and set traits
MitchTurner Dec 4, 2024
37235ae
Fix broken test
MitchTurner Dec 5, 2024
96f6b9d
Cleanup extra code, remove unused imports, hide imlementations.
MitchTurner Dec 5, 2024
f43f00d
Merge branch 'master' into feature/init-task-for-v1-gas-price-service
MitchTurner Dec 6, 2024
e80edd5
Merge branch 'feature/init-task-for-v1-gas-price-service' into chore/…
MitchTurner Dec 6, 2024
51d41c7
Correct typo
MitchTurner Dec 6, 2024
17a56d3
Merge branch 'feature/init-task-for-v1-gas-price-service' into chore/…
MitchTurner Dec 6, 2024
a642cb7
Use relevant config for v1 constructor
MitchTurner Dec 6, 2024
8b027b9
Merge branch 'feature/init-task-for-v1-gas-price-service' into chore/…
MitchTurner Dec 6, 2024
abf0cca
Add todo
MitchTurner Dec 6, 2024
2469bf8
Merge remote-tracking branch 'origin' into chore/add-unrecorded-block…
MitchTurner Dec 7, 2024
288f51f
Add missed things
MitchTurner Dec 7, 2024
00daced
Merge branch 'master' into chore/add-unrecorded-blocks-trait
MitchTurner Dec 9, 2024
9fe913c
Remove extra code from merge
MitchTurner Dec 9, 2024
009a847
Merge branch 'master' into chore/add-unrecorded-blocks-trait
MitchTurner Dec 9, 2024
2eba245
Suggestions to simplify the V1 storage requirements and initializatio…
xgreenx Dec 9, 2024
1b74ff5
Small simplificaitons
xgreenx Dec 12, 2024
986c6c6
Fix the setting of the sequence number
MitchTurner Dec 12, 2024
ffce782
Format
MitchTurner Dec 12, 2024
4fc7184
Remove comment, move mutation, rename field
MitchTurner Dec 12, 2024
ee1c886
Fix compilation
MitchTurner Dec 12, 2024
43e19f2
Appease Clippy-sama
MitchTurner Dec 12, 2024
93876fa
Rename sequence number to bundle id
MitchTurner Dec 12, 2024
398ecec
Merge branch 'master' into chore/add-unrecorded-blocks-trait
MitchTurner Dec 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2485](https://github.com/FuelLabs/fuel-core/pull/2485): Hardcode the timestamp of the genesis block and version of `tai64` to avoid breaking changes for us.

### Changed
- [2468](https://github.com/FuelLabs/fuel-core/pull/2468): Abstract unrecorded blocks concept for V1 algorithm, create new storage impl. Introduce `TransactionableStorage` trait to allow atomic changes to the storage.
- [2295](https://github.com/FuelLabs/fuel-core/pull/2295): `CombinedDb::from_config` now respects `state_rewind_policy` with tmp RocksDB.
- [2378](https://github.com/FuelLabs/fuel-core/pull/2378): Use cached hash of the topic instead of calculating it on each publishing gossip message.
- [2429](https://github.com/FuelLabs/fuel-core/pull/2429): Introduce custom enum for representing result of running service tasks
Expand Down
4 changes: 2 additions & 2 deletions crates/fuel-core/src/service/sub_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ pub fn init_sub_services(
let genesis_block_height = *genesis_block.header().height();
let settings = consensus_parameters_provider.clone();
let block_stream = importer_adapter.events_shared_result();
let metadata = StructuredStorage::new(database.gas_price().clone());
let metadata = database.gas_price().clone();

let gas_price_service_v0 = new_gas_price_service_v0(
config.clone().into(),
genesis_block_height,
settings,
block_stream,
database.gas_price().clone(),
metadata,
StructuredStorage::new(metadata),
database.on_chain().clone(),
)?;

Expand Down
84 changes: 71 additions & 13 deletions crates/fuel-gas-price-algorithm/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub enum Error {
FailedToIncludeL2BlockData(String),
#[error("L2 block expected but not found in unrecorded blocks: {height}")]
L2BlockExpectedNotFound { height: u32 },
#[error("Could not insert unrecorded block: {0}")]
CouldNotInsertUnrecordedBlock(String),
#[error("Could not remove unrecorded block: {0}")]
CouldNotRemoveUnrecordedBlock(String),
}

// TODO: separate exec gas price and DA gas price into newtypes for clarity
Expand Down Expand Up @@ -59,6 +63,27 @@ impl AlgorithmV1 {
}
}

pub type Height = u32;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub type Height = u32;
pub struct Height(u32);

nit: I usually try do advocate for the Newtype pattern (see https://www.lurklurk.org/effective-rust/newtype.html).
In this case I think that we have this type defined somewhere else already

pub struct DaBlockHeight(pub u64);

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I don't want to use foreign types here. I want to keep the domain independent.

I'm not completely sold on using newtypes for situations like this. I mostly just added this for readability on the BTreeMap generics, otherwise people would just be guessing what the u32 and u64 represent. But the newtype would add an additional construct or From conversion for the caller, an that's noisy IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the newtype would add an additional construct or From conversion for the caller.

I also agree newtype is overkill here, but this particular issue can be solved by taking impl Into<Height> as arguments instead of specifying the type in the function signature.

pub type Bytes = u64;

pub trait UnrecordedBlocks {
fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), String>;

fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, String>;
}

impl UnrecordedBlocks for BTreeMap<Height, Bytes> {
fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), String> {
self.insert(height, bytes);
Ok(())
}

fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, String> {
let value = self.remove(height);
Ok(value)
}
}

/// 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
Expand Down Expand Up @@ -96,8 +121,6 @@ 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
Expand Down Expand Up @@ -143,8 +166,6 @@ pub struct AlgorithmUpdaterV1 {
pub latest_da_cost_per_byte: u128,
/// Activity of L2
pub l2_activity: L2ActivityTracker,
/// The unrecorded blocks that are used to calculate the projected cost of recording blocks
pub unrecorded_blocks: BTreeMap<Height, Bytes>,
/// Total unrecorded block bytes
pub unrecorded_blocks_bytes: u128,
}
Expand Down Expand Up @@ -269,10 +290,28 @@ impl L2ActivityTracker {
pub fn current_activity(&self) -> u16 {
self.chain_activity
}

pub fn max_activity(&self) -> u16 {
self.max_activity
}

pub fn capped_activity_threshold(&self) -> u16 {
self.capped_activity_threshold
}

pub fn decrease_activity_threshold(&self) -> u16 {
self.decrease_activity_threshold
}

pub fn block_activity_threshold(&self) -> ClampedPercentage {
self.block_activity_threshold
}
}

/// A value that represents a value between 0 and 100. Higher values are clamped to 100
#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, PartialOrd)]
#[derive(
serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, PartialOrd,
)]
pub struct ClampedPercentage {
value: u8,
}
Expand Down Expand Up @@ -300,27 +339,34 @@ impl core::ops::Deref for ClampedPercentage {
}

impl AlgorithmUpdaterV1 {
pub fn update_da_record_data(
pub fn update_da_record_data<U: UnrecordedBlocks>(
&mut self,
heights: &[u32],
recorded_bytes: u32,
recording_cost: u128,
unrecorded_blocks: &mut U,
) -> Result<(), Error> {
if !heights.is_empty() {
self.da_block_update(heights, recorded_bytes as u128, recording_cost)?;
self.da_block_update(
heights,
recorded_bytes as u128,
recording_cost,
unrecorded_blocks,
)?;
self.recalculate_projected_cost();
self.update_da_gas_price();
}
Ok(())
}

pub fn update_l2_block_data(
pub fn update_l2_block_data<U: UnrecordedBlocks>(
&mut self,
height: u32,
used: u64,
capacity: NonZeroU64,
block_bytes: u64,
fee_wei: u128,
unrecorded_blocks: &mut U,
) -> Result<(), Error> {
let expected = self.l2_block_height.saturating_add(1);
if height != expected {
Expand Down Expand Up @@ -351,7 +397,9 @@ impl AlgorithmUpdaterV1 {
self.update_da_gas_price();

// metadata
self.unrecorded_blocks.insert(height, block_bytes);
unrecorded_blocks
.insert(height, block_bytes)
.map_err(Error::CouldNotInsertUnrecordedBlock)?;
self.unrecorded_blocks_bytes = self
.unrecorded_blocks_bytes
.saturating_add(block_bytes as u128);
Expand Down Expand Up @@ -512,13 +560,14 @@ impl AlgorithmUpdaterV1 {
.saturating_div(100)
}

fn da_block_update(
fn da_block_update<U: UnrecordedBlocks>(
&mut self,
heights: &[u32],
recorded_bytes: u128,
recording_cost: u128,
unrecorded_blocks: &mut U,
) -> Result<(), Error> {
self.update_unrecorded_block_bytes(heights);
self.update_unrecorded_block_bytes(heights, unrecorded_blocks)?;

let new_da_block_cost = self
.latest_known_total_da_cost_excess
Expand All @@ -540,10 +589,17 @@ impl AlgorithmUpdaterV1 {

// 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 update_unrecorded_block_bytes(&mut self, heights: &[u32]) {
fn update_unrecorded_block_bytes<U: UnrecordedBlocks>(
&mut self,
heights: &[u32],
unrecorded_blocks: &mut U,
) -> Result<(), Error> {
let mut total: u128 = 0;
for expected_height in heights {
let maybe_bytes = self.unrecorded_blocks.remove(expected_height);
let maybe_bytes = unrecorded_blocks
.remove(expected_height)
.map_err(Error::CouldNotRemoveUnrecordedBlock)?;

if let Some(bytes) = maybe_bytes {
total = total.saturating_add(bytes as u128);
} else {
Expand All @@ -554,6 +610,8 @@ impl AlgorithmUpdaterV1 {
}
}
self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total);

Ok(())
}

fn recalculate_projected_cost(&mut self) {
Expand Down
27 changes: 14 additions & 13 deletions crates/fuel-gas-price-algorithm/src/v1/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

use crate::v1::{
AlgorithmUpdaterV1,
Bytes,
Height,
L2ActivityTracker,
};
use std::collections::BTreeMap;

#[cfg(test)]
mod algorithm_v1_tests;
Expand Down Expand Up @@ -38,7 +41,7 @@ pub struct UpdaterBuilder {
da_cost_per_byte: u128,
project_total_cost: u128,
latest_known_total_cost: u128,
unrecorded_blocks: Vec<BlockBytes>,
unrecorded_blocks_bytes: u64,
last_profit: i128,
second_to_last_profit: i128,
da_gas_price_factor: u64,
Expand All @@ -65,7 +68,7 @@ impl UpdaterBuilder {
da_cost_per_byte: 0,
project_total_cost: 0,
latest_known_total_cost: 0,
unrecorded_blocks: vec![],
unrecorded_blocks_bytes: 0,
last_profit: 0,
second_to_last_profit: 0,
da_gas_price_factor: 1,
Expand Down Expand Up @@ -146,8 +149,14 @@ impl UpdaterBuilder {
self
}

fn with_unrecorded_blocks(mut self, unrecorded_blocks: Vec<BlockBytes>) -> Self {
self.unrecorded_blocks = unrecorded_blocks;
fn with_unrecorded_blocks(
mut self,
unrecorded_blocks: &BTreeMap<Height, Bytes>,
) -> Self {
let unrecorded_block_bytes = unrecorded_blocks
.iter()
.fold(0u64, |acc, (_, bytes)| acc + bytes);
self.unrecorded_blocks_bytes = unrecorded_block_bytes;
self
}

Expand Down Expand Up @@ -180,11 +189,6 @@ impl UpdaterBuilder {
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,
unrecorded_blocks: self
.unrecorded_blocks
.iter()
.map(|b| (b.height, b.block_bytes))
.collect(),
last_profit: self.last_profit,
second_to_last_profit: self.second_to_last_profit,
min_da_gas_price: self.min_da_gas_price,
Expand All @@ -193,10 +197,7 @@ 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)),
unrecorded_blocks_bytes: self.unrecorded_blocks_bytes as u128,
}
}
}
Expand Down
Loading
Loading