Conversation
WalkthroughAdds a combined CLI and TUI application, configuration and context layers, service scaffolding (wallet, stake, transfer, subnet, weights, root, crowd), display utilities, comprehensive error/result types, multiple TUI components/views, and bittensor-rs API additions (DynamicSubnetInfo, get_all_dynamic_info, Service::client). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (clap)
participant AppCtx as AppContext
participant Config as Config
participant Service as Service Layer
participant Output as Formatter
User->>CLI: invoke command
CLI->>CLI: parse args
CLI->>AppCtx: build context (network,wallet,hotkey)
AppCtx->>Config: load defaults
Config-->>AppCtx: return Config
AppCtx-->>CLI: context ready
CLI->>Service: dispatch handler (wallet/stake/...)
Service->>Service: perform operation (fs or network call)
Service-->>Output: produce result (Text/JSON)
Output->>User: render output
sequenceDiagram
participant User
participant Terminal as TUI
participant App as App (state)
participant Views as Views
participant Async as AsyncWorker
User->>Terminal: start TUI
Terminal->>App: App::new(ctx)
App->>App: load wallets (sync/async)
loop event loop
Terminal->>Views: draw(frame, app)
Views->>Terminal: rendered frame
User->>Terminal: key input
Terminal->>App: handle input -> update state
App->>Async: spawn tasks (balances, subnets)
Async-->>App: send AsyncResult
App->>Views: update state with results
end
User->>Terminal: quit
Terminal->>User: restore terminal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
lightning-tensor/src/tui/components/popup.rs (1)
92-111: Duplicatecentered_recthelper.As noted in the
views/mod.rsreview, this function is duplicated. Consider consolidating into a shared utility module.
🧹 Nitpick comments (27)
lightning-tensor/src/tui/components/input.rs (1)
13-18: Consider makingis_passwordandis_focusedprivate to enforce builder usage.The struct exposes all fields as
pub, but the builder methodspassword()andfocused()suggest these should be encapsulated. Making them private (is_password: bool) would enforce consistent construction patterns.🔎 Proposed change
pub struct InputField<'a> { pub prompt: &'a str, pub value: &'a str, - pub is_password: bool, - pub is_focused: bool, + is_password: bool, + is_focused: bool, }lightning-tensor/src/services/crowd.rs (1)
5-18: Consider using#[derive(Default)]to reduce boilerplate.Since
new()simply returnsSelf, theDefaultimpl can be derived. This applies to all similar service structs in this PR.🔎 Proposed change
+#[derive(Default)] /// Service for crowdfunding operations pub struct CrowdService; impl CrowdService { pub fn new() -> Self { Self } } - -impl Default for CrowdService { - fn default() -> Self { - Self::new() - } -}lightning-tensor/src/tui/views/stake.rs (1)
14-29: Consider&Appinstead of&mut Appif no mutation is needed.The
drawfunction and its helpers only read fromapp. If the signature is meant to match other views that may mutate, this is fine; otherwise, prefer&Appfor clarity.🔎 Proposed signature
-pub fn draw(f: &mut Frame, app: &mut App, area: Rect) { +pub fn draw(f: &mut Frame, app: &App, area: Rect) {lightning-tensor/src/tui/views/subnet.rs (1)
91-92: Note: Table state management incomplete.The comment indicates that proper
TableStatemanagement isn't implemented yet. While this works for rendering, consider implementing full table state management for features like selection and scrolling when you expand this view.lightning-tensor/src/cli/transfer.rs (1)
17-19: Consider using a more precise type for monetary amounts.Using
f64for cryptocurrency amounts can lead to precision loss. Blockchain systems typically use integer types (e.g.,u64,u128, orDecimal) to represent amounts in the smallest denomination (like satoshis or wei) to avoid floating-point arithmetic issues.💡 Alternative approach
Consider using a string or integer type with a known decimal precision, or a dedicated decimal type like
rust_decimal::Decimal:- /// Amount in TAO - #[arg(short, long)] - pub amount: f64, + /// Amount in TAO (will be converted to RAO internally) + #[arg(short, long)] + pub amount: String,This allows parsing the amount with precise decimal handling when the actual implementation is added.
lightning-tensor/src/tui/views/home.rs (1)
108-130: Consider more accurate messaging for lock contention.The messages rendering correctly uses
try_lockto avoid blocking the UI thread. However, displaying "Loading..." when the lock is unavailable might be slightly misleading, as it suggests an async operation rather than temporary lock contention.💡 Optional improvement
Consider a more specific message:
} else { - "Loading...".to_string() + "Updating...".to_string() };This better reflects that messages are being updated rather than initially loaded. However, the current implementation is acceptable given that lock contention should be rare.
lightning-tensor/src/tui/views/wallet.rs (1)
138-144: Potential panic on non-ASCII input and code duplication.Byte slicing (
&addr[..10]) can panic if the string contains multi-byte UTF-8 characters and the slice boundary falls within a character. While SS58 addresses are typically ASCII-safe, this could fail on unexpected input.Additionally, there's a similar
truncate_addressfunction inlightning-tensor/src/models/display.rswith a more flexible signature acceptingprefix_lenandsuffix_lenparameters. Consider reusing that implementation for consistency.🔎 Suggested safer implementation
fn truncate_address(addr: &str) -> String { if addr.len() > 20 { - format!("{}...{}", &addr[..10], &addr[addr.len()-8..]) + let chars: Vec<char> = addr.chars().collect(); + if chars.len() > 20 { + let prefix: String = chars[..10].iter().collect(); + let suffix: String = chars[chars.len()-8..].iter().collect(); + format!("{}...{}", prefix, suffix) + } else { + addr.to_string() + } } else { addr.to_string() } }Or reuse the existing helper:
use crate::models::display::truncate_address; // Call as: truncate_address(addr, 10, 8)lightning-tensor/src/tui/views/mod.rs (1)
144-163: Duplicatecentered_rectimplementation.This exact implementation is duplicated in
lightning-tensor/src/tui/components/popup.rs(lines 92-111). Consider extracting this utility to a shared location (e.g.,tui/utils.rsortui/components/mod.rs) and reusing it in both places.lightning-tensor/src/cli/weights.rs (1)
25-28: Unusedyesflag parameter.The
yesflag is defined in theSetvariant (line 27) but explicitly discarded in the match (yes: _on line 67). Either implement the confirmation skip logic or remove the flag if it's not needed for the current stub implementation.Also applies to: 67-67
lightning-tensor/src/cli/wallet.rs (2)
19-21: Consider validating mnemonic word count.The
wordsparameter accepts anyu8, but BIP-39 only supports specific word counts (12, 15, 18, 21, 24). Invalid values will likely cause errors deeper in the wallet creation flow. Consider adding validation or using an enum.🔎 Suggested improvement
+ /// Number of mnemonic words (12, 15, 18, 21, or 24) - /// Number of mnemonic words (12 or 24) #[arg(short, long, default_value = "12")] + #[arg(value_parser = clap::builder::PossibleValuesParser::new(["12", "15", "18", "21", "24"]))] words: u8,
134-139: Consider adding password confirmation for wallet creation.When creating a new wallet, users should confirm their password to prevent accidental typos that would lock them out of their funds. The
dialoguer::Passwordtype supports.with_confirmation().🔎 Suggested fix
let password = if no_password { String::new() } else { - prompt_password("Enter wallet password: ")? + dialoguer::Password::new() + .with_prompt("Enter wallet password") + .with_confirmation("Confirm password", "Passwords do not match") + .interact() + .map_err(|e| crate::errors::Error::ui(e.to_string()))? };lightning-tensor/src/models/display.rs (1)
17-28: UTF-8 safety concern with byte slicing.Similar to the TUI wallet view, byte slicing on strings (
&address[..prefix_len]) can panic on multi-byte UTF-8 characters. While SS58 addresses are ASCII, this function has a generic signature that could be called with arbitrary strings.🔎 Suggested safer implementation
pub fn truncate_address(address: &str, prefix_len: usize, suffix_len: usize) -> String { if address.len() <= prefix_len + suffix_len + 3 { return address.to_string(); } - format!( - "{}...{}", - &address[..prefix_len], - &address[address.len() - suffix_len..] - ) + let chars: Vec<char> = address.chars().collect(); + if chars.len() <= prefix_len + suffix_len + 3 { + return address.to_string(); + } + + let prefix: String = chars[..prefix_len].iter().collect(); + let suffix: String = chars[chars.len() - suffix_len..].iter().collect(); + format!("{}...{}", prefix, suffix) }lightning-tensor/src/cli/stake.rs (2)
233-251: Delegate/Undelegate output messages are misleading.
delegate_stakeandundelegate_stakedirectly calladd_stake/remove_stake, which output messages about "staking" rather than "delegating". When these stubs are implemented, the user will see confusing output like "Would stake X TAO" instead of "Would delegate X TAO".Consider either:
- Passing a descriptive action string to the shared implementation
- Creating separate output logic for delegation operations
113-121: Consider validatingtakeparameter range.The
takeparameter forSetTakeis documented as "0.0 to 1.0" but there's no validation. Invalid values could cause unexpected behavior when the actual implementation is added.🔎 Suggested validation with clap
SetTake { /// Take percentage (0.0 to 1.0) + #[arg(value_parser = clap::value_parser!(f64).range(0.0..=1.0))] take: f64,lightning-tensor/src/cli/crowd.rs (2)
12-67: Consider using a fixed-point type for TAO amounts in production.Using
f64for currency amounts can lead to precision issues with large values. TAO is divisible to 9 decimal places (rao), and floating-point representation may lose precision. For placeholder code this is fine, but when implementing the actual functionality, consider using a fixed-point representation oru128rao values.
108-114: Consider using a consistent "status" key for JSON output.The JSON output uses
"error"as the key, buttransfer.rsuses"status": "not_implemented". Using a consistent structure across modules would improve API consistency for consumers parsing JSON output.🔎 Suggested consistent JSON structure
OutputFormat::Json => { let output = serde_json::json!({ - "error": "Crowdfunding not yet implemented" + "status": "not_implemented", + "message": "Crowdfunding not yet implemented" }); println!("{}", serde_json::to_string_pretty(&output)?); }lightning-tensor/src/tui/app.rs (4)
44-96: Consider reducing public field exposure.The
Appstruct exposes many fields aspub. While this allows views to access state directly, it reduces encapsulation. Consider whether some fields should be private with accessor methods, especially sensitive ones likeasync_tx/async_rxandmessages.
140-148: Inefficient removal from front of Vec.Using
messages.remove(0)is O(n). Consider usingVecDequefor O(1) removal from the front, especially if message frequency is high.🔎 Suggested improvement using VecDeque
-use std::sync::Arc; +use std::sync::Arc; +use std::collections::VecDeque; ... - pub messages: Arc<Mutex<Vec<String>>>, + pub messages: Arc<Mutex<VecDeque<String>>>, ... - messages: Arc::new(Mutex::new(Vec::new())), + messages: Arc::new(Mutex::new(VecDeque::new())), ... pub async fn add_message(&self, msg: String) { let mut messages = self.messages.lock().await; - messages.push(msg); + messages.push_back(msg); // Keep last 10 messages if messages.len() > 10 { - messages.remove(0); + messages.pop_front(); } }
263-301: Navigation doesn't initialize selection when list state is None.If
wallet_list_state.selected()isNoneand the user presses Up/Down, nothing happens. Consider initializing to the first item (for Down) or last item (for Up) in this case to improve UX.🔎 Suggested fix
KeyCode::Up | KeyCode::Char('k') => { if let Some(selected) = self.wallet_list_state.selected() { if selected > 0 { self.wallet_list_state.select(Some(selected - 1)); } else if !self.wallets.is_empty() { self.wallet_list_state.select(Some(self.wallets.len() - 1)); } + } else if !self.wallets.is_empty() { + self.wallet_list_state.select(Some(self.wallets.len() - 1)); } } KeyCode::Down | KeyCode::Char('j') => { if let Some(selected) = self.wallet_list_state.selected() { if selected < self.wallets.len().saturating_sub(1) { self.wallet_list_state.select(Some(selected + 1)); } else { self.wallet_list_state.select(Some(0)); } + } else if !self.wallets.is_empty() { + self.wallet_list_state.select(Some(0)); } }
395-399: Input is discarded without processing.The
process_inputmethod discards the input and just logs a generic message. Consider adding a TODO comment to indicate this needs context-specific implementation, or at minimum preserve the input for debugging.🔎 Suggested improvement
/// Process input after Enter - async fn process_input(&mut self, _input: String) { - // Context-specific input processing - self.add_message("Input received".to_string()).await; + async fn process_input(&mut self, input: String) { + // TODO: Implement context-specific input processing based on self.state and self.input_prompt + self.add_message(format!("Input received: {}", input)).await; }lightning-tensor/src/services/wallet.rs (1)
20-35: Consider usingu32forwordsparameter directly.The
wordsparameter isu8but immediately cast tou32at line 32. Since the downstream API expectsu32, usingu32directly avoids the unnecessary cast and provides more flexibility for word counts.lightning-tensor/src/config/settings.rs (1)
35-66: Consider usingimpl AsRef<Path>for path parameters.The
loadandsavemethods take&PathBufwhich is less flexible thanimpl AsRef<Path>. This would allow callers to pass&str,&Path,PathBuf, etc.🔎 Suggested improvement
- pub fn load(path: &PathBuf) -> Result<Self> { + pub fn load(path: impl AsRef<Path>) -> Result<Self> { + let path = path.as_ref(); if !path.exists() { return Ok(Self::default()); } ... } - pub fn save(&self, path: &PathBuf) -> Result<()> { + pub fn save(&self, path: impl AsRef<Path>) -> Result<()> { + let path = path.as_ref(); ... }lightning-tensor/src/context.rs (2)
105-126: Unnecessary cloning of strings.The
connect_with_defaultsmethod cloneswallet_nameandhotkey_namebefore passing them toconnect, butconnectonly borrows them. This allocation is unnecessary.🔎 Suggested fix
pub async fn connect_with_defaults(&self) -> Result<Arc<Service>> { let wallet_name = self .config .wallet .default_wallet .as_ref() - .ok_or_else(|| Error::config("No default wallet configured"))? - .clone(); + .ok_or_else(|| Error::config("No default wallet configured"))?; let hotkey_name = self .config .wallet .default_hotkey .as_ref() - .ok_or_else(|| Error::config("No default hotkey configured"))? - .clone(); + .ok_or_else(|| Error::config("No default hotkey configured"))?; let netuid = self.config.wallet.default_netuid; - self.connect(&wallet_name, &hotkey_name, netuid).await + self.connect(wallet_name, hotkey_name, netuid).await }
168-187: Duplicate logic withWalletService::list_wallets.The
list_walletsmethod inAppContextduplicates the same directory-reading logic found inWalletService::list_wallets(lightning-tensor/src/services/wallet.rs, lines 48-67). Consider delegating toWalletServiceto avoid maintaining duplicate code.🔎 Suggested refactor
/// List available wallets pub fn list_wallets(&self) -> Result<Vec<String>> { - if !self.wallet_dir.exists() { - return Ok(Vec::new()); - } - - let mut wallets = Vec::new(); - - for entry in std::fs::read_dir(&self.wallet_dir)? { - let entry = entry?; - if entry.file_type()?.is_dir() { - if let Some(name) = entry.file_name().to_str() { - wallets.push(name.to_string()); - } - } - } - - wallets.sort(); - Ok(wallets) + use crate::services::WalletService; + let service = WalletService::new(self.wallet_dir.clone()); + service.list_wallets() }lightning-tensor/src/errors.rs (2)
64-65: Consider precision implications for balance display.
InsufficientBalanceusesf64forrequiredandavailable, which may display with floating-point artifacts (e.g., "0.10000000000000001"). Consider formatting to a fixed number of decimal places in the Display impl or using integer rao values.
119-167: Consider adding a helper forInvalidNetwork.There are constructors for most error variants (
config,wallet,network, etc.) but not forInvalidNetwork. If this variant is used, adding a constructor would maintain consistency.🔎 Suggested addition
+ /// Create an invalid network error + pub fn invalid_network(network: impl Into<String>) -> Self { + Self::InvalidNetwork { + network: network.into(), + } + }lightning-tensor/src/cli/mod.rs (1)
38-40: Remove unusedverboseflag or implement verbose output functionality.The
verboseflag is declared as a global CLI option but is never passed to subcommand handlers or referenced in the execute method. Either implement verbose output (e.g., controlling logging verbosity) or remove the unused flag entirely.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
lightning-tensor/Cargo.tomllightning-tensor/src/cli/crowd.rslightning-tensor/src/cli/mod.rslightning-tensor/src/cli/root.rslightning-tensor/src/cli/stake.rslightning-tensor/src/cli/subnet.rslightning-tensor/src/cli/transfer.rslightning-tensor/src/cli/wallet.rslightning-tensor/src/cli/weights.rslightning-tensor/src/config/mod.rslightning-tensor/src/config/settings.rslightning-tensor/src/context.rslightning-tensor/src/errors.rslightning-tensor/src/lib.rslightning-tensor/src/main.rslightning-tensor/src/models/display.rslightning-tensor/src/models/mod.rslightning-tensor/src/services/crowd.rslightning-tensor/src/services/mod.rslightning-tensor/src/services/root.rslightning-tensor/src/services/stake.rslightning-tensor/src/services/subnet.rslightning-tensor/src/services/transfer.rslightning-tensor/src/services/wallet.rslightning-tensor/src/services/weights.rslightning-tensor/src/tui/app.rslightning-tensor/src/tui/components/animation.rslightning-tensor/src/tui/components/input.rslightning-tensor/src/tui/components/mod.rslightning-tensor/src/tui/components/popup.rslightning-tensor/src/tui/components/spinner.rslightning-tensor/src/tui/components/table.rslightning-tensor/src/tui/mod.rslightning-tensor/src/tui/views/home.rslightning-tensor/src/tui/views/metagraph.rslightning-tensor/src/tui/views/mod.rslightning-tensor/src/tui/views/stake.rslightning-tensor/src/tui/views/subnet.rslightning-tensor/src/tui/views/wallet.rs
🧰 Additional context used
🧬 Code graph analysis (26)
lightning-tensor/src/services/weights.rs (5)
lightning-tensor/src/services/crowd.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/root.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/stake.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/subnet.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/transfer.rs (2)
new(9-11)default(15-17)
lightning-tensor/src/tui/views/stake.rs (3)
lightning-tensor/src/services/stake.rs (2)
default(15-17)new(9-11)lightning-tensor/src/services/wallet.rs (1)
new(16-18)lightning-tensor/src/tui/app.rs (1)
new(100-133)
lightning-tensor/src/tui/views/subnet.rs (3)
lightning-tensor/src/tui/views/mod.rs (1)
draw(22-55)lightning-tensor/src/tui/app.rs (1)
new(100-133)lightning-tensor/src/tui/components/table.rs (1)
new(23-37)
lightning-tensor/src/tui/components/spinner.rs (2)
lightning-tensor/src/tui/app.rs (1)
new(100-133)lightning-tensor/src/tui/components/animation.rs (3)
new(23-28)spinner_char(39-42)default(17-19)
lightning-tensor/src/cli/wallet.rs (1)
lightning-tensor/src/services/wallet.rs (8)
new(16-18)create_wallet(21-35)list_wallets(49-67)sign_message(102-109)verify_signature(112-145)regen_wallet(148-158)create_hotkey(93-99)list_hotkeys(70-90)
lightning-tensor/src/tui/components/animation.rs (2)
lightning-tensor/src/tui/app.rs (1)
new(100-133)lightning-tensor/src/tui/components/spinner.rs (1)
new(20-22)
lightning-tensor/src/services/transfer.rs (5)
lightning-tensor/src/services/crowd.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/root.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/stake.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/subnet.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/weights.rs (2)
new(9-11)default(15-17)
lightning-tensor/src/tui/views/mod.rs (2)
lightning-tensor/src/tui/app.rs (1)
new(100-133)lightning-tensor/src/tui/components/input.rs (1)
new(21-28)
lightning-tensor/src/cli/crowd.rs (3)
lightning-tensor/src/cli/mod.rs (1)
execute(94-137)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)lightning-tensor/src/cli/wallet.rs (1)
execute(93-125)
lightning-tensor/src/cli/subnet.rs (5)
lightning-tensor/src/cli/mod.rs (1)
execute(94-137)lightning-tensor/src/cli/root.rs (1)
execute(45-60)lightning-tensor/src/cli/stake.rs (1)
execute(125-149)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)lightning-tensor/src/cli/wallet.rs (1)
execute(93-125)
lightning-tensor/src/models/display.rs (1)
bittensor-rs/src/types/balance.rs (2)
as_tao(108-110)as_rao(94-96)
lightning-tensor/src/main.rs (1)
lightning-tensor/src/cli/mod.rs (1)
run(141-144)
lightning-tensor/src/tui/views/home.rs (2)
lightning-tensor/src/tui/components/animation.rs (3)
default(17-19)node_char(45-48)new(23-28)lightning-tensor/src/tui/app.rs (1)
new(100-133)
lightning-tensor/src/cli/root.rs (4)
lightning-tensor/src/cli/mod.rs (1)
execute(94-137)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)lightning-tensor/src/cli/wallet.rs (1)
execute(93-125)lightning-tensor/src/cli/weights.rs (1)
execute(65-77)
lightning-tensor/src/tui/app.rs (5)
lightning-tensor/src/context.rs (5)
new(34-43)new(209-216)wallet_dir(57-59)default(261-263)wallet(134-136)lightning-tensor/src/services/wallet.rs (1)
new(16-18)lightning-tensor/src/tui/components/animation.rs (2)
new(23-28)default(17-19)lightning-tensor/src/tui/mod.rs (1)
run(22-49)lightning-tensor/src/tui/views/wallet.rs (1)
draw(15-29)
lightning-tensor/src/cli/stake.rs (2)
lightning-tensor/src/cli/mod.rs (1)
execute(94-137)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)
lightning-tensor/src/cli/transfer.rs (2)
lightning-tensor/src/cli/mod.rs (1)
execute(94-137)bittensor-rs/src/extrinsics/transfer.rs (1)
keep_alive(60-63)
lightning-tensor/src/config/mod.rs (2)
lightning-tensor/src/context.rs (1)
config(52-54)lightning-tensor/src/errors.rs (1)
config(121-125)
lightning-tensor/src/services/root.rs (5)
lightning-tensor/src/services/crowd.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/stake.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/subnet.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/transfer.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/weights.rs (2)
new(9-11)default(15-17)
lightning-tensor/src/lib.rs (2)
lightning-tensor/src/context.rs (1)
config(52-54)lightning-tensor/src/errors.rs (1)
config(121-125)
lightning-tensor/src/cli/mod.rs (8)
lightning-tensor/src/context.rs (3)
wallet(134-136)new(34-43)new(209-216)lightning-tensor/src/cli/crowd.rs (1)
execute(70-91)lightning-tensor/src/cli/root.rs (1)
execute(45-60)lightning-tensor/src/cli/stake.rs (1)
execute(125-149)lightning-tensor/src/cli/subnet.rs (1)
execute(57-78)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)lightning-tensor/src/cli/wallet.rs (1)
execute(93-125)lightning-tensor/src/cli/weights.rs (1)
execute(65-77)
lightning-tensor/src/config/settings.rs (2)
lightning-tensor/src/errors.rs (3)
config(121-125)network(135-139)wallet(128-132)lightning-tensor/src/config/mod.rs (2)
config_file_path(31-33)default_wallet_dir(24-28)
lightning-tensor/src/tui/components/popup.rs (1)
lightning-tensor/src/tui/views/mod.rs (1)
centered_rect(145-163)
lightning-tensor/src/services/stake.rs (5)
lightning-tensor/src/services/crowd.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/root.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/subnet.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/transfer.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/weights.rs (2)
new(9-11)default(15-17)
lightning-tensor/src/tui/views/metagraph.rs (2)
lightning-tensor/src/tui/views/mod.rs (1)
draw(22-55)lightning-tensor/src/tui/app.rs (1)
new(100-133)
lightning-tensor/src/errors.rs (1)
bittensor-wallet/src/errors.rs (1)
from(108-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Coverage
- GitHub Check: Test
🔇 Additional comments (61)
lightning-tensor/src/tui/components/input.rs (1)
40-66: LGTM!The rendering logic is clean. Password masking, cursor display, and focus-based border coloring are correctly implemented using ratatui primitives.
lightning-tensor/src/models/mod.rs (1)
1-8: LGTM!Standard module scaffold with appropriate re-exports for the display utilities.
lightning-tensor/src/services/weights.rs (1)
5-18: LGTM!Consistent with other service stubs. The same
#[derive(Default)]suggestion fromCrowdServiceapplies here if you choose to reduce boilerplate.lightning-tensor/src/tui/components/spinner.rs (1)
14-33: LGTM!Clean spinner component implementation. The rendering correctly delegates character selection to
AnimationState::spinner_char()and composes the display text appropriately.lightning-tensor/src/services/subnet.rs (1)
5-18: LGTM!Consistent service stub following the established pattern.
lightning-tensor/src/services/stake.rs (1)
5-18: LGTM!Consistent service stub following the established pattern.
lightning-tensor/src/tui/views/stake.rs (1)
31-83: LGTM!The stake view layout and placeholder content are well-structured. The action hints provide good UX guidance, and the implementation follows ratatui patterns correctly.
lightning-tensor/src/tui/components/mod.rs (1)
1-16: LGTM! Clean component module structure.The module declarations and re-exports follow standard Rust conventions, providing a clean API surface for TUI components.
lightning-tensor/src/tui/views/metagraph.rs (1)
14-43: LGTM! Consistent view implementation.The metagraph view follows the established pattern used by other TUI views, with clear placeholder messaging for future functionality.
lightning-tensor/src/services/transfer.rs (1)
5-18: LGTM! Consistent service scaffolding.The
TransferServicefollows the same pattern as other service stubs in this PR (crowd, root, stake, etc.), providing a foundation for future business logic implementation.lightning-tensor/src/services/root.rs (1)
5-18: LGTM! Consistent service scaffolding.The
RootServicefollows the established pattern for service stubs, providing a clean foundation for root network operations.lightning-tensor/src/lib.rs (1)
1-30: LGTM! Well-structured crate root.The module declarations and convenience re-exports provide a clean, well-organized API surface for the Lightning Tensor crate. The structure follows Rust library conventions effectively.
lightning-tensor/Cargo.toml (1)
1-61: LGTM! Well-organized dependency manifest.The Cargo.toml changes properly support the new CLI/TUI functionality:
- Clear binary target definition
- Dependencies logically grouped with helpful comments
- Appropriate use of workspace-managed versions
- Metadata (description, license) properly added
lightning-tensor/src/cli/subnet.rs (4)
1-54: LGTM! Well-structured CLI command definitions.The SubnetCommand enum is well-documented and properly structured with appropriate clap attributes. The command structure follows the pattern established in other CLI modules.
56-78: LGTM! Execute function properly dispatches commands.The dispatch pattern correctly routes each SubnetCommand variant to its corresponding handler, consistent with other CLI modules in the PR. The unused
yesparameters are appropriate for these placeholder implementations.
80-96: Placeholder implementation is appropriately structured.The handler correctly provides feedback for both output formats and directs users to the TUI. The implementation is ready for future network integration.
98-185: LGTM! Consistent placeholder implementations across all handlers.All remaining handlers follow a consistent pattern with appropriate placeholder messaging for both Text and JSON output formats. The structure is ready for future network-enabled implementations.
lightning-tensor/src/cli/transfer.rs (1)
30-51: LGTM! Placeholder implementation is well-structured.The execute function properly handles both output formats and includes a clear TODO indicating the dependency on bittensor-rs Service exposure. The implementation is ready for future network integration.
lightning-tensor/src/tui/views/home.rs (3)
1-33: LGTM! Clean separation of home view rendering.The main draw function properly structures the home view layout into three distinct sections with appropriate size allocation. The delegation to helper functions maintains good code organization.
35-59: LGTM! Animated logo rendering is well-implemented.The animated network logo effectively uses the animation state and creates a visually appealing representation. The styling choices are appropriate for a home view.
61-106: LGTM! Menu is clear and well-organized.The menu effectively presents navigation options with intuitive keyboard shortcuts and appropriate visual styling. The color-coded shortcuts enhance usability.
lightning-tensor/src/tui/components/animation.rs (3)
1-20: LGTM! Animation state structure is straightforward.The AnimationState struct is well-defined with appropriate defaults. The 100ms frame duration provides smooth animation without excessive redraws.
22-36: LGTM! Animation update logic is correct.The update method properly manages frame advancement based on elapsed time, and the modulo operation ensures the frame index stays within bounds.
38-49: LGTM! Character selection methods are well-implemented.Both animation character sets are visually appropriate for their respective use cases (spinner for loading, nodes for network visualization), and the array-based selection is efficient.
lightning-tensor/src/tui/mod.rs (2)
1-19: LGTM! Module structure is well-organized.The module properly declares submodules and re-exports the necessary public types. The imports for terminal management are appropriate.
21-49: LGTM! Terminal lifecycle is properly managed.The function correctly sets up and tears down the terminal environment, ensuring raw mode, alternate screen, and mouse capture are properly handled. Error handling and terminal restoration are appropriate.
lightning-tensor/src/main.rs (1)
1-31: LGTM! Clean transition to CLI-first entry point.The refactoring properly shifts the entry point to a CLI-first approach while maintaining TUI access via the
tuisubcommand. The logging setup is appropriate with a sensible default level, and the documentation provides helpful usage examples.lightning-tensor/src/tui/components/table.rs (2)
1-37: LGTM! Table component structure is appropriate.The DataTable struct is well-designed for reusable table rendering with appropriate lifetimes for borrowed references. The public fields allow flexibility for callers while the constructor provides a clean initialization pattern.
39-73: LGTM! Table rendering is well-implemented.The render method properly constructs a styled, scrollable table with clear visual hierarchy. The use of
render_stateful_widgetenables proper scroll state management.lightning-tensor/src/services/mod.rs (1)
1-21: LGTM! Services module is well-organized.The module provides a clean centralized hub for service re-exports with consistent naming and clear documentation. The structure facilitates easy access to the business logic layer.
lightning-tensor/src/tui/views/wallet.rs (2)
31-68: Consider using&Appinstead of&mut Appfor read-only rendering.
draw_wallet_listtakes&mut Appbut only needs mutable access towallet_list_stateforrender_stateful_widget. The function doesn't modify other app state. This is acceptable as-is since the parent function requires mutability anyway.
70-136: Wallet details rendering looks good.The implementation correctly handles the case when no wallet is selected and when the selected index is out of bounds. The use of
unwrap_or_elsefor coldkey retrieval provides a safe fallback.lightning-tensor/src/tui/views/mod.rs (2)
22-55: View dispatcher structure is well-organized.The three-part layout (header, content, status bar) and state-based view dispatching is clean and maintainable. The placeholder handling for unimplemented views is appropriate.
114-127: Input overlay implementation is correct.The overlay properly clears the background before rendering the input field, and correctly passes the password mode and focus state to the
InputFieldcomponent.lightning-tensor/src/config/mod.rs (1)
16-33: Configuration path helpers are well-implemented.The functions correctly use the
dirscrate for cross-platform directory resolution and provide appropriate error handling when directories cannot be determined. The hardcoded paths (.bittensor/wallets,lightning-tensor) align with Bittensor conventions.lightning-tensor/src/cli/weights.rs (2)
117-122: Salt generation approach is acceptable.Using
rand::thread_rng()for salt generation is appropriate. For commit-reveal schemes in production, you may want to ensurerandis configured with a cryptographically secure RNG backend (e.g.,getrandom), which is the default.
64-77: Execute function structure is correct.The async dispatch pattern is consistent with other CLI modules. The
_ctxprefix correctly indicates the context is unused in the current stub implementation.lightning-tensor/src/cli/wallet.rs (1)
92-125: Execute dispatcher is well-structured.The command routing is clean and consistent with other CLI modules. Service instantiation with the wallet directory from context is appropriate.
lightning-tensor/src/models/display.rs (1)
40-54:format_blocksimplementation is correct.The comma-grouping logic correctly handles the reversal and placement of separators. Unit tests cover expected cases well.
lightning-tensor/src/tui/components/popup.rs (1)
27-89: Popup component implementation is well-designed.The factory methods provide a clean API for creating typed popups, and the render method correctly handles background clearing, border coloring by type, and text wrapping. The
Wrap { trim: true }setting is appropriate for message display.lightning-tensor/src/cli/stake.rs (1)
124-149: Execute dispatcher is correctly structured.The command routing follows the established pattern for CLI modules. All branches are covered and properly delegate to internal handlers.
lightning-tensor/src/cli/crowd.rs (2)
1-11: LGTM on module structure and imports.The module structure follows the established pattern from other CLI modules like
transfer.rsandwallet.rs, and the command enum is well-documented with clear subcommand variants.
69-91: LGTM on the execute dispatcher.The command dispatch is correctly implemented and follows the same pattern as other CLI modules. All subcommands are properly routed to their helper functions.
lightning-tensor/src/tui/app.rs (3)
1-19: LGTM on imports and module documentation.The imports are well-organized and the module documentation clearly describes the purpose.
150-189: LGTM on the main event loop.The event loop structure is solid - it properly handles UI rendering, event polling with timeout for responsiveness, async result processing, and animation updates. The global quit shortcuts (Ctrl+C/Q) are correctly prioritized.
446-465: LGTM on wallet loading.The
load_walletsfunction appropriately handles filesystem errors gracefully by returning an empty list, which is suitable for TUI initialization where a missing or inaccessible wallet directory shouldn't crash the app.lightning-tensor/src/cli/mod.rs (3)
1-48: LGTM on CLI structure and global options.The CLI structure is well-designed with proper use of clap's global arguments, sensible defaults, and clear documentation for each option.
92-137: LGTM on command dispatch.The execute method properly builds the
AppContextusing the builder pattern and correctly dispatches to all subcommand handlers. The ownership model is appropriate - consumingselfensures CLI args aren't accidentally reused.
140-144: LGTM on the run entry point.Simple and clean entry point that parses arguments and delegates to execute.
lightning-tensor/src/services/wallet.rs (2)
1-18: LGTM on WalletService structure.Clean service struct with a single responsibility of managing wallet operations against a directory.
111-145: LGTM on signature verification.The
verify_signatureimplementation correctly validates hex input, checks byte lengths, and usessp_core::sr25519for cryptographic verification.lightning-tensor/src/config/settings.rs (4)
1-33: LGTM on Config struct definition.The nested configuration structure is well-organized with serde defaults, making the TOML configuration file optional with sensible defaults.
68-87: LGTM onto_bittensor_configconversion.The network name matching correctly handles known networks and falls back to treating unknown strings as custom endpoints.
166-209: LGTM on WalletConfig.The
get_wallet_dirmethod correctly falls back to the default wallet directory when no custom path is configured.
250-269: LGTM on tests.The tests validate default values and round-trip serialization, which are the essential behaviors to verify.
lightning-tensor/src/context.rs (3)
14-43: LGTM on AppContext structure.The context properly encapsulates thread-safe access to the service and wallet using
RwLock, allowing concurrent reads and exclusive writes.
208-216: Builder usesConfig::default()instead of loading from file.
AppContextBuilder::new()initializes withConfig::default()rather thanConfig::load_or_default(). This means builder-based construction ignores any existing configuration file. If this is intentional (e.g., for testing or CLI overrides), consider documenting it; otherwise, consider loading from file.
266-287: LGTM on builder tests.The test validates that builder options correctly override config values.
lightning-tensor/src/errors.rs (3)
7-8: LGTM on the Result type alias.The centralized
Result<T>type alias simplifies error handling across the crate.
10-117: LGTM on the Error enum definition.Comprehensive error categorization with clear variants for each domain (config, wallet, network, staking, subnet, weights, UI). The
#[from]derives correctly enable automatic conversion from external library errors.
169-191: LGTM on utility methods.The
is_recoverableandis_user_facingpredicates provide useful categorization for control flow and error presentation decisions.
| ) -> Result<()> { | ||
| let name = wallet_name.unwrap_or("default"); | ||
| let password = prompt_password("Enter wallet password: ")?; |
There was a problem hiding this comment.
Inconsistent default wallet resolution.
sign_message uses a hardcoded "default" fallback (line 260), while show_balance (lines 192-195) correctly uses ctx.config().wallet.default_wallet. This inconsistency could lead to confusion when the user's default wallet has a different name.
🔎 Suggested fix
async fn sign_message(
service: &WalletService,
message: &str,
wallet_name: Option<&str>,
format: OutputFormat,
+ ctx: &AppContext,
) -> Result<()> {
- let name = wallet_name.unwrap_or("default");
+ let name = wallet_name
+ .map(String::from)
+ .or_else(|| ctx.config().wallet.default_wallet.clone())
+ .ok_or_else(|| crate::errors::Error::wallet("No wallet specified"))?;
+ let name = name.as_str();
let password = prompt_password("Enter wallet password: ")?;Also update the call site at line 110 to pass ctx.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lightning-tensor/src/cli/wallet.rs around lines 259 to 261, sign_message
currently uses a hardcoded "default" fallback for wallet name instead of reading
ctx.config().wallet.default_wallet like show_balance does; update sign_message
to resolve the wallet name from
wallet_name.unwrap_or(ctx.config().wallet.default_wallet.as_str()) (or
equivalent) so it uses the configured default, and also update the call site at
line 110 to pass ctx into sign_message.
| /// Network configuration | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct NetworkConfig { | ||
| /// Network name: "finney", "test", "local", or custom URL | ||
| #[serde(default = "default_network")] | ||
| pub network: String, | ||
|
|
||
| /// Connection timeout in seconds | ||
| #[serde(default = "default_timeout")] | ||
| pub timeout_secs: u64, | ||
|
|
||
| /// Number of retry attempts | ||
| #[serde(default = "default_retries")] | ||
| pub retries: u32, | ||
|
|
||
| /// Custom chain endpoints (optional) | ||
| #[serde(default)] | ||
| pub endpoints: Vec<String>, | ||
| } | ||
|
|
||
| fn default_network() -> String { | ||
| "finney".to_string() | ||
| } | ||
|
|
||
| fn default_timeout() -> u64 { | ||
| 30 | ||
| } | ||
|
|
||
| fn default_retries() -> u32 { | ||
| 3 | ||
| } | ||
|
|
||
| impl Default for NetworkConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| network: default_network(), | ||
| timeout_secs: default_timeout(), | ||
| retries: default_retries(), | ||
| endpoints: Vec::new(), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of timeout_secs and retries in the entire codebase
rg -n 'timeout_secs|retries' lightning-tensor/src/Repository: one-covenant/bittensor-rs
Length of output: 634
🏁 Script executed:
#!/bin/bash
# Check the test context around line 258
sed -n '250,265p' lightning-tensor/src/config/settings.rs
# Also search more broadly for retries usage
echo "=== Searching for retries usage ==="
rg -n 'retries' lightning-tensor/src/ -A 2 -B 2
# Search for to_bittensor_config function
echo "=== Searching for to_bittensor_config ==="
rg -n 'to_bittensor_config' lightning-tensor/src/ -A 10Repository: one-covenant/bittensor-rs
Length of output: 3396
retries is defined but unused; timeout_secs is used in tests.
NetworkConfig defines a retries field that does not appear to be used anywhere in the codebase. The timeout_secs field, however, is validated in the test_default_config test. Neither field is currently passed to BittensorConfig in the to_bittensor_config function. Consider either:
- Removing the
retriesfield if it's not needed - Adding a TODO comment if it's planned for future use
🤖 Prompt for AI Agents
In lightning-tensor/src/config/settings.rs around lines 89 to 130, the
NetworkConfig defines a retries field that is unused and timeout_secs is defined
but not propagated into BittensorConfig in to_bittensor_config; either remove
retries (or add a TODO explaining planned future use) and update
to_bittensor_config to pass timeout_secs (and retries if you keep it) into the
BittensorConfig so tests and runtime actually use the configured timeout and
optionally retries, ensuring defaults remain the same.
| #[test] | ||
| fn test_truncate_address() { | ||
| let addr = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; | ||
| assert_eq!(truncate_address(addr, 6, 4), "5Grwva...tQY"); | ||
| } |
There was a problem hiding this comment.
Test assertion appears incorrect.
With suffix_len=4, the last 4 characters of the address "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY" are "utQY", not "tQY". The expected output should be "5Grwva...utQY".
🔎 Suggested fix
#[test]
fn test_truncate_address() {
let addr = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
- assert_eq!(truncate_address(addr, 6, 4), "5Grwva...tQY");
+ assert_eq!(truncate_address(addr, 6, 4), "5Grwva...utQY");
}📝 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.
| #[test] | |
| fn test_truncate_address() { | |
| let addr = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; | |
| assert_eq!(truncate_address(addr, 6, 4), "5Grwva...tQY"); | |
| } | |
| #[test] | |
| fn test_truncate_address() { | |
| let addr = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; | |
| assert_eq!(truncate_address(addr, 6, 4), "5Grwva...utQY"); | |
| } |
🤖 Prompt for AI Agents
In lightning-tensor/src/models/display.rs around lines 60 to 64, the unit test’s
expected truncated address is wrong: with suffix_len=4 the last four characters
of the address are "utQY", not "tQY". Update the test assertion to expect
"5Grwva...utQY" (i.e., change the expected string in assert_eq! to
"5Grwva...utQY") so the test matches the actual function behavior.
| /// Regenerate wallet from mnemonic | ||
| pub fn regen_wallet(&self, name: &str, mnemonic: &str, password: &str) -> Result<Wallet> { | ||
| let wallet_path = self.wallet_dir.join(name); | ||
|
|
||
| // Create wallet directory if needed | ||
| std::fs::create_dir_all(&wallet_path)?; | ||
|
|
||
| let mut wallet = Wallet::new(name, wallet_path); | ||
| wallet.regenerate_wallet(mnemonic, password)?; | ||
|
|
||
| Ok(wallet) | ||
| } |
There was a problem hiding this comment.
regen_wallet may silently overwrite an existing wallet.
The function calls create_dir_all without checking if the wallet already exists. If a wallet with the same name exists, regenerating will overwrite it. Consider checking for existence and either returning an error or requiring explicit confirmation.
🔎 Suggested fix
/// Regenerate wallet from mnemonic
pub fn regen_wallet(&self, name: &str, mnemonic: &str, password: &str) -> Result<Wallet> {
let wallet_path = self.wallet_dir.join(name);
+ // Check if wallet already exists to prevent accidental overwrite
+ if wallet_path.exists() {
+ return Err(Error::WalletAlreadyExists { name: name.to_string() });
+ }
+
// Create wallet directory if needed
std::fs::create_dir_all(&wallet_path)?;
let mut wallet = Wallet::new(name, wallet_path);
wallet.regenerate_wallet(mnemonic, password)?;
Ok(wallet)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lightning-tensor/src/services/wallet.rs around lines 147 to 158, regen_wallet
currently calls create_dir_all unconditionally which can silently overwrite an
existing wallet; change it to check if wallet_path.exists() (or use
std::fs::metadata) before creating directories and regenerating: if the path
already exists return an Err with a clear message (or a specific error variant)
asking the caller to remove or confirm overwrite, otherwise call create_dir_all
and proceed to construct and regenerate the Wallet; ensure the error type
matches the function's Result.
| /// Delete a wallet | ||
| pub fn delete_wallet(&self, name: &str) -> Result<()> { | ||
| let wallet_path = self.wallet_dir.join(name); | ||
|
|
||
| if !wallet_path.exists() { | ||
| return Err(Error::WalletNotFound { name: name.to_string() }); | ||
| } | ||
|
|
||
| std::fs::remove_dir_all(wallet_path)?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
delete_wallet permanently removes data without safeguards.
Using remove_dir_all permanently deletes the wallet directory and all its contents. Consider:
- Adding a confirmation parameter (like
force: bool) - Moving to trash/backup instead of permanent deletion
- At minimum, logging this destructive action
This is especially critical since wallets contain cryptographic keys.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
lightning-tensor/src/config/settings.rs (1)
68-89:timeout_secs,retries, andendpointsare still not propagated toBittensorConfig.The
NetworkConfigfieldstimeout_secs,retries, andendpointsare defined (lines 99-108) and validated in tests (line 260), but this method doesn't pass them toBittensorConfig. The constructed configs use only default values or no timeout/retry configuration at all.Verify whether the
bittensor_rs::BittensorConfigAPI supports timeout and retry parameters:#!/bin/bash # Search for BittensorConfig struct definition and its fields rg -n "struct BittensorConfig" -A 20 # Search for constructor signatures rg -n "impl.*BittensorConfig" -A 30 | rg -A 10 "pub fn (finney|testnet|local|new)"lightning-tensor/src/services/wallet.rs (3)
101-110:sign_messagestill returns the public key, not a signature.This issue was previously flagged: the function returns the hex-encoded public key (
public.0) instead of an actual signature. This remains misleading and could cause bugs if callers expect a real signature. As suggested in the previous review, consider either returning a clear error (e.g.,Err(Error::wallet("Message signing not yet implemented"))) or a string explicitly labeled as a placeholder.
148-159:regen_walletmay still silently overwrite an existing wallet.This issue was previously flagged: the function calls
create_dir_allwithout checking if the wallet already exists, which could silently overwrite it. This is inconsistent withcreate_wallet(lines 24-26) which explicitly checks for existence. Consider adding a similar check or requiring explicit confirmation for regeneration.🔎 Suggested fix
pub fn regen_wallet(&self, name: &str, mnemonic: &str, password: &str) -> Result<Wallet> { let wallet_path = self.wallet_dir.join(name); + // Check if wallet already exists to prevent accidental overwrite + if wallet_path.exists() { + return Err(Error::WalletAlreadyExists { name: name.to_string() }); + } + // Create wallet directory if needed std::fs::create_dir_all(&wallet_path)?; let mut wallet = Wallet::new(name, wallet_path); wallet.regenerate_wallet(mnemonic, password)?; Ok(wallet) }
161-171:delete_walletstill permanently removes data without safeguards.This issue was previously flagged: using
remove_dir_allpermanently deletes the wallet directory and all its contents without confirmation, backup, or logging. While safeguards might exist at the CLI/UI layer, consider adding at least logging for this destructive operation, especially since wallets contain cryptographic keys.
🧹 Nitpick comments (7)
lightning-tensor/src/config/settings.rs (3)
60-66: Consider logging config path resolution failures.The method silently falls back to defaults when
config_file_path()fails, which provides robustness but could hide issues like permission problems or invalid home directories. Consider logging the error for debugging purposes while maintaining the graceful fallback behavior.
213-250: Consider validatingrefresh_rate_msrange.While the default of 100ms is reasonable, there's no validation to prevent setting it to 0 or extremely large values during deserialization. Consider adding bounds checking if this value is used in timing-critical code.
252-271: Consider adding tests forto_bittensor_configconversion.The existing tests validate defaults and TOML serialization but don't test the
to_bittensor_configmethod. Adding tests for different network types (finney/test/local/custom) would help ensure the conversion logic works correctly.lightning-tensor/src/services/wallet.rs (3)
48-67: Consider handling non-UTF8 directory names explicitly.Directory names with non-UTF8 characters are silently skipped (line 59). This is likely fine since wallet names should be ASCII/UTF-8, but you could log a warning or return an error if non-UTF8 names are encountered to make the behavior more explicit.
69-90: Consider handling non-UTF8 filenames explicitly.Similar to
list_wallets, filenames with non-UTF8 characters are silently skipped (line 82). Consider logging warnings or returning errors for non-UTF8 names for more explicit behavior.
112-146: Consider makingpubkeya required parameter.The
pubkeyparameter isOption<&str>but the function always returns an error when it'sNone(line 144). If a public key is always required, consider changing the signature topubkey: &strto make this requirement explicit at compile time rather than runtime.🔎 Proposed change
- pub fn verify_signature(&self, message: &str, signature: &str, pubkey: Option<&str>) -> Result<bool> { + pub fn verify_signature(&self, message: &str, signature: &str, pubkey: &str) -> Result<bool> { let sig_bytes = hex::decode(signature.trim_start_matches("0x")) .map_err(|_| Error::wallet("Invalid signature hex"))?; if sig_bytes.len() != 64 { return Err(Error::wallet("Signature must be 64 bytes")); } let mut sig_array = [0u8; 64]; sig_array.copy_from_slice(&sig_bytes); let signature = sp_core::sr25519::Signature::from_raw(sig_array); - if let Some(pk) = pubkey { - // Decode public key - let pk_str = pk.trim_start_matches("0x"); - let pk_bytes = hex::decode(pk_str) - .map_err(|_| Error::wallet("Invalid public key hex"))?; - - if pk_bytes.len() != 32 { - return Err(Error::wallet("Public key must be 32 bytes")); - } - - let mut pk_array = [0u8; 32]; - pk_array.copy_from_slice(&pk_bytes); - - let public = sp_core::sr25519::Public::from_raw(pk_array); - - use sp_core::Pair; - Ok(sp_core::sr25519::Pair::verify(&signature, message.as_bytes(), &public)) - } else { - Err(Error::wallet("Public key required for verification")) - } + // Decode public key + let pk_str = pubkey.trim_start_matches("0x"); + let pk_bytes = hex::decode(pk_str) + .map_err(|_| Error::wallet("Invalid public key hex"))?; + + if pk_bytes.len() != 32 { + return Err(Error::wallet("Public key must be 32 bytes")); + } + + let mut pk_array = [0u8; 32]; + pk_array.copy_from_slice(&pk_bytes); + + let public = sp_core::sr25519::Public::from_raw(pk_array); + + use sp_core::Pair; + Ok(sp_core::sr25519::Pair::verify(&signature, message.as_bytes(), &public)) }lightning-tensor/src/tui/views/subnet.rs (1)
51-67: Past field name issues resolved, but emission data hardcoded.The previous review correctly identified that wrong field names were used. The current code now correctly uses
subnet.nandsubnet.max_n, which aligns with the SubnetInfo struct definition.However, line 63 hardcodes "N/A" for the Emission column. Consider whether:
- Emission data should be added to SubnetInfo (if available from the substrate query), OR
- The Emission column should be removed from the table if this data is not accessible
The manual row styling (lines 52-57) provides visual feedback for selection, but note that it doesn't leverage the table's built-in highlight mechanism (see separate comment on lines 84-92).
Based on past review comments, the field name corrections address the previous critical compilation issues.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Cargo.tomllightning-tensor/src/config/settings.rslightning-tensor/src/services/wallet.rslightning-tensor/src/tui/views/subnet.rs
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (2)
lightning-tensor/src/services/wallet.rs (1)
lightning-tensor/src/errors.rs (1)
wallet(128-132)
lightning-tensor/src/tui/views/subnet.rs (2)
lightning-tensor/src/tui/components/table.rs (1)
new(23-37)lightning-tensor/src/tui/app.rs (1)
new(100-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Test
- GitHub Check: Coverage
🔇 Additional comments (4)
lightning-tensor/src/services/wallet.rs (4)
9-18: LGTM!The struct definition and constructor are clean and straightforward.
20-35: LGTM!The existence check prevents accidental overwrites, and error handling is appropriate.
37-46: LGTM!Appropriate existence check and error handling for loading wallets.
92-99: LGTM!Clean delegation to wallet methods with proper error propagation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lightning-tensor/src/ui/wallet.rs (1)
161-166: Fix type error:WalletErroris not imported.Line 164 uses
WalletErrorin the function signature, but onlyBittensorErroris imported at line 11. This will cause a compilation error.🔎 Proposed fix
fn get_wallet_public_key( wallet: &Wallet, password: &str, -) -> Result<sp_core::sr25519::Public, WalletError> { +) -> Result<sp_core::sr25519::Public, BittensorError> { wallet.get_coldkey(password) }
♻️ Duplicate comments (3)
lightning-tensor/src/cli/wallet.rs (1)
282-286: Inconsistent default wallet resolution.
sign_messageuses a hardcoded"default"fallback (line 283), whileshow_balance(lines 215-218) correctly usesctx.config().wallet.default_wallet. This inconsistency could lead to confusion when the user's default wallet has a different name.🔎 Suggested fix
async fn sign_message( service: &WalletService, message: &str, wallet_name: Option<&str>, format: OutputFormat, + ctx: &AppContext, ) -> Result<()> { - let name = wallet_name.unwrap_or("default"); + let name = wallet_name + .map(String::from) + .or_else(|| ctx.config().wallet.default_wallet.clone()) + .ok_or_else(|| crate::errors::Error::wallet("No wallet specified"))?; + let name = name.as_str(); let password = prompt_password("Enter wallet password: ")?;Also update the call site at line 132-133 to pass
ctx.lightning-tensor/src/services/wallet.rs (2)
164-181: Still lacks protection against overwriting existing wallets.The previous review comment about checking for existing wallets before regeneration was not addressed. Lines 169-170 call
create_dir_allunconditionally, which could overwrite an existing wallet and cause data loss.🔎 Add existence check
pub fn regen_wallet(&self, name: &str, mnemonic: &str, _password: &str) -> Result<Wallet> { let wallet_path = self.wallet_dir.join(name); + // Prevent accidental overwrites + if wallet_path.exists() { + return Err(Error::WalletAlreadyExists { name: name.to_string() }); + } + // Create wallet directory if needed std::fs::create_dir_all(&wallet_path)?;
183-193: Still lacks safeguards for destructive wallet deletion.The previous review comment about adding safeguards to
delete_walletwas not addressed. The method still usesremove_dir_allto permanently delete wallet data, including cryptographic keys that could represent significant value.Consider:
- Adding a
force: boolparameter requiring explicit confirmation- Moving to trash/backup instead of permanent deletion
- At minimum, logging the deletion with timestamp and wallet name
- Verifying the wallet has zero balance before allowing deletion
This is especially critical since wallets contain keys that control funds on the blockchain.
🧹 Nitpick comments (13)
lightning-tensor/src/cli/wallet.rs (2)
150-156: Consider removing unnecessaryasyncfrom functions that don't await.Functions like
create_wallet,list_wallets,show_wallet_info,sign_message,verify_signature,regen_wallet,create_hotkey, andlist_hotkeysare markedasyncbut perform only synchronous operations. This adds minor overhead from the async state machine.If these are intentionally async for future network operations or API consistency, this is fine to keep.
12-32: Consider returningOption<String>instead of magic "N/A" strings.The
read_coldkey_addressfunction returns special strings like"N/A"or"N/A (coldkeypub.txt not found)"on failure. These could potentially be mistaken for valid addresses in downstream code or display logic.🔎 Suggested refactor
-fn read_coldkey_address(wallet_path: &Path) -> String { +fn read_coldkey_address(wallet_path: &Path) -> Option<String> { let coldkeypub_path = wallet_path.join("coldkeypub.txt"); - if coldkeypub_path.exists() { - match std::fs::read_to_string(&coldkeypub_path) { - Ok(content) => { - if let Ok(json) = serde_json::from_str::<serde_json::Value>(&content) { - if let Some(addr) = json.get("ss58Address").and_then(|v| v.as_str()) { - return addr.to_string(); - } - } - content.trim().to_string() - } - Err(_) => "N/A".to_string(), - } - } else { - "N/A (coldkeypub.txt not found)".to_string() - } + std::fs::read_to_string(&coldkeypub_path).ok().map(|content| { + serde_json::from_str::<serde_json::Value>(&content) + .ok() + .and_then(|json| json.get("ss58Address").and_then(|v| v.as_str()).map(String::from)) + .unwrap_or_else(|| content.trim().to_string()) + }) }Then update call sites to use
.unwrap_or_else(|| "N/A".to_string())for display.bittensor-rs/src/queries/subnet.rs (1)
367-370: Use.flatten()instead of.filter_map(|opt| opt).This is a minor style improvement for readability.
let subnets: Vec<DynamicSubnetInfo> = result .into_iter() - .filter_map(|opt| opt) + .flatten() .map(|info| {lightning-tensor/src/tui/views/mod.rs (1)
146-165: Duplicatecentered_rectimplementation.This function is identical to the one in
lightning-tensor/src/tui/components/popup.rs(lines 92-110). Consider extracting it to a shared utility module to avoid duplication.🔎 Suggested approach
Create a shared utility, e.g., in
components/mod.rsor a newutils.rs:// In components/mod.rs or a new utils.rs pub fn centered_rect(percent_x: u16, percent_y: u16, r: Rect) -> Rect { // ... implementation }Then import and use it in both
views/mod.rsandcomponents/popup.rs.lightning-tensor/src/tui/views/home.rs (1)
35-59: Animation state update in render function has side effects.
draw_logocallsapp.animation_state.update()(line 36), mutating state during rendering. While this works, it couples animation timing to render frequency, which could cause inconsistent animation speeds if frame rates vary.Consider updating animation state in the main event loop instead, or accept this approach if consistent animation isn't critical.
lightning-tensor/src/tui/views/wallet.rs (2)
101-106: Address truncation may panic on edge cases.While SS58 addresses are typically ASCII, the byte slicing approach could fail on unexpected input. Consider using the same char-boundary-safe truncation recommended for other views.
245-251: Duplicatetruncatehelper with UTF-8 slicing issue.This function duplicates similar helpers in
subnet.rsandmetagraph.rs. Consider:
- Extracting to a shared utility module
- Fixing the multi-byte character slicing issue (same as noted for
subnet.rs)🔎 Suggested shared utility
Create a shared helper in
tui/components/mod.rsor a newtui/utils.rs:/// Truncate string for display, respecting UTF-8 boundaries pub fn truncate(s: &str, max_len: usize) -> String { let char_count = s.chars().count(); if char_count <= max_len { s.to_string() } else { let truncated: String = s.chars().take(max_len - 1).collect(); format!("{}…", truncated) } }Then import and use across all views.
lightning-tensor/src/tui/app.rs (2)
735-844: Pattern: Handle channel send errors across all async fetch methods.Multiple async fetch methods (
fetch_metagraph,fetch_subnets,fetch_balance,fetch_all_balances,fetch_all_stakes) uselet _ = tx.send(...)to ignore send errors. This pattern silently drops results if the channel is closed, preventing the UI from updating.Consider creating a helper method to centralize error handling:
🔎 Suggested helper method
/// Helper to send async results with logging async fn send_result(tx: &Sender<AsyncResult>, result: AsyncResult) { if tx.send(result).await.is_err() { eprintln!("Failed to send async result: receiver dropped"); } }Then update all spawned tasks to use this helper instead of
let _ = tx.send(...).
993-1000: Consider improving the address truncation logic for clarity.The current logic checks
addr.len() <= len * 2 + 3to decide whether to truncate, but the relationship between this check and the slicing operations isn't immediately clear. For better readability and to avoid potential edge cases:🔎 Clearer implementation
fn truncate_address(addr: &str, len: usize) -> String { - if addr.len() <= len * 2 + 3 { + let min_display_len = len * 2 + 3; // len chars + "..." + len chars + if addr.len() <= min_display_len { addr.to_string() } else { format!("{}...{}", &addr[..len], &addr[addr.len()-len..]) } }lightning-tensor/src/context.rs (2)
87-103: Consider validating wallet/hotkey existence before attempting connection.The method creates a service with the provided
wallet_nameandhotkey_namewithout checking if they exist in the wallet directory. WhileService::new()will likely fail if the wallet doesn't exist, early validation would provide clearer error messages.🔎 Suggested validation
pub async fn connect( &self, wallet_name: &str, hotkey_name: &str, netuid: u16, ) -> Result<Arc<Service>> { + // Validate wallet exists + let wallet_path = self.wallet_dir.join(wallet_name); + if !wallet_path.exists() { + return Err(Error::WalletNotFound { name: wallet_name.to_string() }); + } + + // Validate hotkey exists + let hotkey_path = wallet_path.join("hotkeys").join(hotkey_name); + if !hotkey_path.exists() { + return Err(Error::HotkeyNotFound { name: hotkey_name.to_string() }); + } + let bittensor_config = self .config .to_bittensor_config(wallet_name, hotkey_name, netuid);
106-127: Fallback to "default" wallet may fail silently without clear guidance.When
default_walletordefault_hotkeyare not configured, the method falls back to"default". If this wallet doesn't exist, the connection will fail with a potentially confusing error. Consider checking for wallet existence first or providing clearer error messages.🔎 Improved error handling
pub async fn connect_with_defaults(&self) -> Result<Arc<Service>> { // Use defaults from config, or fallback to sensible defaults for read-only access let wallet_name = self .config .wallet .default_wallet .as_ref() .map(|s| s.as_str()) .unwrap_or("default"); let hotkey_name = self .config .wallet .default_hotkey .as_ref() .map(|s| s.as_str()) .unwrap_or("default"); + // Validate that the wallet exists + let wallet_path = self.wallet_dir.join(wallet_name); + if !wallet_path.exists() { + return Err(Error::config(format!( + "Default wallet '{}' not found. Create one with 'lt wallet create {}' or set default_wallet in config", + wallet_name, wallet_name + ))); + } + let netuid = self.config.wallet.default_netuid; self.connect(wallet_name, hotkey_name, netuid).await }lightning-tensor/src/errors.rs (1)
169-191: Consider expanding error classification helpers for better UX.The
is_recoverableandis_user_facinghelpers are conservative but might miss some cases:
- Recoverable errors: Some
Ioerrors like connection timeouts, refused connections, or temporary unavailability could be marked recoverable.- User-facing errors: Errors like
Config,Subnet, andWeightscontain user-actionable messages and could be classified as user-facing.🔎 Expanded classifications
pub fn is_recoverable(&self) -> bool { matches!( self, Error::Network { .. } | Error::ConnectionFailed { .. } | Error::Rpc { .. } | Error::TransactionTimeout { .. } + | Error::Io(e) if is_transient_io_error(e) ) } pub fn is_user_facing(&self) -> bool { matches!( self, Error::InvalidPassword | Error::UserCancelled | Error::WalletNotFound { .. } | Error::WalletAlreadyExists { .. } | Error::InsufficientBalance { .. } | Error::InvalidNetwork { .. } + | Error::Config { .. } + | Error::Subnet { .. } + | Error::Weights { .. } + | Error::HotkeyNotFound { .. } ) } + + fn is_transient_io_error(e: &std::io::Error) -> bool { + matches!( + e.kind(), + std::io::ErrorKind::ConnectionRefused + | std::io::ErrorKind::ConnectionReset + | std::io::ErrorKind::TimedOut + | std::io::ErrorKind::Interrupted + ) + }lightning-tensor/src/services/wallet.rs (1)
112-119: Password parameter is unused in hotkey creation.The
_passwordparameter is ignored, meaning hotkeys are not encrypted. If this is intentional (hotkeys are often stored unencrypted for validator operations), document this behavior clearly. Otherwise, implement encryption.Add documentation:
/// Create a new hotkey for a wallet + /// + /// Note: Currently creates an unencrypted hotkey. Password parameter is reserved + /// for future encryption support. pub fn create_hotkey(&self, wallet_name: &str, hotkey_name: &str, _password: &str) -> Result<String> {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
Cargo.tomlbittensor-rs/src/lib.rsbittensor-rs/src/queries/mod.rsbittensor-rs/src/queries/subnet.rsbittensor-rs/src/service.rslightning-tensor/Cargo.tomllightning-tensor/src/app.rslightning-tensor/src/cli/root.rslightning-tensor/src/cli/wallet.rslightning-tensor/src/context.rslightning-tensor/src/errors.rslightning-tensor/src/handlers/wallet.rslightning-tensor/src/services/wallet.rslightning-tensor/src/tui/app.rslightning-tensor/src/tui/mod.rslightning-tensor/src/tui/views/home.rslightning-tensor/src/tui/views/metagraph.rslightning-tensor/src/tui/views/mod.rslightning-tensor/src/tui/views/stake.rslightning-tensor/src/tui/views/subnet.rslightning-tensor/src/tui/views/wallet.rslightning-tensor/src/ui/wallet.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- lightning-tensor/src/cli/root.rs
- Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (12)
lightning-tensor/src/app.rs (2)
lightning-tensor/src/context.rs (1)
wallet(135-137)lightning-tensor/src/errors.rs (1)
wallet(128-132)
bittensor-rs/src/lib.rs (3)
bittensor-rs/src/queries/subnet.rs (4)
get_all_dynamic_info(351-422)get_subnet_info(86-155)get_total_subnets(299-320)subnet_exists(323-345)bittensor-rs/src/queries/metagraph.rs (1)
get_metagraph(48-83)bittensor-rs/src/queries/neuron.rs (3)
get_neuron(33-60)get_neuron_lite(65-92)get_uid_for_hotkey(105-134)
lightning-tensor/src/tui/views/stake.rs (2)
lightning-tensor/src/tui/views/wallet.rs (1)
draw(15-29)lightning-tensor/src/tui/app.rs (1)
new(164-214)
bittensor-rs/src/queries/mod.rs (1)
bittensor-rs/src/queries/subnet.rs (5)
get_all_dynamic_info(351-422)get_subnet_hyperparameters(158-296)get_subnet_info(86-155)get_total_subnets(299-320)subnet_exists(323-345)
bittensor-rs/src/queries/subnet.rs (2)
bittensor-rs/metadata/test.rs (2)
get_all_dynamic_info(3368-3385)apis(78-80)bittensor-rs/metadata/local.rs (2)
get_all_dynamic_info(3368-3385)apis(78-80)
lightning-tensor/src/services/wallet.rs (6)
lightning-tensor/src/context.rs (5)
wallet(135-137)new(34-43)new(216-223)wallet_dir(57-59)load_wallet(149-151)lightning-tensor/src/errors.rs (1)
wallet(128-132)lightning-tensor/src/app.rs (1)
new(105-139)lightning-tensor/src/tui/app.rs (1)
new(164-214)lightning-tensor/src/tui/components/spinner.rs (1)
new(20-22)bittensor-rs/src/wallet/mod.rs (4)
create_random(169-180)hotkey(281-285)load_from_path(122-148)from_mnemonic(199-220)
lightning-tensor/src/tui/views/wallet.rs (1)
lightning-tensor/src/tui/app.rs (1)
new(164-214)
lightning-tensor/src/tui/views/mod.rs (3)
lightning-tensor/src/tui/app.rs (1)
new(164-214)lightning-tensor/src/tui/components/input.rs (1)
new(21-28)lightning-tensor/src/tui/components/popup.rs (1)
centered_rect(93-111)
lightning-tensor/src/ui/wallet.rs (2)
lightning-tensor/src/context.rs (1)
wallet(135-137)lightning-tensor/src/errors.rs (1)
wallet(128-132)
lightning-tensor/src/tui/views/subnet.rs (2)
lightning-tensor/src/tui/views/metagraph.rs (3)
draw(15-28)draw_header(30-69)draw_footer(167-193)lightning-tensor/src/tui/components/table.rs (1)
new(23-37)
lightning-tensor/src/errors.rs (8)
lightning-tensor/src/tui/components/popup.rs (1)
error(44-50)bittensor-wallet/src/errors.rs (1)
from(108-110)bittensor-wallet/src/python_bindings.rs (1)
from(183-185)bittensor-rs/src/service.rs (1)
serde_json(43-43)lightning-tensor/src/cli/wallet.rs (1)
serde_json(19-19)lightning-tensor/src/tui/app.rs (1)
serde_json(903-903)lightning-tensor/src/context.rs (2)
wallet(135-137)config(52-54)lightning-tensor/src/app.rs (1)
ui(163-166)
lightning-tensor/src/handlers/wallet.rs (2)
lightning-tensor/src/context.rs (1)
wallet(135-137)lightning-tensor/src/errors.rs (1)
wallet(128-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Test
- GitHub Check: Coverage
🔇 Additional comments (24)
bittensor-rs/src/service.rs (1)
776-809: LGTM! Useful addition for advanced queries.The new
client()method provides direct access to the underlying Substrate client for custom queries not covered by the Service API. The implementation correctly retrieves a healthy client from the pool and maps errors consistently.lightning-tensor/Cargo.toml (3)
5-6: LGTM! Good metadata additions.Adding description and license fields improves package discoverability and legal clarity.
8-10: LGTM! Binary target configured correctly.The binary target "lt" is properly configured with the appropriate entry point at
src/main.rs, aligning with the CLI/TUI application objectives mentioned in the PR.
12-58: LGTM! Dependencies align well with CLI/TUI functionality.The added dependencies (indicatif for progress bars, dialoguer for prompts, console/tabled for formatting, ratatui/crossterm for TUI) are appropriate for the CLI/TUI application described in the PR objectives.
lightning-tensor/src/app.rs (1)
3-3: LGTM! Import path correctly updated.The wallet import path has been updated to align with the new bittensor-rs dependency structure.
lightning-tensor/src/handlers/wallet.rs (1)
3-3: LGTM! Import path correctly updated.The wallet import path has been updated consistently with other files to use bittensor-rs.
lightning-tensor/src/ui/wallet.rs (1)
10-11: LGTM! Import paths correctly updated.The imports have been updated to use bittensor-rs types.
bittensor-rs/src/queries/mod.rs (1)
19-22: LGTM! Public API expanded consistently.The addition of
get_all_dynamic_infoandDynamicSubnetInfoexports aligns with the new dynamic subnet information functionality.lightning-tensor/src/tui/views/stake.rs (3)
15-29: LGTM! Layout structure is clean.The stake view is properly structured with a vertical split for summary and positions sections.
65-83: LGTM! Placeholder implementation is appropriate.The stake positions placeholder with instructions is clear and consistent with the TUI pattern.
31-63: No issues found. Theselected_wallet_info()method exists inlightning-tensor/src/tui/app.rsat line 217 and correctly returnsOption<&WalletInfo>. TheWalletInfostruct has aname: Stringfield, making the code snippet valid and correct.Likely an incorrect or invalid review comment.
bittensor-rs/src/lib.rs (1)
113-119: LGTM! Public exports updated consistently.The query exports now include
get_all_dynamic_infoandDynamicSubnetInfo, consistent with the additions inqueries/mod.rs.bittensor-rs/src/queries/subnet.rs (3)
28-58: LGTM! Well-structuredDynamicSubnetInfowith clear documentation.The struct fields are well-documented with comments explaining units (RAO) and purpose. The inclusion of DTAO-specific fields like
moving_price,price_tao, and pool information provides comprehensive subnet data.
385-401: LGTM! Price and fixed-point calculations are correctly implemented.The code properly:
- Converts RAO to TAO using the 10^9 factor
- Handles the root subnet special case (netuid 0 → price 1.0)
- Guards against division by zero
- Correctly interprets FixedI128 by dividing by 2^32
424-428: LGTM! Simple and effective compact bytes decoder.Using
from_utf8_lossyis appropriate here as it gracefully handles any malformed UTF-8 data from the chain.lightning-tensor/src/tui/views/mod.rs (1)
21-55: LGTM! Clean dispatcher pattern with proper layout structure.The draw function correctly:
- Sets up a three-part vertical layout (header, content, status bar)
- Dispatches to appropriate view based on AppState
- Handles input overlay as a modal layer on top
lightning-tensor/src/tui/views/home.rs (1)
125-147: LGTM! Non-blocking message rendering.Using
try_lock()(line 128) is the right approach for UI rendering to avoid blocking the main thread. The fallback to "Loading..." when the lock is held is a sensible UX choice.lightning-tensor/src/tui/views/subnet.rs (1)
108-208: LGTM! Subnet table rendering is well-implemented.The table correctly:
- Uses
render_stateful_widgetfor proper selection highlighting (line 207)- Calculates emission percentages based on actual TAO emission
- Applies appropriate color coding for prices and emissions
- Handles the root subnet (netuid 0) exclusion in emission calculations
lightning-tensor/src/tui/views/metagraph.rs (2)
14-28: LGTM! Clean three-pane layout with proper delegation.The draw function correctly sets up the vertical layout and delegates to focused helper functions.
150-164: LGTM! Conditional scrollbar implementation.The scrollbar is appropriately added only when content exceeds visible area (>10 neurons), and correctly syncs its position with the table selection state.
lightning-tensor/src/tui/views/wallet.rs (2)
14-29: LGTM! Clear two-pane layout with proper structure.The horizontal 40/60 split for wallet list and details is well-designed for the wallet management use case.
177-243: LGTM! Stakes table with proper empty and loading states.The implementation correctly handles:
- Empty stakes with appropriate messaging
- Connection status awareness
- Total stake calculation and display in the title
lightning-tensor/src/tui/app.rs (1)
32-65: No issues found. These structs are defined only inlightning-tensor/src/tui/app.rswith no duplicates in the models module or elsewhere in the codebase.lightning-tensor/src/services/wallet.rs (1)
121-126: Sign_message implementation is correct.The method properly calls
wallet.sign()and returns a hex-encoded signature. The underlyingWallet::sign()method from bittensor-rs does not require a password parameter—it operates directly on the already-loaded wallet struct.The unused
_passwordparameter is consistent across multiple wallet service methods (create_wallet,create_hotkey,regen_wallet), indicating this is a planned API design rather than an implementation oversight. The parameter can remain as-is pending future password-based wallet encryption support.
| let wallet = service.create_wallet(name, words, &password)?; | ||
| let address = wallet.hotkey().to_string(); // Hotkey address (coldkey needs unlock) | ||
|
|
||
| match format { | ||
| OutputFormat::Text => { | ||
| println!("✓ Wallet '{}' created successfully", name); | ||
| println!(" Coldkey: {}", address); | ||
| } | ||
| OutputFormat::Json => { | ||
| let output = serde_json::json!({ | ||
| "success": true, | ||
| "wallet": name, | ||
| "coldkey": address | ||
| }); |
There was a problem hiding this comment.
Misleading output: displaying hotkey address as "Coldkey".
Line 164 retrieves wallet.hotkey().to_string() (the hotkey address), but the output labels it as "Coldkey" (lines 169, 175). This will confuse users who need the actual coldkey address for transfers or other operations.
🔎 Suggested fix
Either display the correct coldkey address or change the label:
let wallet = service.create_wallet(name, words, &password)?;
- let address = wallet.hotkey().to_string(); // Hotkey address (coldkey needs unlock)
+ let hotkey_address = wallet.hotkey().to_string();
+ let coldkey_address = read_coldkey_address(&wallet.path);
match format {
OutputFormat::Text => {
println!("✓ Wallet '{}' created successfully", name);
- println!(" Coldkey: {}", address);
+ println!(" Coldkey: {}", coldkey_address);
+ println!(" Hotkey: {}", hotkey_address);
}
OutputFormat::Json => {
let output = serde_json::json!({
"success": true,
"wallet": name,
- "coldkey": address
+ "coldkey": coldkey_address,
+ "hotkey": hotkey_address
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lightning-tensor/src/cli/wallet.rs around lines 163 to 176, the code
retrieves wallet.hotkey() but prints it labeled as "Coldkey", which is
misleading; either fetch and print the real coldkey (e.g., replace the retrieval
with wallet.coldkey().to_string() and use that variable in both Text and Json
outputs) or keep the hotkey value but change the labels to "Hotkey" (and
"hotkey" in JSON) so the output accurately reflects the value being shown.
| /// Create a new wallet with random mnemonic | ||
| /// Note: The new bittensor_rs wallet API creates a wallet with hotkey in one step | ||
| pub fn create_wallet(&self, name: &str, _words: u8, _password: &str) -> Result<Wallet> { | ||
| let wallet_path = self.wallet_dir.join(name); | ||
|
|
||
| if wallet_path.exists() { | ||
| return Err(Error::WalletAlreadyExists { name: name.to_string() }); | ||
| } | ||
|
|
||
| // Create wallet directory structure | ||
| std::fs::create_dir_all(&wallet_path)?; | ||
| std::fs::create_dir_all(wallet_path.join("hotkeys"))?; | ||
|
|
||
| // Create random wallet with default hotkey | ||
| let wallet = Wallet::create_random(name, "default") | ||
| .map_err(|e| Error::wallet(&format!("Failed to create wallet: {}", e)))?; | ||
|
|
||
| // Save the hotkey address to coldkeypub.txt for compatibility | ||
| // Note: bittensor_rs wallet stores hotkey, not coldkey by default | ||
| let coldkeypub_path = wallet_path.join("coldkeypub.txt"); | ||
| std::fs::write(&coldkeypub_path, wallet.hotkey().to_string()) | ||
| .map_err(|e| Error::wallet(&format!("Failed to save coldkeypub: {}", e)))?; | ||
|
|
||
| Ok(wallet) | ||
| } |
There was a problem hiding this comment.
Hotkey is incorrectly saved as coldkeypub, and wallet parameters are ignored.
This method has two issues:
-
Lines 42-46: The hotkey address is saved to
coldkeypub.txt, but coldkey and hotkey are semantically different in Bittensor. Coldkeys are for identity/balance, while hotkeys are for subnet operations. This mismatch will confuse users and break tools expecting coldkey addresses. -
Lines 27-27: The
_wordsand_passwordparameters are ignored, so users cannot control mnemonic length or encrypt their wallets.
Based on learnings, address the semantic issue by either:
- Using bittensor_rs wallet APIs that support coldkey creation, or
- Documenting clearly that this wallet service only manages hotkeys (and renaming coldkeypub.txt to hotkeypub.txt), or
- Implementing coldkey creation separately from hotkey creation
🤖 Prompt for AI Agents
In lightning-tensor/src/services/wallet.rs around lines 25 to 49, the function
currently ignores the _words and _password parameters and incorrectly writes the
hotkey pubkey into coldkeypub.txt; fix it by (1) using the bittensor_rs APIs to
explicitly create or derive a coldkey (and its pubkey) when creating the wallet
and save that coldkey pubkey to coldkeypub.txt (or if this service is only meant
to manage hotkeys, rename coldkeypub.txt to hotkeypub.txt and update docs/usage
accordingly), and (2) wire the _words and _password parameters into the wallet
creation call so mnemonic length and optional encryption are applied (pass
words/password to the appropriate Wallet creation API or generate the mnemonic
of requested length and encrypt with the password), and ensure error
handling/logging remains consistent when saving the correct pubkey file.
| async fn auto_connect(&mut self) { | ||
| // Check if we have wallets - if not, show guidance | ||
| if self.wallets.is_empty() { | ||
| self.add_message("No wallets found. Create one with 'lt wallet create <name>'".to_string()).await; | ||
| self.add_message("Press 'c' to try connecting after creating a wallet.".to_string()).await; | ||
| return; | ||
| } | ||
|
|
||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
| let network = ctx.network_name().to_string(); | ||
|
|
||
| self.add_message(format!("⏳ Connecting to {}...", network)).await; | ||
| self.start_loading(&format!("Connecting to {}...", network)); | ||
|
|
||
| tokio::spawn(async move { | ||
| match ctx.connect_with_defaults().await { | ||
| Ok(_) => { | ||
| let _ = tx.send(AsyncResult::Connected(Ok(()))).await; | ||
| } | ||
| Err(e) => { | ||
| let _ = tx.send(AsyncResult::Connected(Err(format!("{}", e)))).await; | ||
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Handle channel send errors to avoid silently dropping connection results.
The spawned task uses let _ = tx.send(...) on lines 404 and 407, which silently ignores send failures. If the channel is closed or the receiver is dropped, connection results won't reach the UI, leaving users without feedback.
🔎 Proposed fix
tokio::spawn(async move {
match ctx.connect_with_defaults().await {
Ok(_) => {
- let _ = tx.send(AsyncResult::Connected(Ok(()))).await;
+ if tx.send(AsyncResult::Connected(Ok(()))).await.is_err() {
+ eprintln!("Failed to send connection success: receiver dropped");
+ }
}
Err(e) => {
- let _ = tx.send(AsyncResult::Connected(Err(format!("{}", e)))).await;
+ if tx.send(AsyncResult::Connected(Err(format!("{}", e)))).await.is_err() {
+ eprintln!("Failed to send connection error: receiver dropped");
+ }
}
}
});🤖 Prompt for AI Agents
In lightning-tensor/src/tui/app.rs around lines 386 to 411, the spawned task
currently uses `let _ = tx.send(...).await`, which ignores failures to send the
connection result to the UI; change this to await and match the send result and
handle Err properly — e.g. replace `let _ = tx.send(...).await` with a match on
the `.await` result and on Err call a logging facility (tracing::error! or
eprintln!) including the underlying channel error and the connection outcome,
and optionally fall back to notifying the UI directly via the shared ctx (Arc)
if a suitable method exists; ensure both success and failure branches are
explicit so connection results are never silently dropped.
| /// Process input after Enter | ||
| async fn process_input(&mut self, _input: String) { | ||
| // Context-specific input processing | ||
| self.add_message("Input received".to_string()).await; | ||
| } |
There was a problem hiding this comment.
Incomplete input processing logic.
The process_input method is a placeholder that doesn't process input based on the current context. Multiple handlers (stake, subnet, transfer, weights) set input_mode and input_prompt expecting this method to process the input, but it only logs a generic message.
Do you want me to help implement the context-specific input processing logic based on the current AppState and input_prompt?
| /// Fetch metagraph data from network | ||
| async fn fetch_metagraph_data(ctx: &AppContext, netuid: u16) -> Result<Vec<NeuronDisplay>> { | ||
| let service = ctx.require_service().await?; | ||
| let metagraph = service.get_metagraph(netuid).await?; | ||
|
|
||
| let discovery = NeuronDiscovery::new(&metagraph); | ||
| let neurons = discovery.get_all_neurons() | ||
| .map_err(|e| crate::errors::Error::network(&format!("Failed to discover neurons: {}", e)))?; | ||
|
|
||
| let displays: Vec<NeuronDisplay> = neurons.into_iter().map(|n| { | ||
| NeuronDisplay { | ||
| uid: n.uid, | ||
| hotkey: truncate_address(&n.hotkey, 8), | ||
| coldkey: truncate_address(&n.coldkey, 8), | ||
| stake: n.stake as f64 / 1_000_000_000.0, // Convert from rao to TAO | ||
| is_validator: n.is_validator, | ||
| ip: n.axon_info.as_ref().map(|a| a.ip.clone()).unwrap_or_default(), | ||
| port: n.axon_info.as_ref().map(|a| a.port).unwrap_or(0), | ||
| incentive: 0.0, // TODO: Extract from metagraph when available | ||
| emission: 0.0, | ||
| trust: 0.0, | ||
| consensus: 0.0, | ||
| dividends: 0.0, | ||
| } | ||
| }).collect(); | ||
|
|
||
| Ok(displays) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find where metagraph is defined or imported
rg "struct.*Metagraph" --type rustRepository: one-covenant/bittensor-rs
Length of output: 859
🏁 Script executed:
# Also search for imports of Metagraph
rg "use.*Metagraph|from.*metagraph" --type rustRepository: one-covenant/bittensor-rs
Length of output: 1144
🏁 Script executed:
# Check the service.get_metagraph implementation to see what it returns
rg "fn get_metagraph" --type rust -A 10Repository: one-covenant/bittensor-rs
Length of output: 3361
🏁 Script executed:
# Look for where these metrics might be defined (incentive, emission, trust, consensus, dividends)
rg "incentive|emission|consensus|dividends" --type rust | head -50Repository: one-covenant/bittensor-rs
Length of output: 4504
🏁 Script executed:
# Check the NeuronDisplay struct definition
rg "struct NeuronDisplay" --type rust -A 20Repository: one-covenant/bittensor-rs
Length of output: 1255
🏁 Script executed:
# Find the Metagraph struct definition in the queries module
rg "pub struct Metagraph" --type rust -A 30Repository: one-covenant/bittensor-rs
Length of output: 5418
🏁 Script executed:
# Look for Neuron struct that might contain these metrics
rg "pub struct Neuron" --type rust -A 30Repository: one-covenant/bittensor-rs
Length of output: 31258
🏁 Script executed:
# Check the SelectiveMetagraph fields to understand what can be queried
rg "SelectiveMetagraph" --type rust -A 20 | head -80Repository: one-covenant/bittensor-rs
Length of output: 5044
🏁 Script executed:
# Look for where incentive, consensus, dividends, emission fields are used in discovery or metagraph queries
rg "\.incentive|\.consensus|\.dividends" --type rustRepository: one-covenant/bittensor-rs
Length of output: 51
🏁 Script executed:
# Check the discovery module to see what fields are available from discovered neurons
cat lightning-tensor/src/tui/app.rs | sed -n '910,950p'Repository: one-covenant/bittensor-rs
Length of output: 1744
🏁 Script executed:
# Look for get_neurons method in the service
rg "pub async fn get_neurons|pub fn get_neurons" --type rust -A 5Repository: one-covenant/bittensor-rs
Length of output: 2962
🏁 Script executed:
# Check if NeuronDiscovery has a method to extract metrics
cat bittensor-rs/src/discovery.rs | head -100Repository: one-covenant/bittensor-rs
Length of output: 2859
🏁 Script executed:
# Check what methods are available in Metagraph struct definition
rg "impl.*Metagraph|fn.*neurons|fn.*metrics" --type rust -A 3 | head -50Repository: one-covenant/bittensor-rs
Length of output: 3162
🏁 Script executed:
# Search for methods in Service that might return neuron metrics
rg "pub async fn.*neuron" --type rust bittensor-rs/src/service.rsRepository: one-covenant/bittensor-rs
Length of output: 97
🏁 Script executed:
# Look at the full queries module to see what's exported
rg "pub.*fn.*neuron|pub.*struct.*Neuron" --type rust bittensor-rs/src/queries/Repository: one-covenant/bittensor-rs
Length of output: 193
🏁 Script executed:
# Check the get_neuron function signature and what it returns
rg "pub async fn get_neuron" --type rust -A 15 bittensor-rs/src/queries/neuron.rsRepository: one-covenant/bittensor-rs
Length of output: 946
🏁 Script executed:
# Check if Service exposes get_neuron method
rg "pub async fn get_neuron" --type rust -A 10 bittensor-rs/src/service.rsRepository: one-covenant/bittensor-rs
Length of output: 426
🏁 Script executed:
# Look at the full Metagraph struct fields to confirm it doesn't have metrics arrays
rg "pub.*incentive|pub.*emission|pub.*consensus|pub.*dividends|pub.*trust" --type rust bittensor-rs/src/queries/metagraph.rsRepository: one-covenant/bittensor-rs
Length of output: 51
🏁 Script executed:
# Check what NeuronInfo contains
rg "pub struct NeuronInfo" --type rust -A 20 bittensor-rs/src/queries/neuron.rsRepository: one-covenant/bittensor-rs
Length of output: 51
🏁 Script executed:
# Look for NeuronInfo definition in the queries module
fd "neuron.rs" bittensor-rs/src/queries/ --exec cat {}Repository: one-covenant/bittensor-rs
Length of output: 3944
Neuron metrics are hardcoded to zero values.
Lines 931-935 return hardcoded 0.0 values for incentive, emission, trust, consensus, and dividends. These metrics are available via the get_neuron() runtime API (which returns NeuronInfo containing these fields), but fetching them would require individual RPC calls per neuron rather than extracting from the metagraph. Consider whether the performance cost of additional RPC calls is justified for the data completeness benefit, or if these metrics should remain stubbed until a batch query endpoint becomes available.
| pub async fn run(ctx: AppContext) -> Result<()> { | ||
| // Setup terminal | ||
| enable_raw_mode()?; | ||
| let mut stdout = io::stdout(); | ||
| execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; | ||
| let backend = CrosstermBackend::new(stdout); | ||
| let mut terminal = Terminal::new(backend)?; | ||
|
|
||
| // Create app and run it (wrap context in Arc for async sharing) | ||
| let mut app = App::new(Arc::new(ctx))?; | ||
| let res = app.run(&mut terminal).await; | ||
|
|
||
| // Restore terminal | ||
| disable_raw_mode()?; | ||
| execute!( | ||
| terminal.backend_mut(), | ||
| LeaveAlternateScreen, | ||
| DisableMouseCapture | ||
| )?; | ||
| terminal.show_cursor()?; | ||
|
|
||
| if let Err(err) = res { | ||
| eprintln!("Error: {:?}", err); | ||
| return Err(err); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Terminal cleanup may not run on panic.
If app.run() panics, the terminal restoration code (lines 36-42) won't execute, leaving the terminal in raw mode with the alternate screen active. Users would need to run reset to fix their terminal.
🔎 Suggested fix using panic hook or guard pattern
pub async fn run(ctx: AppContext) -> Result<()> {
// Setup terminal
enable_raw_mode()?;
let mut stdout = io::stdout();
execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?;
let backend = CrosstermBackend::new(stdout);
let mut terminal = Terminal::new(backend)?;
+ // Ensure terminal is restored on panic
+ let original_hook = std::panic::take_hook();
+ std::panic::set_hook(Box::new(move |panic| {
+ let _ = disable_raw_mode();
+ let _ = execute!(io::stdout(), LeaveAlternateScreen, DisableMouseCapture);
+ let _ = crossterm::cursor::Show;
+ original_hook(panic);
+ }));
// Create app and run it
let mut app = App::new(Arc::new(ctx))?;
let res = app.run(&mut terminal).await;
+ // Restore original panic hook
+ let _ = std::panic::take_hook();
// Restore terminal
disable_raw_mode()?;Alternatively, use a RAII guard struct for cleanup.
Committable suggestion skipped: line range outside the PR's diff.
| /// Truncate name for display | ||
| fn truncate_name(name: &str, max_len: usize) -> String { | ||
| if name.len() <= max_len { | ||
| name.to_string() | ||
| } else { | ||
| format!("{}…", &name[..max_len - 1]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential panic on multi-byte UTF-8 characters.
truncate_name uses byte slicing (&name[..max_len - 1]) which will panic if the slice boundary falls within a multi-byte UTF-8 character. Subnet names could contain emoji or non-ASCII characters.
🔎 Suggested fix using char boundaries
fn truncate_name(name: &str, max_len: usize) -> String {
- if name.len() <= max_len {
+ if name.chars().count() <= max_len {
name.to_string()
} else {
- format!("{}…", &name[..max_len - 1])
+ let truncated: String = name.chars().take(max_len - 1).collect();
+ format!("{}…", truncated)
}
}📝 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.
| /// Truncate name for display | |
| fn truncate_name(name: &str, max_len: usize) -> String { | |
| if name.len() <= max_len { | |
| name.to_string() | |
| } else { | |
| format!("{}…", &name[..max_len - 1]) | |
| } | |
| } | |
| /// Truncate name for display | |
| fn truncate_name(name: &str, max_len: usize) -> String { | |
| if name.chars().count() <= max_len { | |
| name.to_string() | |
| } else { | |
| let truncated: String = name.chars().take(max_len - 1).collect(); | |
| format!("{}…", truncated) | |
| } | |
| } |
🤖 Prompt for AI Agents
In lightning-tensor/src/tui/views/subnet.rs around lines 239 to 246, the
function truncates by byte-slicing which can panic on multi-byte UTF-8
characters; change it to operate on character boundaries (e.g., handle max_len
== 0, then if name.chars().count() <= max_len return name.to_string(), else
return the first max_len-1 chars via name.chars().take(max_len -
1).collect::<String>() and append the ellipsis) so slicing is safe for non-ASCII
and emoji.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (8)
lightning-tensor/src/cli/wallet.rs (2)
275-284: Inconsistent default wallet resolution.This function uses a hardcoded
"default"fallback (line 281), whileshow_balancecorrectly usesctx.config().wallet.default_wallet. This inconsistency could confuse users whose default wallet has a different name.
161-177: Misleading output: hotkey address labeled as "Coldkey".Line 162 retrieves
wallet.hotkey().to_string()but both Text (line 167) and Json (line 173) outputs label it as "Coldkey". This will confuse users who need the actual coldkey address.lightning-tensor/src/tui/mod.rs (1)
25-52: Terminal cleanup on panic remains unaddressed.If
app.run()panics, terminal restoration (lines 37-44) won't execute, leaving the terminal in raw mode. This was flagged in a previous review.🔎 Suggested fix using a panic hook
pub async fn run(ctx: AppContext) -> Result<()> { // Setup terminal enable_raw_mode()?; let mut stdout = io::stdout(); execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; let backend = CrosstermBackend::new(stdout); let mut terminal = Terminal::new(backend)?; + // Ensure terminal is restored on panic + let original_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |panic| { + let _ = disable_raw_mode(); + let _ = execute!(io::stdout(), LeaveAlternateScreen, DisableMouseCapture); + original_hook(panic); + })); // Create app and run it (wrap context in Arc for async sharing) let mut app = App::new(Arc::new(ctx))?; let res = app.run(&mut terminal).await; + // Restore original panic hook + let _ = std::panic::take_hook(); // Restore terminal disable_raw_mode()?;lightning-tensor/src/tui/views/subnet.rs (1)
311-318: Potential panic on multi-byte UTF-8 characters intruncate_name.This was flagged in a previous review. Byte slicing will panic if boundaries fall within a multi-byte character.
🔎 Suggested fix
fn truncate_name(name: &str, max_len: usize) -> String { - if name.len() <= max_len { + if name.chars().count() <= max_len { name.to_string() } else { - format!("{}…", &name[..max_len - 1]) + let truncated: String = name.chars().take(max_len - 1).collect(); + format!("{}…", truncated) } }lightning-tensor/src/tui/app.rs (3)
417-427: Handle channel send errors to avoid silently dropping connection results.The spawned task ignores send failures on lines 420 and 423. If the receiver is dropped, connection results won't reach the UI.
630-634: Incomplete input processing logic.This placeholder doesn't process input based on context. Multiple handlers (stake, transfer, weights) set
input_modeexpecting this method to handle their input.
954-989: Neuron metrics are hardcoded to zero values.Lines 979-983 return hardcoded
0.0forincentive,emission,trust,consensus, anddividends. These require individualget_neuron()RPC calls per neuron, which may not justify the performance cost.lightning-tensor/src/config/settings.rs (1)
83-124:retriesis defined but unused;timeout_secsis not propagated toBittensorConfig.The
retriesfield (line 96) is not used anywhere in the codebase. Thetimeout_secsfield (line 92) is validated in tests but not passed toBittensorConfigin theto_bittensor_configmethod (lines 61-81).
🧹 Nitpick comments (12)
lightning-tensor/src/cli/stake.rs (2)
237-255: Delegation functions are aliases - intentional?
delegate_stakeandundelegate_stakesimply forward toadd_stakeandremove_stake. While acceptable for placeholders, the output messages will say "stake/unstake" instead of "delegate/undelegate", which may confuse users.When implementing the actual logic, ensure these produce distinct outputs or factor out a shared helper with a configurable action label.
82-122: Consider adding validation forproportionsandtakeparameters.Similar to the validation added in
root.rsfor comma-separated weights, theproportionsfield should be validated:
- Parse as comma-separated floats
- Verify they sum to 1.0 (within floating-point tolerance)
- Ensure count matches the number of children
The
takeparameter should validate the range[0.0, 1.0].🔎 Example validation for SetTake
ChildrenAction::SetTake { take, netuid } => { + if !(0.0..=1.0).contains(&take) { + return Err(crate::errors::Error::config( + "Take must be between 0.0 and 1.0" + )); + } println!( " Would set childkey take to {:.2}% on subnet {:?}", take * 100.0, netuid ); }lightning-tensor/src/tui/views/root.rs (1)
81-85: Hardcoded validator count.The value
"64"is hardcoded with a comment noting it's a placeholder. Consider either:
- Adding a
TODOcomment to make it more discoverable- Using a constant with a descriptive name like
PLACEHOLDER_VALIDATOR_COUNT- Span::styled( - "64", // Placeholder - would come from root metagraph - Style::default().fg(colors::VALIDATOR), - ), + // TODO: Fetch actual validator count from root metagraph + Span::styled("64", Style::default().fg(colors::VALIDATOR)),lightning-tensor/src/tui/views/stake.rs (1)
191-194: Document the emission rate multiplier.The magic number
7200.0represents blocks per day (~12s block time), but this isn't documented. Consider extracting to a named constant.+// Blocks per day at ~12 second block time (86400 / 12 = 7200) +const BLOCKS_PER_DAY: f64 = 7200.0; + ... - format!("{:.6} {}/day", total_emission * 7200.0, symbols::TAO), + format!("{:.6} {}/day", total_emission * BLOCKS_PER_DAY, symbols::TAO),lightning-tensor/src/tui/views/metagraph.rs (1)
310-317: Potential panic on multi-byte UTF-8 characters intruncate_key.Byte slicing (
&key[..6]and&key[key.len() - 5..]) will panic if boundaries fall within a multi-byte character. SS58 addresses are typically ASCII, but defensive coding is recommended.🔎 Suggested fix using char-safe slicing
fn truncate_key(key: &str) -> String { if key.len() <= 14 { key.to_string() } else { - format!("{}…{}", &key[..6], &key[key.len() - 5..]) + let start: String = key.chars().take(6).collect(); + let end: String = key.chars().rev().take(5).collect::<String>().chars().rev().collect(); + format!("{}…{}", start, end) } }lightning-tensor/src/tui/views/home.rs (1)
250-289: Consider logging or differentiating thetry_lockfailure state.When
try_lock()fails (line 252), the UI shows "Loading..." which may be misleading—it indicates lock contention, not actual loading. This could confuse users if it persists.🔎 Suggested improvement
} else { - vec![Line::from(vec![Span::styled( - "Loading...", - Style::default().fg(colors::TEXT_TERTIARY), - )])] + vec![Line::from(vec![Span::styled( + "Updating...", + Style::default().fg(colors::TEXT_TERTIARY), + )])] };lightning-tensor/src/tui/views/weights.rs (1)
136-139: Placeholder weight values need implementation.The weight percentages are hardcoded (0.0 for validators, 10.0 for miners) rather than derived from actual weight data. This should be connected to real weight values.
Would you like me to help implement the actual weight calculation, or should this be tracked as a follow-up issue?
lightning-tensor/src/tui/components/animation.rs (1)
35-40: Consider making fields private with accessor methods.The
AnimationStatestruct exposes all fields as public. While this works, encapsulating the internal state with private fields and accessor methods would provide better API control and allow internal changes without breaking consumers.📝 Optional refactor
pub struct AnimationState { - pub frame: usize, - pub last_update: Instant, - pub slow_frame: usize, - pub last_slow_update: Instant, + frame: usize, + last_update: Instant, + slow_frame: usize, + last_slow_update: Instant, }Then add accessor methods if external code needs to read these values.
lightning-tensor/src/tui/components/input.rs (1)
14-32: Consider encapsulating internal state fields.The
InputFieldstruct exposesis_password,is_focused, andplaceholderas public fields. While this works, using private fields with the existing builder methods (password(),focused(),placeholder()) would provide better encapsulation and API clarity.lightning-tensor/src/tui/components/popup.rs (1)
150-176: Consider validating selected_button index.If
selected_buttonis out of bounds (e.g., greater thanbuttons.len()), no button will appear selected, which could confuse users. Consider clamping or validating the index when set viaselect_button().📝 Optional improvement
pub fn select_button(mut self, idx: usize) -> Self { - self.selected_button = idx; + if let Some(buttons) = &self.buttons { + self.selected_button = idx.min(buttons.len().saturating_sub(1)); + } else { + self.selected_button = idx; + } self }lightning-tensor/src/tui/app.rs (1)
1053-1060: Eliminate code duplication by using the theme module's formatting helper.This
truncate_addressfunction duplicates the logic inlightning-tensor/src/tui/theme.rs::format_address(lines 349-356). Both truncate addresses with ellipsis for display.🔎 Proposed refactor
Remove the local
truncate_addressfunction and import the theme module'sformat_addressinstead:+use crate::tui::theme::format_address; // Inside fetch_metagraph_data: - hotkey: truncate_address(&n.hotkey, 8), - coldkey: truncate_address(&n.coldkey, 8), + hotkey: format_address(&n.hotkey, 8), + coldkey: format_address(&n.coldkey, 8),Then delete lines 1053-1060.
bittensor-rs/src/queries/subnet.rs (1)
384-384: Consider handling invalid UTF-8 explicitly instead of lossy conversion.Using
String::from_utf8_lossy(lines 384, 443) replaces invalid UTF-8 sequences with the replacement character (�), which could silently hide data corruption issues. While this is safe and prevents panics, it may make debugging difficult if the chain state contains unexpected data.For production code displaying user-facing information, lossy conversion is likely acceptable. However, consider whether logging a warning when invalid UTF-8 is encountered would help with debugging.
Also applies to: 443-443
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
bittensor-rs/build.rsbittensor-rs/src/bin/generate-metadata.rsbittensor-rs/src/queries/subnet.rsbittensor-rs/src/service.rsbittensor-rs/src/utils.rsbittensor-rs/src/wallet/keyfile.rsbittensor-rs/src/wallet/mod.rsbittensor-rs/src/wallet/signer.rslightning-tensor/src/cli/crowd.rslightning-tensor/src/cli/mod.rslightning-tensor/src/cli/root.rslightning-tensor/src/cli/stake.rslightning-tensor/src/cli/subnet.rslightning-tensor/src/cli/transfer.rslightning-tensor/src/cli/wallet.rslightning-tensor/src/cli/weights.rslightning-tensor/src/config/mod.rslightning-tensor/src/config/settings.rslightning-tensor/src/context.rslightning-tensor/src/lib.rslightning-tensor/src/models/display.rslightning-tensor/src/models/mod.rslightning-tensor/src/services/crowd.rslightning-tensor/src/services/mod.rslightning-tensor/src/services/root.rslightning-tensor/src/services/stake.rslightning-tensor/src/services/subnet.rslightning-tensor/src/services/transfer.rslightning-tensor/src/services/wallet.rslightning-tensor/src/services/weights.rslightning-tensor/src/tui/app.rslightning-tensor/src/tui/components/animation.rslightning-tensor/src/tui/components/input.rslightning-tensor/src/tui/components/mod.rslightning-tensor/src/tui/components/popup.rslightning-tensor/src/tui/components/spinner.rslightning-tensor/src/tui/components/table.rslightning-tensor/src/tui/mod.rslightning-tensor/src/tui/theme.rslightning-tensor/src/tui/views/home.rslightning-tensor/src/tui/views/metagraph.rslightning-tensor/src/tui/views/mod.rslightning-tensor/src/tui/views/root.rslightning-tensor/src/tui/views/stake.rslightning-tensor/src/tui/views/subnet.rslightning-tensor/src/tui/views/transfer.rslightning-tensor/src/tui/views/wallet.rslightning-tensor/src/tui/views/weights.rs
✅ Files skipped from review due to trivial changes (2)
- bittensor-rs/src/utils.rs
- bittensor-rs/src/wallet/keyfile.rs
🚧 Files skipped from review as they are similar to previous changes (13)
- lightning-tensor/src/services/mod.rs
- lightning-tensor/src/services/stake.rs
- lightning-tensor/src/services/subnet.rs
- lightning-tensor/src/cli/weights.rs
- lightning-tensor/src/lib.rs
- lightning-tensor/src/services/transfer.rs
- lightning-tensor/src/models/mod.rs
- lightning-tensor/src/services/weights.rs
- lightning-tensor/src/tui/components/spinner.rs
- lightning-tensor/src/tui/components/mod.rs
- lightning-tensor/src/models/display.rs
- lightning-tensor/src/cli/crowd.rs
- lightning-tensor/src/services/wallet.rs
🧰 Additional context used
🧬 Code graph analysis (17)
lightning-tensor/src/tui/views/transfer.rs (1)
lightning-tensor/src/tui/views/mod.rs (2)
draw(26-59)draw_header(62-101)
lightning-tensor/src/cli/wallet.rs (2)
lightning-tensor/src/errors.rs (2)
wallet(128-132)ui(163-167)lightning-tensor/src/services/wallet.rs (8)
new(16-18)create_wallet(27-51)list_wallets(73-91)sign_message(131-140)verify_signature(143-185)regen_wallet(188-204)create_hotkey(117-128)list_hotkeys(94-114)
lightning-tensor/src/tui/views/weights.rs (1)
lightning-tensor/src/tui/views/mod.rs (2)
draw(26-59)draw_header(62-101)
lightning-tensor/src/tui/components/animation.rs (2)
lightning-tensor/src/tui/app.rs (1)
new(173-223)lightning-tensor/src/tui/components/spinner.rs (3)
new(33-39)new(86-88)pulse(99-104)
lightning-tensor/src/services/root.rs (6)
lightning-tensor/src/services/crowd.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/stake.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/subnet.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/transfer.rs (2)
new(9-11)default(15-17)lightning-tensor/src/services/wallet.rs (1)
new(16-18)lightning-tensor/src/services/weights.rs (2)
new(9-11)default(15-17)
lightning-tensor/src/cli/transfer.rs (3)
lightning-tensor/src/cli/mod.rs (1)
execute(94-120)lightning-tensor/src/cli/wallet.rs (1)
execute(116-146)bittensor-rs/src/extrinsics/transfer.rs (1)
keep_alive(60-63)
bittensor-rs/src/queries/subnet.rs (1)
bittensor-rs/src/service.rs (1)
client(799-808)
lightning-tensor/src/cli/stake.rs (2)
lightning-tensor/src/cli/mod.rs (1)
execute(94-120)lightning-tensor/src/cli/wallet.rs (1)
execute(116-146)
lightning-tensor/src/config/mod.rs (2)
lightning-tensor/src/context.rs (1)
config(52-54)lightning-tensor/src/errors.rs (1)
config(121-125)
lightning-tensor/src/tui/views/stake.rs (1)
lightning-tensor/src/tui/app.rs (1)
new(173-223)
lightning-tensor/src/tui/views/root.rs (2)
lightning-tensor/src/tui/views/metagraph.rs (3)
draw(19-32)draw_header(34-100)draw_footer(286-308)lightning-tensor/src/tui/views/subnet.rs (3)
draw(18-37)draw_header(39-107)draw_footer(285-309)
lightning-tensor/src/tui/views/wallet.rs (2)
lightning-tensor/src/tui/theme.rs (1)
selected(108-112)lightning-tensor/src/tui/app.rs (1)
new(173-223)
lightning-tensor/src/cli/mod.rs (8)
lightning-tensor/src/context.rs (4)
wallet(131-133)default(264-266)new(34-43)new(212-219)lightning-tensor/src/cli/crowd.rs (1)
execute(70-91)lightning-tensor/src/cli/root.rs (1)
execute(45-56)lightning-tensor/src/cli/subnet.rs (1)
execute(57-70)lightning-tensor/src/cli/transfer.rs (1)
execute(31-51)lightning-tensor/src/cli/wallet.rs (1)
execute(116-146)lightning-tensor/src/cli/weights.rs (1)
execute(65-86)lightning-tensor/src/tui/app.rs (1)
new(173-223)
lightning-tensor/src/tui/views/subnet.rs (4)
lightning-tensor/src/tui/views/metagraph.rs (3)
draw(19-32)draw_header(34-100)draw_footer(286-308)lightning-tensor/src/tui/views/wallet.rs (1)
draw(16-27)lightning-tensor/src/tui/app.rs (1)
new(173-223)lightning-tensor/src/tui/theme.rs (1)
selected(108-112)
lightning-tensor/src/config/settings.rs (3)
bittensor-rs/src/wallet/mod.rs (1)
load(99-102)lightning-tensor/src/errors.rs (2)
config(121-125)wallet(128-132)lightning-tensor/src/config/mod.rs (2)
config_file_path(31-33)default_wallet_dir(24-28)
lightning-tensor/src/cli/root.rs (3)
lightning-tensor/src/cli/mod.rs (1)
execute(94-120)lightning-tensor/src/context.rs (1)
config(52-54)lightning-tensor/src/errors.rs (1)
config(121-125)
bittensor-rs/src/service.rs (1)
bittensor-rs/src/discovery.rs (1)
format(267-267)
🔇 Additional comments (59)
bittensor-rs/src/wallet/mod.rs (1)
32-32: LGTM — import reordering is consistent with convention.This formatting change places the
std::pathimport after crate-level imports, which aligns with the common Rust convention of grouping imports by origin (external crates, thenstd, or vice versa with a blank line separator).bittensor-rs/src/bin/generate-metadata.rs (1)
106-108: LGTM! Non-functional import reordering.The import reordering has no runtime impact—all imports remain available before use.
bittensor-rs/src/wallet/signer.rs (2)
39-40: LGTM: Formatting refinement improves readability.The collapsed statement is more concise while preserving the same logic and error handling.
47-47: LGTM: Removed extraneous blank line.This formatting improvement enhances code consistency.
bittensor-rs/build.rs (1)
32-32: LGTM! Clear and informative error message.The single-line panic message is concise and clearly communicates both the condition (offline mode) and the problem (missing cached code). The use of
{:?}for path formatting is appropriate.bittensor-rs/src/service.rs (2)
776-808: LGTM - Clean delegation to connection pool.The
client()method provides appropriate access to the underlying chain client for custom queries. The implementation correctly delegates to the connection pool and handles errors properly.
776-808: No duplicateclient()method exists in this file.The AI summary incorrectly claimed duplicate client() methods. Verification confirms only one
pub async fn client()method exists in src/service.rs. The code is correct.lightning-tensor/src/services/crowd.rs (1)
1-18: LGTM!The
CrowdServicefollows the same pattern as other service stubs in this PR. The manualDefaultimplementation is consistent with sibling services.Consider using
#[derive(Default)]in a future cleanup pass to reduce boilerplate across all unit-struct services.lightning-tensor/src/cli/subnet.rs (2)
1-88: LGTM!The subnet CLI module is well-structured with consistent patterns across all subcommand handlers. The placeholder implementations properly distinguish between Text and Json output formats.
90-185: Consistent placeholder implementations.All subnet handlers follow the established pattern with proper Text/Json branching and error propagation.
lightning-tensor/src/cli/stake.rs (1)
1-80: Well-structured command definitions.The
StakeCommandandChildrenActionenums are comprehensive and follow clap best practices with appropriate argument annotations.lightning-tensor/src/tui/views/root.rs (1)
223-303: Registration panel and footer look good.The conditional wallet status rendering is clean, and the footer appropriately shows available actions for this view.
lightning-tensor/src/services/root.rs (1)
1-18: LGTM!The
RootServicefollows the established pattern for service stubs in this PR.lightning-tensor/src/cli/wallet.rs (2)
12-32: Good fallback handling inread_coldkey_address.The function gracefully handles both JSON format (extracting
ss58Address) and plain text format, with appropriate fallbacks for missing files.
420-433: Helper functions are well-implemented.The
prompt_passwordandprompt_inputhelpers properly use dialoguer and convert errors to the crate's error type.lightning-tensor/src/tui/views/stake.rs (2)
1-28: Clean layout structure.The main
drawfunction properly splits the area into summary (35%) and positions (65%) sections with delegated rendering.
132-168: Good empty/loading state handling.The stake positions view properly handles three states: loading (with spinner), empty, and populated, providing clear user guidance in each case.
lightning-tensor/src/cli/root.rs (2)
74-119: Good input validation for weights command.The validation for comma-separated netuids and weights is well-implemented:
- Parses netuids as
u16values- Parses weights as
f64values- Validates that counts match
This addresses the previous review feedback.
1-56: LGTM!The root CLI module is well-structured with consistent patterns across all subcommand handlers.
lightning-tensor/src/tui/views/metagraph.rs (1)
18-32: LGTM!Clean layout orchestration with appropriate constraint allocation for header, table, and footer sections.
lightning-tensor/src/tui/views/home.rs (1)
17-35: LGTM!Well-structured three-panel layout with sensible percentage allocations.
lightning-tensor/src/config/mod.rs (1)
17-33: LGTM!Clean helper functions with appropriate error handling for missing directories. The wallet path convention (
.bittensor/wallets) aligns with standard bittensor tooling.lightning-tensor/src/tui/views/subnet.rs (1)
153-283: LGTM!Table rendering now correctly uses
render_stateful_widgetwithsubnet_list_state, addressing the previous review concern. The emission percentage calculation and color-coding logic are well-implemented.lightning-tensor/src/tui/views/mod.rs (1)
200-217: LGTM!Clean implementation of
centered_rectutility for popup positioning using percentage-based constraints.lightning-tensor/src/tui/components/animation.rs (5)
58-72: LGTM! Clean dual-cadence animation update.The time-based animation update mechanism with fast (80ms) and slow (400ms) cadences is well-implemented. The modulo operations safely wrap frame indices, and the duration checks prevent unnecessary updates.
99-120: LGTM! Elegant gradient-based loading bar.The distance-based character selection creates a nice visual effect. The use of
unsigned_abs()for the distance calculation is appropriate and safe.
210-228: LGTM! Robust sparkline rendering with proper edge case handling.The implementation correctly handles empty data, zero range, and bounds checking. The normalization and clamping logic ensures safe array access.
274-295: LGTM! Safe progress bar rendering with gradient effect.The use of
saturating_suband bounds checking with.min(self.width)ensures safe arithmetic. The gradient rendering logic creates a polished visual effect.
232-257: LGTM! Consistent zone-based rendering.The zone calculation logic is clear and consistent with the base
render()method. The threshold-based coloring will provide good visual feedback.lightning-tensor/src/cli/transfer.rs (2)
30-51: LGTM! Appropriate placeholder implementation.The stub implementation with TODO comment and structured output (both text and JSON) is consistent with other placeholder commands in this PR. The messaging clearly indicates the feature is not yet implemented.
17-19: The f64 choice aligns with bittensor-rs API design and is consistent across the codebase.The underlying bittensor-rs transfer API (
TransferParams::new_tao) accepts f64 TAO amounts and internally converts to u64 RAO via(amount_tao * 1_000_000_000.0) as u64. This same pattern is used throughout bittensor-rs for staking (StakeParams::new_tao) and balance conversions (Balance::from_tao). Since the entire lightning-tensor codebase consistently uses f64 for TAO display and input (wallet balances, stake amounts, emissions), maintaining this type here is appropriate and matches the established API contract. When the transfer function is implemented (currently TODO), it will follow the same f64→u64 RAO conversion pattern.lightning-tensor/src/tui/views/wallet.rs (3)
29-92: LGTM! Safe wallet list rendering with proper bounds checking.The wallet list iteration uses safe access patterns with
.get()and.and_then()to handle optional balances. The styling logic for active vs. selected wallets is clear and appropriate.
94-188: LGTM! Wallet details with safe optional access.The wallet details rendering correctly handles missing wallets and balances using safe optional access patterns. The address truncation logic is appropriate for display purposes.
213-311: Well-structured stakes table with safe data access.The stakes table rendering properly handles empty states, connection status, and calculates the total stake correctly. The alternating row styles enhance readability. Note: The
truncate()helper used on Line 275 has a UTF-8 safety issue flagged separately.lightning-tensor/src/tui/components/input.rs (2)
49-84: LGTM! Clear input rendering logic with password masking.The
display_valuecalculation correctly handles password masking, placeholders, and normal text. The cursor effect and value styling appropriately reflect the input state.
85-110: LGTM! Well-structured input field rendering.The focus-dependent border styling and block configuration provide clear visual feedback. The component integrates well with the theme system.
lightning-tensor/src/tui/components/popup.rs (3)
14-82: LGTM! Clean popup type definitions and constructors.The popup type variants and constructor methods provide a clear, type-safe API. The
confirm()constructor appropriately defaults to the "Confirm" button (index 1).
94-148: LGTM! Robust popup rendering with proper layout.The rendering logic correctly handles popups with and without buttons, applies type-specific styling, and properly clears the background. The layout constraints adapt appropriately to content.
179-198: LGTM! Clean centered rectangle calculation.The centering logic using Layout constraints is straightforward and works well for typical percentage values. The function assumes reasonable input ranges (0-100), which is appropriate for its internal use.
lightning-tensor/src/cli/mod.rs (3)
16-90: LGTM! Well-structured CLI interface with sensible defaults.The CLI structure with global options and subcommands is clean and follows clap best practices. The default values (finney network, text output) are appropriate for typical usage.
92-121: LGTM! Clean command dispatch with proper context building.The context builder pattern is used effectively to construct
AppContextfrom CLI options. The command dispatch logic is straightforward and routes to the appropriate subcommand handlers with consistent error handling.
123-127: LGTM! Simple and effective CLI entry point.The
run()function provides a clean entry point that delegates to clap for parsing and theexecute()method for dispatch.lightning-tensor/src/tui/components/table.rs (3)
14-111: LGTM! Flexible table component with safe rendering.The
DataTablecomponent provides a reusable, well-structured table with optional per-row styling. The rendering logic safely handles optional row styles and applies alternating backgrounds for readability.
116-132: LGTM! User-friendly TAO amount formatting.The threshold-based formatting with K/M suffixes makes large amounts more readable while preserving precision for small amounts. The logic is clear and handles all reasonable value ranges.
134-181: Well-designed cell formatting helpers.The
StyledCellmethods provide consistent, reusable formatting across the TUI. The color-coding for percentages and role indicators enhances data readability. Note: Theaddress()method has a UTF-8 safety issue flagged separately.lightning-tensor/src/context.rs (6)
14-54: LGTM! Well-designed context with appropriate thread safety.The use of
RwLock<Option<...>>forserviceandwalletprovides thread-safe, lazy initialization. The separation of concerns between config, service, and wallet state is clean.
61-78: LGTM! Clean service access pattern with guards.The optional and required accessor patterns (
service()vsrequire_service()) provide flexibility while preventing unwrap panics. The error messages are descriptive and actionable.
80-128: LGTM! Robust connection management with sensible defaults.The connection methods handle both explicit parameters and config-based defaults. The fallback to
"default"wallet/hotkey values provides reasonable behavior for read-only access scenarios.
130-169: LGTM! Consistent wallet management with proper validation.The wallet loading logic validates existence before attempting to load, provides descriptive errors, and follows the same optional/required accessor pattern as the service methods.
171-201: LGTM! Safe wallet listing with proper error handling.The wallet listing logic gracefully handles missing directories, filters appropriately for wallet folders, and sorts results for consistent output. The network and netuid accessors provide simple, direct access to config values.
203-290: LGTM! Clean builder pattern with test coverage.The
AppContextBuilderprovides a fluent API for constructing contexts with custom settings. The test validates that builder options correctly propagate to the config. The pattern is idiomatic and well-implemented.lightning-tensor/src/tui/theme.rs (1)
1-356: Well-structured theming module with consistent visual design.The theme module provides a cohesive cyberpunk/lightning aesthetic with proper separation of concerns:
- Color palette with semantic naming and clear hierarchy
- Style constructors for consistent UI elements
- Comprehensive symbol set with Unicode glyphs
- Utility functions (sparkline, gradient, formatting) with correct edge-case handling
The sparkline normalization (lines 230-246), progress bar gradient (lines 265-286), and formatting helpers (lines 322-356) all handle boundary conditions appropriately.
bittensor-rs/src/queries/subnet.rs (7)
7-7: LGTM!The codec import is necessary for the new compact byte decoding functionality.
28-58: LGTM!The struct is well-documented and comprehensively covers dynamic subnet information including DTAO pricing data.
395-404: Price calculation logic is sound.The price calculation correctly:
- Converts RAO to TAO for precision
- Handles the root subnet (netuid 0) as a special 1:1 case
- Guards against division by zero
- Returns 0.0 for empty pools
440-444: LGTM!The helper function correctly extracts the inner bytes from the
Compact<u8>wrappers and converts to a string. The lossy UTF-8 conversion concern was already noted in a previous comment.
373-373: No issue here—the.flatten()usage is intentional and correct.The RPC API intentionally returns
Vec<Option<DynamicInfo>>to support sparse data where some subnets may be uninitialized. The.flatten()call correctly filters these empty slots, convertingVec<Option<T>>to a dense iterator of only populated subnets. This is standard Rust pattern for handling sparse collections.Error handling is already in place: actual RPC failures are caught and propagated as
BittensorErrorat lines 359–361 and 367–369.Nonevalues are not errors—they're expected empty slots that don't need logging.Likely an incorrect or invalid review comment.
429-430: No action needed — the code correctly converts AccountId types to SS58 format.Lines 429-430 use
format!("{}")to convertAccountId(AccountId32) types to strings. This is both idiomatic and correct: the AccountId32 Display trait implementation produces the SS58 address format, as documented in the codebase ataccount.rs:40. The same pattern is used elsewhere in the codebase (e.g.,account_id_to_hotkey()ataccount.rs:41), confirming consistency and correctness.
409-409: The conversion formula is correct. The runtime metadata confirms thatmoving_priceusesFixedI128<U32>(32 fractional bits), and dividing the rawbitsvalue by 2^32 is the proper way to convert a fixed-point number to f64 in Substrate. The code and comments accurately reflect this.
| /// Fetch metagraph for current netuid | ||
| pub fn fetch_metagraph(&mut self) { | ||
| let netuid = self.current_netuid; | ||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
|
|
||
| self.start_loading(&format!("Loading metagraph for subnet {}...", netuid)); | ||
|
|
||
| tokio::spawn(async move { | ||
| let result = fetch_metagraph_data(&ctx, netuid).await; | ||
| let _ = tx | ||
| .send(AsyncResult::MetagraphLoaded { | ||
| netuid, | ||
| neurons: result, | ||
| }) | ||
| .await; | ||
| }); | ||
| } | ||
|
|
||
| /// Fetch subnets | ||
| pub fn fetch_subnets(&mut self) { | ||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
|
|
||
| self.start_loading("Loading subnets..."); | ||
|
|
||
| tokio::spawn(async move { | ||
| let result = fetch_subnet_data(&ctx).await; | ||
| let _ = tx.send(AsyncResult::SubnetsLoaded(result)).await; | ||
| }); | ||
| } | ||
|
|
||
| /// Fetch wallet balance | ||
| pub fn fetch_balance(&mut self, wallet_idx: usize) { | ||
| if wallet_idx >= self.wallets.len() { | ||
| return; | ||
| } | ||
|
|
||
| let wallet = self.wallets[wallet_idx].clone(); | ||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
|
|
||
| self.start_loading(&format!("Fetching balance for {}...", wallet.name)); | ||
|
|
||
| tokio::spawn(async move { | ||
| let result = fetch_wallet_balance(&ctx, &wallet).await; | ||
| let _ = tx | ||
| .send(AsyncResult::BalanceLoaded { | ||
| wallet_idx, | ||
| balance: result, | ||
| }) | ||
| .await; | ||
| }); | ||
| } | ||
|
|
||
| /// Fetch all wallet balances in parallel (batched for performance) | ||
| pub fn fetch_all_balances(&mut self) { | ||
| if self.wallets.is_empty() { | ||
| return; | ||
| } | ||
|
|
||
| let wallets: Vec<_> = self.wallets.iter().cloned().enumerate().collect(); | ||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
| let count = wallets.len(); | ||
|
|
||
| self.add_message_sync(format!("⏳ Fetching {} wallet balances...", count)); | ||
|
|
||
| // Spawn parallel fetches for all wallets | ||
| tokio::spawn(async move { | ||
| use futures::future::join_all; | ||
|
|
||
| let futures: Vec<_> = wallets | ||
| .into_iter() | ||
| .map(|(idx, wallet)| { | ||
| let ctx = Arc::clone(&ctx); | ||
| let tx = tx.clone(); | ||
| async move { | ||
| let result = fetch_wallet_balance(&ctx, &wallet).await; | ||
| let _ = tx | ||
| .send(AsyncResult::BalanceLoaded { | ||
| wallet_idx: idx, | ||
| balance: result, | ||
| }) | ||
| .await; | ||
| } | ||
| }) | ||
| .collect(); | ||
|
|
||
| join_all(futures).await; | ||
| let _ = tx | ||
| .send(AsyncResult::Message(format!("✓ Loaded {} balances", count))) | ||
| .await; | ||
| }); | ||
| } | ||
|
|
||
| /// Sync version of add_message for use in non-async context | ||
| fn add_message_sync(&self, msg: String) { | ||
| let messages = self.messages.clone(); | ||
| tokio::spawn(async move { | ||
| messages.lock().await.push(msg); | ||
| }); | ||
| } | ||
|
|
||
| /// Fetch all wallet stakes in parallel | ||
| pub fn fetch_all_stakes(&mut self) { | ||
| if self.wallets.is_empty() { | ||
| return; | ||
| } | ||
|
|
||
| let wallets: Vec<_> = self.wallets.iter().cloned().enumerate().collect(); | ||
| let tx = self.async_tx.clone(); | ||
| let ctx = Arc::clone(&self.ctx); | ||
|
|
||
| // Spawn parallel fetches for all wallets | ||
| tokio::spawn(async move { | ||
| use futures::future::join_all; | ||
|
|
||
| let futures: Vec<_> = wallets | ||
| .into_iter() | ||
| .map(|(idx, wallet)| { | ||
| let ctx = Arc::clone(&ctx); | ||
| let tx = tx.clone(); | ||
| async move { | ||
| let result = fetch_wallet_stakes(&ctx, &wallet).await; | ||
| let _ = tx | ||
| .send(AsyncResult::StakeLoaded { | ||
| wallet_idx: idx, | ||
| stakes: result, | ||
| }) | ||
| .await; | ||
| } | ||
| }) | ||
| .collect(); | ||
|
|
||
| join_all(futures).await; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consistent error handling needed for all async channel sends.
Multiple async tasks (lines 758, 776, 794, 827, 873) use let _ = tx.send(...).await, silently ignoring send failures. This pattern is repeated across fetch_metagraph, fetch_subnets, fetch_balance, fetch_all_balances, and fetch_all_stakes. If the channel closes, data fetches complete but results are lost without notification.
🔎 Suggested approach
Implement consistent error handling across all async spawns. Options:
- Log failures (recommended for this "Chill" review): Replace
let _ =with logging on error - Propagate via fallback: Send an
AsyncResult::Errormessage when the primary send fails - Structured tracing: Use
tracing::warn!ortracing::error!for production observability
Example for fetch_metagraph (line 758):
- let _ = tx
- .send(AsyncResult::MetagraphLoaded {
- netuid,
- neurons: result,
- })
- .await;
+ if tx
+ .send(AsyncResult::MetagraphLoaded {
+ netuid,
+ neurons: result,
+ })
+ .await
+ .is_err()
+ {
+ eprintln!("Failed to send metagraph result: receiver dropped");
+ }Apply the same pattern to lines 776, 794-799, 827-832, 838-840, and 873-878.
🤖 Prompt for AI Agents
In lightning-tensor/src/tui/app.rs around lines 748–885, the spawned async tasks
currently use "let _ = tx.send(...).await", which silently drops send failures;
change each send to handle the Result and log failures (e.g., using
tracing::warn! or tracing::error!) including contextual info (netuid,
wallet_idx, message type) so dropped messages are observable; ensure you add the
tracing import if missing and apply this pattern to the send calls in
fetch_metagraph, fetch_subnets, fetch_balance, fetch_all_balances (including the
per-wallet sends and final Message), and fetch_all_stakes.
| /// Generate bouncing loading indicator | ||
| pub fn bouncing_loader(&self, width: usize) -> String { | ||
| let mut bar = String::with_capacity(width); | ||
| // Bounce effect using sine-like pattern | ||
| let bounce_frames = width * 2 - 2; | ||
| let pos = self.frame % bounce_frames.max(1); | ||
| let actual_pos = if pos < width { | ||
| pos | ||
| } else { | ||
| bounce_frames - pos | ||
| }; | ||
|
|
||
| for i in 0..width { | ||
| if i == actual_pos { | ||
| bar.push('⚡'); | ||
| } else { | ||
| bar.push('·'); | ||
| } | ||
| } | ||
|
|
||
| bar | ||
| } |
There was a problem hiding this comment.
Guard against width = 0 to prevent underflow.
If width is 0, line 126 calculates bounce_frames = 0 * 2 - 2, which would underflow (usize wrapping to a very large number). While .max(1) on line 127 prevents modulo-by-zero, the logic would still be incorrect. Consider adding an early return or assertion.
🔎 Suggested fix
pub fn bouncing_loader(&self, width: usize) -> String {
+ if width == 0 {
+ return String::new();
+ }
let mut bar = String::with_capacity(width);
// Bounce effect using sine-like pattern
let bounce_frames = width * 2 - 2;📝 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.
| /// Generate bouncing loading indicator | |
| pub fn bouncing_loader(&self, width: usize) -> String { | |
| let mut bar = String::with_capacity(width); | |
| // Bounce effect using sine-like pattern | |
| let bounce_frames = width * 2 - 2; | |
| let pos = self.frame % bounce_frames.max(1); | |
| let actual_pos = if pos < width { | |
| pos | |
| } else { | |
| bounce_frames - pos | |
| }; | |
| for i in 0..width { | |
| if i == actual_pos { | |
| bar.push('⚡'); | |
| } else { | |
| bar.push('·'); | |
| } | |
| } | |
| bar | |
| } | |
| /// Generate bouncing loading indicator | |
| pub fn bouncing_loader(&self, width: usize) -> String { | |
| if width == 0 { | |
| return String::new(); | |
| } | |
| let mut bar = String::with_capacity(width); | |
| // Bounce effect using sine-like pattern | |
| let bounce_frames = width * 2 - 2; | |
| let pos = self.frame % bounce_frames.max(1); | |
| let actual_pos = if pos < width { | |
| pos | |
| } else { | |
| bounce_frames - pos | |
| }; | |
| for i in 0..width { | |
| if i == actual_pos { | |
| bar.push('⚡'); | |
| } else { | |
| bar.push('·'); | |
| } | |
| } | |
| bar | |
| } |
🤖 Prompt for AI Agents
In lightning-tensor/src/tui/components/animation.rs around lines 122 to 143,
guard against width == 0 to prevent usize underflow when computing
bounce_frames; add an early return (e.g., return an empty String) at the start
of bouncing_loader if width == 0, then compute bounce_frames safely and proceed
as-is so modulo and indexing logic never operates on an invalid width.
| /// Create a cell with address styling (truncated) | ||
| pub fn address(addr: &str) -> Cell<'static> { | ||
| let truncated = if addr.len() > 16 { | ||
| format!("{}…{}", &addr[..8], &addr[addr.len() - 6..]) | ||
| } else { | ||
| addr.to_string() | ||
| }; | ||
|
|
||
| Cell::from(truncated).style(Style::default().fg(colors::TEXT_ACCENT)) | ||
| } |
There was a problem hiding this comment.
Fix potential panic on multi-byte UTF-8 characters in address truncation.
Similar to the truncate() function in wallet.rs, the string slicing &addr[..8] and &addr[addr.len() - 6..] on lines 151-152 can panic if the slice boundaries fall within multi-byte UTF-8 characters.
🔎 Suggested fix
pub fn address(addr: &str) -> Cell<'static> {
let truncated = if addr.len() > 16 {
- format!("{}…{}", &addr[..8], &addr[addr.len() - 6..])
+ let start: String = addr.chars().take(8).collect();
+ let end: String = addr.chars().rev().take(6).collect::<String>()
+ .chars().rev().collect();
+ format!("{}…{}", start, end)
} else {
addr.to_string()
};📝 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.
| /// Create a cell with address styling (truncated) | |
| pub fn address(addr: &str) -> Cell<'static> { | |
| let truncated = if addr.len() > 16 { | |
| format!("{}…{}", &addr[..8], &addr[addr.len() - 6..]) | |
| } else { | |
| addr.to_string() | |
| }; | |
| Cell::from(truncated).style(Style::default().fg(colors::TEXT_ACCENT)) | |
| } | |
| /// Create a cell with address styling (truncated) | |
| pub fn address(addr: &str) -> Cell<'static> { | |
| let truncated = if addr.len() > 16 { | |
| let start: String = addr.chars().take(8).collect(); | |
| let end: String = addr.chars().rev().take(6).collect::<String>() | |
| .chars().rev().collect(); | |
| format!("{}…{}", start, end) | |
| } else { | |
| addr.to_string() | |
| }; | |
| Cell::from(truncated).style(Style::default().fg(colors::TEXT_ACCENT)) | |
| } |
| match app.state { | ||
| AppState::Home => home::draw(f, app, chunks[1]), | ||
| AppState::Wallet => wallet::draw(f, app, chunks[1]), | ||
| AppState::Stake => stake::draw(f, app, chunks[1]), | ||
| AppState::Subnet => subnet::draw(f, app, chunks[1]), | ||
| AppState::Metagraph => metagraph::draw(f, app, chunks[1]), | ||
| AppState::Transfer => transfer::draw(f, app, chunks[1]), | ||
| AppState::Weights => weights::draw(f, app, chunks[1]), | ||
| AppState::Root => root::draw(f, app, chunks[1]), | ||
| } |
There was a problem hiding this comment.
Async/sync mismatch with wallet view.
The dispatcher calls wallet::draw(f, app, chunks[1]) synchronously, but based on the relevant code snippet from wallet.rs, its draw function is declared as async fn. This will cause a compilation error since async functions return a Future that must be awaited.
Either make the wallet draw function synchronous (recommended for consistency with other views), or make this dispatcher async and await the wallet draw call.
🔎 Verification script
#!/bin/bash
# Check if wallet::draw is async
rg -n 'pub\s+(async\s+)?fn\s+draw' lightning-tensor/src/tui/views/wallet.rs -A 1| for (i, subnet) in sorted_subnets.iter().take(display_count).enumerate() { | ||
| if subnet.netuid == 0 { | ||
| continue; // Skip root | ||
| } | ||
|
|
||
| let emission_pct = if total_emission > 0 { | ||
| (subnet.tao_in_emission as f64 / total_emission as f64) * 100.0 | ||
| } else { | ||
| 0.0 | ||
| }; | ||
|
|
||
| let bar = GradientProgress::new(emission_pct, 15).render(); | ||
| let is_last = i == display_count.saturating_sub(1); | ||
| let branch = if is_last { | ||
| symbols::BRANCH_END | ||
| } else { | ||
| symbols::BRANCH | ||
| }; |
There was a problem hiding this comment.
Potential incorrect branch symbol when root subnet is skipped.
The loop uses enumerate() before filtering out netuid == 0, so i counts all subnets including root. However, is_last compares against display_count - 1, which doesn't account for the skipped root subnet.
If root is in the first position of sorted_subnets, i will be off by one for all subsequent subnets, potentially showing the wrong branch symbol.
🔎 Suggested fix
- for (i, subnet) in sorted_subnets.iter().take(display_count).enumerate() {
- if subnet.netuid == 0 {
- continue; // Skip root
- }
-
- let emission_pct = if total_emission > 0 {
- (subnet.tao_in_emission as f64 / total_emission as f64) * 100.0
- } else {
- 0.0
- };
-
- let bar = GradientProgress::new(emission_pct, 15).render();
- let is_last = i == display_count.saturating_sub(1);
+ let non_root_subnets: Vec<_> = sorted_subnets
+ .iter()
+ .filter(|s| s.netuid != 0)
+ .take(display_count)
+ .collect();
+
+ for (i, subnet) in non_root_subnets.iter().enumerate() {
+ let emission_pct = if total_emission > 0 {
+ (subnet.tao_in_emission as f64 / total_emission as f64) * 100.0
+ } else {
+ 0.0
+ };
+
+ let bar = GradientProgress::new(emission_pct, 15).render();
+ let is_last = i == non_root_subnets.len().saturating_sub(1);🤖 Prompt for AI Agents
In lightning-tensor/src/tui/views/root.rs around lines 165 to 182, the
enumerator runs before skipping subnets with netuid == 0 so the index i includes
skipped entries and causes incorrect is_last/branch selection; fix by
enumerating only the visible subnets (e.g., filter out netuid == 0 first and
then .take(display_count).enumerate()) or maintain a separate counter that
increments only for non-root subnets and use that counter for the is_last
comparison so branch selection is based on the filtered list.
| fn draw_transfer_form(f: &mut Frame, app: &App, area: Rect) { | ||
| let form_chunks = Layout::default() | ||
| .direction(Direction::Vertical) | ||
| .margin(1) | ||
| .constraints([ | ||
| Constraint::Length(2), // Destination | ||
| Constraint::Length(2), // Amount | ||
| Constraint::Length(2), // Action hint | ||
| ]) | ||
| .split(area); | ||
|
|
||
| // Destination field | ||
| let dest_display = if app.transfer_dest.is_empty() { | ||
| Span::styled( | ||
| "Press [d] to enter destination address", | ||
| Style::default().fg(colors::TEXT_TERTIARY), | ||
| ) | ||
| } else { | ||
| Span::styled(&app.transfer_dest, Style::default().fg(colors::TEXT_ACCENT)) | ||
| }; | ||
|
|
||
| let dest_line = Line::from(vec![ | ||
| Span::styled( | ||
| format!("{} To: ", symbols::ARROW_RIGHT), | ||
| Style::default() | ||
| .fg(colors::LIGHTNING) | ||
| .add_modifier(Modifier::BOLD), | ||
| ), | ||
| dest_display, | ||
| ]); | ||
|
|
||
| f.render_widget(Paragraph::new(dest_line), form_chunks[0]); | ||
|
|
||
| // Amount field | ||
| let amount_display = if app.transfer_amount.is_empty() { | ||
| Span::styled( | ||
| "Press [a] to enter amount", | ||
| Style::default().fg(colors::TEXT_TERTIARY), | ||
| ) | ||
| } else { | ||
| Span::styled( | ||
| format!("{} {}", app.transfer_amount, symbols::TAO), | ||
| Style::default().fg(colors::TAO), | ||
| ) | ||
| }; | ||
|
|
||
| let amount_line = Line::from(vec![ | ||
| Span::styled( | ||
| format!("{} Amount: ", symbols::DIAMOND), | ||
| Style::default() | ||
| .fg(colors::VOLT) | ||
| .add_modifier(Modifier::BOLD), | ||
| ), | ||
| amount_display, | ||
| ]); | ||
|
|
||
| f.render_widget(Paragraph::new(amount_line), form_chunks[1]); | ||
|
|
||
| // Action hint | ||
| let can_transfer = !app.transfer_dest.is_empty() | ||
| && !app.transfer_amount.is_empty() | ||
| && app.selected_wallet.is_some(); | ||
|
|
||
| let action_hint = if can_transfer { | ||
| Line::from(vec![ | ||
| Span::styled( | ||
| format!("{} ", symbols::SUCCESS), | ||
| Style::default().fg(colors::SUCCESS), | ||
| ), | ||
| Span::styled( | ||
| "Press [Enter] to confirm transfer", | ||
| Style::default().fg(colors::SUCCESS), | ||
| ), | ||
| ]) | ||
| } else { | ||
| Line::from(vec![Span::styled( | ||
| "Complete all fields to transfer", | ||
| Style::default().fg(colors::TEXT_TERTIARY), | ||
| )]) | ||
| }; | ||
|
|
||
| f.render_widget(Paragraph::new(action_hint), form_chunks[2]); | ||
|
|
||
| // Form container | ||
| let form_block = Block::default() | ||
| .borders(Borders::ALL) | ||
| .border_type(ratatui::widgets::BorderType::Rounded) | ||
| .border_style(Style::default().fg(colors::LIGHTNING_DIM)) | ||
| .title(Span::styled( | ||
| format!(" {} Transfer Details ", symbols::DIAMOND), | ||
| Style::default() | ||
| .fg(colors::VOLT) | ||
| .add_modifier(Modifier::BOLD), | ||
| )) | ||
| .style(Style::default().bg(colors::BG_DEEP)); | ||
|
|
||
| f.render_widget(form_block, area); | ||
| } |
There was a problem hiding this comment.
Form block overwrites field content.
The form fields are rendered to form_chunks[0..2] (lines 128, 153, 178), but then form_block is rendered to the entire area (line 193), which will overwrite the previously rendered fields.
🔎 Suggested fix
Render the block first before the inner content, or render fields to the block's inner area:
fn draw_transfer_form(f: &mut Frame, app: &App, area: Rect) {
+ // Form container - render first
+ let form_block = Block::default()
+ .borders(Borders::ALL)
+ .border_type(ratatui::widgets::BorderType::Rounded)
+ .border_style(Style::default().fg(colors::LIGHTNING_DIM))
+ .title(Span::styled(
+ format!(" {} Transfer Details ", symbols::DIAMOND),
+ Style::default()
+ .fg(colors::VOLT)
+ .add_modifier(Modifier::BOLD),
+ ))
+ .style(Style::default().bg(colors::BG_DEEP));
+
+ let inner_area = form_block.inner(area);
+ f.render_widget(form_block, area);
+
let form_chunks = Layout::default()
.direction(Direction::Vertical)
.margin(1)
.constraints([
Constraint::Length(2), // Destination
Constraint::Length(2), // Amount
Constraint::Length(2), // Action hint
])
- .split(area);
+ .split(inner_area);
// ... rest of field rendering ...
- // Form container
- let form_block = Block::default()
- // ...
- f.render_widget(form_block, area);🤖 Prompt for AI Agents
In lightning-tensor/src/tui/views/transfer.rs around lines 97 to 194 the form
Block is rendered after the field widgets so it draws over them; either render
the form_block first (f.render_widget(form_block, area)) and then render the
dest/amount/action widgets into its inner area, or compute let inner =
form_block.inner(area) (or use block.inner(area) from ratatui) and render the
three field widgets into form_chunks derived from inner before finally rendering
the block; adjust margins/constraints to use inner so borders do not overlap
field content.
| fn truncate(s: &str, len: usize) -> String { | ||
| if s.len() <= len { | ||
| s.to_string() | ||
| } else { | ||
| format!("{}…", &s[..len - 1]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix potential panic on multi-byte UTF-8 characters.
The string slice &s[..len - 1] on Line 317 uses byte indexing, which will panic if it falls in the middle of a multi-byte UTF-8 character. Use character-boundary-aware slicing or a safer approach.
🔎 Suggested fix
fn truncate(s: &str, len: usize) -> String {
if s.len() <= len {
s.to_string()
} else {
- format!("{}…", &s[..len - 1])
+ let truncated: String = s.chars().take(len - 1).collect();
+ format!("{}…", truncated)
}
}🤖 Prompt for AI Agents
In lightning-tensor/src/tui/views/wallet.rs around lines 313 to 319, the current
byte-based slice &s[..len - 1] can panic on multi-byte UTF-8 characters; change
to a character-boundary-safe implementation: count characters with
s.chars().count(), return s.to_string() if chars <= len, otherwise collect the
first (len.saturating_sub(1)) chars with s.chars().take(n).collect::<String>()
and append '…'; also handle len == 0 or len == 1 safely (e.g., return "" for len
== 0, return "…" for len == 1).
| Style::default().fg(colors::TEXT_SECONDARY), | ||
| ), | ||
| Span::raw(" "), | ||
| Span::styled(&neuron.hotkey, Style::default().fg(colors::TEXT_TERTIARY)), |
There was a problem hiding this comment.
Hotkey displayed without truncation may overflow.
The full neuron.hotkey is rendered without truncation. SS58 addresses are ~48 characters and may overflow the display area.
🔎 Suggested fix using truncation
- Span::styled(&neuron.hotkey, Style::default().fg(colors::TEXT_TERTIARY)),
+ Span::styled(truncate_key(&neuron.hotkey, 16), Style::default().fg(colors::TEXT_TERTIARY)),Add a truncation helper similar to the one in metagraph.rs.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lightning-tensor/src/tui/views/weights.rs around line 168, the full
neuron.hotkey is rendered and can overflow (SS58 ~48 chars); add or reuse a
truncation helper (like the one in metagraph.rs) and apply it to neuron.hotkey
before creating the Span so long keys are shortened (e.g., show prefix+…+suffix
or truncate with ellipsis to fit available width). Import or implement the
helper in this file, call it where Span::styled currently uses &neuron.hotkey,
and ensure the displayed string is the truncated result.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.