Added validate_domain in electrum option by Rahamath-unnisa · Pull Request #223 · bitcoindevkit/bdk-cli

Rahamath-unnisa · 2025-10-20T15:12:27Z

Description:
This PR adds the validate_domain option to the Electrum client in bdk-cli.
This resolves #134 .
Changes included:

Added --validate-domain flag in command.rs.

Updated Electrum client handling in handlers.rs and utils.rs to respect the flag.

Verified functionality locally using:

RUST_LOG=debug,rusqlite=info,rustls=info cargo run --features electrum -- wallet --client-type electrum --database-type sqlite --url ssl://electrum.blockstream.info:60002 --validate-domain true --ext-descriptor "wpkh(.../*)" sync

Motivation:
Allows users to enable or disable domain validation for Electrum servers, useful for self-hosted/custom servers while preserving security by default.

Checklist:

Tested locally with Electrum client

No unrelated changes included

va-an · 2025-10-21T08:53:54Z

@Rahamath-unnisa pls mention in PR description that this resolves #134.

110CodingP · 2025-10-21T09:46:49Z

src/commands.rs


-#![allow(clippy::large_enum_variant)]
+
+#[allow(clippy::large_enum_variant)]


ig this is not required.

110CodingP · 2025-10-21T10:20:39Z

src/handlers.rs

 //!
 //! This module describes all the command handling logic used by bdk-cli.
-
+ use crate::debug;


ig this is not needed either.

110CodingP · 2025-10-21T10:22:35Z

src/commands.rs

    },
    /// Syncs with the chosen blockchain server.
-    Sync,
-    /// Broadcasts a transaction to the network. Takes either a raw transaction or a PSBT to extract.


This doc comment should not be removed.

110CodingP · 2025-10-21T10:30:29Z

src/handlers.rs

                        .populate_tx_cache(wallet.tx_graph().full_txs().map(|tx_node| tx_node.tx));

-                    let update = client.sync(request, batch_size, false)?;
+                    let update = client.sync(request, batch_size, validate_domain)?;


ig this is not the right place to pass validate_domain looking at the defn here. I think we need to change how the client is built using this.

Rahamath-unnisa · 2025-10-23T11:02:02Z

Hi @110CodingP,
I have made the requested cleanups in commands.rs and handlers.rs per your review comments
All changes are pushed to the validate_domain-config branch.
Please let me know if anything else is needed. Thanks!

tvpeter

Hi @Rahamath-unnisa,
Thank you for the eagerness to contribute, but please understand the project fully before attempting to do so as it is easier to make progress that way.
Thank you.

tvpeter · 2025-11-03T10:05:16Z

Cargo.toml

 shlex = {  version = "1.3.0", optional = true }
 tracing = "0.1.41"
 tracing-subscriber = "0.3.20"
+electrum-client = "0.24.0"


bdk_electrum is built on this crate, so no need pulling it here.

tvpeter · 2025-11-03T10:06:01Z

src/commands.rs

    #[cfg(feature = "electrum")]
    #[arg(env = "ELECTRUM_BATCH_SIZE", short = 'b', long, default_value = "10")]
    pub batch_size: usize,
+    ///Electrum validate domain option.


this comment is not needed

tvpeter · 2025-11-03T10:06:37Z

src/commands.rs

+    ///Electrum validate domain option.
+    #[cfg(feature = "electrum")]
+    #[arg(env="VALIDATE_DOMAIN",long = "validate-domain", action = clap::ArgAction::Set, default_value_t = true)]
+    pub validate_domain: bool,


a user do not need to specifically indicate whether the domain should be validated

sorry if I misunderstood but doesn't the issue want us to provide validate_domain as an option? Because otherwise config.validate_domain is true by default anyways.

va-an

Could you rebase and squash the commits into one, please?

Rahamath-unnisa · 2026-03-28T12:28:12Z

Hi @tvpeter, @110CodingP, @va-an,

I’ve updated the PR based on your feedback:

Removed CLI option for validate_domain
Enabled validation internally
Cleaned up code and dependencies
Rebased and resolved conflicts

