feat: implement tui #2
Conversation
|
Warning Rate limit exceeded@lgalabru has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughThis pull request updates dependency management, command handling, and UI features across several modules. Changes include adding and updating dependencies in the CLI package, enhancing the CLI command structure with a completions command, refactoring the simulation network command to use a synchronous multi-threaded approach, and introducing a full terminal user interface for simnet monitoring. Additionally, core module functions now send simulation events via channels and adjust timing and error handling. Minor modifications in macro imports and RPC imports are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as CLI App
participant GCH as generate_completion_helpers
participant FS as File System
U->>CLI: Invoke "completions" command
CLI->>GCH: Call generate_completion_helpers(cmd)
GCH->>FS: Write completion script
FS-->>GCH: Script saved confirmation
GCH-->>CLI: Return success message
CLI-->>U: Display completion instructions
sequenceDiagram
participant C as CLI Command
participant H as Simnet Handler
participant T as Background Thread
participant Core as Simnet Core
participant L as Event Logger
C->>H: Invoke start_simnet command
H->>T: Spawn background thread with channel (Tx/Rx)
T->>Core: Run start_simnet(simnet_events_tx)
Core->>T: Send SimnetEvent messages
T->>L: Relay events to log_events function
L-->>C: Log processed events
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
crates/cli/src/cli/simnet/mod.rs (2)
15-18: Consider more graceful fallback instead of forcing process exit.
Currently, ifnestable_block_onfails, the thread logs the error, sleeps, and then callsstd::process::exit(1). This approach abruptly terminates the entire process, which might hinder debugging or partial recovery scenarios.Consider returning the error to the main thread or providing a retry mechanism before exiting:
if let Err(e) = hiro_system_kit::nestable_block_on(future) { error!(ctx_cloned.expect_logger(), "{e}"); std::thread::sleep(std::time::Duration::from_millis(500)); - std::process::exit(1); + // Possibly propagate this error to the main thread or support a retry count: + return Err(e); }
24-29: Separate the TUI from logging logic more cleanly.
The current code toggles betweenlog_eventsandtui::simnet::start_appbased oncmd.no_tui. Although this works, merging the logic might help unify how events are handled (e.g., common filtering, shared event queue).crates/core/src/simnet/mod.rs (1)
71-72: Provide additional logs or fallback for server startup failure.
This block forcibly exits the process if the simulation server cannot start. While it may be appropriate, consider logging the cause or providing a fallback mechanism.crates/cli/src/tui/simnet.rs (2)
158-225: Limit or rotate stored log events to prevent unbounded memory usage.
Currently, the TUI stores incoming events in aVecDequewithout an explicit maximum size, which can lead to high memory usage over time if the simulation runs for an extended period.To mitigate this:
// For example, keep only the latest 10,000 events: const MAX_EVENTS: usize = 10_000; if app.events.len() >= MAX_EVENTS { app.events.pop_back(); }
211-223: Validate keyboard input.
All key events are assumed to be valid within the TUI, but spurious or unknown keys might appear. Consider a default case for logging unknown keys or ignoring them gracefully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
crates/cli/Cargo.toml(1 hunks)crates/cli/src/cli/mod.rs(5 hunks)crates/cli/src/cli/simnet/mod.rs(1 hunks)crates/cli/src/macros.rs(6 hunks)crates/cli/src/main.rs(1 hunks)crates/cli/src/tui/mod.rs(1 hunks)crates/cli/src/tui/simnet.rs(1 hunks)crates/core/src/lib.rs(1 hunks)crates/core/src/rpc/minimal.rs(1 hunks)crates/core/src/simnet/mod.rs(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- crates/cli/src/tui/mod.rs
- crates/cli/src/macros.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (15)
crates/cli/src/tui/simnet.rs (1)
185-186: Align with the renamedErrorLogevent.
If you rename the variant in the core simnet enum toErrorLog, ensure this usage is also updated here.- SimnetEvent::ErroLog(dt, log) => { + SimnetEvent::ErrorLog(dt, log) => {crates/core/src/lib.rs (2)
14-15: LGTM! Good separation of concerns.The addition of
SenderandSimnetEventimports indicates a well-designed event-driven architecture for monitoring simnet events.Also applies to: 19-19
23-27: LGTM! Clean function signature update.The updated
start_simnetsignature now accepts an event channel, enabling real-time monitoring of simnet events. This is a good architectural decision that:
- Decouples the simnet core from UI concerns
- Enables real-time monitoring
- Allows for multiple consumers of simnet events
crates/core/src/rpc/minimal.rs (1)
4-4: LGTM! Clean import statement.Good cleanup of unused
Errorimport while retainingResult.crates/cli/src/main.rs (1)
8-8: LGTM! Clean module organization.Good addition of the public
tuimodule, making the TUI functionality accessible to other parts of the application.crates/cli/src/cli/mod.rs (3)
73-77: LGTM! Good CLI options for simnet.The new options provide good flexibility:
--no-tuifor traditional logging output--debugfor detailed debugging
80-85: LGTM! Clean shell completion support.Good use of
clapfor shell completion generation with case-insensitive shell selection.
117-126: Verify file permissions and path safety.The completion script generation looks good but needs additional safety checks.
Consider adding these safety improvements:
fn generate_completion_helpers(cmd: &Completions) -> Result<(), String> { let mut app = Opts::command(); let file_name = cmd.shell.file_name("surfpool"); + // Ensure the file doesn't exist or is writable + if std::path::Path::new(&file_name).exists() { + return Err(format!("File {} already exists", file_name)); + } let mut file = File::create(file_name.clone()) .map_err(|e| format!("unable to create file {}: {}", file_name, e))?; + // Set restrictive permissions + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + file.set_permissions(std::fs::Permissions::from_mode(0o644)) + .map_err(|e| format!("unable to set file permissions: {}", e))?; + } clap_complete::generate(cmd.shell, &mut app, "surfpool", &mut file); println!("{} {}", green!("Created file"), file_name.clone()); println!("Check your shell's docs for how to enable completions for surfpool."); Ok(()) }crates/cli/Cargo.toml (7)
20-20: New Dependency: ansi_term Added
The dependencyansi_term = "0.12.1"has been added to support terminal styling. Please verify that its functionalities complement the new TUI features introduced viaratatuiand that there is no redundancy or conflict with other styling libraries.
24-24: Update: Simplified Format for clap_generate Dependency
Theclap_generatedependency is now specified using a simple string format"3.0.3"instead of the previous object format. Ensure that downstream references (e.g., incrates/cli/src/cli/mod.rs) are consistent with this change in dependency specification.
25-25: New Dependency: clap_complete Introduced
The addition ofclap_complete = "4.5.44"supports enhanced CLI completions. Confirm that the CLI command structure leveragesclap_completeappropriately and that this integration aligns with the updated command handling logic.
26-26: New Dependency: crossterm Added for Terminal I/O
The inclusion ofcrossterm = "0.28.1"provides powerful terminal I/O capabilities which are essential for the new TUI implementation.
27-27: New Dependency: ratatui for Terminal UI
The dependencyratatui = "0.29.0"has been added to support the new terminal user interface functionality. This is central to the TUI improvements outlined in the PR objectives.
28-28: New Dependency: chrono Included for Date/Time Functions
Addingchrono = "0.4"enriches the package with robust date and time handling. This will be useful for logging or event timestamping in the TUI and other modules.
32-32: Feature Update: Adjusted CLI Feature Configuration
Theclifeature has been updated to remove the dependency onclap_generate, now listing only["clap", "toml", "ctrlc", "hiro-system-kit/log"]. Ensure that any CLI code sections and documentation reflect this modified feature configuration.
crates/cli/src/cli/simnet/mod.rs
Outdated
| SimnetEvent::ErroLog(_, log) => { | ||
| error!(ctx.expect_logger(), "{} ", log); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename ErroLog variant for consistency.
The variant SimnetEvent::ErroLog is likely intended to be ErrorLog. This small naming mismatch can be confusing and deviates from typical naming conventions.
| let epoch_info = rpc_client.get_epoch_info().unwrap(); | ||
| let _ = simnet_events_tx.send(SimnetEvent::EpochInfoUpdate(epoch_info.clone())); |
There was a problem hiding this comment.
Avoid unwrap() to prevent panics on epoch info retrieval.
Calling rpc_client.get_epoch_info().unwrap() will panic on failure. Consider handling the Result to gracefully recover or report a more controlled error.
-let epoch_info = rpc_client.get_epoch_info().unwrap();
+let epoch_info = match rpc_client.get_epoch_info() {
+ Ok(info) => info,
+ Err(e) => return Err(format!("Failed to get epoch info: {}", e).into()),
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let epoch_info = rpc_client.get_epoch_info().unwrap(); | |
| let _ = simnet_events_tx.send(SimnetEvent::EpochInfoUpdate(epoch_info.clone())); | |
| let epoch_info = match rpc_client.get_epoch_info() { | |
| Ok(info) => info, | |
| Err(e) => return Err(format!("Failed to get epoch info: {}", e).into()), | |
| }; | |
| let _ = simnet_events_tx.send(SimnetEvent::EpochInfoUpdate(epoch_info.clone())); |
crates/core/src/simnet/mod.rs
Outdated
| pub enum SimnetEvent { | ||
| ClockUpdate(Clock), | ||
| EpochInfoUpdate(EpochInfo), | ||
| BlockHashExpired, | ||
| InfoLog(DateTime<Local>, String), | ||
| ErroLog(DateTime<Local>, String), | ||
| WarnLog(DateTime<Local>, String), | ||
| DebugLog(DateTime<Local>, String), | ||
| TransactionReceived(DateTime<Local>, VersionedTransaction), | ||
| AccountUpdate(DateTime<Local>, Pubkey), | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent naming for error logging.
The ErroLog variant name is potentially a typo. Rename it to ErrorLog to align with conventional naming:
-pub enum SimnetEvent {
+pub enum SimnetEvent {
ClockUpdate(Clock),
EpochInfoUpdate(EpochInfo),
BlockHashExpired,
- InfoLog(DateTime<Local>, String),
- ErroLog(DateTime<Local>, String),
+ InfoLog(DateTime<Local>, String),
+ ErrorLog(DateTime<Local>, String),
WarnLog(DateTime<Local>, String),
DebugLog(DateTime<Local>, String),
TransactionReceived(DateTime<Local>, VersionedTransaction),
AccountUpdate(DateTime<Local>, Pubkey),
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub enum SimnetEvent { | |
| ClockUpdate(Clock), | |
| EpochInfoUpdate(EpochInfo), | |
| BlockHashExpired, | |
| InfoLog(DateTime<Local>, String), | |
| ErroLog(DateTime<Local>, String), | |
| WarnLog(DateTime<Local>, String), | |
| DebugLog(DateTime<Local>, String), | |
| TransactionReceived(DateTime<Local>, VersionedTransaction), | |
| AccountUpdate(DateTime<Local>, Pubkey), | |
| } | |
| pub enum SimnetEvent { | |
| ClockUpdate(Clock), | |
| EpochInfoUpdate(EpochInfo), | |
| BlockHashExpired, | |
| InfoLog(DateTime<Local>, String), | |
| ErrorLog(DateTime<Local>, String), | |
| WarnLog(DateTime<Local>, String), | |
| DebugLog(DateTime<Local>, String), | |
| TransactionReceived(DateTime<Local>, VersionedTransaction), | |
| AccountUpdate(DateTime<Local>, Pubkey), | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
crates/cli/src/tui/simnet.rs (3)
24-35: Consider supporting multiple theme variations or external configuration.
Currently, theColorThemestruct is hard-coded to a specific palette (tailwind::EMERALD). If theming becomes more advanced in the future or if users request color customization, you might consider allowing a parameter or a configuration-driven approach to handle multiple themes.
234-234: Avoid callingapp.set_colors()on every render.
app.set_colors()is invoked on every UI render loop, but it only reassigns a static color palette. Consider moving it to the initialization phase, or call it only when the user actually changes the theme.- app.set_colors(); + // Only call app.set_colors() if theming is toggled or changed at runtime
304-320: Potential performance overhead inrender_slots.
For large terminal dimensions, generating and concatenating a big string of▮/▯characters might be expensive. Though acceptable for smaller screens, consider short-circuiting or limiting the output if performance becomes an issue for extremely large terminals.crates/cli/src/cli/simnet/mod.rs (2)
16-18: Avoid hard process exit.
std::process::exit(1)bypasses typical cleanup, dropping, and unwinding. If feasible, consider returning an error instead and letting upstream logic handle it gracefully.
37-75: Consider a mechanism to stop logging gracefully.
log_eventsloops until the channel is closed, which can be fine, but in situations where the simulator needs to stop early or the user cancels, some interruption mechanism may be desirable.crates/cli/src/cli/mod.rs (2)
73-77: Ensure consistent naming of flags and clarity in help text.
The flagsno-tuianddebugare straightforward, but consider clarifying them in the help text:
--no-tui: "Use plain logs instead of the TUI dashboard."--debug: "Enable debug-level logs."
This can improve usability for new users.
117-125: Enhance user guidance for generated completion scripts.
In addition to printing the file name, you could provide an explicit shell command snippet or link to the official docs for each shell so users can quickly enable completions.crates/core/src/simnet/mod.rs (1)
86-117: Refactor transaction processing into a separate function.The transaction processing logic is complex and could benefit from being moved into a dedicated function for better maintainability.
+fn process_transaction( + tx: VersionedTransaction, + ctx: &mut GlobalState, + rpc_client: &RpcClient, + simnet_events_tx: &Sender<SimnetEvent>, +) { + tx.verify_with_results(); + let tx = tx.into_legacy_transaction().unwrap(); + let message = &tx.message; + + for instruction in &message.instructions { + if instruction.program_id_index as usize >= message.account_keys.len() { + unreachable!(); + } + let program_id = &message.account_keys[instruction.program_id_index as usize]; + if ctx.svm.get_account(&program_id).is_none() { + let res = rpc_client.get_account(&program_id); + let event = match res { + Ok(account) => { + let _ = ctx.svm.set_account(*program_id, account); + SimnetEvent::AccountUpdate(Local::now(), program_id.clone()) + } + Err(e) => SimnetEvent::ErroLog( + Local::now(), + format!("unable to retrieve account: {}", e), + ), + }; + if let Err(e) = simnet_events_tx.send(event) { + eprintln!("Failed to send account event: {}", e); + } + } + } + let res = ctx.svm.send_transaction(tx); +}Then use it in the main loop:
while let Ok(tx) = mempool_rx.try_recv() { - // ... existing transaction processing code ... + process_transaction(tx, &mut ctx, &rpc_client, &simnet_events_tx); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/cli/Cargo.toml(1 hunks)crates/cli/src/cli/mod.rs(5 hunks)crates/cli/src/cli/simnet/mod.rs(1 hunks)crates/cli/src/tui/simnet.rs(1 hunks)crates/core/src/rpc/minimal.rs(1 hunks)crates/core/src/simnet/mod.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/core/src/rpc/minimal.rs
- crates/cli/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
crates/cli/src/tui/simnet.rs (1)
62-71: Potential unbounded event queue growth.
Storing events in an always-growingVecDeque<(EventType, DateTime<Local>, String)>without a retention policy can eventually lead to high memory usage. Consider a maximum capacity or expiration strategy (e.g., discarding the oldest events).crates/cli/src/cli/simnet/mod.rs (1)
56-57: Use consistent naming for the variant.
This repeats a past review comment about theErroLogvariant spelling. If you decide to rectify it, renameErroLogtoErrorLogto align with typical naming conventions.crates/core/src/simnet/mod.rs (3)
18-18: Verify impact of reduced slot time.The
DEFAULT_SLOT_TIMEhas been reduced from 2000ms to 400ms, which could significantly increase CPU usage and affect simulation fidelity.Please ensure this change:
- Doesn't overwhelm system resources
- Maintains accurate simulation behavior
- Aligns with real network timing requirements
26-36: Use consistent naming for error logging.The
ErroLogvariant name appears to be a typo and should be renamed toErrorLogto maintain consistent naming conventions.
45-45: Avoidunwrap()to prevent panics on epoch info retrieval.Using
unwrap()on the RPC client call could cause panics. Consider proper error handling instead.
|
|
||
| fn run_app<B: Backend>(terminal: &mut Terminal<B>, mut app: App) -> io::Result<()> { | ||
| loop { | ||
| while let Ok(event) = app.simnet_events_rx.try_recv() { |
There was a problem hiding this comment.
💡 Codebase verification
Investigation Outcome: TUI Shutdown Behavior
The grep results confirm that in the TUI code the event channel is polled via a non‐blocking call (try_recv()) in a loop. This loop continuously processes available events, which raises the concern noted: if new events keep arriving, the loop would keep processing them without an obvious opportunity for an orderly shutdown unless an explicit exit condition (or closed channel) is reached.
Points to Note:
- The channel is created in
crates/cli/src/cli/simnet/mod.rsand passed to the TUI (incrates/cli/src/tui/simnet.rs). - In the TUI, the code uses
while let Ok(event) = app.simnet_events_rx.try_recv()to pull events. - There is no evidence in this snippet that the TUI loop also checks for an external shutdown signal (e.g., a user input or a flag) before continuing to process events.
It is therefore advisable to verify that the design includes another mechanism—such as an external exit signal—to break out of the event-pumping loop. If not, continuous event input will indeed prevent graceful shutdown.
🔗 Analysis chain
Ensure channel activity doesn't block gracefully shutting down the TUI.
try_recv() is a non-blocking call, but if there are continuous events, the TUI loop will keep running indefinitely unless the channel is closed or some external exit signal is provided. Verify that this behavior matches the desired application flow.
You can investigate references to this simnet_events_rx usage across the repository to confirm no unintentional indefinite blocking:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for simnet_events_rx references throughout the repo
rg -A 5 "simnet_events_rx"
Length of output: 4163
| let _ = | ||
| simnet_events_tx.send(SimnetEvent::TransactionReceived(Local::now(), tx.clone())); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle event sending errors appropriately.
Event sending errors are currently ignored with let _. Consider logging these errors or implementing a retry mechanism.
-let _ = simnet_events_tx.send(SimnetEvent::TransactionReceived(Local::now(), tx.clone()));
+if let Err(e) = simnet_events_tx.send(SimnetEvent::TransactionReceived(Local::now(), tx.clone())) {
+ eprintln!("Failed to send transaction event: {}", e);
+}Also applies to: 132-132
| .cors(DomainsValidation::Disabled) | ||
| .start_http(&"127.0.0.1:8899".parse().unwrap()) | ||
| .expect("Unable to start RPC server"); | ||
| .start_http(&"127.0.0.1:8899".parse().unwrap()); |
There was a problem hiding this comment.
Handle address parsing errors gracefully.
Using unwrap() on address parsing could cause panics. Consider proper error handling:
-"127.0.0.1:8899".parse().unwrap()
+match "127.0.0.1:8899".parse() {
+ Ok(addr) => addr,
+ Err(e) => {
+ eprintln!("Failed to parse server address: {}", e);
+ std::process::exit(1);
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .start_http(&"127.0.0.1:8899".parse().unwrap()); | |
| .start_http(&match "127.0.0.1:8899".parse() { | |
| Ok(addr) => addr, | |
| Err(e) => { | |
| eprintln!("Failed to parse server address: {}", e); | |
| std::process::exit(1); | |
| } | |
| }); |
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Nitpick comments (4)
crates/cli/src/cli/simnet/mod.rs (1)
8-30: Improve error handling with more context.The error handling could be enhanced by providing more context about the failure points:
Apply this diff to improve error handling:
- .map_err(|e| format!("{}", e))?; + .map_err(|e| format!("Failed to spawn simnet thread: {}", e))?; - handle.join().map_err(|_e| format!("unable to terminate"))? + handle.join().map_err(|e| format!("Failed to join simnet thread: {:?}", e))?crates/cli/src/cli/mod.rs (1)
110-115: Remove unnecessary async marker.The function is marked as async but doesn't use any async operations. The implementation is synchronous.
Apply this diff to remove the async marker:
-async fn handle_command(opts: Opts, ctx: &Context) -> Result<(), String> { +fn handle_command(opts: Opts, ctx: &Context) -> Result<(), String> {crates/core/src/simnet/mod.rs (1)
70-73: Handle server startup errors gracefully.The server startup error handling could be improved by providing more context about the failure.
Apply this diff to improve error handling:
- let Ok(server) = res else { - std::process::exit(1); - }; + let server = res.map_err(|e| { + eprintln!("Failed to start server: {}", e); + std::process::exit(1); + })?;crates/cli/src/tui/simnet.rs (1)
322-366: Limit event queue size and make column widths configurable.Consider the following improvements:
- Add a maximum size for the event queue to prevent unbounded memory growth.
- Make column widths configurable or calculate them based on terminal width.
Apply this diff to limit queue size:
impl App { fn new(simnet_events_rx: Receiver<SimnetEvent>, include_debug_logs: bool) -> App { + const MAX_EVENTS: usize = 1000; App { state: TableState::default().with_selected(0), scroll_state: ScrollbarState::new(5 * ITEM_HEIGHT), colors: ColorTheme::new(&palette::tailwind::EMERALD), simnet_events_rx, clock: Clock::default(), epoch_info: EpochInfo { epoch: 0, slot_index: 0, slots_in_epoch: 0, absolute_slot: 0, block_height: 0, transaction_count: None, }, - events: VecDeque::new(), + events: VecDeque::with_capacity(MAX_EVENTS), include_debug_logs, } } + + fn add_event(&mut self, event_type: EventType, dt: DateTime<Local>, log: String) { + self.events.push_front((event_type, dt, log)); + if self.events.len() > MAX_EVENTS { + self.events.pop_back(); + } + } }
🛑 Comments failed to post (3)
crates/cli/src/cli/simnet/mod.rs (1)
32-76: 🛠️ Refactor suggestion
Handle logger access safely.
The function uses
expect_logger()which could panic at runtime. Consider handling the case where logger is not available.Apply this diff to handle logger access safely:
- info!( - ctx.expect_logger(), - "Surfpool: The best place to train before surfing Solana" - ); + if let Some(logger) = ctx.logger.as_ref() { + info!( + logger, + "Surfpool: The best place to train before surfing Solana" + ); + }Apply similar changes to other logger usages in the function.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn log_events(simnet_events_rx: Receiver<SimnetEvent>, include_debug_logs: bool, ctx: &Context) { if let Some(logger) = ctx.logger.as_ref() { info!( logger, "Surfpool: The best place to train before surfing Solana" ); } while let Ok(event) = simnet_events_rx.recv() { match event { SimnetEvent::AccountUpdate(_, account) => { info!( ctx.expect_logger(), "Account retrieved from Mainnet {}", account ); } SimnetEvent::EpochInfoUpdate(epoch_info) => { info!( ctx.expect_logger(), "Connection established. Epoch {}, Slot {}.", epoch_info.epoch, epoch_info.slot_index ); } SimnetEvent::ClockUpdate(clock) => { info!(ctx.expect_logger(), "Slot #{} ", clock.slot); } SimnetEvent::ErroLog(_, log) => { error!(ctx.expect_logger(), "{} ", log); } SimnetEvent::InfoLog(_, log) => { info!(ctx.expect_logger(), "{} ", log); } SimnetEvent::WarnLog(_, log) => { warn!(ctx.expect_logger(), "{} ", log); } SimnetEvent::DebugLog(_, log) => { if include_debug_logs { debug!(ctx.expect_logger(), "{} ", log); } } SimnetEvent::TransactionReceived(_, _transaction) => { info!(ctx.expect_logger(), "Transaction received"); } SimnetEvent::BlockHashExpired => {} } } }crates/cli/src/cli/mod.rs (1)
117-126:
⚠️ Potential issueAdd missing macro import.
The code uses the
green!macro but it's not imported.Add the following import at the top of the file:
+use crate::utils::green;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.use crate::utils::green; fn generate_completion_helpers(cmd: &Completions) -> Result<(), String> { let mut app = Opts::command(); let file_name = cmd.shell.file_name("surfpool"); let mut file = File::create(file_name.clone()) .map_err(|e| format!("unable to create file {}: {}", file_name, e))?; clap_complete::generate(cmd.shell, &mut app, "surfpool", &mut file); println!("{} {}", green!("Created file"), file_name.clone()); println!("Check your shell’s docs for how to enable completions for surfpool."); Ok(()) }crates/cli/src/tui/simnet.rs (1)
94-96: 🛠️ Refactor suggestion
Handle slot conversion safely.
Using
unwrap()on the conversion could cause panics if the slot value doesn't fit in usize.Apply this diff to handle the conversion safely:
- pub fn slot(&self) -> usize { - self.clock.slot.try_into().unwrap() + pub fn slot(&self) -> usize { + self.clock.slot.try_into().unwrap_or(0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub fn slot(&self) -> usize { self.clock.slot.try_into().unwrap_or(0) }
Summary by CodeRabbit
New Features
Refactor
Chores