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

Allow DA recorded blocks to come out-of-order #2415

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 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
48fe978
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 18, 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
cc90cbd
Fix tracing messages to be emitted at the right time
MitchTurner Nov 20, 2024
7ac01aa
Fix logic to always remove recorded blocks, use predicted cost as bes…
MitchTurner Nov 20, 2024
9f78fa6
Add tests, fix bad bug
MitchTurner Nov 20, 2024
a2c0125
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 24, 2024
57ed9db
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 25, 2024
eff76bf
Merge branch 'master' into feature/allow-DA-blocks-to-be-received-out…
MitchTurner Nov 27, 2024
4d5409a
Use compressed bytes to calculate cost per bytes
MitchTurner Nov 28, 2024
4e8df28
Fix compilation
MitchTurner Nov 28, 2024
757896a
Appease Clippy-sama
MitchTurner Nov 28, 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
14 changes: 8 additions & 6 deletions crates/fuel-gas-price-algorithm/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Height, Bytes>,
/// 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.
Expand Down Expand Up @@ -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);
acerone85 marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}
}
Expand Down Expand Up @@ -538,16 +543,13 @@ impl AlgorithmUpdaterV1 {
)?;
total = total.saturating_add(bytes as u128);
}
self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking about the following scenario, not sure if this is an issue or not:

  1. we have 3 blocks in self.unrecorded_blocks
  2. we remove() 2 of them, but we get an error on 3rd one
  3. now, the unrecorded_blocks_bytes becomes out-of-sync

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. This is the inverse of the problem I mentioned above.

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
Expand Down
4 changes: 4 additions & 0 deletions crates/fuel-gas-price-algorithm/src/v1/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
Loading