Skip to content

Commit

Permalink
test: restart with relayer data (#1866)
Browse files Browse the repository at this point in the history
The relayer service fails to start if the relayer database is already
populated with a `DaHeightTable` entry. When the relayer starts, it
attempts to set the DA height to the config's `da_deploy_height`. If the
`da_deploy_height` defined by the config (where the default is 0) is not
a monotonically increasing value compared to the database entry, the
service panics.

Instead of setting we DA height at relayer start up, we can simply
continue from where the relayer has previously left off. We do not need
to make this invalid database commit at relayer startup. This PR removes
the invalid commit.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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?

---------

Co-authored-by: xgreenx <xgreenx9999@gmail.com>
  • Loading branch information
Brandon Vrooman and xgreenx committed Apr 29, 2024
1 parent fc75538 commit 6f5c4fc
Show file tree
Hide file tree
Showing 16 changed files with 191 additions and 273 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- [#1866](https://github.com/FuelLabs/fuel-core/pull/1866): Fixed a runtime panic that occurred when restarting a node. The panic happens when the relayer database is already populated, and the relayer attempts an empty commit during start up. This invalid commit is removed in this PR.
- [#1871](https://github.com/FuelLabs/fuel-core/pull/1871): Fixed `block` endpoint to return fetch the blocks from both databases after regenesis.
- [#1856](https://github.com/FuelLabs/fuel-core/pull/1856): Replaced instances of `Union` with `Enum` for GraphQL definitions of `ConsensusParametersVersion` and related types. This is needed because `Union` does not support multiple `Version`s inside discriminants or empty variants.

Expand Down
5 changes: 5 additions & 0 deletions crates/fuel-core/src/combined_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ impl CombinedDatabase {
&self.relayer
}

#[cfg(any(feature = "test-helpers", test))]
pub fn relayer_mut(&mut self) -> &mut Database<Relayer> {
&mut self.relayer
}

#[cfg(feature = "test-helpers")]
pub fn read_state_config(&self) -> StorageResult<StateConfig> {
use fuel_core_chain_config::AddTable;
Expand Down
31 changes: 19 additions & 12 deletions crates/fuel-core/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ mod tests {
.unwrap();

// Then
assert_eq!(database.latest_height().unwrap(), None);
assert_eq!(AtomicView::latest_height(&database), None);
}

#[test]
Expand Down Expand Up @@ -782,7 +782,7 @@ mod tests {
.unwrap();

// Then
assert_eq!(database.latest_height().unwrap(), None);
assert_eq!(AtomicView::latest_height(&database), None);
}

#[test]
Expand Down Expand Up @@ -903,10 +903,7 @@ mod tests {
database_description::relayer::Relayer,
DatabaseHeight,
};
use fuel_core_relayer::storage::{
DaHeightTable,
EventsHistory,
};
use fuel_core_relayer::storage::EventsHistory;
use fuel_core_storage::transactional::WriteTransaction;

#[test]
Expand Down Expand Up @@ -939,12 +936,18 @@ mod tests {

// When
database
.storage_as_mut::<DaHeightTable>()
.insert(&(), &DaBlockHeight::default())
.storage_as_mut::<MetadataTable<Relayer>>()
.insert(
&(),
&DatabaseMetadata::<DaBlockHeight>::V1 {
version: Default::default(),
height: Default::default(),
},
)
.unwrap();

// Then
assert_eq!(database.latest_height().unwrap(), None);
assert_eq!(AtomicView::latest_height(&database), None);
}

#[test]
Expand Down Expand Up @@ -1008,9 +1011,13 @@ mod tests {
.unwrap();

// When
let result = database
.storage_as_mut::<DaHeightTable>()
.insert(&(), &DaBlockHeight::default());
let result = database.storage_as_mut::<MetadataTable<Relayer>>().insert(
&(),
&DatabaseMetadata::<DaBlockHeight>::V1 {
version: Default::default(),
height: Default::default(),
},
);

// Then
assert!(result.is_err());
Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-core/src/service/adapters/consensus_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl RelayerPort for MaybeRelayerAdapter {
#[cfg(feature = "relayer")]
{
if let Some(sync) = self.relayer_synced.as_ref() {
let current_height = sync.get_finalized_da_height()?;
let current_height = sync.get_finalized_da_height();
anyhow::ensure!(
da_height.saturating_sub(*current_height) <= **_max_da_lag,
"Relayer is too far out of sync"
Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-core/src/service/adapters/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter {
{
if let Some(sync) = self.relayer_synced.as_ref() {
sync.await_at_least_synced(height).await?;
let highest = sync.get_finalized_da_height()?;
let highest = sync.get_finalized_da_height();
Ok(highest)
} else {
Ok(*height)
Expand Down
6 changes: 6 additions & 0 deletions crates/fuel-core/src/service/adapters/relayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ use crate::database::{
};
use fuel_core_relayer::ports::Transactional;
use fuel_core_storage::transactional::{
AtomicView,
IntoTransaction,
StorageTransaction,
};
use fuel_core_types::blockchain::primitives::DaBlockHeight;

impl Transactional for Database<Relayer> {
type Transaction<'a> = StorageTransaction<&'a mut Self> where Self: 'a;

fn transaction(&mut self) -> Self::Transaction<'_> {
self.into_transaction()
}

fn latest_da_height(&self) -> Option<DaBlockHeight> {
AtomicView::latest_height(self)
}
}
2 changes: 1 addition & 1 deletion crates/fuel-core/src/service/sub_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
schema::build_schema,
service::{
adapters::{
consensus_parameters_provider,
BlockImporterAdapter,
BlockProducerAdapter,
ConsensusParametersProvider,
Expand All @@ -32,7 +33,6 @@ use tokio::sync::Mutex;

#[cfg(feature = "relayer")]
use crate::relayer::Config as RelayerConfig;
use crate::service::adapters::consensus_parameters_provider;
#[cfg(feature = "relayer")]
use fuel_core_types::blockchain::primitives::DaBlockHeight;

Expand Down
34 changes: 14 additions & 20 deletions crates/services/relayer/src/mock_db.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
#![allow(missing_docs)]

use crate::ports::RelayerDb;
use fuel_core_storage::{
not_found,
Result as StorageResult,
};
use fuel_core_storage::Result as StorageResult;
use fuel_core_types::{
blockchain::primitives::DaBlockHeight,
entities::{
Expand Down Expand Up @@ -92,6 +89,17 @@ impl MockDb {
.map(|map| map.iter().map(|(_, tx)| tx).cloned().collect())
.unwrap_or_default()
}

#[cfg(any(test, feature = "test-helpers"))]
pub fn set_finalized_da_height_to_at_least(
&mut self,
height: &DaBlockHeight,
) -> StorageResult<()> {
let mut lock = self.data.lock().unwrap();
let max = lock.finalized_da_height.get_or_insert(0u64.into());
*max = (*max).max(*height);
Ok(())
}
}

impl RelayerDb for MockDb {
Expand Down Expand Up @@ -122,21 +130,7 @@ impl RelayerDb for MockDb {
Ok(())
}

fn set_finalized_da_height_to_at_least(
&mut self,
height: &DaBlockHeight,
) -> StorageResult<()> {
let mut lock = self.data.lock().unwrap();
let max = lock.finalized_da_height.get_or_insert(0u64.into());
*max = (*max).max(*height);
Ok(())
}

fn get_finalized_da_height(&self) -> StorageResult<DaBlockHeight> {
self.data
.lock()
.unwrap()
.finalized_da_height
.ok_or(not_found!("FinalizedDaHeight for test"))
fn get_finalized_da_height(&self) -> Option<DaBlockHeight> {
self.data.lock().unwrap().finalized_da_height
}
}
12 changes: 4 additions & 8 deletions crates/services/relayer/src/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,9 @@ pub trait RelayerDb: Send + Sync {
events: &[Event],
) -> StorageResult<()>;

/// Set finalized da height that represent last block from da layer that got finalized.
/// This will only set the value if it is greater than the current.
fn set_finalized_da_height_to_at_least(
&mut self,
block: &DaBlockHeight,
) -> StorageResult<()>;

/// Get finalized da height that represent last block from da layer that got finalized.
/// Panics if height is not set as of initialization of database.
fn get_finalized_da_height(&self) -> StorageResult<DaBlockHeight>;
fn get_finalized_da_height(&self) -> Option<DaBlockHeight>;
}

/// The trait that should be implemented by the database transaction returned by the database.
Expand All @@ -49,4 +42,7 @@ pub trait Transactional {

/// Returns the storage transaction.
fn transaction(&mut self) -> Self::Transaction<'_>;

/// Returns the latest da block height.
fn latest_da_height(&self) -> Option<DaBlockHeight>;
}
88 changes: 20 additions & 68 deletions crates/services/relayer/src/ports/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,21 @@ use crate::{
RelayerDb,
Transactional,
},
storage::{
DaHeightTable,
EventsHistory,
},
storage::EventsHistory,
Config,
};
use fuel_core_storage::test_helpers::{
MockBasic,
MockStorage,
};
use fuel_core_types::{
blockchain::primitives::DaBlockHeight,
entities::{
Message,
RelayedTransaction,
},
services::relayer::Event,
};
use std::borrow::Cow;
use test_case::test_case;

type DBTx = MockStorage<MockBasic, MockDatabaseTransaction>;
type ReturnDB = Box<dyn Fn() -> DBTx + Send + Sync>;
Expand All @@ -41,27 +38,23 @@ impl Transactional for MockDatabase {
fn transaction(&mut self) -> Self::Transaction<'_> {
(self.data)()
}

fn latest_da_height(&self) -> Option<DaBlockHeight> {
Some(Config::DEFAULT_DA_DEPLOY_HEIGHT.into())
}
}

#[test]
fn test_insert_events() {
let same_height = 12;
// Given
let same_height = 12u64;
let return_db_tx = move || {
let mut db = DBTx::default();
db.storage
.expect_insert::<EventsHistory>()
.times(1)
.returning(|_, _| Ok(None));
db.storage
.expect_insert::<DaHeightTable>()
.times(1)
.withf(move |_, v| **v == same_height)
.returning(|_, _| Ok(None));
db.data.expect_commit().returning(|| Ok(()));
db.storage
.expect_get::<DaHeightTable>()
.once()
.returning(|_| Ok(Some(std::borrow::Cow::Owned(9u64.into()))));
db
};

Expand All @@ -73,12 +66,18 @@ fn test_insert_events() {
let mut m = Message::default();
m.set_amount(10);
m.set_da_height(same_height.into());

let mut m2 = m.clone();
m2.set_nonce(1.into());
assert_ne!(m.id(), m2.id());

let messages = [m.into(), m2.into()];
db.insert_events(&same_height.into(), &messages[..])
.unwrap();

// When
let result = db.insert_events(&same_height.into(), &messages[..]);

// Then
assert!(result.is_ok());
}

#[test]
Expand All @@ -100,24 +99,16 @@ fn insert_always_raises_da_height_monotonically() {
db.storage
.expect_insert::<EventsHistory>()
.returning(|_, _| Ok(None));
db.storage
.expect_insert::<DaHeightTable>()
.once()
.withf(move |_, v| *v == same_height)
.returning(|_, _| Ok(None));
db.data.expect_commit().returning(|| Ok(()));
db.storage
.expect_get::<DaHeightTable>()
.once()
.returning(|_| Ok(None));
db
};

// When
let mut db = MockDatabase {
data: Box::new(return_db_tx),
storage: Default::default(),
};

// When
let result = db.insert_events(&same_height, &events);

// Then
Expand Down Expand Up @@ -175,6 +166,7 @@ fn insert_fails_for_events_same_height_but_on_different_height() {
data: Box::new(DBTx::default),
storage: Default::default(),
};

let next_height = last_height + 1;
let result = db.insert_events(&next_height.into(), &events);

Expand Down Expand Up @@ -207,43 +199,3 @@ fn insert_fails_for_events_same_height_but_on_different_height() {
last_height,
);
}

#[test_case(None, 0, 0; "can set DA height to 0 when there is none available")]
#[test_case(None, 10, 10; "can set DA height to 10 when there is none available")]
#[test_case(0, 10, 10; "can set DA height to 10 when it is 0")]
#[test_case(0, None, 0; "inserts are bypassed when height goes from 0 to 0")]
#[test_case(10, 11, 11; "can set DA height to 11 when it is 10")]
#[test_case(11, None, 11; "inserts are bypassed when height goes from 11 to 11")]
#[test_case(11, None, 10; "inserts are bypassed when height reverted from 11 to 10")]
fn set_raises_da_height_monotonically(
get: impl Into<Option<u64>>,
inserts: impl Into<Option<u64>>,
new_height: u64,
) {
let inserts = inserts.into();
let get = get.into();
let return_db_tx = move || {
let mut db = DBTx::default();
if let Some(h) = inserts {
db.storage
.expect_insert::<DaHeightTable>()
.once()
.withf(move |_, v| **v == h)
.returning(|_, _| Ok(None));
}
let get = get.map(|g| Cow::Owned(g.into()));
db.storage
.expect_get::<DaHeightTable>()
.once()
.returning(move |_| Ok(get.clone()));
db.data.expect_commit().returning(|| Ok(()));
db
};

let mut db = MockDatabase {
data: Box::new(return_db_tx),
storage: Default::default(),
};
db.set_finalized_da_height_to_at_least(&new_height.into())
.unwrap();
}

0 comments on commit 6f5c4fc

Please sign in to comment.