enha: add global node mode

[gabriel-aranha-cw]

PR Type

Enhancement

---

Description

• Implemented a global node mode system with Leader and Follower modes
• Refactored miner and importer configurations to support different node modes
• Updated main application flow to initialize node mode based on configuration
• Modified importer and miner initialization to use the new node mode system
• Simplified external mode handling in various components
• Updated tests to reflect changes in miner initialization
• Added new GlobalState methods for managing and querying node mode

---

Changes walkthrough 📝

	Relevant files
Enhancement	importer_offline.rs Update miner initialization in importer_offline                    src/bin/importer_offline.rs Updated init_external_mode to init_with_mode with MinerMode::External +2/-1      importer_config.rs Refactor importer config for node modes                                    src/eth/follower/importer/importer_config.rs Added GlobalState and NodeMode imports Modified init method to handle different node modes Added init_follower method for follower node initialization +15/-11  miner_config.rs Refactor miner config for node modes                                          src/eth/miner/miner_config.rs Removed init_external_mode method Updated init method to use node mode for determining miner mode Added init_with_mode method for specific mode initialization +14/-9    globals.rs Implement global node mode management                                        src/globals.rs Added NodeMode enum with Leader and Follower variants Implemented GlobalState methods for managing node mode Added IS_LEADER atomic boolean for tracking node mode +42/-0    lib.rs Export NodeMode from lib                                                                  src/lib.rs Added NodeMode to public exports +1/-0      main.rs Implement node mode in main application flow                          src/main.rs Added node mode initialization based on config Simplified miner and importer initialization Removed explicit follower mode handling +10/-18
Tests	test_import_external_snapshot_common.rs Update test for new miner initialization                                  tests/test_import_external_snapshot_common.rs Updated miner initialization to use init_with_mode with MinerMode::External +2/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 2678576)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Atomic Operations The use of AtomicBool with Ordering::Relaxed for IS_LEADER might not provide sufficient synchronization in a multi-threaded environment. Consider using Ordering::SeqCst for stronger guarantees. Error Handling The init method returns Ok(None) for NodeMode::Leader without any logging or explanation. Consider adding a log message to explain why no consensus is initialized for the leader mode. Logging The node mode initialization is not logged. Consider adding a log message to indicate which mode the node is starting in.

[gabriel-aranha-cw]

/describe

[github-actions]

PR Description updated to latest commit (70d8833)

[gabriel-aranha-cw]

/describe

[github-actions]

PR Description updated to latest commit (2c8febf)

[github-actions]

Persistent review updated to latest commit 2678576

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 2678576