Please review again. Thanks!

va-an

Make sure you've done the rebase correctly.
Also, I'll repeat my request to squash into a single commit.

va-an · 2026-03-28T21:03:56Z

Cargo.toml

What's the idea behind these changes?
Moreover, because of these changes, Cargo.toml has become invalid:

-> % just pre-push error: failed to parse manifest at `/home/vaan/src/bitcoindevkit/bdk-cli-tmp/Cargo.toml` Caused by: feature `_payjoin-dependencies` includes `payjoin` which is neither a dependency nor another feature error: Recipe `pre-push` failed on line 29 with exit code 101

va-an · 2026-03-28T21:05:53Z

src/handlers.rs

+<<<<<<< HEAD
+<<<<<<< HEAD
+=======


Suggested change

<<<<<<< HEAD

<<<<<<< HEAD

=======

va-an · 2026-03-28T21:06:10Z

src/handlers.rs

+>>>>>>> 7931dbd (Added validate_domain in electrum option)
+=======
+>>>>>>> a2f5954 (Clean up commands.rs and handlers.rs per review comments)


Suggested change

>>>>>>> 7931dbd (Added validate_domain in electrum option)

=======

>>>>>>> a2f5954 (Clean up commands.rs and handlers.rs per review comments)

va-an · 2026-03-28T21:06:27Z

src/utils.rs

+use std::fmt::Display;
+use std::path::{Path, PathBuf};
+use std::str::FromStr;
+>>>>>>> 80e132f (Added validate-domain option config in utils.rs and made changes required to that.)


Suggested change

>>>>>>> 80e132f (Added validate-domain option config in utils.rs and made changes required to that.)

va-an

Also, please fill out the PR template provided in this repo - it includes checklists that help catch issues like, you know, the code not actually compiling.

Rahamath-unnisa · 2026-03-29T15:04:31Z

Hi @va-an,

Thanks for your feedback.

I’ve now:

Squashed all changes into a single commit
Fixed Cargo.toml by restoring upstream version
Removed leftover conflict markers and cleaned up code

Please review again. Thanks!

github-project-automation bot added this to BDK-CLI Oct 20, 2025

110CodingP reviewed Oct 21, 2025

View reviewed changes

110CodingP suggested changes Oct 21, 2025

View reviewed changes

Rahamath-unnisa force-pushed the validate_domain-config branch from d18f668 to 31a3fbf Compare October 30, 2025 13:25

Rahamath-unnisa requested a review from 110CodingP October 30, 2025 13:25

tvpeter requested changes Nov 3, 2025

View reviewed changes

Rahamath-unnisa requested a review from tvpeter November 4, 2025 16:56

va-an suggested changes Feb 22, 2026

View reviewed changes

tvpeter assigned Rahamath-unnisa Feb 23, 2026

Rahamath-unnisa force-pushed the validate_domain-config branch from 818a744 to c697136 Compare March 28, 2026 12:26

va-an suggested changes Mar 28, 2026

View reviewed changes

fix(electrum): always enable domain validation internally

546948e

Rahamath-unnisa force-pushed the validate_domain-config branch from 1fc13e4 to 546948e Compare March 29, 2026 15:02


		#![allow(clippy::large_enum_variant)]

		#[allow(clippy::large_enum_variant)]

	>>>>>>> 7931dbd (Added validate_domain in electrum option)
	=======
	>>>>>>> a2f5954 (Clean up commands.rs and handlers.rs per review comments)

Conversation

Rahamath-unnisa commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

va-an commented Oct 21, 2025

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!

Rahamath-unnisa commented Oct 23, 2025

Uh oh!

tvpeter 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!

va-an left a comment

Choose a reason for hiding this comment

Uh oh!

Rahamath-unnisa commented Mar 28, 2026

Uh oh!

va-an 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!

va-an left a comment

Choose a reason for hiding this comment

Uh oh!

Rahamath-unnisa commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Rahamath-unnisa commented Oct 20, 2025 •

edited

Loading