feat(net): support domain names in peer configuration#75
feat(net): support domain names in peer configuration#75
Conversation
📝 WalkthroughWalkthroughThis pull request implements DNS domain name resolution for backup members and seed nodes. A new utility class handles hostname-to-IP conversion with IPv4/IPv6 fallback and parallel resolution. BackupManager now caches resolved IPs and periodically refreshes them via a scheduled executor. Configuration parsing is centralized and validates all backup member entries at startup. Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as Node Startup
participant Args as Args.checkBackupMembers()
participant InetUtil as InetUtil
participant DNS as DNS Resolver
participant BackupMgr as BackupManager
participant Scheduler as ScheduledExecutor
Startup->>Args: Load node.backup.members config
activate Args
Args->>Args: Iterate each member entry
Args->>InetUtil: resolveInetAddress(member)
activate InetUtil
InetUtil->>DNS: Resolve IPv4 (or IPv6 fallback)
DNS-->>InetUtil: Return InetAddress or null
InetUtil-->>Args: InetAddress or null
deactivate InetUtil
alt Resolution failed
Args->>Args: Throw TronError(PARAMETER_INIT)
else Resolution succeeded
Args->>Args: Add to validated members list
end
Args-->>Startup: Validated members with resolved IPs
deactivate Args
Startup->>BackupMgr: Initialize with resolved members
activate BackupMgr
BackupMgr->>BackupMgr: Populate domainIpCache
BackupMgr->>Scheduler: Schedule periodic refresh task
deactivate BackupMgr
Scheduler->>Scheduler: Fixed interval (e.g., 10 min)
loop Periodic Re-resolution
Scheduler->>InetUtil: resolveInetAddress(domain)
InetUtil->>DNS: Resolve IPv4/IPv6
DNS-->>InetUtil: New InetAddress or null
alt IP changed
InetUtil->>BackupMgr: Update domainIpCache & members
BackupMgr->>BackupMgr: Atomically swap IP in member set
else IP unchanged
InetUtil->>InetUtil: No-op
else Resolution failed
BackupMgr->>BackupMgr: Retain last known IP, log warning
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
gradle/verification-metadata.xml (1)
250-257: Remove the stale verification entry forio.github.tronprotocol:libp2p:2.2.7.The old coordinate is commented out in
common/build.gradle(line 25) and replaced withcom.github.317787106:libp2p:v0.10.0(line 24). The verification block at lines 857–867 inverification-metadata.xmlfor the 2.2.7 entry is no longer reachable and should be removed. Stale entries can obscure real version drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle/verification-metadata.xml` around lines 250 - 257, Remove the stale verification entry for the old coordinate io.github.tronprotocol:libp2p:2.2.7 from gradle/verification-metadata.xml; since common/build.gradle now uses com.github.317787106:libp2p:v0.10.0, locate the verification block that references the io.github.tronprotocol:libp2p:2.2.7 component and delete that entire component/artifact SHA256 entries so only the current com.github.317787106:libp2p:v0.10.0 verification remains.framework/src/main/java/org/tron/core/config/args/Args.java (3)
1675-1683: Sequential DNS on startup path; also duplicates work done later inBackupManager.init().Two concerns:
- Each member is resolved serially with IPv4-then-IPv6 fallback. If any entry is slow/timing out, startup is blocked for
N × (ipv4_timeout + ipv6_timeout). For a small backup cluster the wall-clock cost is usually fine, but a single misconfigured entry can stall the node for tens of seconds before theTronErroris thrown. Consider usingInetUtil.resolveInetSocketAddressList(or a similar parallel helper) and asserting every entry resolved.BackupManager.init()re-resolves the same list immediately afterwards. Caching the results once onPARAMETER(e.g. aList<InetAddress>sibling tobackupMembers) would let both paths share the lookups and avoid a second round of DNS on startup.Non-blocking for correctness, but worth considering for operational UX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/tron/core/config/args/Args.java` around lines 1675 - 1683, The checkBackupMembers method does serial DNS lookups via resolveInetAddress causing startup blocking and duplicates work in BackupManager.init; replace the sequential loop by using a parallel resolver such as InetUtil.resolveInetSocketAddressList to resolve PARAMETER.backupMembers in parallel, assert every input resolved, and store the resolved InetAddress results on PARAMETER (e.g. add a sibling List<InetAddress> like resolvedBackupMembers) so BackupManager.init can reuse the cached addresses instead of re-resolving; update checkBackupMembers to populate and validate that cached list rather than calling resolveInetAddress per entry.
1333-1339: Error message reads awkwardly when wrapping a runtime exception.
String.format("config %s contains %s", path, e.getMessage())produces messages likeconfig node.active contains Invalid host/port ..., which is hard to parse. Prefer something like"Failed to parse %s: %s"and include the cause:🛠️ Suggested fix
try { socketAddressList = InetUtil.resolveInetSocketAddressList(list); } catch (RuntimeException e) { - throw new TronError(String.format("config %s contains %s", path, e.getMessage()), - TronError.ErrCode.PARAMETER_INIT); + throw new TronError( + String.format("Failed to parse %s: %s", path, e.getMessage()), + e, TronError.ErrCode.PARAMETER_INIT); }(Assumes a
TronError(String, Throwable, ErrCode)constructor exists; if not, at least propagatee.getMessage()in the same sentence structure.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/tron/core/config/args/Args.java` around lines 1333 - 1339, The catch block that wraps InetUtil.resolveInetSocketAddressList(list) currently throws a TronError with an awkward message via String.format("config %s contains %s", path, e.getMessage()); change this to a clearer message like "Failed to parse %s: %s" and pass the original exception as the cause when creating the TronError (i.e., use the TronError constructor that accepts a Throwable) so the thrown error includes both the clearer text and the original exception; update the throw in Args (the catch around InetUtil.resolveInetSocketAddressList) to use the new message format and include e as the cause, or at minimum reorder message formatting to "%s: %s" and include e.getMessage().
1326-1340: Typo in helper name:getInetSockerAddress→getInetSocketAddress.The private helper is misspelled (
Socker). Since the public overload isgetInetSocketAddress, the two names differ by only a character, which is both confusing and easy to miss at a call-site. Given it's private, renaming is risk-free.🛠️ Suggested fix
- private static List<InetSocketAddress> getInetSockerAddress( + private static List<InetSocketAddress> resolveInetSocketAddresses( final com.typesafe.config.Config config, String path) {(And update the two internal callers on lines 1308 and 1345.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/tron/core/config/args/Args.java` around lines 1326 - 1340, Private helper getInetSockerAddress is misspelled; rename it to getInetSocketAddress to match the public overload and avoid confusion. Change the method name getInetSockerAddress to getInetSocketAddress, update all internal callers that reference the old name to the new name, ensure there is no duplicate signature/conflict with the existing public getInetSocketAddress overload, and run a build to confirm all references compile.framework/src/main/java/org/tron/common/backup/BackupManager.java (1)
54-56: Minor: allocate the DNS-refresh executor lazily.
dnsExecutorServiceis created unconditionally at field-init time, but it's only scheduled whendomainIpCacheis non-empty (line 130). For deployments that stick to IP literals (the vast majority today), this spins up — and later shuts down — an unused scheduled-executor thread. Consider deferring creation to inside theif (!domainIpCache.isEmpty())block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/tron/common/backup/BackupManager.java` around lines 54 - 56, The dnsExecutorService is created eagerly at field initialization (dnsExecutorService = ExecutorServiceManager.newSingleThreadScheduledExecutor(dnsEsName)) but only used when domainIpCache is non-empty; change to lazy allocation by replacing the field with a nullable/optional reference (e.g., private ScheduledExecutorService dnsExecutorService = null) and instantiate it inside the existing if (!domainIpCache.isEmpty()) block using ExecutorServiceManager.newSingleThreadScheduledExecutor(dnsEsName); update any shutdown/cleanup logic to check for null before calling shutdown on dnsExecutorService and keep dnsEsName and domainIpCache references unchanged so the allocator and lifecycle remain consistent.framework/src/main/java/org/tron/core/config/args/InetUtil.java (1)
51-97: Parse each entry once and reuse.
NetUtil.parseInetSocketAddress(item)is invoked on line 54 (filter), line 88 (rebuild), and a third time insideresolveInetSocketAddressfor each submitted domain. Besides the redundant work, any malformed entry will also throw from two different code paths, making error messages inconsistent. Parsing once and threadingInetSocketAddressobjects through the pipeline is both cleaner and more robust.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/tron/core/config/args/InetUtil.java` around lines 51 - 97, The code repeatedly calls NetUtil.parseInetSocketAddress(item) in the filter loop, when submitting DNS tasks, and again when rebuilding the result; parse each input string once up-front into an InetSocketAddress (e.g., produce a List<Pair<String,InetSocketAddress>> or Map<String,InetSocketAddress> from ipOrDomainWithPortList), use the parsed address to decide whether it is an IP literal (isIpLiteral(parsed.getHostString())) and only submit unresolved domain InetSocketAddress objects to resolveInetSocketAddress, store resolved results in resolvedDomains keyed by the original input string, and then build the final result from those parsed/resolved addresses so parsing is not duplicated and malformed-entry parsing errors occur in a single place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/build.gradle`:
- Around line 24-25: The build currently depends on a personal JitPack fork
(com.github.317787106:libp2p:v0.10.0) instead of the official
io.github.tronprotocol libp2p artifact; revert common/build.gradle to use the
official dependency (replace the com.github... coordinate with
io.github.tronprotocol:libp2p at the appropriate released version) or, if the
forked API (LookUpTxt.lookUpIp) is truly required and not yet upstream, move the
fork dependency behind a clearly named non-default Gradle profile (e.g.,
"jitpack-fork") and document that exception, and remove the commented-out
io.github.tronprotocol line so no dead config remains; also create a ticket to
coordinate with the libp2p maintainers to upstream LookUpTxt.lookUpIp (or get an
official 2.2.8+ release) so the fork can be removed.
In `@framework/src/main/java/org/tron/common/backup/BackupManager.java`:
- Around line 86-100: The loop over parameter.getBackupMembers() skips caching
domain names that resolve to localIp, so later DNS changes are never detected by
refreshMemberIps(); change the logic in the loop (around resolveInetAddress,
localIp, domainIpCache, members and NetUtil.validIpV4/validIpV6) to always
populate domainIpCache.put(ipOrDomain, ip) for domain names (i.e., when
!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) before the
localIp check/continue, and only then skip adding the ip to members when
ip.equals(localIp).
- Around line 195-214: refreshMemberIps currently unconditionally removes oldIp
from members which can drop an IP still referenced by another domain; change the
removal logic to only remove oldIp when no other domain in domainIpCache maps to
that same IP and the oldIp is not present as an explicit IP-literal member or
the localIp. Specifically, inside refreshMemberIps (using domainIpCache,
members, localIp), before calling members.remove(oldIp) check: (1) iterate
domainIpCache.values() to see if any other domain (key != current domain) still
resolves to oldIp, and (2) ensure oldIp is not equal to localIp and not present
in members as an explicit IP-literal that should be preserved; only call
members.remove(oldIp) if both checks indicate it is no longer referenced. Also
ensure you still add newIp (unless it equals localIp) and update
domainIpCache.put(domain, newIp) as before.
In `@framework/src/main/java/org/tron/core/config/args/InetUtil.java`:
- Around line 60-84: The single-domain branch in InetUtil currently calls
resolveInetSocketAddress(entry) directly and can throw/propagate runtime
exceptions; change it to mirror the multi-domain path by wrapping the
single-domain resolution in a try/catch, call resolveInetSocketAddress(entry)
inside the try, only put into resolvedDomains on success, and catch and log
failures (e.g., catch RuntimeException/Exception and logger.warn("Failed to
resolve address, skip: {}", entry)); this keeps behavior consistent with the
parallel path (resolveInetSocketAddress, resolvedDomains) and prevents startup
failures observed in Args.getInetSockerAddress.
---
Nitpick comments:
In `@framework/src/main/java/org/tron/common/backup/BackupManager.java`:
- Around line 54-56: The dnsExecutorService is created eagerly at field
initialization (dnsExecutorService =
ExecutorServiceManager.newSingleThreadScheduledExecutor(dnsEsName)) but only
used when domainIpCache is non-empty; change to lazy allocation by replacing the
field with a nullable/optional reference (e.g., private ScheduledExecutorService
dnsExecutorService = null) and instantiate it inside the existing if
(!domainIpCache.isEmpty()) block using
ExecutorServiceManager.newSingleThreadScheduledExecutor(dnsEsName); update any
shutdown/cleanup logic to check for null before calling shutdown on
dnsExecutorService and keep dnsEsName and domainIpCache references unchanged so
the allocator and lifecycle remain consistent.
In `@framework/src/main/java/org/tron/core/config/args/Args.java`:
- Around line 1675-1683: The checkBackupMembers method does serial DNS lookups
via resolveInetAddress causing startup blocking and duplicates work in
BackupManager.init; replace the sequential loop by using a parallel resolver
such as InetUtil.resolveInetSocketAddressList to resolve PARAMETER.backupMembers
in parallel, assert every input resolved, and store the resolved InetAddress
results on PARAMETER (e.g. add a sibling List<InetAddress> like
resolvedBackupMembers) so BackupManager.init can reuse the cached addresses
instead of re-resolving; update checkBackupMembers to populate and validate that
cached list rather than calling resolveInetAddress per entry.
- Around line 1333-1339: The catch block that wraps
InetUtil.resolveInetSocketAddressList(list) currently throws a TronError with an
awkward message via String.format("config %s contains %s", path,
e.getMessage()); change this to a clearer message like "Failed to parse %s: %s"
and pass the original exception as the cause when creating the TronError (i.e.,
use the TronError constructor that accepts a Throwable) so the thrown error
includes both the clearer text and the original exception; update the throw in
Args (the catch around InetUtil.resolveInetSocketAddressList) to use the new
message format and include e as the cause, or at minimum reorder message
formatting to "%s: %s" and include e.getMessage().
- Around line 1326-1340: Private helper getInetSockerAddress is misspelled;
rename it to getInetSocketAddress to match the public overload and avoid
confusion. Change the method name getInetSockerAddress to getInetSocketAddress,
update all internal callers that reference the old name to the new name, ensure
there is no duplicate signature/conflict with the existing public
getInetSocketAddress overload, and run a build to confirm all references
compile.
In `@framework/src/main/java/org/tron/core/config/args/InetUtil.java`:
- Around line 51-97: The code repeatedly calls
NetUtil.parseInetSocketAddress(item) in the filter loop, when submitting DNS
tasks, and again when rebuilding the result; parse each input string once
up-front into an InetSocketAddress (e.g., produce a
List<Pair<String,InetSocketAddress>> or Map<String,InetSocketAddress> from
ipOrDomainWithPortList), use the parsed address to decide whether it is an IP
literal (isIpLiteral(parsed.getHostString())) and only submit unresolved domain
InetSocketAddress objects to resolveInetSocketAddress, store resolved results in
resolvedDomains keyed by the original input string, and then build the final
result from those parsed/resolved addresses so parsing is not duplicated and
malformed-entry parsing errors occur in a single place.
In `@gradle/verification-metadata.xml`:
- Around line 250-257: Remove the stale verification entry for the old
coordinate io.github.tronprotocol:libp2p:2.2.7 from
gradle/verification-metadata.xml; since common/build.gradle now uses
com.github.317787106:libp2p:v0.10.0, locate the verification block that
references the io.github.tronprotocol:libp2p:2.2.7 component and delete that
entire component/artifact SHA256 entries so only the current
com.github.317787106:libp2p:v0.10.0 verification remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8388a78-066b-4989-9f2c-c77171921972
📒 Files selected for processing (5)
common/build.gradleframework/src/main/java/org/tron/common/backup/BackupManager.javaframework/src/main/java/org/tron/core/config/args/Args.javaframework/src/main/java/org/tron/core/config/args/InetUtil.javagradle/verification-metadata.xml
| api group: 'com.github.317787106', name: 'libp2p', version: 'v0.10.0',{ | ||
| //api group: 'io.github.tronprotocol', name: 'libp2p', version: '2.2.7',{ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the io.github.tronprotocol:libp2p Maven artifact have a 2.2.8 release that exposes LookUpTxt.lookUpIp?
💡 Result:
No, the io.github.tronprotocol:libp2p Maven artifact does not have a 2.2.8 release. The latest available version is 2.2.7, released on November 12, 2025. No sources mention a 2.2.8 release on Maven Central, Sonatype Central, mvnrepository.com, or the project's GitHub releases page. No evidence was found for a class or method named LookUpTxt.lookUpIp in the libp2p codebase or documentation across all versions.
Citations:
- 1: https://mvnrepository.com/artifact/io.github.tronprotocol/libp2p
- 2: https://github.com/tronprotocol/libp2p
- 3: https://central.sonatype.com/artifact/io.github.tronprotocol/libp2p/2.2.0
- 4: https://central.sonatype.com/artifact/io.github.tronprotocol/libp2p
- 5: https://mvnrepository.com/artifact/io.github.tronprotocol/libp2p/versions
- 6: https://mvnrepository.com/artifact/io.github.tronprotocol/libp2p/2.2.7
- 7: https://github.com/tronprotocol/libp2p/releases
🏁 Script executed:
#!/bin/bash
# Check what the new fork coordinate actually resolves to on JitPack and whether
# LookUpTxt.lookUpIp is referenced anywhere in this repo's sources.
rg -nP -C2 '\bLookUpTxt\b|lookUpIp\s*\('
# Show every Gradle file referencing libp2p to confirm no other module pins the old coord.
rg -nP --type=gradle -C1 'libp2p'Repository: 317787106/java-tron
Length of output: 2290
Do not ship a personal JitPack fork of libp2p as the production dependency.
This replaces the official io.github.tronprotocol:libp2p:2.2.7 with com.github.317787106:libp2p:v0.10.0. The new group coordinate com.github.317787106 is a JitPack path that resolves to a personal GitHub account (user id 317787106), not the tronprotocol organization. Several critical problems:
-
PR summary is incorrect. The PR description states the bump is
libp2p 2.2.7 -> 2.2.8to acquireLookUpTxt.lookUpIp, but this is misleading:io.github.tronprotocol:libp2phas no 2.2.8 release (latest is 2.2.7), andLookUpTxt.lookUpIpdoes not exist in the official libp2p codebase. The method exists only in the fork (com.github.317787106). The actual change is2.2.7 -> v0.10.0from a personal account — this is a material difference from what the commit message promises. -
The fork is not optional, making supply chain risk acute. Because
LookUpTxt.lookUpIpis used in this codebase (seeframework/src/main/java/org/tron/core/config/args/InetUtil.javalines 112, 114, 141, 143) and exists nowhere else, the fork dependency is not a temporary workaround—it is a production requirement for a mainnet node's core P2P configuration. This means the trust anchor is permanently anchored to an individual contributor's GitHub account rather than the Tron Protocol organization. -
Commented-out coordinate. Line 25 retains the old
io.github.tronprotocoldependency as a comment. Dead config should not be merged. Its presence is a strong signal that this change was a development workaround that was never properly resolved upstream.
Recommended: Coordinate with the libp2p maintainers to release LookUpTxt.lookUpIp under the official io.github.tronprotocol org (e.g., as 2.2.8 or later), then revert to the official dependency. If that is not feasible in the release timeline, gate the fork behind a clearly labeled non-default profile and document the limitation and timeline for resolution in the PR. Do not merge commented-out code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/build.gradle` around lines 24 - 25, The build currently depends on a
personal JitPack fork (com.github.317787106:libp2p:v0.10.0) instead of the
official io.github.tronprotocol libp2p artifact; revert common/build.gradle to
use the official dependency (replace the com.github... coordinate with
io.github.tronprotocol:libp2p at the appropriate released version) or, if the
forked API (LookUpTxt.lookUpIp) is truly required and not yet upstream, move the
fork dependency behind a clearly named non-default Gradle profile (e.g.,
"jitpack-fork") and document that exception, and remove the commented-out
io.github.tronprotocol line so no dead config remains; also create a ticket to
coordinate with the libp2p maintainers to upstream LookUpTxt.lookUpIp (or get an
official 2.2.8+ release) so the fork can be removed.
| for (String ipOrDomain : parameter.getBackupMembers()) { | ||
| InetAddress inetAddress = resolveInetAddress(ipOrDomain); | ||
| if (inetAddress == null) { | ||
| logger.warn("Failed to resolve backup member domain: {}", ipOrDomain); | ||
| continue; | ||
| } | ||
| String ip = inetAddress.getHostAddress(); | ||
| if (localIp.equals(ip)) { | ||
| continue; | ||
| } | ||
| if (!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) { | ||
| domainIpCache.put(ipOrDomain, ip); | ||
| } | ||
| members.add(ip); | ||
| } |
There was a problem hiding this comment.
Domains resolving to localIp at startup are never tracked for DNS refresh.
When a configured backup member is a domain name that currently resolves to localIp, the continue at line 94 fires before domainIpCache.put(...). If that DNS record later points to a non-local IP, the change will never be picked up by refreshMemberIps() — the only way to recover is a process restart.
Unless that's intentional, cache the domain first and only skip adding it to members:
🛠️ Suggested fix
for (String ipOrDomain : parameter.getBackupMembers()) {
InetAddress inetAddress = resolveInetAddress(ipOrDomain);
if (inetAddress == null) {
logger.warn("Failed to resolve backup member domain: {}", ipOrDomain);
continue;
}
String ip = inetAddress.getHostAddress();
- if (localIp.equals(ip)) {
- continue;
- }
if (!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) {
domainIpCache.put(ipOrDomain, ip);
}
+ if (localIp.equals(ip)) {
+ continue;
+ }
members.add(ip);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@framework/src/main/java/org/tron/common/backup/BackupManager.java` around
lines 86 - 100, The loop over parameter.getBackupMembers() skips caching domain
names that resolve to localIp, so later DNS changes are never detected by
refreshMemberIps(); change the logic in the loop (around resolveInetAddress,
localIp, domainIpCache, members and NetUtil.validIpV4/validIpV6) to always
populate domainIpCache.put(ipOrDomain, ip) for domain names (i.e., when
!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) before the
localIp check/continue, and only then skip adding the ip to members when
ip.equals(localIp).
| private void refreshMemberIps() { | ||
| for (Map.Entry<String, String> entry : domainIpCache.entrySet()) { | ||
| String domain = entry.getKey(); | ||
| String oldIp = entry.getValue(); | ||
| InetAddress inetAddress = resolveInetAddress(domain); | ||
| if (inetAddress == null) { | ||
| logger.warn("DNS refresh: failed to re-resolve backup member domain {}, keep it", domain); | ||
| continue; | ||
| } | ||
| String newIp = inetAddress.getHostAddress(); | ||
| if (!newIp.equals(oldIp)) { | ||
| logger.info("DNS refresh: backup member {} IP changed {} -> {}", domain, oldIp, newIp); | ||
| members.remove(oldIp); | ||
| if (!localIp.equals(newIp)) { | ||
| members.add(newIp); | ||
| } | ||
| domainIpCache.put(domain, newIp); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
refreshMemberIps may drop an IP that is still in use by another tracked domain.
members.remove(oldIp) is unconditional. If two backup-member domains currently resolve to the same IP and only one of them rotates to a new address, the shared IP is removed from members even though the second domain still depends on it — keepalive messages will stop flowing to that peer until the other domain is also refreshed (or a restart happens).
Consider keeping oldIp only if no other entry in domainIpCache (or a plain IP-literal member) still maps to it:
🛠️ Sketch
if (!newIp.equals(oldIp)) {
logger.info("DNS refresh: backup member {} IP changed {} -> {}", domain, oldIp, newIp);
- members.remove(oldIp);
+ boolean stillReferenced = domainIpCache.entrySet().stream()
+ .anyMatch(e -> !e.getKey().equals(domain) && oldIp.equals(e.getValue()));
+ if (!stillReferenced) {
+ members.remove(oldIp);
+ }
if (!localIp.equals(newIp)) {
members.add(newIp);
}
domainIpCache.put(domain, newIp);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@framework/src/main/java/org/tron/common/backup/BackupManager.java` around
lines 195 - 214, refreshMemberIps currently unconditionally removes oldIp from
members which can drop an IP still referenced by another domain; change the
removal logic to only remove oldIp when no other domain in domainIpCache maps to
that same IP and the oldIp is not present as an explicit IP-literal member or
the localIp. Specifically, inside refreshMemberIps (using domainIpCache,
members, localIp), before calling members.remove(oldIp) check: (1) iterate
domainIpCache.values() to see if any other domain (key != current domain) still
resolves to oldIp, and (2) ensure oldIp is not equal to localIp and not present
in members as an explicit IP-literal that should be preserved; only call
members.remove(oldIp) if both checks indicate it is no longer referenced. Also
ensure you still add newIp (unless it equals localIp) and update
domainIpCache.put(domain, newIp) as before.
| Map<String, InetSocketAddress> resolvedDomains = new HashMap<>(); | ||
| if (domainEntries.size() > 1) { | ||
| int poolSize = StrictMath.min(domainEntries.size(), DNS_POOL_MAX_SIZE); | ||
| ExecutorService dnsPool = ExecutorServiceManager | ||
| .newFixedThreadPool(DNS_POOL_NAME, poolSize, true); | ||
| List<Future<InetSocketAddress>> futures = new ArrayList<>(domainEntries.size()); | ||
| for (String entry : domainEntries) { | ||
| futures.add(dnsPool.submit(() -> resolveInetSocketAddress(entry))); | ||
| } | ||
| for (int i = 0; i < domainEntries.size(); i++) { | ||
| String entry = domainEntries.get(i); | ||
| try { | ||
| resolvedDomains.put(entry, futures.get(i).get()); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| logger.warn("DNS lookup interrupted for: {}", entry); | ||
| } catch (ExecutionException e) { | ||
| logger.warn("Failed to resolve address, skip: {}", entry); | ||
| } | ||
| } | ||
| ExecutorServiceManager.shutdownAndAwaitTermination(dnsPool, DNS_POOL_NAME); | ||
| } else if (domainEntries.size() == 1) { | ||
| String entry = domainEntries.get(0); | ||
| resolvedDomains.put(entry, resolveInetSocketAddress(entry)); | ||
| } |
There was a problem hiding this comment.
Single-domain path skips the try/catch that the parallel path has.
When domainEntries.size() == 1, resolveInetSocketAddress(entry) is called inline (line 83). If the libp2p lookup throws a RuntimeException for any reason, it propagates up and turns into a TronError(PARAMETER_INIT) in Args.getInetSockerAddress — i.e. startup fails. The multi-domain path catches ExecutionException, logs a warning, and skips. The behavior for seed/active/passive/fastForward should be "skip on failure" per the PR objectives regardless of whether there's 1 or N domains.
🛠️ Suggested fix
} else if (domainEntries.size() == 1) {
String entry = domainEntries.get(0);
- resolvedDomains.put(entry, resolveInetSocketAddress(entry));
+ try {
+ resolvedDomains.put(entry, resolveInetSocketAddress(entry));
+ } catch (RuntimeException e) {
+ logger.warn("Failed to resolve address, skip: {}", entry);
+ }
}📝 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.
| Map<String, InetSocketAddress> resolvedDomains = new HashMap<>(); | |
| if (domainEntries.size() > 1) { | |
| int poolSize = StrictMath.min(domainEntries.size(), DNS_POOL_MAX_SIZE); | |
| ExecutorService dnsPool = ExecutorServiceManager | |
| .newFixedThreadPool(DNS_POOL_NAME, poolSize, true); | |
| List<Future<InetSocketAddress>> futures = new ArrayList<>(domainEntries.size()); | |
| for (String entry : domainEntries) { | |
| futures.add(dnsPool.submit(() -> resolveInetSocketAddress(entry))); | |
| } | |
| for (int i = 0; i < domainEntries.size(); i++) { | |
| String entry = domainEntries.get(i); | |
| try { | |
| resolvedDomains.put(entry, futures.get(i).get()); | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| logger.warn("DNS lookup interrupted for: {}", entry); | |
| } catch (ExecutionException e) { | |
| logger.warn("Failed to resolve address, skip: {}", entry); | |
| } | |
| } | |
| ExecutorServiceManager.shutdownAndAwaitTermination(dnsPool, DNS_POOL_NAME); | |
| } else if (domainEntries.size() == 1) { | |
| String entry = domainEntries.get(0); | |
| resolvedDomains.put(entry, resolveInetSocketAddress(entry)); | |
| } | |
| Map<String, InetSocketAddress> resolvedDomains = new HashMap<>(); | |
| if (domainEntries.size() > 1) { | |
| int poolSize = StrictMath.min(domainEntries.size(), DNS_POOL_MAX_SIZE); | |
| ExecutorService dnsPool = ExecutorServiceManager | |
| .newFixedThreadPool(DNS_POOL_NAME, poolSize, true); | |
| List<Future<InetSocketAddress>> futures = new ArrayList<>(domainEntries.size()); | |
| for (String entry : domainEntries) { | |
| futures.add(dnsPool.submit(() -> resolveInetSocketAddress(entry))); | |
| } | |
| for (int i = 0; i < domainEntries.size(); i++) { | |
| String entry = domainEntries.get(i); | |
| try { | |
| resolvedDomains.put(entry, futures.get(i).get()); | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| logger.warn("DNS lookup interrupted for: {}", entry); | |
| } catch (ExecutionException e) { | |
| logger.warn("Failed to resolve address, skip: {}", entry); | |
| } | |
| } | |
| ExecutorServiceManager.shutdownAndAwaitTermination(dnsPool, DNS_POOL_NAME); | |
| } else if (domainEntries.size() == 1) { | |
| String entry = domainEntries.get(0); | |
| try { | |
| resolvedDomains.put(entry, resolveInetSocketAddress(entry)); | |
| } catch (RuntimeException e) { | |
| logger.warn("Failed to resolve address, skip: {}", entry); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@framework/src/main/java/org/tron/core/config/args/InetUtil.java` around lines
60 - 84, The single-domain branch in InetUtil currently calls
resolveInetSocketAddress(entry) directly and can throw/propagate runtime
exceptions; change it to mirror the multi-domain path by wrapping the
single-domain resolution in a try/catch, call resolveInetSocketAddress(entry)
inside the try, only put into resolvedDomains on success, and catch and log
failures (e.g., catch RuntimeException/Exception and logger.warn("Failed to
resolve address, skip: {}", entry)); this keeps behavior consistent with the
parallel path (resolveInetSocketAddress, resolvedDomains) and prevents startup
failures observed in Args.getInetSockerAddress.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="framework/src/main/java/org/tron/common/backup/BackupManager.java">
<violation number="1" location="framework/src/main/java/org/tron/common/backup/BackupManager.java:93">
P1: Domains resolving to `localIp` at startup are skipped before being cached, so later DNS changes for those domains are never picked up.</violation>
<violation number="2" location="framework/src/main/java/org/tron/common/backup/BackupManager.java:207">
P1: Removing `oldIp` unconditionally can drop a still-active backup member when multiple domains share the same IP.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (localIp.equals(ip)) { | ||
| continue; | ||
| } | ||
| if (!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) { | ||
| domainIpCache.put(ipOrDomain, ip); | ||
| } | ||
| members.add(ip); |
There was a problem hiding this comment.
P1: Domains resolving to localIp at startup are skipped before being cached, so later DNS changes for those domains are never picked up.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/common/backup/BackupManager.java, line 93:
<comment>Domains resolving to `localIp` at startup are skipped before being cached, so later DNS changes for those domains are never picked up.</comment>
<file context>
@@ -78,10 +83,20 @@ public void init() {
+ continue;
+ }
+ String ip = inetAddress.getHostAddress();
+ if (localIp.equals(ip)) {
+ continue;
+ }
</file context>
| if (localIp.equals(ip)) { | |
| continue; | |
| } | |
| if (!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) { | |
| domainIpCache.put(ipOrDomain, ip); | |
| } | |
| members.add(ip); | |
| if (!NetUtil.validIpV4(ipOrDomain) && !NetUtil.validIpV6(ipOrDomain)) { | |
| domainIpCache.put(ipOrDomain, ip); | |
| } | |
| if (localIp.equals(ip)) { | |
| continue; | |
| } | |
| members.add(ip); |
| String newIp = inetAddress.getHostAddress(); | ||
| if (!newIp.equals(oldIp)) { | ||
| logger.info("DNS refresh: backup member {} IP changed {} -> {}", domain, oldIp, newIp); | ||
| members.remove(oldIp); |
There was a problem hiding this comment.
P1: Removing oldIp unconditionally can drop a still-active backup member when multiple domains share the same IP.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/common/backup/BackupManager.java, line 207:
<comment>Removing `oldIp` unconditionally can drop a still-active backup member when multiple domains share the same IP.</comment>
<file context>
@@ -162,4 +188,28 @@ public enum BackupStatusEnum {
+ String newIp = inetAddress.getHostAddress();
+ if (!newIp.equals(oldIp)) {
+ logger.info("DNS refresh: backup member {} IP changed {} -> {}", domain, oldIp, newIp);
+ members.remove(oldIp);
+ if (!localIp.equals(newIp)) {
+ members.add(newIp);
</file context>
| members.remove(oldIp); | |
| if (domainIpCache.entrySet().stream().noneMatch(e -> !e.getKey().equals(domain) && oldIp.equals(e.getValue()))) { | |
| members.remove(oldIp); | |
| } |
1 participant
Summary
Allows
node.seed.ip.list,node.active,node.passive,node.fastForward, andnode.backup.membersconfiguration entries to specify domain names in addition to IP literals. Domains are resolved to IPs at startup; backup member IPs are refreshed automatically every 60 seconds when DNS returns a different address.Closes tronprotocol#6634
Problem
All network peer configuration fields previously required numeric IP addresses. Operators running nodes behind dynamic DNS or cloud load-balancers had to manually update config files whenever IPs changed. Backup members in particular need continuous accuracy because they participate in keep-alive health checks.
Changes
New:
InetUtil(framework/src/main/java/org/tron/core/config/args/InetUtil.java)Central DNS resolution utility used by both startup parsing and runtime refresh:
resolveInetSocketAddressList(List<String>)— converts a list ofipOrDomain:portstrings toInetSocketAddressobjects. IP literals are used as-is; domain entries are resolved viaLookUpTxt.lookUpIp(IPv4 first, IPv6 fallback). When there are multiple domain entries, resolution is parallelised using a bounded thread pool (up to 10 threads). Unresolvable entries are silently skipped.resolveInetAddress(String)— resolves a bare host (no port) toInetAddress; IP literals take the fast path without a DNS lookup.BackupManager(framework)init(), each configured backup member is resolved viaInetUtil.resolveInetAddress. The resolved IP is stored inmembers; entries with a domain host are additionally tracked in adomainIpCache.dnsExecutorServicescheduler runsrefreshMemberIps()every 60 seconds whendomainIpCacheis non-empty. If DNS returns a new IP, the old IP is removed frommembersand the new IP is added;localIpis never added.dnsExecutorServiceis shut down alongsideexecutorServiceinstop().Args(framework)getInetSockerAddress(config, path)helper that callsInetUtil.resolveInetSocketAddressList, used by bothgetInetSocketAddress(seed/active/passive nodes) andgetInetAddress. Unresolvable domain entries in seed lists are silently dropped; unresolvable backup member entries throwTronError(PARAMETER_INIT)at startup via the newcheckBackupMembers()validator.Dependency
libp2pfromio.github.tronprotocol:libp2p:2.2.7toio.github.tronprotocol:libp2p:2.2.8, which exposes theLookUpTxt.lookUpIpAPI used for DNS resolution.Behaviour Summary
node.backup.membersTronErrorthrown at startupTest
InetUtilresolution paths andBackupManagerDNS refresh logic.Summary by CodeRabbit
New Features
Improvements
Chores
Summary by cubic
Add domain name support to peer config fields (
node.seed.ip.list,node.active,node.passive,node.fastForward,node.backup.members). Domains resolve at startup, and backup member IPs auto-refresh every 60s.New Features
InetUtilfor DNS resolution (IPv4-first, IPv6 fallback) with parallel lookups for multiple domains; IP literals bypass DNS.TronError(PARAMETER_INIT)if unresolved); track domain→IP and refresh every 60s, updatingmemberson change and never adding the local IP.Args: shared resolver forInetSocketAddress/InetAddressused across peer config parsing.Dependencies
com.github.317787106:libp2p@v0.10.0to useLookUpTxt.lookUpIp; updatedverification-metadata.xml.Written for commit 634f111. Summary will update on new commits.