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

Fixed the rollback functionality to work with empty gas price database #2105

Merged
merged 8 commits into from
Aug 20, 2024
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
#### Breaking
- [2051](https://github.com/FuelLabs/fuel-core/pull/2051): Misdocumented `CONSENSUS_KEY` environ variable has been removed, use `CONSENSUS_KEY_SECRET` instead. Also raises MSRV to `1.79.0`.

### Fixed

- [2105](https://github.com/FuelLabs/fuel-core/pull/2105): Fixed the rollback functionality to work with empty gas price database.

## [Version 0.33.0]

Expand Down
28 changes: 17 additions & 11 deletions crates/fuel-core/src/combined_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,15 @@ impl CombinedDatabase {
.ok_or(anyhow::anyhow!("off-chain database doesn't have height"))?;

let gas_price_chain_height =
self.gas_price().latest_height_from_metadata()?.ok_or(
anyhow::anyhow!("gas-price-chain database doesn't have height"),
)?;
self.gas_price().latest_height_from_metadata()?;

let gas_price_roll_backed = gas_price_chain_height.is_none()
|| gas_price_chain_height.expect("We checked height before")
== target_block_height;

if on_chain_height == target_block_height
&& off_chain_height == target_block_height
&& gas_price_chain_height == target_block_height
&& gas_price_roll_backed
{
break;
}
Expand All @@ -277,11 +279,13 @@ impl CombinedDatabase {
));
}

if gas_price_chain_height < target_block_height {
return Err(anyhow::anyhow!(
"gas-price-chain database height({gas_price_chain_height}) \
is less than target height({target_block_height})"
));
if let Some(gas_price_chain_height) = gas_price_chain_height {
if gas_price_chain_height < target_block_height {
return Err(anyhow::anyhow!(
"gas-price-chain database height({gas_price_chain_height}) \
is less than target height({target_block_height})"
));
}
}

if on_chain_height > target_block_height {
Expand All @@ -292,8 +296,10 @@ impl CombinedDatabase {
self.off_chain().rollback_last_block()?;
}

if gas_price_chain_height > target_block_height {
self.gas_price().rollback_last_block()?;
if let Some(gas_price_chain_height) = gas_price_chain_height {
if gas_price_chain_height > target_block_height {
self.gas_price().rollback_last_block()?;
}
}
}

Expand Down
42 changes: 41 additions & 1 deletion tests/tests/state_rewind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ use rand::{
SeedableRng,
};
use std::collections::BTreeSet;
use test_helpers::fuel_core_driver::FuelCoreDriver;
use test_helpers::{
fuel_core_driver::FuelCoreDriver,
produce_block_with_tx,
};

fn transfer_transaction(min_amount: u64, rng: &mut StdRng) -> Transaction {
let mut builder = TransactionBuilder::script(vec![], vec![]);
Expand Down Expand Up @@ -251,3 +254,40 @@ async fn rollback_chain_to_same_height_1000() -> anyhow::Result<()> {
)
.await
}

#[tokio::test(flavor = "multi_thread")]
async fn rollback_to__should_work_with_empty_gas_price_database() -> anyhow::Result<()> {
let mut rng = StdRng::seed_from_u64(1234);
let driver = FuelCoreDriver::spawn_feeless(&[
"--debug",
"--poa-instant",
"true",
"--state-rewind-duration",
"7d",
])
.await?;

// Given
const TOTAL_BLOCKS: u64 = 500;
for _ in 0..TOTAL_BLOCKS {
produce_block_with_tx(&mut rng, &driver.client).await;
}
let temp_dir = driver.kill().await;
std::fs::remove_dir_all(temp_dir.path().join("gas_price")).unwrap();

// When
let args = [
"_IGNORED_",
Copy link
Member

Choose a reason for hiding this comment

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

I've seen this being used in other tests too, what does this mean?

Copy link
Member

Choose a reason for hiding this comment

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

That's the first (shell) command argument, which is usually the path to the binary. Since the executable name isn't used for anything and pointing to the real fuel-core binary would make no sense, just having a dummy string here is fine.

"--db-path",
temp_dir.path().to_str().unwrap(),
"--target-block-height",
"1",
];
let command = fuel_core_bin::cli::rollback::Command::parse_from(args);
let result = fuel_core_bin::cli::rollback::exec(command).await;

// Then
result.expect("Rollback should succeed");

Ok(())
}
Loading