fix(combined_database): syncing auxiliary databases on startup with custom behaviour

[rymnc]

Linked Issues/PRs

Fixes #2239

Description

• Doesn't swallow the error blindly now and errors where necessary, this should fix the flakiness as well

Checklist

• Breaking changes are clearly marked as such in the PR description and changelog
• New behavior is reflected in tests
• The specification 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
• Sway compiler
• Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
• Someone else?

[MitchTurner]

Approved but with a thought.

[MitchTurner]

What if the rollback_last_block fails after rolling back a couple blocks. Are we okay with mutating the state without correcting it?

Is there a way to commit the rollback atomically?

[rymnc]

good question ~ if we include this refactor, we will also have to ensure the existing function on CombinedDatabase::rollback_to will have to be refactored -

fuel-core/crates/fuel-core/src/combined_database.rs

Lines 234 to 313 in 103ecf6

	/// Rollbacks the state of the blockchain to a specific block height.
	pub fn rollback_to<S>(
	&self,
	target_block_height: BlockHeight,
	shutdown_listener: &mut S,
	) -> anyhow::Result<()>
	where
	S: ShutdownListener,
	{
	while !shutdown_listener.is_cancelled() {
	let on_chain_height = self
	.on_chain()
	.latest_height_from_metadata()?
	.ok_or(anyhow::anyhow!("on-chain database doesn't have height"))?;
	let off_chain_height = self
	.off_chain()
	.latest_height_from_metadata()?
	.ok_or(anyhow::anyhow!("off-chain database doesn't have height"))?;
	let gas_price_chain_height =
	self.gas_price().latest_height_from_metadata()?;
	let gas_price_rolled_back = 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_rolled_back
	{
	break;
	}
	if on_chain_height < target_block_height {
	return Err(anyhow::anyhow!(
	"on-chain database height({on_chain_height}) \
	is less than target height({target_block_height})"
	));
	}
	if off_chain_height < target_block_height {
	return Err(anyhow::anyhow!(
	"off-chain database height({off_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 {
	self.on_chain().rollback_last_block()?;
	}
	if off_chain_height > target_block_height {
	self.off_chain().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()?;
	}
	}
	}
	if shutdown_listener.is_cancelled() {
	return Err(anyhow::anyhow!(
	"Stop the rollback due to shutdown signal received"
	));
	}
	Ok(())
	}

a simple way would perhaps be to use a WriteTransaction and then commit once done.

[rymnc]

how does this look @MitchTurner? - 54457c2

[rymnc]

eh, looks like the internal function doesn't actually rollback atomically. we'll have to keep the loop. it rolls back the blocks one-by-one, which makes the rollback_to quite misleading as a function name in

fuel-core/crates/fuel-core/src/state.rs

Line 61 in b1f2bf4

pub trait TransactableStorage<Height>: IterableStore + Debug + Send + Sync {

cc: @xgreenx for confirmation

[rymnc]

I'm going to revert 54457c2 for now

[xgreenx]

I think we are okay with mutating state since during next launch we continue to rollback the state. It doesn't live as in corrupted state.

[xgreenx]

I think we are okay with mutating state since during next launch we continue to rollback the state. It doesn't live as in corrupted state.

[xgreenx]

I'm curious: why don't we want to continue the use of the rollback_to? Looks like it does what we need

[rymnc]

I'm curious: why don't we want to continue the use of the rollback_to? Looks like it does what we need

I tried using it and a lot of the tests break because there are instances where the height of the auxiliary databases are lower than onchain height and rollback_to throws an error. we explicitly define this behaviour in the docstring :)