feat: use timeout instead of yield

[dinhani-cw]

No description provided.

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes in concurrency and synchronization mechanisms which are critical and can be error-prone. The changes include renaming of global variables, replacing yield_now with sleep, and modifying how block numbers are updated. These changes require careful consideration to ensure they do not introduce race conditions or logic errors.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The use of Ordering::Relaxed in fetch_update and load might not be sufficient for ensuring memory visibility across threads, which could lead to stale reads or writes in a multi-threaded context.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/bin/importer_online.rs
suggestion	Consider using Ordering::SeqCst in fetch_update and load for EXTERNAL_RPC_CURRENT_BLOCK to ensure proper memory ordering and visibility across threads. This change is important to prevent potential data races or stale data issues in a multi-threaded environment. [important]
relevant line	let _ = EXTERNAL_RPC_CURRENT_BLOCK.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current_number| {

---

relevant file	src/bin/importer_online.rs
suggestion	Replace the custom if_else! macro with a standard Rust conditional expression to improve code readability and maintainability. Custom macros can obscure the logic and make the code harder to understand for new developers or contributors. [medium]
relevant line	if_else!(new_number_u64 >= current_number, Some(new_number_u64), None)

---

relevant file	src/bin/importer_online.rs
suggestion	Add error handling for the fetch_update operation on EXTERNAL_RPC_CURRENT_BLOCK. Currently, the result is ignored with let _ =. Handling this result could prevent silent failures and make the system more robust. [important]
relevant line	let _ = EXTERNAL_RPC_CURRENT_BLOCK.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current_number| {

---

relevant file	src/bin/importer_online.rs
suggestion	Consider reducing the magic number 1000 in blocks_to_fetch calculation to a named constant to improve code readability and maintainability. This helps in understanding the significance of the number and makes it easier to modify if needed. [medium]
relevant line	let mut blocks_to_fetch = min(blocks_behind, 1000); // avoid spawning millions of tasks (not parallelism), at least until we know it is safe

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Improve thread safety by using stronger memory ordering in atomic operations Replace the use of Ordering::Relaxed with Ordering::SeqCst in the fetch_update method to ensure a stronger memory ordering guarantee, which is crucial for correct synchronization across threads. src/bin/importer_online.rs [46-48] -let _ = EXTERNAL_RPC_CURRENT_BLOCK.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current_number| { +let _ = EXTERNAL_RPC_CURRENT_BLOCK.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |current_number| { if_else!(new_number_u64 >= current_number, Some(new_number_u64), None) }); Suggestion importance[1-10]: 9 Why: This suggestion improves thread safety by using a stronger memory ordering, which is crucial for correct synchronization across threads. It addresses a significant issue in concurrent programming.	9
Maintainability	Simplify conditional logic using standard Rust syntax Replace the custom if_else! macro with a standard Rust conditional expression to improve code readability and maintainability. src/bin/importer_online.rs [47] -if_else!(new_number_u64 >= current_number, Some(new_number_u64), None) +if new_number_u64 >= current_number { Some(new_number_u64) } else { None } Suggestion importance[1-10]: 8 Why: Simplifying the conditional logic using standard Rust syntax improves code readability and maintainability. This is a good practice for long-term code maintenance.	8
Enhancement	Improve variable naming for better code readability Consider using a more descriptive variable name than new_number_u64 to enhance code clarity, especially since it represents a block number converted to u64. src/bin/importer_online.rs [45] -let new_number_u64 = new_number.as_u64(); +let block_number_u64 = new_number.as_u64(); Suggestion importance[1-10]: 7 Why: Improving variable naming enhances code readability, making it easier for future developers to understand the code. While not critical, it is a beneficial enhancement.	7
Performance	Optimize the wait duration for fetching receipts to enhance performance Adjust the BACKOFF_RECEIPTS duration to match the previously used value more closely, considering the potential impact on system performance and user experience. src/bin/importer_online.rs [67] -const BACKOFF_RECEIPTS: Duration = Duration::from_millis(50); +const BACKOFF_RECEIPTS: Duration = Duration::from_millis(45); Suggestion importance[1-10]: 6 Why: This suggestion aims to optimize the wait duration for fetching receipts, potentially enhancing performance. However, the impact may be minor, so it is not as crucial as other suggestions.	6