Conversation
…() method Problem: - connectLoop() runs every 5 seconds, recentConnections cache expires after 30 seconds - After cache expiration, it attempts to reconnect to already connected nodes - Results in duplicate TCP and P2P handshakes every 30 seconds Solution: - Add hasActiveConnectionTo() method in ChannelManager - Check connectedNodeIds Map for existing active connections - Verify actual Netty Channel active state (not just flag) - Support loopback address special handling (for local testing) - Add checks in both connectLoop() and connectAsync() Modified Files: - ChannelManager.java: Add active connection detection logic - PeerClient.java: Clean up debug logging - XdagMessageHandler.java: Optimize application layer message handling - MessageCode.java, MessageFactory.java: Support message routing Test Results: - ✅ 0 duplicate connections in 90-second test (previously 3) - ✅ Only initial handshake, no extra handshake overhead - ✅ Works for both local testing and production environments Version: 0.1.5 -> 0.1.6
Problem: - Commit 61b0d17 added hasActiveConnectionTo() method but without unit tests - Missing test coverage for critical duplicate connection prevention logic - No verification of loopback address handling and edge cases Solution: - Add 10 comprehensive test cases for hasActiveConnectionTo() - Test null input handling and no connection scenarios - Test exact address matching in channels Map with active/inactive states - Test connectedNodeIds Map matching logic - Test loopback address special handling (with/without/empty nodeId) - Test non-loopback different port scenarios - Test edge cases (null context, empty nodeId) - All tests use Java Reflection to access private method - All tests use Mockito for proper mocking Test Results: - ✅ All 59 tests pass (49 existing + 10 new) - ✅ Covers all code paths in hasActiveConnectionTo() - ✅ Tests both fast path (channels Map) and slow path (connectedNodeIds) - ✅ Verifies loopback special logic for local testing - ✅ All tests follow JUnit 5 conventions Coverage: - Before: 49 test cases - After: 59 test cases (+10 new) - New tests: ChannelManagerTest.java:823-1091 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Problem: - APP_TEST was moved from 0x20 to 0x16 (commit 61b0d17) to avoid conflict with application message range - Tests still expected APP_TEST at 0x20 and treated as application message - 3 tests failed: IMessageCodeTest (2 failures) and MessageTypeTest (1 failure) Solution: - Update IMessageCodeTest.testMessageCodeEnum_applicationProtocol() - Now expects APP_TEST at 0x16 as framework message - Update IMessageCodeTest.testAllMessageCodesAreInCorrectRange() - Simplified to check all MessageCode enums are framework messages (0x00-0x1F) - Update MessageTypeTest.testOf_KnownTypes() - Changed MessageCode.of(0x20) to MessageCode.of(0x16) Test Results: - ✅ All 883 tests pass (previously 880/883) - ✅ IMessageCodeTest: 13/13 pass - ✅ MessageTypeTest: 3/3 pass - ✅ No regression in existing tests Rationale: APP_TEST is now at 0x16 in Node protocol range, making it a framework message for testing/debugging purposes, leaving 0x20-0xFF range for application layers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes: - CHANGELOG.md: Added v0.1.6 release notes - Documented hasActiveConnectionTo() duplicate connection fix - Documented 10 new unit tests for hasActiveConnectionTo() - Documented APP_TEST test fixes (moved to 0x16) - Added version comparison links - README.md: Updated all version references from 0.1.5 to 0.1.6 - Maven dependency version - JAR file names in examples - "What's New" section - docs/EXAMPLES.md: Updated JAR filenames to 0.1.6 - docs/USER_GUIDE.md: Updated all version references and JAR paths - docs/NODE_DISCOVERY.md: Updated JAR filenames and version info Summary: All documentation now reflects v0.1.6 release with: - Duplicate connection prevention improvements - 883 passing tests (880 → 883) - Comprehensive test coverage for hasActiveConnectionTo() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove duplicate explicit channelActivated() call in DiscoverServer.start() (Netty already handles this via MessageHandler.channelActive()) - Change KadService.inited from volatile boolean to AtomicBoolean - Use compareAndSet() for atomic check-and-set to prevent race conditions - Add isInited() method to maintain API compatibility - Bump version to 0.1.7 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… (BUG-P2P-002) Problem: - Multiple connections from same NodeId accumulated in channels map - Broadcasting sent to 28 "peers" when only 1 actual node existed - Caused by ephemeral ports being used as map keys Solution: - Use ConcurrentHashMap.compute() for atomic check-and-update on connectedNodeIds - Add getUniqueConnectedChannels() for deduplicated broadcasting - Properly clean up stale connections before accepting new ones - Handle null/empty nodeId gracefully Also includes: - Remove redundant channelActivated() call in DiscoverServer.start() - Change KadService.inited to AtomicBoolean for thread safety - Add 8 comprehensive unit tests for new deduplication logic Test Results: - All 891 tests pass (8 new tests added) - Thread safety verified with concurrent test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
When two nodes connect to each other simultaneously, both detect "duplicate connection" and previously used first-come-first-served which could result in both closing each other's connections. This fix implements deterministic tie-breaking based on NodeId: - Node with smaller NodeId prefers OUTBOUND connections - Node with larger NodeId prefers INBOUND connections - Both sides make the SAME decision, keeping the SAME connection Changes: - Add localNodeId field to cache local node identifier - Initialize localNodeId in start() from nodeKey - Implement deterministic tie-breaking in onChannelActive() - Add 9 unit tests covering all scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Root cause of ReadTimeoutException: - IdleStateHandler only checked WRITER_IDLE, not READER_IDLE - Scenario: Node A sends to B, B only receives (no data to send back) - B never triggers write-idle, never sends Ping - A waits 60s with no data → ReadTimeoutException Fixes: 1. Add READER_IDLE detection (30s) to trigger Ping when no data received 2. KeepAliveHandler now sends Ping on both read and write idle 3. Add channelInactive handler to properly cleanup disconnected channels This ensures bidirectional heartbeat and proper reconnection on disconnect.
1. XdagBusinessHandler: Handle PING/PONG before channel lookup - PING/PONG are network-layer keepalive, don't need Channel object - Fixes ReadTimeoutException caused by early PING/PONG being ignored 2. ChannelManager: Add isOutbound parameter to markHandshakeSuccess() - Correctly identify connection direction (outbound vs inbound) - Fixes duplicate connection detection logic 3. HandshakeHandler: Pass isOutbound flag to markHandshakeSuccess() 4. KeepAliveHandler: Change PING log level to info for debugging 5. ChannelManagerTest: Add test for inbound connection handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1. KeepAliveHandlerTest: Fix mock setup for ctx.channel() and writeAndFlush() - Add Channel and ChannelFuture mocks to prevent NPE - Update testReaderIdleAlsoTriggersPing to match production behavior - Fix testMixedEventSequence expected call count (2→3) - Fix testContextNotNullBeforeWrite verification 2. XdagBusinessHandlerTest: Add 4 tests for channelInactive() method - testChannelInactiveCallsOnChannelInactiveWhenChannelExists - testChannelInactiveDoesNotCallOnChannelInactiveWhenChannelNotFound - testChannelInactiveAlwaysCallsSuperMethod - testChannelInactiveWithNullRemoteAddress 3. XdagBusinessHandler: Fix null check bug in channelInactive() - Add null check for remoteAddress to prevent NPE - Discovered by new unit test Tests: 909 → 913 (all passing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1. ChannelManager: Add detailed debug logging for connection detection - Log hasActiveConnectionTo() decision path for debugging - Add connectedNodeIds state dump for troubleshooting - Improve comments for duplicate connection handling 2. XdagMessageHandler: Change PING/PONG log level to INFO - Easier debugging of keepalive mechanism 3. KeepAliveHandler: Change log level to INFO for debugging 4. ChannelManagerTest: Add comprehensive tests for connection handling 5. ConnectionFlappingRootCauseTest: New test for connection flapping analysis 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- xdagj-crypto: 0.1.2 → 0.1.5 - commons-lang3: 3.18.0 → 3.20.0 - commons-cli: 1.9.0 → 1.11.0 - aws-dns: 2.31.52 → 2.39.5 - maven-assembly-plugin: 3.7.1 → 3.8.0 - maven-source-plugin: 3.3.1 → 3.4.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- README.md: Update test count 873 → 913 (6 places) - docs/AI_USAGE_GUIDE.md: Update version 0.1.5 → 0.1.6 - docs/REPUTATION.md: Update version 0.1.5 → 0.1.6 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Resolve pom.xml conflicts by selecting latest versions for each dependency: - xdagj-crypto: 0.1.5 (develop) - lombok: 1.18.42 (master) - guava: 33.5.0-jre (master) - commons-lang3: 3.20.0 (develop) - commons-cli: 1.11.0 (develop) - snappy-java: 1.1.10.8 (master) - aws-dns: 2.39.5 (develop) - maven-assembly-plugin: 3.8.0 (develop) - maven-source-plugin: 3.4.0 (develop) - maven-javadoc-plugin: 3.12.0 (master) - central-publishing-plugin: 0.9.0 (master) - nexus-staging-plugin: 1.7.0 (master) - maven-gpg-plugin: 3.2.8 (master) - jacoco-maven-plugin: 0.8.14 (master) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sync code