Skip to content

Commit c6bda0a

Browse files
authored
fix(tests): flakiness across a few different tests (#2982)
> [!NOTE] > discovered the issue, bob doesn't have the delegate keys, and therefore when he receives preconfirmations he can't verify the signature, and then rejects the messages. the rejections reduce the peer score of the authority node and then the two are disconnected. maybe the preconfirmation service should just `GossipsubMessageAcceptance::Ignore` if it has no delegate keys instead of `GossipsubMessageAcceptance::Reject`. focusing on improving the configuration of nodes, enhancing consistency checks, and cleaning up unused imports. ### Configuration Enhancements: * Added a new configuration option `pre_confirmation_signature_service.echo_delegation_interval` to set an interval of 100 milliseconds for echo delegation in `make_nodes`. This improves configurability for testing scenarios. * Introduced conditional database initialization in `make_config`, using `CombinedDatabase` when the `default` feature is enabled, and falling back to an in-memory database otherwise. This provides flexibility for different environments. ### Consistency Check Improvements: * Refactored the `consistency` method to use a `tokio::time::interval` for periodic checks, replacing the previous loop and event-based logic. This simplifies the code and ensures consistent behavior. * Consolidated consistency methods by introducing `consistency_with_duration`, which allows specifying a custom timeout duration. The existing `consistency_10s` and `consistency_20s` methods now delegate to this new method for better code reuse. ### Cleanup: * Removed an unused import `schema::tx::types::TransactionStatus` to tidy up the codebase.
1 parent 47b84ca commit c6bda0a

3 files changed

Lines changed: 49 additions & 37 deletions

File tree

crates/fuel-core/src/p2p_test_helpers.rs

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::{
1212
},
1313
fuel_core_graphql_api::storage::transactions::TransactionStatuses,
1414
p2p::Multiaddr,
15-
schema::tx::types::TransactionStatus,
1615
service::{
1716
Config,
1817
FuelService,
@@ -401,6 +400,12 @@ pub async fn make_nodes(
401400

402401
node_config.consensus_signer = SignMode::Key(Secret::new(secret.into()));
403402

403+
if !txs.is_empty() {
404+
node_config
405+
.pre_confirmation_signature_service
406+
.echo_delegation_interval = Duration::from_millis(200);
407+
}
408+
404409
test_txs = txs;
405410
}
406411

@@ -507,7 +512,14 @@ pub fn make_config(
507512
}
508513

509514
pub async fn make_node(node_config: Config, test_txs: Vec<Transaction>) -> Node {
515+
#[cfg(feature = "default")]
516+
let db = CombinedDatabase::from_config(&node_config.combined_db_config)
517+
.unwrap()
518+
.on_chain()
519+
.clone();
520+
#[cfg(not(feature = "default"))]
510521
let db = Database::in_memory();
522+
511523
// Test coverage slows down the execution a lot, and while running all tests,
512524
// it may require a lot of time to start the node. We have a
513525
// timeout here to watch infinity loops, so it is okay to use 120 seconds.
@@ -573,55 +585,55 @@ impl Node {
573585
/// Wait for the node to reach consistency with the given transactions.
574586
pub async fn consistency(&mut self, txs: &HashMap<Bytes32, Transaction>) {
575587
let db = self.node.shared.database.off_chain();
576-
loop {
577-
let not_found = not_found_txs(db, txs);
578-
579-
if not_found.is_empty() {
580-
break;
581-
}
582-
583-
let tx_id = not_found[0];
584-
let mut wait_transaction =
585-
self.node.transaction_status_change(tx_id).await.unwrap();
588+
let mut interval = tokio::time::interval(tokio::time::Duration::from_millis(100));
586589

587-
loop {
588-
tokio::select! {
589-
result = wait_transaction.next() => {
590-
let status = result.unwrap().unwrap();
590+
// first tick completes immediately
591+
interval.tick().await;
591592

592-
if matches!(status, TransactionStatus::Failure { .. })
593-
|| matches!(status, TransactionStatus::Success { .. }) {
594-
break
595-
}
593+
loop {
594+
tokio::select! {
595+
_ = interval.tick() => {
596+
let not_found = not_found_txs(db, txs);
596597

597-
if matches!(status, TransactionStatus::SqueezedOut(_)) {
598-
panic!("Transaction was squeezed out: {:?}", status);
599-
}
600-
}
601-
_ = self.node.await_shutdown() => {
602-
panic!("Got a stop signal")
598+
if not_found.is_empty() {
599+
break;
603600
}
604601
}
602+
_ = self.node.await_shutdown() => {
603+
panic!("Got a stop signal")
604+
}
605605
}
606606
}
607607
}
608608

609-
/// Wait for the node to reach consistency with the given transactions within 10 seconds.
610-
pub async fn consistency_10s(&mut self, txs: &HashMap<Bytes32, Transaction>) {
611-
tokio::time::timeout(Duration::from_secs(10), self.consistency(txs))
609+
pub async fn consistency_with_duration(
610+
&mut self,
611+
txs: &HashMap<Bytes32, Transaction>,
612+
duration: Duration,
613+
) {
614+
tokio::time::timeout(duration, self.consistency(txs))
612615
.await
613616
.unwrap_or_else(|_| {
614617
panic!("Failed to reach consistency for {:?}", self.config.name)
615618
});
616619
}
617620

621+
/// Wait for the node to reach consistency with the given transactions within 10 seconds.
622+
pub async fn consistency_10s(&mut self, txs: &HashMap<Bytes32, Transaction>) {
623+
self.consistency_with_duration(txs, Duration::from_secs(10))
624+
.await;
625+
}
626+
618627
/// Wait for the node to reach consistency with the given transactions within 20 seconds.
619628
pub async fn consistency_20s(&mut self, txs: &HashMap<Bytes32, Transaction>) {
620-
tokio::time::timeout(Duration::from_secs(20), self.consistency(txs))
621-
.await
622-
.unwrap_or_else(|_| {
623-
panic!("Failed to reach consistency for {:?}", self.config.name)
624-
});
629+
self.consistency_with_duration(txs, Duration::from_secs(20))
630+
.await;
631+
}
632+
633+
/// Wait for the node to reach consistency with the given transactions within 30 seconds.
634+
pub async fn consistency_30s(&mut self, txs: &HashMap<Bytes32, Transaction>) {
635+
self.consistency_with_duration(txs, Duration::from_secs(30))
636+
.await;
625637
}
626638

627639
/// Insert the test transactions into the node's transaction pool.

tests/tests/peering.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async fn max_discovery_peers_connected__node_will_not_discover_new_nodes_if_full
5757
assert!(actual <= expected);
5858
}
5959

60-
#[tokio::test]
60+
#[tokio::test(flavor = "multi_thread")]
6161
async fn max_functional_peers_connected__nodes_will_discover_new_peers_if_first_peer_is_full()
6262
{
6363
let mut rng = StdRng::seed_from_u64(1234);
@@ -87,7 +87,7 @@ async fn max_functional_peers_connected__nodes_will_discover_new_peers_if_first_
8787
None,
8888
)
8989
.await;
90-
tokio::time::sleep(Duration::from_secs(1)).await;
90+
tokio::time::sleep(Duration::from_secs(10)).await;
9191

9292
// then
9393
let Nodes {
@@ -98,7 +98,7 @@ async fn max_functional_peers_connected__nodes_will_discover_new_peers_if_first_
9898
let producer = producers.pop().unwrap();
9999
let client = FuelClient::from(producer.node.bound_address);
100100
client.produce_blocks(new_blocks, None).await.unwrap();
101-
tokio::time::sleep(Duration::from_secs(10)).await;
101+
tokio::time::sleep(Duration::from_secs(20)).await;
102102

103103
for validator in validators {
104104
let client = FuelClient::from(validator.node.bound_address);

tests/tests/sync.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ async fn test_multiple_producers_different_keys() {
294294
for (expected, mut validators) in expected.iter().zip(groups) {
295295
assert_eq!(group_size, validators.len());
296296
for v in &mut validators {
297-
v.consistency_20s(expected).await;
297+
v.consistency_30s(expected).await;
298298
}
299299
}
300300
}

0 commit comments

Comments
 (0)