Skip to content
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

[ship-3979] remove ws validation #1318

Merged
merged 12 commits into from
Nov 13, 2024
Merged

Conversation

davidcauchi
Copy link
Contributor

@davidcauchi davidcauchi commented Nov 11, 2024

Adds the possibility for networks and Seth to not have WSs.

Possible to run with HTTP only or WS. WS can only run when provided HTTP also.


Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.

Why

The changes improve flexibility in network configuration, especially for environments where WebSocket (WS) endpoints may not be available. They enable Seth and the broader system to operate with only HTTP endpoints, aligning with practical deployment scenarios and enhancing compatibility and configuration validation.

What

  • lib/.changeset/v1.50.14.md
    • Added documentation for the new version, including updates on dependencies, and adjustments to network initialization and RPC URL validation to better support HTTP-only environments.
  • lib/config/network.go
    • Modified network validation logic to allow configurations with only HTTP endpoints and added checks to ensure WS endpoints are accompanied by HTTP endpoints.
  • lib/config/testconfig_test.go
    • Added unit tests for validating network configurations, ensuring the system behaves as expected with various combinations of HTTP and WS endpoints.
  • lib/go.mod & lib/go.sum
    • Updated dependencies, including the Seth version, to incorporate the latest changes and improvements.
  • lib/networks/known_networks.go & lib/networks/known_networks_test.go
    • Adjusted network setup logic and added tests to reflect the new capability of initializing networks with only HTTP endpoints.
  • lib/utils/seth/seth.go & lib/utils/seth/seth_test.go
    • Updated Seth utility functions to align with the changes in network configuration handling, including support for HTTP-only setups, and added tests to cover these scenarios.

@davidcauchi davidcauchi marked this pull request as draft November 12, 2024 09:50

// Ensure at least one of WS or HTTP URLs is available
if !wsOk && !httpOk {
return nil, fmt.Errorf("no RPC URLs found for '%s' network; at least one HTTP or WS RPC URL must be set", selectedNetworks[i])
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will fail if no ws endpoints are provided, no? why not use the same condition as in lib/config/network.go?

Copy link
Contributor Author

@davidcauchi davidcauchi Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it passes due to the !wsOk && !httpOk condition. But this does not take into consideration "only HTTP or both HTTP and WS are allowed". So yes I should use the same condition as in lib/config/network.go.

Will update thanks.

Copy link
Contributor

@Tofel Tofel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work!

@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
22.1% Duplication on New Code (required ≤ 5%)

See analysis details on SonarQube

@davidcauchi davidcauchi merged commit ed6103e into main Nov 13, 2024
46 of 49 checks passed
@davidcauchi davidcauchi deleted the ship-3979-remove-ws-validation branch November 13, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants