feat: autonat support #1334
feat: autonat support #1334
Conversation
emizzle
left a comment
emizzle
left a comment
There was a problem hiding this comment.
Shaping up. Just a few suggestions so far
| askNewConnectedPeers = true, | ||
| numPeersToAsk = 5, | ||
| maxQueueSize = 10, | ||
| minConfidence = 0.1, |
There was a problem hiding this comment.
Not sure if this will work, but could be useful so maxQueueSize can be changed without worrying about having to change minConfidence correctly.
| minConfidence = 0.1, | |
| minConfidence = 1 div maxQueueSize, |
codex/nat/port_mapping.nim
Outdated
| return failure("No internal ports to be mapped were supplied") | ||
|
|
||
| strategy = initProtocols(strategy) | ||
| if strategy == PortMappingStrategy.None: |
There was a problem hiding this comment.
duplicate from line 253-254?
There was a problem hiding this comment.
Yeah, it would seem like that, but no. The initProtocols() will return None if it did not manage to initialize any of the protocols. So, need to double-check that edge case.
| except CancelledError as exc: | ||
| raise exc | ||
| except CatchableError as exc: | ||
| info "Failed to dial bootstrap nodes", err = exc.msg |
There was a problem hiding this comment.
Should be at least a warning, no? Or does this only impact relay?
There was a problem hiding this comment.
I guess we can change it to warn, but I guess there should be some errors coming from the DHT component when it can't actually bootstrap the connectivity to the bootstrap nodes...
codex/nat/port_mapping.nim
Outdated
| renewalThread = Thread[RenewelThreadArgs]() | ||
| renewalThread.createThread(renewPortMapping, (strategy, portMapping)) |
There was a problem hiding this comment.
Maybe we should utilise the taskpool started on the codex node, that is also used for erasure? This allows the user to set the number of threads from the command line.
Closes #1319
This PR:
ReachabilityManagerwhich drives the decision making to make the node reachable by other peers in the networkReachabilityManagerReachabilityManagerCurrent state of the PR is very WIP. Implementation is done, now make it compile and test it.
cc @emizzle