Skip to content

Commit

Permalink
fix(gas_price_service_v0): bring back removed fields, causing UB when…
Browse files Browse the repository at this point in the history
… trying to access (#2511)

## Linked Issues/PRs
<!-- List of related issues/PRs -->

## Description
<!-- List of detailed changes -->
Detected UB when trying to access the `UpdaterMetdata`table in the gas
price db, because the newer version of fuel-core doesn't have fields
that existed in older versions, it thought that the `min_exec_gas_price`
was the same as the `l2_block_height`, leading to some very weird issues
when trying to sync a v0.40.2 node initially, and then starting a
`master`-based branch

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [ ] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
  • Loading branch information
rymnc authored Dec 20, 2024
1 parent 825e18d commit 4bf049d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2479](https://github.com/FuelLabs/fuel-core/pull/2479): Fix an error on the last iteration of the read and write sequential opcodes on contract storage.
- [2478](https://github.com/FuelLabs/fuel-core/pull/2478): Fix proof created by `message_receipts_proof` function by ignoring the receipts from failed transactions to match `message_outbox_root`.
- [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.
- [2511](https://github.com/FuelLabs/fuel-core/pull/2511): Fix backward compatibility of V0Metadata in gas price db.

### 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.
Expand Down
16 changes: 16 additions & 0 deletions crates/services/gas_price_service/src/v0/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
use fuel_gas_price_algorithm::v0::AlgorithmUpdaterV0;

/// DO NOT TOUCH! DEPLOYED TO MAINNET
/// FIELDS PREFIXED WITH _ ARE UNUSED BUT EXIST ON MAINNET DB's
/// IF YOU WANT TO MODIFY THIS, MAKE A MIGRATION IN crates/fuel-core/src/service/genesis/importer/gas_price.rs
#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)]
pub struct V0Metadata {
/// The gas price to cover the execution of the next block
pub new_exec_price: u64,
// Execution
/// The lowest the algorithm allows the exec gas price to go
pub _min_exec_gas_price: u64,
/// The Percentage the execution gas price will change in a single block, either increase or decrease
/// based on the fullness of the last L2 block
pub _exec_gas_price_change_percent: u64,
/// The height for which the `new_exec_price` is calculated, which should be the _next_ block
pub l2_block_height: u32,
/// The threshold of gas usage above and below which the gas price will increase or decrease
/// This is a percentage of the total capacity of the L2 block
pub _l2_block_fullness_threshold_percent: u64,
}

pub struct V0AlgorithmConfig {
Expand All @@ -20,6 +32,10 @@ impl From<AlgorithmUpdaterV0> for V0Metadata {
Self {
new_exec_price: updater.new_exec_price,
l2_block_height: updater.l2_block_height,
_min_exec_gas_price: updater.min_exec_gas_price,
_exec_gas_price_change_percent: updater.exec_gas_price_change_percent,
_l2_block_fullness_threshold_percent: updater
.l2_block_fullness_threshold_percent,
}
}
}
4 changes: 4 additions & 0 deletions crates/services/gas_price_service/src/v0/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ fn arbitrary_metadata() -> V0Metadata {
V0Metadata {
new_exec_price: 100,
l2_block_height: 0,
_min_exec_gas_price: 0,
_exec_gas_price_change_percent: 0,
_l2_block_fullness_threshold_percent: 0,
}
}

Expand Down Expand Up @@ -339,6 +342,7 @@ async fn uninitialized_task__new__if_exists_already_reload_old_values_with_overr
let V0Metadata {
new_exec_price,
l2_block_height,
..
} = original_metadata;
let UninitializedTask { algo_updater, .. } = service;
assert_eq!(algo_updater.new_exec_price, new_exec_price);
Expand Down

0 comments on commit 4bf049d

Please sign in to comment.