-
Notifications
You must be signed in to change notification settings - Fork 39
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
fix(dapi): invalid mainnet seed ports #2173
Conversation
WalkthroughThe changes involve the modification of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NetworkConfig
participant SeedNode
Client->>NetworkConfig: Request mainnet configuration
NetworkConfig->>SeedNode: Provide seed node addresses
SeedNode-->>NetworkConfig: Return hostnames without port
NetworkConfig-->>Client: Send updated seed node addresses
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/js-dapi-client/lib/networkConfigs.js (1)
Line range hint
3-33
: Consider reviewing testnet configuration and updating commentsWhile the changes to the mainnet configuration are correct, I noticed a couple of points that might need attention:
The testnet configuration still includes the port 1443 for its seed addresses. This is consistent with the PR's focus on mainnet changes, but it might be worth considering if a similar update is needed for testnet in the future.
There's a comment about the absence of PoSe (Proof of Service) and the use of hardcoded DCG testnet masternodes. It might be beneficial to review if this comment is still relevant or if it can be updated or removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/js-dapi-client/lib/networkConfigs.js (1 hunks)
🔇 Additional comments (2)
packages/js-dapi-client/lib/networkConfigs.js (2)
Line range hint
1-62
: Overall assessment: Changes are correct and focusedThe modifications to remove the port specification from mainnet seed addresses are implemented correctly and consistently. The changes align with the PR objectives and don't introduce any apparent issues. The file structure and other configurations remain intact, maintaining the overall functionality of the network configurations.
56-59
: LGTM: Port removal from mainnet seed addressesThe changes correctly remove the
:1443
port specification from all mainnet seed addresses, aligning with the PR objective to revert to the default port configuration. This modification is consistent across all four mainnet seed entries.To ensure this change doesn't introduce any unintended consequences, let's verify if there are any other occurrences of hardcoded ports for mainnet:
✅ Verification successful
Hardcoded Ports Removal Verified Successfully
All mainnet seed addresses are free from hardcoded ports, aligning with the PR objective and ensuring default port configurations. No further hardcoded ports were found in the mainnet configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining hardcoded ports in mainnet configuration # Test: Search for any remaining port specifications in mainnet config rg --type js 'mainnet.*:\d+' packages/js-dapi-client/lib/networkConfigs.jsLength of output: 76
Issue being fixed or feature implemented
Invalid mainnet seed node ports
What was done?
How Has This Been Tested?
None
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit