refactor: remove unused Insight API and show_in_ui config fields

[lklimek]

Removes unused insight_api_url and show_in_ui fields from NetworkConfig, along with related serialization, environment variable examples, and tests. Simplifies configuration initialization by eliminating dead settings.

Summary by CodeRabbit

• Chores
  • Removed Insight API configuration options from environment files and app network settings.
  • Simplified configuration initialization and related test setup by eliminating those settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change removes Insight API fields (insight_api_url, show_in_ui) from the configuration and environment examples, updates save/validation logic and tests to the reduced NetworkConfig shape, and adjusts related test environment setup and one import in the UI module.

Changes

Cohort / File(s)	Summary
Core configuration .env.example, src/config.rs	Deleted *_insight_api_url and *_show_in_ui environment keys and removed insight_api_url and show_in_ui fields from NetworkConfig; updated save/write and validation logic to omit these fields.
Test updates src/spv/tests.rs, src/ui/tokens/tokens_screen/mod.rs	Removed construction and environment setup references to the removed fields in test helpers and test environment variable initialization.
UI import tweak src/ui/wallets/wallets_screen/dialogs.rs	Adjusted imports to explicitly include Color32; no behavioral changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐰 I hopped through configs, tidy and spry,
Two fields dropped softly — out they fly.
Env lines cleared, tests hopped along,
A leaner layout, neat and strong.
✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: removing unused Insight API and show_in_ui configuration fields across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/remove-insight-api-VvsdM

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

src/config.rs (3)

80-133: Consider extracting repeated .map_err boilerplate in save().

Every writeln! call in the closure chains the same .map_err(|e| ConfigError::LoadError(e.to_string())). A small helper (or a From<std::io::Error> impl on ConfigError) would reduce noise and make future field additions less error-prone. Pre-existing pattern, so fine to defer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 80 - 133, The closure write_network_config inside
save() repeats .map_err(|e| ConfigError::LoadError(e.to_string())) on every
writeln!; extract that boilerplate by adding a small helper (e.g., a function
like map_io_err or write_env_line) or implement From<std::io::Error> for
ConfigError so you can simply use ? after writeln! calls; update the writeln!
invocations in write_network_config to call the helper or rely on the From impl
and ? to remove repeated .map_err(...) occurrences while keeping the same
ConfigError::LoadError semantics.

---

294-302: is_valid() is still marked #[allow(dead_code)] — verify if it's used now or plan to use it.

If the method is still unused in production code, consider either removing it (consistent with the cleanup spirit of this PR) or adding a concrete call site. If it's kept for future validation, the current annotation is fine.

#!/bin/bash
# Check if is_valid() is called anywhere outside tests
rg -n --type=rust 'is_valid\(\)' -g '!**/test*' -g '!**/mod.rs'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 294 - 302, The method NetworkConfig::is_valid()
is still annotated with #[allow(dead_code)]; either remove the unused method for
cleanup or wire it up where config validation occurs. Locate the impl
NetworkConfig::is_valid method and either delete it if not referenced, or remove
the attribute and add a call (e.g., in your configuration load/validation flow
such as the config loader/initializer function) to check and handle invalid
configs (logging/error-return) so the method is actually exercised.

---

533-566: Save-format test only covers mainnet — consider extending coverage.

The test_save_format_contains_expected_env_vars test builds a Config with both mainnet and testnet (line 539), but the simulated output block (lines 547–559) only formats and asserts the mainnet section. This means testnet env-var formatting is untested here. Consider adding assertions for TESTNET_ lines as well, or dropping the unused testnet_config from the test fixture to avoid giving a false sense of coverage.

