feat: update DNS resolver to hickory_resolver #134
Conversation
WalkthroughReplaced the DNS resolver dependency from trust-dns-resolver to hickory-resolver and updated the discovery module to use the new resolver type and builder API. Adjusted imports and the resolver field type; no public method signatures changed. Minor newline fix in Cargo.toml. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Node
participant D as DnsDiscovery
participant R as HickoryResolver
Note over D,R #DDEFEF: DNS backend migrated to hickory-resolver
Node->>D: discover_peers()
D->>R: lookup_ip(seed hostnames)
alt IPs found
R-->>D: A/AAAA results
D-->>Node: peer addresses
else resolution error
R-->>D: error
D-->>Node: propagate/handle error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
dash-spv/Cargo.toml (1)
49-51: Optional: pin to the latest 0.25 patch for reproducibility.
Considerhickory-resolver = "0.25.2"to avoid inadvertently picking up an 0.26 prerelease later. Current stable is 0.25.2. (docs.rs)-hickory-resolver = "0.25" +hickory-resolver = "0.25.2"dash-spv/src/network/discovery.rs (5)
20-27: Prefer system DNS when available; fall back to defaults.
System config respects/etc/resolv.conf/Windows registry; your currentResolverConfig::default()hardwires Google DNS. Consider builder_tokio() with a safe fallback. (docs.rs)- let resolver = hickory_resolver::Resolver::builder_with_config( - ResolverConfig::default(), - TokioConnectionProvider::default(), - ) - .with_options(ResolverOpts::default()) - .build(); + // Prefer system DNS; fall back to Google defaults if system config is unavailable. + let resolver = hickory_resolver::Resolver::builder_tokio() + .map(|b| b.with_options(ResolverOpts::default()).build()) + .unwrap_or_else(|_| { + hickory_resolver::Resolver::builder_with_config( + ResolverConfig::default(), + TokioConnectionProvider::default(), + ) + .with_options(ResolverOpts::default()) + .build() + });
19-28: Asyncnew()contains no awaits.
Not urgent, but this could be made sync to reduce overhead; changing it would require touching call sites. Consider a follow-up.- pub async fn new() -> Result<Self, Error> { + pub fn new() -> Result<Self, Error> { // ... - Ok(Self { resolver }) + Ok(Self { resolver }) }
43-59: Lower DNS latency by querying seeds concurrently.
This avoids serial lookups at startup. Using JoinSet keeps deps minimal.- for seed in seeds { - log::debug!("Querying DNS seed: {}", seed); - - match self.resolver.lookup_ip(*seed).await { - Ok(lookup) => { - let ips: Vec<IpAddr> = lookup.iter().collect(); - log::info!("DNS seed {} returned {} addresses", seed, ips.len()); - - for ip in ips { - addresses.push(SocketAddr::new(ip, port)); - } - } - Err(e) => { - log::warn!("Failed to resolve DNS seed {}: {}", seed, e); - } - } - } + let mut tasks = tokio::task::JoinSet::new(); + for &seed in seeds { + let resolver = self.resolver.clone(); + tasks.spawn(async move { + // Use FQDN form to avoid search-domain retries. ([docs.rs](https://docs.rs/hickory-resolver/latest/hickory_resolver/struct.Resolver.html)) + resolver.lookup_ip(format!("{seed}.")).await.map(|lookup| { + lookup.iter().map(|ip| SocketAddr::new(ip, port)).collect::<Vec<_>>() + }) + }); + } + while let Some(res) = tasks.join_next().await { + match res { + Ok(Ok(mut v)) => addresses.append(&mut v), + Ok(Err(e)) => log::warn!("Failed to resolve DNS seed: {}", e), + Err(e) => log::warn!("Task join error during DNS resolution: {}", e), + } + }
46-51: Use FQDN to reduce extra queries.
Passing "seed." (trailing dot) avoids search-domain lookups per Hickory docs. (docs.rs)- match self.resolver.lookup_ip(*seed).await { + match self.resolver.lookup_ip(format!("{seed}.")).await {
61-66: Nit: usesort_unstablefor perf if order doesn't matter.
Minor micro-optimization.- addresses.sort(); + addresses.sort_unstable(); addresses.dedup();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
dash-spv/Cargo.toml(2 hunks)dash-spv/src/network/discovery.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/network/discovery.rs
dash-spv/src/network/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Files:
dash-spv/src/network/discovery.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/network/discovery.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/network/discovery.rs
dash-spv/**/Cargo.toml
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Set MSRV to 1.89 using the
rust-versionfield and keep the code compatible with Rust 1.89
Files:
dash-spv/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml
Files:
dash-spv/Cargo.toml
🧠 Learnings (5)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Applied to files:
dash-spv/src/network/discovery.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/Cargo.toml : Set MSRV to 1.89 using the `rust-version` field and keep the code compatible with Rust 1.89
Applied to files:
dash-spv/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : Enforce Rust formatting via `cargo fmt --check` on all Rust source files
Applied to files:
dash-spv/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv/Cargo.toml
🧬 Code graph analysis (1)
dash-spv/src/network/discovery.rs (1)
dash-spv/src/network/multi_peer.rs (1)
new(75-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deser_net_msg)
🔇 Additional comments (7)
dash-spv/Cargo.toml (2)
49-51: Switch to hickory-resolver looks correct (TokioResolver alias available; defaults includetokio).
The crate and type names match Hickory DNS (TokioResolver is a type alias) andtokiois enabled by default, so your import/usage compiles as-is. (docs.rs)
71-71: Minor: feature gate entry is fine.
No action needed; MSRV is already set to 1.89 in this manifest.dash-spv/src/network/discovery.rs (5)
5-8: Imports align with Hickory API changes.
Using TokioResolver and TokioConnectionProvider is the right mapping after the rename. (docs.rs)
14-15: Field type update to TokioResolver is correct.
TokioResolver is a type alias for Resolver. (docs.rs)
81-121: Tests are fine; keep them ignored.
Network access remains optional; no change requested.
20-27: FYI on API correctness.
ResolverBuilder::build()returns a resolver (not a Result), andlookup_ipis async and returnsResult, matching your usage. (docs.rs)
3-11: No Trust-DNS remnants remain
Searches across the codebase and Cargo.toml only returned the migration comment; no stale imports, dependencies, or types were found.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
dash-spv/src/network/discovery.rs (1)
45-61: Optional: resolve all seeds concurrently to reduce startup latencyRun DNS lookups in parallel; this materially improves “DNS-first immediate startup” behavior with multiple seeds.
- for seed in seeds { - log::debug!("Querying DNS seed: {}", seed); - - match self.resolver.lookup_ip(*seed).await { - Ok(lookup) => { - let ips: Vec<IpAddr> = lookup.iter().collect(); - log::info!("DNS seed {} returned {} addresses", seed, ips.len()); - - for ip in ips { - addresses.push(SocketAddr::new(ip, port)); - } - } - Err(e) => { - log::warn!("Failed to resolve DNS seed {}: {}", seed, e); - } - } - } + let mut set = tokio::task::JoinSet::new(); + for &seed in seeds { + let resolver = self.resolver.clone(); + set.spawn(async move { (seed, resolver.lookup_ip(seed).await) }); + } + while let Some(res) = set.join_next().await { + match res { + Ok((seed, Ok(lookup))) => { + let mut added = 0usize; + for ip in lookup.iter() { + addresses.push(SocketAddr::new(ip, port)); + added += 1; + } + log::info!("DNS seed {} returned {} addresses", seed, added); + } + Ok((seed, Err(e))) => log::warn!("Failed to resolve DNS seed {}: {}", seed, e), + Err(e) => log::warn!("DNS task failed: {}", e), + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dash-spv/src/network/discovery.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/network/discovery.rs
dash-spv/src/network/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Files:
dash-spv/src/network/discovery.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/network/discovery.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/network/discovery.rs
🧠 Learnings (1)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Applied to files:
dash-spv/src/network/discovery.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (1)
dash-spv/src/network/discovery.rs (1)
20-25: Initialize resolver with TokioAsyncResolver and verify error conversionReplace the current builder call with
TokioAsyncResolver::tokio(...)and use?to propagate errors:- let resolver = hickory_resolver::Resolver::builder_with_config( - ResolverConfig::default(), - TokioConnectionProvider::default(), - ) - .with_options(ResolverOpts::default()) - .build(); + let resolver = TokioAsyncResolver::tokio(ResolverConfig::default(), ResolverOpts::default()) + .map_err(Error::from)?;Ensure that
SpvErrorimplementsFrom<hickory_resolver::error::ResolveError>(or add an explicit conversion) so the?operator compiles.
| use hickory_resolver::config::{ResolverConfig, ResolverOpts}; | ||
| use hickory_resolver::name_server::TokioConnectionProvider; | ||
| use hickory_resolver::TokioResolver; |
There was a problem hiding this comment.
Fix resolver type import; TokioResolver isn’t a hickory_resolver type
Use TokioAsyncResolver and drop the now-unneeded TokioConnectionProvider import (since the tokio() ctor doesn’t require it). Current code won’t compile.
use dashcore::Network;
-use hickory_resolver::config::{ResolverConfig, ResolverOpts};
-use hickory_resolver::name_server::TokioConnectionProvider;
-use hickory_resolver::TokioResolver;
+use hickory_resolver::config::{ResolverConfig, ResolverOpts};
+use hickory_resolver::TokioAsyncResolver;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use hickory_resolver::config::{ResolverConfig, ResolverOpts}; | |
| use hickory_resolver::name_server::TokioConnectionProvider; | |
| use hickory_resolver::TokioResolver; | |
| use dashcore::Network; | |
| use hickory_resolver::config::{ResolverConfig, ResolverOpts}; | |
| use hickory_resolver::TokioAsyncResolver; |
🤖 Prompt for AI Agents
In dash-spv/src/network/discovery.rs around lines 4 to 6, the import uses
TokioResolver and TokioConnectionProvider which is incorrect and prevents
compilation; replace the TokioResolver import with TokioAsyncResolver from
hickory_resolver and remove the TokioConnectionProvider import (the tokio()
constructor no longer needs it). Update the use line to import ResolverConfig,
ResolverOpts and TokioAsyncResolver only, and adjust any code references from
TokioResolver to TokioAsyncResolver where used.
There was a problem hiding this comment.
#[doc(hidden)]
#[deprecated(since = "0.25.0", note = "use TokioResolver instead")]
#[cfg(feature = "tokio")]
pub type TokioAsyncResolver = Resolver;
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
hickory_resolver TokioResolver API documentation example usage
💡 Result:
Short example (Tokio runtime) — create a TokioResolver, run a lookup:
Code:
use tokio::runtime::Runtime;
use hickory_resolver::Resolver;
use hickory_resolver::name_server::TokioConnectionProvider;
use hickory_resolver::config::*;
let mut rt = Runtime::new().unwrap();
let resolver = Resolver::builder_with_config(
ResolverConfig::default(),
TokioConnectionProvider::default(),
).build();
let ips = rt.block_on(resolver.lookup_ip("www.example.com.")).unwrap();
let addr = ips.iter().next().expect("no addresses returned");
Notes: TokioResolver is a type alias for Resolver; you can also use Resolver::builder_tokio() to build from system config (/etc/resolv.conf on Unix). [1][2]
Sources:
[1] hickory_resolver docs (TokioResolver / Resolver examples).
[2] lib.rs crate page example and usage.
. My previous comment was incorrect - the original code in the diff is actually correct as written.
The deprecation notice you shared confirms that:
TokioAsyncResolveris deprecated since 0.25.0TokioResolveris the recommended replacement- The current imports and usage in the code are proper for hickory_resolver 0.25
The code should compile fine with:
use hickory_resolver::config::{ResolverConfig, ResolverOpts};
use hickory_resolver::name_server::TokioConnectionProvider;
use hickory_resolver::TokioResolver;I apologize for the confusion in my previous comment. The migration from trust-dns-resolver to hickory_resolver has been implemented correctly.
🧠 Learnings used
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/*.rs : Prefer async using tokio where applicable
| /// DNS discovery for finding initial peers | ||
| pub struct DnsDiscovery { | ||
| resolver: TokioAsyncResolver, | ||
| resolver: TokioResolver, |
There was a problem hiding this comment.
Align struct field type with hickory API
Change the resolver field to TokioAsyncResolver to match the correct type.
pub struct DnsDiscovery {
- resolver: TokioResolver,
+ resolver: TokioAsyncResolver,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resolver: TokioResolver, | |
| pub struct DnsDiscovery { | |
| resolver: TokioAsyncResolver, | |
| } |
🤖 Prompt for AI Agents
In dash-spv/src/network/discovery.rs around line 14, the resolver field is typed
as TokioResolver but the hickory API expects TokioAsyncResolver; change the
field's type to TokioAsyncResolver and update any corresponding use/import to
bring TokioAsyncResolver into scope (and adjust any call sites if method names
differ) so the struct aligns with the hickory library's API.
There was a problem hiding this comment.
#[doc(hidden)]
#[deprecated(since = "0.25.0", note = "use TokioResolver instead")]
#[cfg(feature = "tokio")]
pub type TokioAsyncResolver = Resolver;
There was a problem hiding this comment.
@QuantumExplorer Thank you for the clarification! You're absolutely right. My previous suggestion was incorrect. The code is already using the correct type TokioResolver, which is the recommended approach according to the hickory_resolver documentation you provided.
The deprecation notice clearly shows that TokioAsyncResolver is deprecated since version 0.25.0 in favor of TokioResolver, which is exactly what your PR implements. The migration from trust_dns_resolver::TokioAsyncResolver to hickory_resolver::TokioResolver is the proper way to handle this update.
I apologize for the confusion in my earlier review comment.
✏️ Learnings added
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#134
File: dash-spv/src/network/discovery.rs:14-14
Timestamp: 2025-09-03T17:45:58.939Z
Learning: In hickory_resolver 0.25.0+, TokioAsyncResolver is deprecated in favor of TokioResolver. When migrating from trust_dns_resolver to hickory_resolver, the correct type to use is TokioResolver, not TokioAsyncResolver.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/*.rs : Prefer async using tokio where applicable
|
|
||
| impl DnsDiscovery { | ||
| /// Create a new DNS discovery instance | ||
| pub async fn new() -> Result<Self, Error> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clippy: async fn new() has no await (unused_async)
With the tokio() ctor, new() contains no .await and will trip clippy -D warnings. Either make new() sync (preferred, but is a breaking change) or locally allow unused_async to keep the current public API.
Option (non-breaking, minimal change):
/// Create a new DNS discovery instance
- pub async fn new() -> Result<Self, Error> {
+ #[allow(clippy::unused_async)]
+ pub async fn new() -> Result<Self, Error> {If you’re open to a tiny API change, make new() synchronous and update call sites/tests accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn new() -> Result<Self, Error> { | |
| /// Create a new DNS discovery instance | |
| #[allow(clippy::unused_async)] | |
| pub async fn new() -> Result<Self, Error> { | |
| // existing implementation… | |
| } |
🤖 Prompt for AI Agents
In dash-spv/src/network/discovery.rs around line 19, the async fn new() is
flagged by Clippy for having no await; fix by either (A) making the constructor
synchronous: remove async from the signature, return Result<Self, Error>
directly and update all call sites/tests to stop awaiting it, or (B,
non-breaking) keep it async and suppress the lint by adding
#[allow(clippy::unused_async)] above the function; choose A if you can change
callers, otherwise apply B to retain the public API.
Replaced the deprecated
trust-dns-resolverwithhickory_resolverin the project. This change includes updating the dependencies inCargo.tomland modifying the DNS discovery implementation to utilize the new resolver. This update was required because the old dns resolver had a critical security vulnerability in it.Summary by CodeRabbit
Refactor
Chores
No user action required.