Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Portal-Hive client-interop fixes #429

Merged
merged 10 commits into from
Aug 6, 2023
Merged

Portal-Hive client-interop fixes #429

merged 10 commits into from
Aug 6, 2023

Conversation

ScottyPoi
Copy link
Collaborator

@ScottyPoi ScottyPoi commented Aug 5, 2023

This PR will bring the portal hive client interop tests to 100% 98% passing.

  • new ENR's are added to routing table by general packet handler
    • Previously this was only available in the SYN packet handling.
  • findENR functions now use getWithPending, which will return the peerId of peers still in the discv5 pending buffer.
  • Corrections made to uTP sockets:
    • sndId / rcvId / connectionId setup
    • seqNr / ackNr setup
    • FIN handling.
      • Corrected to await end of stream before compiling.
  • Added debug logging throughout.

@ScottyPoi ScottyPoi changed the title Trin-interop Portal-Hive client-interop fixes Aug 6, 2023
@ScottyPoi ScottyPoi marked this pull request as ready for review August 6, 2023 01:06
@ScottyPoi
Copy link
Collaborator Author

Currently 1/73 tests failing: OFFER: Receipt trin --> ultralight

Should be fixable on Ultralight side. The failure has to do with FIN packet handling, and the fact that Trin uTP data packets tend to arrive in an out of order pattern, with the FIN arriving before the final 2 data packets.

Ultralight's uTP should know to wait for all data packets before compiling the stream, but the current solution for that is insufficient.

Personally in favor of merging this PR to update the public Portal-Hive tests. The fix for above can be done in a new PR.

Copy link
Collaborator

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@acolytec3 acolytec3 merged commit 83c8061 into master Aug 6, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants