fix: block-miner does not block on startup and also accepts cancellation signal

[dinhani-cw]

No description provided.

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR involves multiple files and significant changes related to asynchronous operations and cancellation handling, which requires careful consideration to ensure thread safety and proper resource management.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The cancellation token is cloned multiple times across different files and functions. This could lead to scenarios where not all parts of the system respect the cancellation signal if not managed correctly.
	Performance Concern: The use of Arc<AtomicUsize> for managing state across async tasks in interval_miner.rs and interval_miner_ticker.rs might introduce unnecessary overhead and complexity. Consider simplifying the approach to state management.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/block_miner.rs
suggestion	Consider using more specific logging levels for different actions within the spawn_interval_miner function. For example, use tracing::debug! for regular interval actions and tracing::error! when an error occurs during mining. This will help in better monitoring and troubleshooting of the mining process. [important]
relevant line	pub fn spawn_interval_miner(self: Arc, cancellation: CancellationToken) -> anyhow::Result<()> {

---

relevant file	src/bin/importer_offline.rs
suggestion	Ensure proper error handling for the initialization of rpc_storage and storage in the run function. Currently, if an error occurs during their initialization, the function will return an error, but it might leave other initialized resources in an inconsistent state. Consider implementing a more robust cleanup or rollback mechanism. [important]
relevant line	let rpc_storage = config.rpc_storage.init().await?;

---

relevant file	src/eth/block_miner.rs
suggestion	Replace the use of AtomicUsize with a simpler concurrency mechanism that fits better with async programming patterns, such as tokio::sync::Mutex. This change will make the code more idiomatic to Rust's async ecosystem and potentially reduce complexity. [medium]
relevant line	let pending_blocks = Arc::new(AtomicUsize::new(0));

---

relevant file	src/eth/block_miner.rs
suggestion	To improve the robustness of the cancellation handling in interval_miner.rs, consider adding a timeout to the cancellation check to ensure that the loop does not get stuck in a non-responsive state if the cancellation signal is delayed or lost. [medium]
relevant line	if cancellation.is_cancelled() {

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Add error handling for the signal handler initialization Consider handling the error from signal_handler() gracefully to ensure the application does not panic if the signal handling setup fails. src/bin/importer_offline.rs [49] -let cancellation = signal_handler(); +let cancellation = signal_handler().map_err(|e| { + eprintln!("Failed to create cancellation token: {:?}", e); + std::process::exit(1); +})?; Suggestion importance[1-10]: 9 Why: Adding error handling for the signal handler initialization is crucial to prevent the application from panicking, which improves the robustness and reliability of the application.	9
	Use structured error handling for better error propagation Replace the direct logging and returning of an error with a more structured error handling approach using Result to propagate errors. src/eth/block_miner.rs [65] -return log_and_err!("cannot spawn interval miner because it does not have a block time defined"); +return Err(anyhow::anyhow!("cannot spawn interval miner because it does not have a block time defined")); Suggestion importance[1-10]: 8 Why: Using structured error handling with Result improves error propagation and makes the code more idiomatic and maintainable in Rust.	8
Best practice	Add a delay in the loop to handle cancellation more effectively and prevent potential deadlocks To avoid potential deadlocks and ensure that the cancellation token is checked periodically, consider adding a timeout or delay within the loop. src/eth/block_miner.rs [330-348] loop { if cancellation.is_cancelled() { warn_task_cancellation("interval miner"); return; } + tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; let pending = pending_blocks .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |n| Some(n.saturating_sub(1))) .unwrap(); if pending > 0 { tracing::info!(%pending, "interval mining block"); mine_and_commit(&miner).await; } else { tracing::debug!(%pending, "waiting for block interval"); yield_now().await; } } Suggestion importance[1-10]: 7 Why: Adding a delay within the loop helps to handle cancellation more effectively and prevents potential deadlocks, improving the reliability of the code.	7
Enhancement	Enhance logging by adding structured data It is recommended to use structured logging for better traceability and debugging. Replace the direct usage of tracing::info! with a structured log that includes more context about the operation. src/config.rs [246] -tracing::info!(config = ?self, "starting block miner"); +tracing::info!("Starting block miner", { config: ?self, action: "init", status: "begin" }); Suggestion importance[1-10]: 6 Why: Enhancing logging with structured data provides better traceability and debugging capabilities, though it is a minor improvement.	6