Skip to content

Conversation

@vladopajic
Copy link
Member

@vladopajic vladopajic commented Nov 6, 2025

vac:p2p:ift:2025q4-nimlibp2p-ipv6

  • adding ipv6 tests for tcp
  • overall cleanups of tcp specific tests

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

🏁 Performance Summary

Commit: 65f63ae25809dec03da50d1b6251c597fdab5472

Scenario Nodes Total messages sent Total messages received Latency min (ms) Latency max (ms) Latency avg (ms)
✅ Base Test (QUIC) 10 100 900 0.571 4.365 1.720
✅ Base Test (TCP) 10 100 900 0.428 42.531 1.827

📊 View Container Resources in the Workflow Summary

Latency History

🔵 Min • 🟢 Avg • 🔴 Max

%%{init: {"xyChart": {"width": 500, "height": 200}}}%%
xychart-beta
    title "TCP"
    x-axis "PR Number" 1599 --> 1858
    y-axis "Latency (ms)"
    line "Min" [0.315, 0.429, 0.308, 0.349, 0.397, 0.259, 0.317, 0.333, 0.266, 0.278, 0.322, 0.300, 0.301, 0.302, 0.263, 0.338, 0.288, 0.308, 0.318, 0.313, 0.358, 0.317, 0.274, 0.293, 0.278, 0.312, 0.308, 0.280, 0.296, 0.292, 0.296, 0.296, 0.437, 0.265, 0.444, 0.396, 0.323, 0.305, 0.305, 0.293, 0.333, 0.322, 0.370, 0.273, 0.338, 0.275, 0.318, 0.322, 0.405, 0.278, 0.325, 0.297, 0.275, 0.297, 0.280, 0.277, 0.407, 0.304, 0.329, 0.319, 0.372, 0.289, 0.317, 0.262, 0.298, 0.376, 0.331, 0.257, 0.286, 0.299, 0.267, 0.312, 0.277, 0.240, 0.266, 0.424, 0.235, 0.261, 0.364, 0.422, 0.274, 0.297, 0.313, 0.291, 0.317, 0.356, 0.274, 0.222, 0.290, 0.253, 0.319, 0.268, 0.303, 0.246, 0.304, 0.316, 0.217, 0.252, 0.431, 0.247, 0.291, 0.267, 0.297, 0.392, 0.267, 0.335, 0.447, 0.289, 0.251, 0.361, 0.400, 0.393, 0.323, 0.267, 0.357, 0.284, 0.266, 0.319, 0.284, 0.299, 0.340, 0.323, 0.239, 0.300, 0.323, 0.339, 0.274, 0.272, 0.342, 0.594, 0.341, 0.254, 0.492, 0.404, 0.499, 0.418, 0.565, 0.343, 0.537, 0.486, 0.487, 0.497, 0.532, 0.582, 0.544, 0.482, 0.502, 0.464, 0.463, 0.510, 0.496, 0.424, 0.363, 0.437, 0.505, 0.484, 0.449, 0.453, 0.648, 0.572, 0.494, 0.498, 0.466, 0.528, 0.407, 0.503, 0.424, 0.473, 0.380, 0.535, 0.423, 0.493, 0.465, 0.439, 0.428, 0.533, 0.492, 0.461, 0.496, 0.527]
    line "Avg" [1.042, 1.134, 1.015, 1.035, 1.177, 0.899, 1.040, 1.085, 0.860, 1.015, 1.020, 0.945, 0.986, 1.074, 0.899, 0.975, 0.958, 1.027, 0.959, 1.047, 1.103, 1.046, 0.908, 0.882, 0.896, 1.064, 1.011, 0.957, 0.991, 0.923, 0.867, 0.959, 1.126, 0.910, 1.125, 1.085, 0.983, 1.049, 1.053, 1.056, 1.080, 0.989, 1.137, 1.007, 1.062, 0.874, 1.065, 1.137, 1.321, 0.852, 1.080, 0.991, 1.003, 1.017, 0.971, 1.068, 1.026, 1.054, 1.100, 1.054, 1.087, 0.961, 0.886, 0.878, 1.028, 1.247, 1.023, 0.887, 1.015, 0.952, 0.875, 0.985, 1.003, 0.997, 0.982, 1.178, 0.804, 0.839, 1.092, 1.110, 0.850, 1.026, 0.996, 0.959, 1.046, 1.083, 0.907, 0.852, 0.933, 0.953, 1.103, 0.932, 1.033, 0.974, 0.901, 1.042, 0.807, 0.838, 1.146, 1.110, 0.967, 0.981, 0.902, 1.065, 0.980, 1.053, 1.187, 0.955, 0.857, 1.106, 1.137, 1.130, 1.215, 1.003, 1.135, 0.933, 0.939, 0.997, 0.910, 0.949, 1.081, 1.009, 0.860, 1.047, 0.958, 0.999, 1.007, 0.870, 1.585, 1.622, 1.666, 0.923, 1.652, 1.383, 1.573, 1.415, 1.903, 1.399, 1.464, 1.422, 1.637, 1.594, 1.636, 1.531, 1.644, 1.674, 1.504, 1.422, 1.780, 1.681, 1.519, 1.252, 1.365, 1.539, 1.592, 1.591, 1.708, 1.391, 1.909, 1.760, 1.483, 1.432, 1.549, 1.699, 1.448, 1.533, 1.597, 1.353, 1.221, 1.833, 1.523, 1.475, 1.355, 1.485, 1.827, 1.587, 1.602, 1.429, 1.496, 1.569]
    line "Max" [2.298, 2.513, 2.647, 2.370, 2.486, 2.258, 2.316, 2.407, 2.065, 3.120, 2.328, 2.193, 2.359, 2.398, 2.082, 2.241, 2.339, 2.187, 2.267, 2.268, 2.480, 2.591, 2.231, 1.950, 2.493, 2.596, 2.270, 2.198, 2.587, 2.539, 1.811, 2.355, 2.373, 2.516, 2.368, 2.412, 2.166, 2.482, 2.401, 2.505, 2.396, 2.211, 2.686, 2.876, 2.527, 2.543, 2.441, 2.495, 41.679, 2.073, 2.354, 2.257, 2.217, 2.562, 2.383, 3.300, 2.337, 2.394, 2.521, 2.316, 2.490, 2.352, 2.218, 2.233, 2.267, 2.836, 2.352, 2.316, 2.371, 2.306, 1.972, 2.273, 2.457, 2.517, 2.295, 2.685, 1.995, 1.988, 2.495, 2.309, 2.154, 2.482, 2.433, 2.387, 2.387, 2.335, 2.411, 2.018, 2.145, 2.515, 2.441, 2.129, 2.243, 2.354, 2.225, 2.359, 2.232, 2.120, 3.212, 3.323, 2.256, 2.348, 2.230, 2.484, 2.177, 2.245, 2.542, 2.492, 2.067, 2.443, 2.400, 2.524, 2.732, 2.678, 2.575, 2.320, 2.945, 2.775, 2.175, 2.225, 2.827, 2.412, 2.462, 2.354, 2.303, 2.117, 2.404, 2.228, 41.975, 3.422, 41.990, 2.286, 42.363, 41.675, 41.847, 3.112, 43.082, 41.563, 3.349, 3.582, 41.786, 41.041, 41.434, 3.211, 41.756, 41.899, 3.401, 40.973, 42.183, 42.185, 41.292, 2.844, 3.133, 42.154, 41.399, 41.836, 41.808, 41.532, 42.924, 6.031, 41.956, 3.255, 42.249, 42.242, 3.540, 3.694, 42.006, 3.391, 2.740, 41.709, 41.636, 41.093, 3.214, 3.373, 42.531, 41.423, 41.626, 3.161, 3.229, 41.833]
Loading
%%{init: {"xyChart": {"width": 500, "height": 200}}}%%
xychart-beta
    title "QUIC"
    x-axis "PR Number" 1685 --> 1858
    y-axis "Latency (ms)"
    line "Min" [0.000, 0.000, 0.325, 0.229, 0.000, 0.000, 0.208, 0.223, 0.000, 0.000, 0.000, 0.000, 0.271, 0.203, 0.186, 0.000, 0.219, 0.254, 0.000, 0.000, 0.338, 0.000, 0.000, 0.207, 0.000, 0.221, 0.229, 0.000, 0.322, 0.000, 0.205, 0.000, 0.368, 0.213, 0.000, 0.187, 0.263, 0.179, 0.000, 0.323, 0.000, 0.000, 0.000, 0.000, 0.169, 0.197, 0.000, 0.306, 0.000, 0.000, 0.000, 0.448, 0.000, 0.242, 0.423, 0.000, 0.357, 0.392, 0.000, 0.000, 0.404, 0.364, 0.000, 0.000, 0.424, 0.000, 0.000, 0.000, 0.359, 0.449, 0.296, 0.364, 0.286, 0.000, 0.396, 0.000, 0.336, 0.369, 0.381, 0.370, 0.420, 0.451, 0.407, 0.000, 0.483, 0.583, 0.000, 0.588, 0.528, 0.572, 0.220, 0.368, 0.507, 0.549, 0.387, 0.559, 0.571, 0.607, 0.637, 0.441, 0.597, 0.551]
    line "Avg" [1.069, 0.259, 1.020, 0.856, 0.725, 0.602, 0.893, 0.875, 0.661, 0.557, 0.731, 0.796, 0.949, 0.726, 0.849, 0.761, 0.718, 0.939, 0.550, 0.561, 0.943, 0.832, 0.807, 0.867, 0.633, 1.098, 0.735, 0.608, 1.008, 0.530, 0.823, 0.822, 1.001, 0.919, 0.789, 0.785, 0.862, 0.741, 0.384, 0.935, 0.539, 0.664, 0.689, 0.829, 0.677, 0.918, 0.625, 0.880, 0.554, 0.506, 0.520, 1.456, 0.838, 0.873, 1.195, 0.824, 1.267, 1.176, 0.885, 0.661, 1.160, 1.118, 0.990, 1.148, 1.215, 1.187, 1.033, 1.098, 1.268, 1.200, 1.277, 1.181, 1.244, 0.863, 1.411, 0.948, 1.204, 1.203, 1.323, 1.341, 1.366, 1.242, 1.218, 1.116, 1.650, 1.700, 1.018, 1.666, 1.618, 1.699, 1.034, 1.254, 1.552, 1.605, 1.509, 1.613, 1.720, 1.756, 1.781, 1.639, 1.666, 1.637]
    line "Max" [2.783, 0.821, 1.944, 1.917, 1.681, 1.573, 1.837, 1.857, 1.782, 1.274, 1.783, 2.676, 2.006, 1.943, 2.179, 1.918, 1.755, 1.959, 1.371, 1.455, 1.958, 2.546, 1.978, 1.997, 1.535, 2.434, 1.934, 1.541, 2.256, 1.587, 1.879, 1.830, 2.144, 1.966, 1.840, 1.831, 1.849, 1.714, 1.796, 2.375, 1.343, 1.580, 1.815, 2.349, 1.672, 1.771, 1.586, 1.984, 1.678, 1.404, 1.476, 3.783, 2.333, 2.056, 2.873, 2.073, 2.809, 2.498, 2.435, 1.792, 2.426, 2.214, 2.181, 2.954, 2.497, 2.517, 2.223, 2.517, 2.560, 2.569, 2.510, 2.509, 2.615, 2.074, 4.512, 2.230, 2.542, 2.512, 3.394, 2.811, 3.227, 2.571, 2.708, 2.375, 3.946, 4.238, 2.346, 4.349, 4.111, 4.722, 2.563, 2.672, 3.922, 4.296, 3.873, 4.155, 4.365, 4.437, 4.471, 4.182, 4.259, 4.818]