Category	Suggestion	Score
Enhancement	Use an atomic enum representation for better type safety and to avoid potential race conditions Consider using an atomic enum instead of a boolean flag for better type safety and to avoid potential race conditions when setting or getting the node mode. src/globals.rs [101-212] -static IS_LEADER: AtomicBool = AtomicBool::new(false); +use std::sync::atomic::{AtomicU8, Ordering}; + +static NODE_MODE: AtomicU8 = AtomicU8::new(NodeMode::Follower as u8); pub fn set_node_mode(mode: NodeMode) { - IS_LEADER.store(matches!(mode, NodeMode::Leader), Ordering::Relaxed); + NODE_MODE.store(mode as u8, Ordering::Relaxed); } pub fn get_node_mode() -> NodeMode { - if IS_LEADER.load(Ordering::Relaxed) { - NodeMode::Leader - } else { - NodeMode::Follower + match NODE_MODE.load(Ordering::Relaxed) { + 0 => NodeMode::Follower, + 1 => NodeMode::Leader, + _ => panic!("Invalid node mode"), } } Suggestion importance[1-10]: 7 Why: This suggestion offers improved type safety and could prevent potential issues, but the current implementation is functional and the change is not critical.	7
Maintainability	Rename the method to better reflect its purpose of initializing consensus based on node mode Consider using a more descriptive name for the init method to clarify its purpose and differentiate it from the init_follower method. src/eth/follower/importer/importer_config.rs [38-43] -pub async fn init(&self, executor: Arc<Executor>, miner: Arc<Miner>, storage: Arc<StratusStorage>) -> anyhow::Result<Option<Arc<dyn Consensus>>> { +pub async fn init_consensus(&self, executor: Arc<Executor>, miner: Arc<Miner>, storage: Arc<StratusStorage>) -> anyhow::Result<Option<Arc<dyn Consensus>>> { match GlobalState::get_node_mode() { NodeMode::Follower => self.init_follower(executor, miner, storage).await, NodeMode::Leader => Ok(None), } } Suggestion importance[1-10]: 6 Why: The suggestion improves code clarity by using a more descriptive method name, which can enhance maintainability and readability.	6
	Extract node mode initialization logic into a separate function for better code organization Consider moving the node mode initialization logic to a separate function for better code organization and reusability. src/main.rs [11-15] -GlobalState::set_node_mode(if global_services.config.follower { - NodeMode::Follower -} else { - NodeMode::Leader -}); +fn initialize_node_mode(config: &StratusConfig) { + let mode = if config.follower { + NodeMode::Follower + } else { + NodeMode::Leader + }; + GlobalState::set_node_mode(mode); +} +// In main function: +initialize_node_mode(&global_services.config); + Suggestion importance[1-10]: 5 Why: The suggestion improves code organization, but the current implementation is simple and clear enough. The benefit is minor.	5
Best practice	Use a match expression with an exhaustive pattern to handle all possible enum variants Consider using a match expression instead of an if-else statement for better readability and to ensure all possible enum variants are handled. src/eth/miner/miner_config.rs [30-33] let mode = match GlobalState::get_node_mode() { NodeMode::Follower => MinerMode::External, NodeMode::Leader => self.block_mode, + _ => panic!("Unexpected node mode"), }; Suggestion importance[1-10]: 3 Why: The suggestion is valid but unnecessary. The existing code already uses a match expression and covers all possible enum variants.	3

---

Previous suggestions

Suggestions up to commit e34f6b1

Category	Suggestion	Score
Possible issue	Use stricter memory ordering for atomic operations on node mode Consider using Ordering::SeqCst instead of Ordering::Relaxed for atomic operations on IS_LEADER. This ensures stricter memory ordering guarantees, which can be important for maintaining consistency in a distributed system. src/globals.rs [190-201] pub fn set_node_mode(mode: NodeMode) { - IS_LEADER.store(matches!(mode, NodeMode::Leader), Ordering::Relaxed); + IS_LEADER.store(matches!(mode, NodeMode::Leader), Ordering::SeqCst); } pub fn get_node_mode() -> NodeMode { - if IS_LEADER.load(Ordering::Relaxed) { + if IS_LEADER.load(Ordering::SeqCst) { NodeMode::Leader } else { NodeMode::Follower } } Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential concurrency issue by using stricter memory ordering, which is important for maintaining consistency in a distributed system.	8
Performance	Optimize memory usage of the NodeMode enum by specifying its representation Use a more memory-efficient representation for the NodeMode enum by deriving #[repr(u8)]. This can be beneficial for systems with many nodes or frequent mode changes. src/globals.rs [77-81] #[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[repr(u8)] pub enum NodeMode { Leader, Follower, } Suggestion importance[1-10]: 7 Why: This suggestion offers a minor optimization that could be beneficial in systems with many nodes, without introducing significant complexity.	7
Maintainability	Extract node mode initialization into a separate function for improved code organization Consider extracting the node mode initialization logic into a separate function for better code organization and reusability. This can make the main function cleaner and easier to understand. src/main.rs [11-18] -fn main() -> anyhow::Result<()> { - let global_services = GlobalServices::<StratusConfig>::init(); - GlobalState::set_node_mode(if global_services.config.follower { +fn initialize_node_mode(config: &StratusConfig) { + let mode = if config.follower { NodeMode::Follower } else { NodeMode::Leader - }); + }; + GlobalState::set_node_mode(mode); +} + +fn main() -> anyhow::Result<()> { + let global_services = GlobalServices::<StratusConfig>::init(); + initialize_node_mode(&global_services.config); global_services.runtime.block_on(run(global_services.config)) } Suggestion importance[1-10]: 6 Why: The suggestion improves code organization and readability, but the current implementation is not overly complex and the benefit is relatively minor.	6
Enhancement	Replace boolean flag with an atomic pointer to an enum for representing node mode Consider using an enum with associated values instead of separate boolean checks for leader and follower modes. This approach can be more type-safe and easier to extend in the future. src/globals.rs [77-100] pub enum NodeMode { Leader, Follower, } /// Current node mode. -static IS_LEADER: AtomicBool = AtomicBool::new(false); +static NODE_MODE: AtomicPtr<NodeMode> = AtomicPtr::new(Box::into_raw(Box::new(NodeMode::Follower))); Suggestion importance[1-10]: 5 Why: The suggestion proposes a more type-safe approach, but it introduces unnecessary complexity and potential memory management issues with raw pointers.	5