This is pre-existing and not introduced by this PR, so feel free to defer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 533 - 566, The test
test_save_format_contains_expected_env_vars constructs a Config with
testnet_config but only formats and asserts MAINNET_ env lines, leaving TESTNET_
formatting untested; either remove the unused testnet_config from the Config
fixture or expand the simulated output/assertions to include TESTNET_ lines
(e.g., mirror the MAINNET formatting block and add asserts for
TESTNET_dapi_addresses, TESTNET_core_rpc_port, TESTNET_core_rpc_user,
TESTNET_core_rpc_password, and TESTNET_core_zmq_endpoint) so the test accurately
reflects coverage of the Config save-format logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/config.rs`:
- Around line 80-133: The closure write_network_config inside save() repeats
.map_err(|e| ConfigError::LoadError(e.to_string())) on every writeln!; extract
that boilerplate by adding a small helper (e.g., a function like map_io_err or
write_env_line) or implement From<std::io::Error> for ConfigError so you can
simply use ? after writeln! calls; update the writeln! invocations in
write_network_config to call the helper or rely on the From impl and ? to remove
repeated .map_err(...) occurrences while keeping the same ConfigError::LoadError
semantics.
- Around line 294-302: The method NetworkConfig::is_valid() is still annotated
with #[allow(dead_code)]; either remove the unused method for cleanup or wire it
up where config validation occurs. Locate the impl NetworkConfig::is_valid
method and either delete it if not referenced, or remove the attribute and add a
call (e.g., in your configuration load/validation flow such as the config
loader/initializer function) to check and handle invalid configs
(logging/error-return) so the method is actually exercised.
- Around line 533-566: The test test_save_format_contains_expected_env_vars
constructs a Config with testnet_config but only formats and asserts MAINNET_
env lines, leaving TESTNET_ formatting untested; either remove the unused
testnet_config from the Config fixture or expand the simulated output/assertions
to include TESTNET_ lines (e.g., mirror the MAINNET formatting block and add
asserts for TESTNET_dapi_addresses, TESTNET_core_rpc_port,
TESTNET_core_rpc_user, TESTNET_core_rpc_password, and TESTNET_core_zmq_endpoint)
so the test accurately reflects coverage of the Config save-format logic.

[coderabbitai]

🧹 Nitpick comments (1)

src/ui/wallets/wallets_screen/dialogs.rs (1)

19-19: Hardcoded Color32::from_rgb is inconsistent with the DashColors theme used everywhere else in this file.

The import is needed because line 171 uses Color32::from_rgb(255, 100, 100) directly. Every other error/status colour in this file is sourced from DashColors (e.g. DashColors::ERROR on line 196, DashColors::DANGER_RED on line 738, DashColors::error_color(dark_mode) on line 925). Additionally, render_send_dialog is the only render method that never reads dark_mode, so the hardcoded value also ignores theme awareness.

♻️ Proposed fix — align with `DashColors` and add `dark_mode` awareness

 use egui::{Color32, Frame, Margin, RichText, TextureOptions};

The import of Color32 would then no longer be needed at all for this path:

-use egui::{Color32, Frame, Margin, RichText, TextureOptions};
+use egui::{Frame, Margin, RichText, TextureOptions};

In render_send_dialog, add dark_mode near the top of the closure (same pattern as the other dialogs) and replace the hardcoded colour:

+        let dark_mode = ctx.style().visuals.dark_mode;
         // ...
         if let Some(error) = &self.send_dialog.address_error {
-            ui.colored_label(Color32::from_rgb(255, 100, 100), error);
+            ui.colored_label(DashColors::error_color(dark_mode), error);
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/wallets/wallets_screen/dialogs.rs` at line 19, The render_send_dialog
function uses a hardcoded Color32::from_rgb(255,100,100) and doesn't read theme;
update render_send_dialog to determine dark_mode (same pattern used in other
render_* dialogs) and replace the hardcoded color with
DashColors::error_color(dark_mode) (or DashColors::ERROR/DANGER_RED if a
non-theme variant is preferred) so the error/status colour respects the
DashColors theme; after this change remove the now-unused Color32 import from
the use list.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/ui/wallets/wallets_screen/dialogs.rs`:
- Line 19: The render_send_dialog function uses a hardcoded
Color32::from_rgb(255,100,100) and doesn't read theme; update render_send_dialog
to determine dark_mode (same pattern used in other render_* dialogs) and replace
the hardcoded color with DashColors::error_color(dark_mode) (or
DashColors::ERROR/DANGER_RED if a non-theme variant is preferred) so the
error/status colour respects the DashColors theme; after this change remove the
now-unused Color32 import from the use list.