refactor: Decouple net config #2258
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2258 +/- ##
===========================================
+ Coverage 73.94% 74.05% +0.11%
===========================================
Files 256 256
Lines 25770 25748 -22
===========================================
+ Hits 19055 19066 +11
+ Misses 5389 5364 -25
+ Partials 1326 1318 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
net/config.go
Outdated
| ) | ||
|
|
||
| // Options is the node options. | ||
| type Options struct { | ||
| ListenAddrs []ma.Multiaddr | ||
| ListenAddress string |
There was a problem hiding this comment.
todo: We need to be able to listen on multiple addresses. For example, we may want to listen on 0.0.0.0 to expose a port externally but alse listen on 127.0.0.1 so that we can have more than one node locally talking to each other.
There was a problem hiding this comment.
Since one of the addresses will always be localhost could we just add an EnableLocalHost option here?
There was a problem hiding this comment.
That's an option. I'm wondering if there may also be a situation where someone would want to listen on multiple ports 🤔
There was a problem hiding this comment.
I've updated the config to support multiple addresses and set the default to localhost like we discussed.
| @@ -36,7 +36,10 @@ net: | |||
| # Whether the P2P is disabled | |||
| p2pdisabled: {{ .Net.P2PDisabled }} | |||
| # Listening address of the P2P network | |||
| p2paddress: {{ .Net.P2PAddress }} | |||
| p2paddresses: | |||
There was a problem hiding this comment.
thought: p2paddresses: {{ .Net.P2PAddresses }} would also work.
## Relevant issue(s) Resolves sourcenetwork#2263 ## Description This is part of a larger refactor of the config package. ## Tasks - [x] I made sure the code is well commented, particularly hard-to-understand areas. - [x] I made sure the repository-held documentation is changed accordingly. - [x] I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in [tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)). - [x] I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ... ## How has this been tested? make test Specify the platform(s) on which this was tested: - MacOS
Relevant issue(s)
Resolves #2263
Description
This is part of a larger refactor of the config package.
Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: