Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Removing bitcoin_conf_path left several tests in settings.rs with no remaining assertions - some passed trivially with zero checks. Rebuilt using p2pool_conf_path as the differentiating field: - load_settings_returns_default_when_file_missing/for_invalid_toml: restored default-field assertions - load_settings_reads_valid_file: was writing/loading/asserting nothing; now round-trips a real p2pool_conf_path value - save_with_override_writes_to_override_dir_and_default: assert override and default copies match - load_settings_reads_from_override_dir_when_set / load_settings_falls_back_to_default_when_override_unreadable: pointer and authoritative Settings were identical, so nothing could prove which one loaded - gave each a distinct p2pool_conf_path and assert the correct one wins
There was a problem hiding this comment.
Pull request overview
Refocuses PDM on P2Poolv2 by removing Bitcoin Core configuration management while retaining Bitcoin chain and peer visibility through P2Poolv2 RPC settings.
Changes:
- Removes the Bitcoin configuration editor, state, persistence, and navigation.
- Builds the Bitcoin RPC client from P2Poolv2 configuration.
- Updates settings, tests, fixtures, and UI snapshots.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/snapshots/ui_snapshots__home_screen_render.snap |
Updates the sidebar snapshot. |
tests/snapshots/ui_snapshots__config_screen_render.snap |
Reflects the Bitcoin status view. |
tests/fixtures/p2pool.toml |
Adds a P2Poolv2 test configuration. |
src/ui.rs |
Removes Bitcoin config rendering and updates navigation tests. |
src/snapshots/pdm__ui__tests__shares_market_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__settings_screen_render.snap |
Removes the Bitcoin settings field. |
src/snapshots/pdm__ui__tests__p2pool_status_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__p2pool_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__p2pool_config_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__ln_status_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__ln_config_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__ui__tests__home_screen_render.snap |
Updates the home screen sidebar. |
src/snapshots/pdm__ui__tests__bitcoin_status_tab_system_render.snap |
Reflects the reduced Bitcoin tabs. |
src/snapshots/pdm__ui__tests__bitcoin_status_tab_peers_render.snap |
Updates the peers snapshot. |
src/snapshots/pdm__ui__tests__bitcoin_status_tab_logs_render.snap |
Reflects the reduced Bitcoin tabs. |
src/snapshots/pdm__ui__tests__bitcoin_status_screen_render.snap |
Updates Bitcoin status messaging. |
src/snapshots/pdm__ui__tests__bitcoin_screen_render.snap |
Updates sidebar layout. |
src/snapshots/pdm__tests__menu_toggled.snap |
Updates menu navigation output. |
src/snapshots/pdm__tests__home_screen.snap |
Updates the home screen sidebar. |
src/settings.rs |
Removes persisted Bitcoin config paths. |
src/p2poolv2_config.rs |
Updates configuration documentation. |
src/main.rs |
Removes Bitcoin config actions and remaps settings fields. |
src/lib.rs |
Removes the Bitcoin config module export. |
src/components/status_bar.rs |
Removes Bitcoin editor hints and remaps settings. |
src/components/settings_view.rs |
Removes the Bitcoin config setting. |
src/components/mod.rs |
Removes the Bitcoin config view module. |
src/components/bitcoin_status_view.rs |
Retains chain and peer tabs using P2Poolv2 state. |
src/components/bitcoin_config_view.rs |
Deletes the Bitcoin configuration editor. |
src/components/bitcoin_client.rs |
Sources RPC endpoint and credentials from P2Poolv2. |
src/bitcoin_config.rs |
Deletes Bitcoin configuration parsing and writing. |
src/app.rs |
Removes Bitcoin config state and uses P2Poolv2 for RPC requests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| app.sidebar_index = 2; | ||
| app.sidebar_index = 1; | ||
| app.toggle_menu(); | ||
| app.bitcoin_status_tab = 3; |
| } | ||
| }, | ||
| 1 => match P2PoolConfig::load(path.to_str().unwrap_or_default()) { | ||
| 0 => match P2PoolConfig::load(path.to_str().unwrap_or_default()) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR?
PDM is intended to manage and provide visibility into P2Poolv2, rather than manage the Bitcoin Core node itself.
The previous implementation included Bitcoin Core configuration management through a bitcoin.conf file. After discussing the intended scope, we've decided that this adds unnecessary responsibility to PDM and overlaps with Bitcoin Core/node management that should remain with the user.
Instead, PDM will use the existing Bitcoin RPC configuration from the P2Poolv2 configuration to provide the Bitcoin information needed by the P2Poolv2 TUI.
What this changes
Removes the Bitcoin Core configuration editor and bitcoin.conf selection/editing flow.
Uses the existing P2Poolv2 [bitcoinrpc] configuration for Bitcoin RPC connectivity.
Keeps Bitcoin Chain Info functionality.
Keeps Bitcoin Peer information functionality.
Removes Bitcoin-specific configuration code that is no longer needed.
Does not change Bitcoin Core itself or take ownership of its lifecycle.
This keeps PDM focused on its main purpose while still providing the Bitcoin information required by P2Poolv2.