Added validate_domain in electrum option#223
Added validate_domain in electrum option#223Rahamath-unnisa wants to merge 5 commits intobitcoindevkit:masterfrom
Conversation
|
@Rahamath-unnisa pls mention in PR description that this resolves #134. |
src/commands.rs
Outdated
|
|
||
| #![allow(clippy::large_enum_variant)] | ||
|
|
||
| #[allow(clippy::large_enum_variant)] |
There was a problem hiding this comment.
ig this is not required.
src/handlers.rs
Outdated
| //! | ||
| //! This module describes all the command handling logic used by bdk-cli. | ||
|
|
||
| use crate::debug; |
There was a problem hiding this comment.
ig this is not needed either.
| }, | ||
| /// Syncs with the chosen blockchain server. | ||
| Sync, | ||
| /// Broadcasts a transaction to the network. Takes either a raw transaction or a PSBT to extract. |
There was a problem hiding this comment.
This doc comment should not be removed.
src/handlers.rs
Outdated
| .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)?; |
|
Hi @110CodingP, |
Rahamath-unnisa
force-pushed
the
validate_domain-config
branch
from
d18f668 to
31a3fbf
Compare
tvpeter
left a comment
tvpeter
left a comment
There was a problem hiding this comment.
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.
Cargo.toml
Outdated
| shlex = { version = "1.3.0", optional = true } | ||
| tracing = "0.1.41" | ||
| tracing-subscriber = "0.3.20" | ||
| electrum-client = "0.24.0" |
There was a problem hiding this comment.
bdk_electrum is built on this crate, so no need pulling it here.
src/commands.rs
Outdated
| #[cfg(feature = "electrum")] | ||
| #[arg(env = "ELECTRUM_BATCH_SIZE", short = 'b', long, default_value = "10")] | ||
| pub batch_size: usize, | ||
| ///Electrum validate domain option. |
There was a problem hiding this comment.
this comment is not needed
src/commands.rs
Outdated
| ///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, |
There was a problem hiding this comment.
a user do not need to specifically indicate whether the domain should be validated
There was a problem hiding this comment.
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.
Signed-off-by: Rahamath-Unnisa <rahamathunnisa287@gmail.com>
4 participants
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