fix: conform kad-dht interop node to the cross-implementation contract - #265
Merged
Conversation
Two defects kept the kad-dht interop node from interoperating with the other implementations in libp2p/unified-testing. Measured against the py, dotnet and nim nodes, the 37 matrix tests involving haskell went from 8/37 to 37/37 passing. Value payload: the provider stored "<TEST_KEY>-value" and the querier compared for exact equality. Every other implementation stores "hello from <impl> client" and matches on the "hello from" substring, so haskell records were rejected by py/dotnet/nim queriers and records from those implementations were rejected by the haskell querier. Store the conventional payload and match on the shared marker. Identify: no role registered the Identify handlers. dotnet-libp2p awaits /ipfs/id/1.0.0 on every outbound connection before its dial resolves (Libp2pPeerFactory.ConnectedTo calls session.DialAsync<IdentifyProtocol>, and Peer.DialAsyncCore blocks on session.Connected until it returns), so a node that does not serve it is undialable from dotnet. Register the handlers in all three roles.
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
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.
The kad-dht interop node added in #264 was only ever exercised against itself.
Running it against the actual libp2p/unified-testing matrix (py, dotnet, nim)
turned up two defects. Both are in
interop/kad-dht-node/Main.hs; the libraryis untouched.
1. Value payload did not follow the suite's convention
The provider stored
<TEST_KEY>-valueand the querier compared for exactequality. Every other implementation stores
hello from <impl> clientandmatches on the
hello fromsubstring:hello from py clientb"hello from" in valuehello from dotnet clientstrVal.Contains("hello from")<TEST_KEY>-valuepy's node carries the comment "Accept any 'hello from' message for
cross-language interop", so the substring match is the intended contract.
The DHT itself was working the whole time — a py querier found the haskell
provider and retrieved its record, then rejected the payload:
Now stores
hello from haskell clientand matches on the shared marker.2. Identify handlers were never registered
No role called
registerIdentifyHandlers. dotnet-libp2p awaits/ipfs/id/1.0.0on every outbound connection before its dial resolves —Libp2pPeerFactory.ConnectedTocallssession.DialAsync<IdentifyProtocol>(),and
Peer.DialAsyncCoreblocks onsession.Connecteduntil that returns — soa node that does not serve it is undialable from dotnet:
This was directional: haskell dialing dotnet worked, dotnet dialing haskell
failed 100% of the time (7/7 cases). Registered in all three roles, since DHT
peers dial each other in every role, not just the bootstrap.
Results
Full local run of the 37 unified-testing matrix tests that involve haskell
(bootstrap x provider x querier over py, dotnet, nim, haskell):
For reference, the same harness scores 24/27 on the pre-existing py/dotnet/nim
matrix (three dotnet-querier tests fail independently of this change).