Add bitcoin config parsing by framicheli · Pull Request #4 · p2poolv2/pdm

framicheli · 2025-12-15T10:49:40Z

No description provided.

pool2win

Nice work! I left some comments which we will need to address though.

let's remove eyre as a dep
let's use config-rs for parsing config else we will end up writing a bunch of code to parse configs.

pool2win · 2025-12-15T11:02:53Z

src/utils.rs

@@ -0,0 +1,13 @@
+use std::path::PathBuf;


Missing and copyright identifier

pool2win · 2025-12-15T11:04:10Z

src/utils.rs

@@ -0,0 +1,13 @@
+use std::path::PathBuf;
+


We should read the bitcoin config from the TUI. In the wireframes we provide a screen to let user choose the config files. Were you planning to switch to it once we got that in place?

Yes I'll remove it in order to let the user select it

pool2win · 2025-12-15T11:06:05Z

src/config.rs

+    ]
+}
+
+pub fn parse_config(path: &Path) -> Result<Vec<ConfigEntry>> {


Let's add a bitcoin.conf.sample to the repo and add a unit test to read that config file and assert some of the config options. I think @R27-pixel 's other PR (pending review and merge) has CI support in now, so we can start building on it. I think you will need to setup the a non insta-rs based testing framework to test things.

pool2win · 2025-12-15T11:07:39Z

Cargo.toml


 [dependencies]
 anyhow = "1.0.100"
+color-eyre = "0.6.5"


Why this dependency? I think we already included anyhow and I wonder if we will create confusion adding a dependency of eyre?

In the ratatui docs it's used color_eyre

pool2win · 2025-12-15T11:08:45Z

src/config.rs

@@ -0,0 +1,371 @@
+use color_eyre::Result;


Missing license and copyright notices. See other files, we are using SPDX identifiers

src/config.rs

pool2win · 2025-12-27T07:03:09Z

@framicheli we need to rebase on to main. There's a bunch of conflicts we need to resolve.

pool2win

Looking good but a few problems we need to address.

Add comments to structs and functions. All of them.
Add unit test to the config.rs module.
Separate out all UI changes to a separate PR. This one should be limited to implement config.rs and testing it thoroughly. I can understand the temptation to run using the tui and see it working, but you can push it in a separate PR which comes after this PR. Otherwise it is too much to review and will result in an error during the review process.

pool2win · 2025-12-27T14:29:20Z

src/app.rs

@@ -2,23 +2,168 @@
 //


Add comments for all structs and functions.

pool2win · 2025-12-27T14:31:59Z

src/app.rs

+        let i = match self.list_state.selected() {
+            Some(i) => {
+                if i >= self.files.len() - 1 {
+                    0
+                } else {
+                    i + 1
+                }
+            }
+            None => 0,
+        };


I think we can replace this condition with saturating_sub

pool2win · 2025-12-27T14:32:14Z

src/app.rs

+        let i = match self.list_state.selected() {
+            Some(i) => {
+                if i == 0 {
+                    self.files.len() - 1
+                } else {
+                    i - 1
+                }
+            }
+            None => 0,
+        };


Replace with saturating_sub

pool2win · 2025-12-27T14:32:32Z

src/app.rs

        }
    }
+
+    pub fn load_config(&mut self, path: &str) -> Result<()> {


Add comment for all functions and structs

pool2win · 2025-12-27T14:42:15Z

src/config.rs

@@ -0,0 +1,448 @@
+// SPDX-FileCopyrightText: 2024 PDM Authors


This module is doing a lot of heavy lifting, and is important to get right. We should add tests to this module to make sure we can parse a sample config file with default and non default values for all values.

Add bitcoin config parsing

d4c850f

pool2win reviewed Dec 15, 2025

View reviewed changes

framicheli and others added 15 commits December 16, 2025 12:04

Update bitcoin config parsing

077e25f

WIP - Update bitcoin config parsing

4f06d0f

Add insta snapshot tests for TUI

74c29a4

Add github CI

29e19cf

Fix github workflow file inconsistencies

73d1b32

Add just file

02c624f

Add codecov

257bda6

Ignore clippy warnings

8b89e07

Specify rustfmt edition 2024

48e72d3

Add README

8af594f

smoke tes for main.rs

0427237

Add bitcoin config parsing

0c307c0

WIP - Update bitcoin config parsing

ce4dd4a

Add bitcoin config editing

ecff08a

Add Bitcoin config load, edit and save

07603a0

Merge branch 'main' into bitcoin_conf

87d0fc9

pool2win requested changes Dec 27, 2025

View reviewed changes

		@@ -0,0 +1,448 @@
		// SPDX-FileCopyrightText: 2024 PDM Authors

Conversation

framicheli commented Dec 15, 2025

Uh oh!

pool2win left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pool2win commented Dec 27, 2025

Uh oh!

pool2win left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants