Skip to content

chore(autonatv2): utilize protobuf_serialization#2612

Open
vladopajic wants to merge 6 commits into
protobuf-improvementsfrom
protobuf-autonat
Open

chore(autonatv2): utilize protobuf_serialization#2612
vladopajic wants to merge 6 commits into
protobuf-improvementsfrom
protobuf-autonat

Conversation

@vladopajic

Copy link
Copy Markdown
Member

No description provided.

@vladopajic vladopajic changed the title chore(autonatv2): protobuf chore(autonatv2): utilize protobuf_serialization Jun 9, 2026
@codecov-commenter

codecov-commenter commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.42857% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (protobuf-improvements@d18f5cd). Learn more about missing BASE report.

Files with missing lines Patch % Lines
libp2p/protocols/connectivity/autonatv2/client.nim 90.90% 3 Missing ⚠️
libp2p/protocols/connectivity/autonatv2/server.nim 88.88% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                    @@
##             protobuf-improvements    #2612   +/-   ##
========================================================
  Coverage                         ?   85.10%           
========================================================
  Files                            ?      165           
  Lines                            ?    28103           
  Branches                         ?        7           
========================================================
  Hits                             ?    23916           
  Misses                           ?     4187           
  Partials                         ?        0           
Files with missing lines Coverage Δ
...2p/protocols/connectivity/autonatv2/mockserver.nim 84.61% <100.00%> (ø)
...ibp2p/protocols/connectivity/autonatv2/service.nim 95.12% <ø> (ø)
libp2p/protocols/connectivity/autonatv2/types.nim 100.00% <100.00%> (ø)
libp2p/protocols/connectivity/autonatv2/utils.nim 70.83% <100.00%> (ø)
libp2p/protocols/pubsub/rpc/message.nim 73.80% <ø> (ø)
libp2p/protocols/connectivity/autonatv2/client.nim 85.10% <90.90%> (ø)
libp2p/protocols/connectivity/autonatv2/server.nim 84.61% <88.88%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates AutoNAT v2 protobuf encoding/decoding from minprotobuf to protobuf_serialization, updating the AutoNAT v2 message types and their usage across client/server/mock/test code paths.

Changes:

  • Replaced manual minprotobuf encode/decode implementations with protobuf_serialization-generated encode()/decode() for AutoNAT v2 types.
  • Updated AutoNAT v2 client/server/test logic to construct and interpret messages via Opt[...] fields instead of a msgType case object.
  • Removed now-unneeded protobuf/minprotobuf imports where applicable.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/libp2p/protocols/test_autonat_v2.nim Updates AutoNAT v2 encode/decode tests to use Opt[...] fields and new serialization.
tests/libp2p/protocols/test_autonat_v2_service.nim Updates service tests for new DialResponse.status representation.
libp2p/protocols/pubsub/rpc/message.nim Removes unused minprotobuf import.
libp2p/protocols/connectivity/autonatv2/utils.nim Adjusts reachability logic for Opt[ResponseStatus].
libp2p/protocols/connectivity/autonatv2/types.nim Replaces manual protobuf with protobuf_serialization schemas + generated serializers.
libp2p/protocols/connectivity/autonatv2/service.nim Removes minprotobuf import.
libp2p/protocols/connectivity/autonatv2/server.nim Updates message IO to use new encode/decode and Opt[...]-based message selection.
libp2p/protocols/connectivity/autonatv2/mockserver.nim Updates mock to write raw bytes; message validation logic needs correction.
libp2p/protocols/connectivity/autonatv2/mock.nim Updates decode call but still references removed msgType/MsgType and uses ProtoBuffer.
libp2p/protocols/connectivity/autonatv2/client.nim Updates client message handling to the new serialization and Opt[...] fields.
Comments suppressed due to low confidence (2)

libp2p/protocols/connectivity/autonatv2/client.nim:190

  • In sendDialRequest, LPStreamRemoteClosedError is caught and only logged, but execution continues and the function returns dialResp.get(). If the stream was reset before dialResp is set, this will raise due to dialResp being none. Either re-raise as AutonatV2Error or return an explicit failure value.
  except LPStreamRemoteClosedError as exc:
    error "Stream reset by server", description = exc.msg, peer = pid
  finally:
    # rollback any changes
    self.expectedNonces.del(nonce)
  return dialResp.get().asAutonatV2Response(testAddrs)

libp2p/protocols/connectivity/autonatv2/mock.nim:33

  • This mock still uses the removed msgType/MsgType discriminator and stores response as ProtoBuffer, but AutonatV2 messages now decode based on which Opt[...] field is set and encode() returns seq[byte]. As-is this will not compile (no MsgType, no ProtoBuffer in scope) and won't interoperate with the new encode/decode API.
      let msg = AutonatV2Msg.decode(await stream.readLp(AutonatV2MsgLpSize)).valueOr:
        return
      if msg.msgType != MsgType.DialRequest:
        return
    except LPStreamError:

Comment thread libp2p/protocols/connectivity/autonatv2/utils.nim
Comment thread libp2p/protocols/connectivity/autonatv2/server.nim
Comment thread libp2p/protocols/connectivity/autonatv2/client.nim Outdated
Comment thread libp2p/protocols/connectivity/autonatv2/client.nim Outdated
Comment thread libp2p/protocols/connectivity/autonatv2/mockserver.nim Outdated
Comment thread libp2p/protocols/connectivity/autonatv2/types.nim
@vladopajic vladopajic marked this pull request as ready for review June 9, 2026 15:41
@vladopajic vladopajic requested a review from a team as a code owner June 9, 2026 15:41
@vladopajic vladopajic requested review from gmelodie and richard-ramos and removed request for a team June 9, 2026 15:41

@gmelodie gmelodie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

-345 >> +185!!!

@github-project-automation github-project-automation Bot moved this from new to In Progress in nim-libp2p Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants