[Net] Turn net structures into dumb storage classes#1646
Merged
random-zebra merged 13 commits intoPIVX-Project:masterfrom Jun 15, 2020
Merged
[Net] Turn net structures into dumb storage classes#1646random-zebra merged 13 commits intoPIVX-Project:masterfrom
random-zebra merged 13 commits intoPIVX-Project:masterfrom
Conversation
21 tasks
f4fea01 to
5a67de0
Compare
Collaborator
Author
|
rebased now that #1640 has been merged |
furszy
reviewed
May 26, 2020
furszy
left a comment
There was a problem hiding this comment.
Code first impression ACK, overall is looking nice.
src/masternode.cpp
Outdated
There was a problem hiding this comment.
Would be good to pass the service by reference
random-zebra
left a comment
There was a problem hiding this comment.
Code ACK, looking pretty good. Couple irrelevant styling nits that can be safely ignored (or addressed later, maybe with a script).
Will give it some testing.
43089a9 to
a3c6a44
Compare
This command serves no real purpose, offers no checking that the address entered is even a MN (which it shouldn't), and is essentially just a stripped down version of the addnode command.
The checking of the port in masternode.conf files was overly complex. This simplifies it whilst maintaining that the port much match the default P2P port for the current network and removes the reliance on "magic numbers".
This also adds some sanity checking to masternode startup arguments: * -listen can't be 0 when -masternode is 1 * when -masternode is set, require that the client uses the default port * validate the port supplied (if any) for -masternodeaddr Lastly, `CMasternodeBroadcast::CheckDefaultPort` now takes a `CService` as it's first argument instead of a string.
Net functionality is no longer needed for CAddress/CAddrman/etc. now that CNetAddr/CService/CSubNet are dumb storage classes.
Also fix up a few small issues: - Lookup with "badip:port" now sets the port to 0 - Don't allow assert to have side-effects
apply clang-format
a3c6a44 to
b36f743
Compare
furszy
approved these changes
Jun 15, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of bitcoin#8128 as part of #1374. Based on top of #1640
Original Description:
Additional work has been done in the following commits: