fix(p2p): improve address validation rules

[lklimek]

Issue being fixed or feature implemented

We have detected records with invalid IP addresses in the peer DB, hotfixed in #1243

What was done?

Stronger validation rules for peer and validator addresses.

How Has This Been Tested?

GHA

Breaking Changes

None

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

Summary by CodeRabbit

• New Features

  • Improved endpoint equality to better handle representation differences during address resolution.

• Bug Fixes

  • Stricter hostname validation: rejects hostnames with ports and many non-ASCII/emoji forms, preventing invalid addresses.

• Tests

  • Expanded test coverage for invalid hostname scenarios and for endpoint comparison/resolution edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds explicit hostname validation (new ValidateHostname and IsValidHostname) into p2p and validator address validation paths, adds Endpoint.Equal for precise endpoint comparisons, and updates tests to reject emoji/non-ASCII hostnames and relax resolution comparisons.

Changes

Cohort / File(s)	Summary
P2P Address Validation internal/p2p/address.go, internal/p2p/address_test.go	Call hostname validator from NodeAddress.Validate; tests remove emoji-hostname successes, add invalid emoji-hostname cases, and relax resolution assertions (use explicit IPv4 and Endpoint.Equal fallback).
Endpoint Equality internal/p2p/transport.go	Added func (e Endpoint) Equal(other *Endpoint) bool comparing Protocol, IP (via IP.Equal), Port, and Path for strict equality checks.
Validator Address Validation & Tests types/validator_address.go, types/validator_address_test.go	Added ValidateHostname(hostname string) error and IsValidHostname(hostname string) bool; integrated hostname validation into ValidatorAddress.Validate. Tests extended with IPv6 literal, standard hostnames, and various invalid bracket/port cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hop through names both short and long,

No sneaky ports where they don't belong,
I sniff each host for dots and dashes,
Match endpoints clean—no fuzzy clashes,
A tidy hop, validation song.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(p2p): improve address validation rules' accurately describes the main change: implementing stronger validation rules for peer and validator addresses in the p2p module.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/validate-address-host

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

types/validator_address.go (1)

120-133: Solid validation flow; minor note on error specificity.

The three-step check (reject embedded port → accept IP literal → validate DNS name) is clean and correct. One small observation: the error on Line 130 ("invalid hostname") could include the offending hostname value to aid debugging (e.g., fmt.Errorf("invalid hostname: %q", hostname)), but this is a nit.

Include hostname in error message for easier debugging

-	if !IsValidHostname(hostname) {
-		return errors.New("invalid hostname")
-	}
+	if !IsValidHostname(hostname) {
+		return fmt.Errorf("invalid hostname: %q", hostname)
+	}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@internal/p2p/address_test.go`:
- Around line 214-218: The test case in address_test.go uses the external
hostname "one.one.one.one", making the NodeAddress -> resolution path flaky;
update the test entry that constructs p2p.NodeAddress (the case comparing to
&p2p.Endpoint) to use a literal IP-only input (e.g., "1.1.1.1") or otherwise
provide the resolved net.IP directly so the resolver isn't invoked, or modify
the test to mock the resolver used by the address parsing code; target the
specific test case using the p2p.NodeAddress/ p2p.Endpoint values so the
behavior is deterministic without external DNS.

In `@internal/p2p/transport.go`:
- Around line 160-174: The Endpoint.Equal method can panic if the receiver
`other` is nil; add an early nil guard in Endpoint.Equal to return false when
`other == nil` before accessing other.Protocol (and also defensively handle
other.IP being nil before calling other.IP.Equal). Update the function
(Endpoint.Equal) to check `other == nil` first and ensure any subsequent
dereferences like other.IP are nil-checked to avoid nil pointer dereferences.

🧹 Nitpick comments (1)

internal/p2p/address.go (1)

183-193: Code duplication with types/validator_address.go.

This validateHostname function is nearly identical to the one in types/validator_address.go (lines 120-131). Consider consolidating into a single shared implementation to avoid maintenance overhead. Since internal/p2p already imports types, you could reuse or export the validation function from types.

♻️ Suggested approach

Either export a single ValidateHostname function from the types package and call it from both locations, or have internal/p2p/address.go directly call the types package helper:

 func validateHostname(hostname string) error {
-	if _, _, err := net.SplitHostPort(hostname); err == nil {
-		return errors.New("hostname must not include port")
-	}
-	if net.ParseIP(hostname) != nil {
-		return nil
-	}
-	if !types.IsValidHostname(hostname) {
-		return errors.New("invalid hostname")
-	}
-	return nil
+	return types.ValidateHostname(hostname) // after exporting from types
 }