Skip to content

refactor(networks): store feeder and gateway URLs as *url.URL#3769

Merged
rodrodros merged 6 commits into
NethermindEth:mainfrom
Ehsan-saradar:feat/network-url-type
Jun 29, 2026
Merged

refactor(networks): store feeder and gateway URLs as *url.URL#3769
rodrodros merged 6 commits into
NethermindEth:mainfrom
Ehsan-saradar:feat/network-url-type

Conversation

@Ehsan-saradar

Copy link
Copy Markdown
Contributor

What

The Network type stored its feeder and gateway URLs as plain strings. Every consumer that needed an actual URL had to re-parse them, and since #3749 the feeder client takes a *url.URL, node startup parsed the string only to hand it straight back.

This changes Network.FeederURL and Network.GatewayURL to *url.URL and drops the redundant conversions:

  • Predefined networks build their URLs up front (via a small mustParseURL helper).
  • Custom-network URLs are parsed and validated once during config load (validateHTTPURL -> parseHTTPURL, returning the parsed URL).
  • node no longer re-parses the feeder URL before constructing the client.

No behavioural change.

Testing

  • go build ./cmd/juno/
  • go test ./blockchain/networks/... ./cmd/juno/... ./node/... ./clients/...

Closes #3761

The Network type kept its feeder and gateway URLs as plain strings, so
every consumer that needed an actual URL had to re-parse them. Since NethermindEth#3749
the feeder client takes a *url.URL, which meant node startup parsed the
string only to hand it straight back.

Store the URLs as *url.URL on the Network itself and drop the redundant
conversions. Predefined networks build their URLs up front, and custom
network URLs are parsed and validated once during config load.

Closes NethermindEth#3761
Copilot AI review requested due to automatic review settings June 27, 2026 06:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the networks.Network configuration to store feeder/gateway endpoints as parsed *url.URL values instead of raw strings, eliminating repeated parsing and simplifying client construction across the node and CLI config flow.

Changes:

  • Change networks.Network.FeederURL / GatewayURL from string to *url.URL, building predefined network URLs up-front.
  • Parse and validate custom-network URLs once during CLI/config load (validateHTTPURLparseHTTPURL returning *url.URL).
  • Remove redundant feeder URL parsing in node.New, and adapt gateway client initialization to use GatewayURL.String().

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
node/node.go Stops re-parsing feeder URL; uses stored *url.URL for feeder client and .String() for gateway client.
cmd/juno/juno.go Parses custom network URLs once and stores them as *url.URL in config.
cmd/juno/juno_test.go Updates tests to construct/compare URLs using *url.URL and .String().
blockchain/networks/network.go Updates Network URL fields to *url.URL and introduces mustParseURL for predefined networks.
blockchain/networks/network_test.go Updates assertions to compare URL string representations.
Comments suppressed due to low confidence (1)

node/node.go:368

  • cfg.Network.FeederURL / GatewayURL are now pointers; if a user selects a network without URLs (e.g. "sequencer") while running with seq-enable=false, this code will panic (GatewayURL.String() and feeder client URL usage). Add an explicit nil check here and return a configuration error instead of crashing.
		client = feeder.NewClient(cfg.Network.FeederURL).
			WithUserAgent(ua).
			WithLogger(logger).
			WithTimeouts(timeouts, fixed).
			WithAPIKey(cfg.GatewayAPIKey)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread blockchain/networks/network.go Outdated
Use url.ParseRequestURI in mustParseURL so a malformed or scheme-less
hardcoded network URL panics at startup instead of silently producing a
broken URL, and include the offending value in the panic message.
The gateway client still took a string, so node had to call
GatewayURL.String() — keeping the url->string round trip NethermindEth#3761 aimed to
remove. Make gateway.NewClient take *url.URL, mirroring the feeder client.

Also guard against nil feeder/gateway URLs in sync mode: the sequencer
network leaves them unset, so `--network sequencer` without --seq would
nil-panic on GatewayURL.String(). Return a clear error instead.
@Ehsan-saradar Ehsan-saradar force-pushed the feat/network-url-type branch from d9962f7 to 86e0dec Compare June 27, 2026 08:38
Comment thread blockchain/networks/network_test.go Outdated
Comment thread clients/gateway/gateway.go Outdated
Comment thread cmd/juno/juno_test.go Outdated
@ongyimeng

Copy link
Copy Markdown
Contributor

Thanks for your PR!

I left a few comments to tighten the refactor so the tests, URL handling and validation all consistently follow the new *url.URL contract.

Store Client.url as *url.URL and build the add_transaction path with JoinPath,
compare URLs as *url.URL in tests, and make mustParseURL enforce the same
http/https scheme and non-empty host checks as custom networks.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread node/node.go Outdated
Comment thread blockchain/networks/network.go Outdated
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.00%. Comparing base (682a0d2) to head (fd6a8e5).

Files with missing lines Patch % Lines
blockchain/networks/network.go 30.00% 4 Missing and 3 partials ⚠️
node/node.go 20.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3769      +/-   ##
==========================================
- Coverage   75.03%   75.00%   -0.04%     
==========================================
  Files         433      433              
  Lines       38594    38606      +12     
==========================================
- Hits        28960    28955       -5     
- Misses       7632     7645      +13     
- Partials     2002     2006       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: ongyimeng <73429081+ongyimeng@users.noreply.github.com>

@ongyimeng ongyimeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, LGTM!

Co-authored-by: Rodrigo <rodrodpino@gmail.com>
Signed-off-by: ongyimeng <73429081+ongyimeng@users.noreply.github.com>
@ongyimeng ongyimeng force-pushed the feat/network-url-type branch from 01a8301 to fd6a8e5 Compare June 29, 2026 10:56
@rodrodros rodrodros merged commit 1f66f48 into NethermindEth:main Jun 29, 2026
17 checks passed
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.

Make Network type store URLs as a *url.URL type instead of a string

4 participants