Loading

@vladopajic vladopajic requested a review from rlve November 6, 2025 14:48
@vladopajic vladopajic marked this pull request as ready for review November 6, 2025 14:48
@vladopajic vladopajic requested a review from a team as a code owner November 6, 2025 14:48
Comment on lines 27 to 28
zeroMultiaddressStrIP4 = "/ip4/0.0.0.0/tcp/0"
zeroMultiaddressStrIP6 = "/ip6/::/tcp/0"
Copy link
Member Author

Choose a reason for hiding this comment

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

open for better naming suggestions

Copy link
Member Author

@vladopajic vladopajic Nov 7, 2025

Choose a reason for hiding this comment

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

going with this:

  • nameMAStr if variable is multiaddress in string

  • nameMA if variable parsed MultiAddress ( == MultiAddress.init(nameMAStr))

  • namaTAStr if variable is transport address (ip) address in string

  • nameTA if variable is parsed TransportAddress ( == initTAddress(namaTAStr) )

  • all can end with IP4 or IP6 indicating version

Comment on lines 26 to 30
message = "No Backdoors. No Masters. No Silence."
zeroMultiaddressStrIP4 = "/ip4/0.0.0.0/tcp/0"
zeroMultiaddressStrIP6 = "/ip6/::/tcp/0"
zeroAddrIP4 = "0.0.0.0:0"
zeroAddrIP6 = "[::]:0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: capitalized names for consts maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

that should not be nit, if naming convention is to have capital lattes for consts that it should be mandatory change here. having that said, it doesn't seem that this is convention here.

Comment on lines 32 to 33
proc transportProvider(): TcpTransport =
TcpTransport.new(upgrade = Upgrade())
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be inline or template or, my favorite, not exist (just use TcpTransport.new(upgrade = Upgrade()) directly).

Also, why is Upgrade() not the default for TcpTransport.new? There might be a reason but for anyone reading this code it just looks unnecessary.

Copy link
Member Author

@vladopajic vladopajic Nov 7, 2025

Choose a reason for hiding this comment

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

inline or template

this is just a test. and this kind of change here has neglectable impact. also tests in general don't have to have as performant code as possible unlike code in the library /libp2p/**.

why is Upgrade() not the default

qood question. it's required, but that decision is not introduced now, so we have to follow it here.

proc transportProvider(): TcpTransport =
TcpTransport.new(upgrade = Upgrade())

template tcpListenerIPTests(suiteName: string, address: string) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we have the tests listen on both ipv4 and ipv6 addresses? This way we'd only have one test and avoid this weird looking template

Copy link
Member Author

Choose a reason for hiding this comment

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

tests listen on both ipv4 and ipv6 addresses

not sure how you are thinking on implementing that. feel free to open pr with rough draft so we can see that approach.

this weird looking template

get used to that. this is how most tests are implemented in /tests/transports

Copy link
Contributor

Choose a reason for hiding this comment

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

image

@vladopajic vladopajic enabled auto-merge (squash) November 7, 2025 10:46
@github-project-automation github-project-automation bot moved this from new to In Progress in nim-libp2p Nov 7, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 95.10490% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.13%. Comparing base (b77f0bb) to head (65f63ae).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
tests/transports/tcp_tests.nim 95.07% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1850      +/-   ##
==========================================
+ Coverage   87.07%   87.13%   +0.06%     
==========================================
  Files         275      276       +1     
  Lines       42886    43026     +140     
  Branches       14       14              
==========================================
+ Hits        37342    37492     +150     
+ Misses       5544     5534      -10     
Files with missing lines Coverage Δ
tests/transports/testtcp.nim 100.00% <100.00%> (+4.26%) ⬆️
tests/transports/tcp_tests.nim 95.20% <95.07%> (ø)

... and 75 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vladopajic vladopajic merged commit f22169e into master Nov 10, 2025
16 of 17 checks passed
@vladopajic vladopajic deleted the tcp-ipv6 branch November 10, 2025 17:04
@github-project-automation github-project-automation bot moved this from In Progress to done in nim-libp2p Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants