networking: support private IPs#513
Merged
Merged
Conversation
Franziska-Mueller
approved these changes
Aug 21, 2025
| return; | ||
| } | ||
|
|
||
| if (isPrivateIp(address.u8)) |
Collaborator
There was a problem hiding this comment.
is the private IP not anyways already contained in the initial knownPublicPeers? if yes, the function will already return in line 459 for a private IP
J0ET0M
reviewed
Aug 21, 2025
| @@ -6964,7 +6964,8 @@ EFI_STATUS efi_main(EFI_HANDLE imageHandle, EFI_SYSTEM_TABLE* systemTable) | |||
| { | |||
| // randomly select verified public peers | |||
| const unsigned int publicPeerIndex = random(numberOfPublicPeers); | |||
Contributor
There was a problem hiding this comment.
could it be more efficient when we add a lower bound for the random function?
e.g. random(NUMBER_OF_PRIVATE_IP,numberOfPublicPeers)
to get only a random peer from the "public" list. this would avoid the extra check with isPrivateIp(..)
inline static unsigned int random_range(unsigned int min, unsigned int max)
{
unsigned int value;
_rdrand32_step(&value);
// Ensure range is valid
unsigned int range = max - min + 1;
return min + (value % range);
}
Contributor
Author
|
thanks, addressed both comments @Franziska-Mueller @J0ET0M |
J0ET0M
reviewed
Aug 21, 2025
| // randomly select verified public peers | ||
| const unsigned int publicPeerIndex = random(numberOfPublicPeers); | ||
| if (publicPeers[publicPeerIndex].isHandshaked /*&& publicPeers[publicPeerIndex].isFullnode*/) | ||
| // randomly select verified public peers and discard private IPs |
Contributor
There was a problem hiding this comment.
can this lead to an infinite loop when handshaked numberOfPublicPeers - NUMBER_OF_PRIVATE_IP < NUMBER_OF_EXCHANGED_PEERS
if yes we can also fill it with request->peers[j].u32 = 0;
Contributor
Author
There was a problem hiding this comment.
since we don't delete IP from bootstrap list numberOfPublicPeers should always > NUMBER_OF_PRIVATE_IP
Contributor
Author
There was a problem hiding this comment.
but I can add a check for future safety
hackerby888
added a commit
to hackerby888/qubic-core-lite
that referenced
this pull request
Aug 27, 2025
* Verify SC files automatically (qubic#506) * compliance changes for CCF SC * compliance changes for GQMPROP SC * compliance changes for MSVAULT SC * compliance changes for QEARN SC * compliance changes for QBAY SC * compliance changes for QUOTTERY SC * compliance changes for QUTIL SC * compliance changes for QX SC * Qx: add div type explicitly to fix compile errors in test project * compliance changes for TestExampleD SC * add contract verify workflow * Update contract-verify.yml * update branch name in contract-verify.yml * find all contract files to verify * fix typo in contract-verify.yml * print full path to file list * use list of contract files as input for verify action * only trigger contract-verify.yml when contract files or workflow file changed * use published action in contract-verify.yml * Revert "use published action in contract-verify.yml" This reverts commit 6fbd535. * mention contract verification tool in contracts.md * make QPI div and mod constexpr * update contract verify tool text in contracts.md * add STATIC_ASSERT macro to enable use of static asserts in SC files * remove workflow trigger on feature branch before merging into develop * contributing doc: add paragraph about curly braces style * update contract guidelines (qubic#512) * remove unused defines that clash with QPI definitions * fix: fixed gtest bug in qearn (qubic#516) * fix: fixed gtest bug in qearn * fix: removed unnecessary local variables in BEGIN_EPOCH procedure * update contract verify tool to v0.3.3-beta * Fix incorrect vc optimization of FourQ for release mode. (qubic#517) * Add unittest for fourq. * Fix MSVC misoptimization causing incorrect operation ordering. * Unittest: Ensure FourQ is initialized if signature verification is used in contract testing. * Add comment for optimization bug of MSVC. * networking: support private IPs (qubic#513) * nw: support private IPs * remove redundant code * fix warning * add checking boundary when accessing publicPeers * Fix bug relate to invalid mining seed of qpi mining. (qubic#518) * update params for epoch 176 / v1.257.0 * Increase target tick duration The delay function did not work due to bugs. Now the delay function works and tick time should decrease. --------- Co-authored-by: Franziska Mueller <11660876+Franziska-Mueller@users.noreply.github.com> Co-authored-by: TakaYuPP <yutotaka24@gmail.com> Co-authored-by: cyber-pc <165458555+cyber-pc@users.noreply.github.com> Co-authored-by: dkat <39078779+krypdkat@users.noreply.github.com> Co-authored-by: fnordspace <qubic@fnord.space>
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
4 participants
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.
#508
add
#define NUMBER_OF_PRIVATE_IP Xsuch that the node won't share first X IP addresses in the bootstrap list in exchangePeerAddress (handshake) packet, these IPs won't be deleted from public peer.Reason: allowing operators to have private connections to the node, easier to control nodes if it getting DDoS