-
Notifications
You must be signed in to change notification settings - Fork 66
chore(autonat-v2): add utils #1657
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
Conversation
7ca8dbc to
da5c8a6
Compare
🏁 Performance SummaryCommit:
📊 View Latency History and full Container Resources in the Workflow Summary |
| ../../../peerid, | ||
| ../../../protobuf/minprotobuf, | ||
| ../../../switch, | ||
| ../autonat/service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably makes more sense to extract from service the things that are common to autonat v1 and v2 into some file that's shared by both that file and this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder where that file would live
- in
libp2p/protocols/connectivity/file.nim - in
libp2p/protocols/connectivity/autonat/file.nim(in this caseautonatv2/should live insideautonat/, andautonatv1should also be insideautonat/) - Same as 2 but without creating a new
autonatv1folder. Which is the same as just renamingcore.nimtotypes.nimand importing it in v2.
I'll go with 3: change core.nim to types.nim in autonat/ and import autonat/types in v2.
| ## Checks if at least one of the requests' addresses | ||
| ## has the same IP address of observedIPAddr | ||
|
|
||
| let observedIPAddr = observedIPAddr[0].valueOr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no guarantee that the first element of the address is gonna be an ip (i.e. /p2p/somePeerId is a valid multiaddress). Look into how observedaddrmanager extracts IP for inspiration. https://github.com/vacp2p/nim-libp2p/blob/master/libp2p/observedaddrmanager.nim#L30-L67
|
|
||
| (ipv4, ipv6) | ||
|
|
||
| proc isPrivateIP*(ip: string): bool {.raises: [ValueError].} = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AutoTLS has a isPublic. Could we maybe put both that function and this one in some utils file?
b660988 to
69c524a
Compare
20135bb to
efd88bd
Compare
efd88bd to
e0e2942
Compare
Add util functions needed for autonatv2