fix(net): validate decoded peer IDs

[burak33bb]

Summary

AddressInfoFromString now parses the serialized peer ID with libp2p's peer.Decode instead of decoding the string as raw base58 bytes and casting the result to peer.ID.

That keeps the JSON round trip behavior the same for valid peer.AddrInfo values, while rejecting malformed peer IDs such as an empty peerID field.

Changes

• use peer.Decode when rebuilding peer.AddrInfo
• add regression coverage for an empty serialized peer ID
• remove the now-unused direct mr-tron/base58 dependency

Summary by CodeRabbit

• Bug Fixes

  • Improved peer ID parsing when processing network addresses.
  • Empty or malformed peer IDs are now rejected with a clear parsing error.
  • Network address information now preserves decoded peer identities more reliably.

• Tests

  • Added regression coverage to verify that empty peer IDs are rejected.
  • Updated address round-trip coverage for typed peer IDs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7e642d5d-2f1d-4aaf-aab9-d5a7af5a945a

📥 Commits

Reviewing files that changed from the base of the PR and between 2aced99 and 2394425.

📒 Files selected for processing (1)

• pkg/net/multiaddrutil_test.go

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

📝 Walkthrough

Walkthrough

AddressInfoFromString now parses serialized peer IDs with peer.Decode and assigns the decoded peer.ID directly. The unused base58 import was removed. Tests cover valid address round trips and rejection of an empty peerID.

Estimated code review effort: 2 (Simple) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 23944

The PR validates serialized peer IDs while preserving valid address round trips and adds coverage for malformed input; no actionable merge-blocking risk remains.

Suggested reviewers: abergasov, hpsing, swarna1101

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows the required format. It uses the valid type fix, the net domain, an imperative summary, and no trailing punctuation. It is 35 characters long and accurately describes the change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Scope Discipline	✅ Passed	PASS. The PR changes only go.mod, pkg/net/multiaddrutil.go, and its test file. The implementation change directly supports the stated peer-ID parsing goal. The test updates add focused regression …
Behavior Safety	✅ Passed	PASS: The behavior change is explicit and intentional. AddressInfoFromString now uses peer.Decode, returns a wrapped error when parsing fails, and assigns the validated peer.ID. The round-trip t…
Over-Engineering	✅ Passed	The change does not introduce an unnecessary cache, helper layer, or signature churn. It replaces direct base58 decoding with the existing peer.Decode API and removes the unused direct dependency. T…
Security	✅ Passed	PASS. The diff replaces raw base58 byte casting with peer.Decode, which adds peer-ID validation and rejects empty or malformed IDs. The change adds no credentials, secrets, cryptographic weakening, …

Full details: Scope Discipline

Explanation

PASS. The PR changes only go.mod, pkg/net/multiaddrutil.go, and its test file. The implementation change directly supports the stated peer-ID parsing goal. The test updates add focused regression coverage. The removed github.com/mr-tron/base58 dependency was used only by the replaced implementation. No unrelated files, abstractions, or new dependencies were added.

Full details: Behavior Safety

Explanation

PASS: The behavior change is explicit and intentional. AddressInfoFromString now uses peer.Decode, returns a wrapped error when parsing fails, and assigns the validated peer.ID. The round-trip test uses a decoded valid ID, and a focused regression test covers an empty peerID. The removed base58 import has no remaining source callers. No silent invariant break is evident.

Full details: Over-Engineering

Explanation

The change does not introduce an unnecessary cache, helper layer, or signature churn. It replaces direct base58 decoding with the existing peer.Decode API and removes the unused direct dependency. The added tests check observable behavior: valid round-trip parsing and rejection of an empty peer ID. They do not assert an implementation detail.

Full details: Security

Explanation

PASS. The diff replaces raw base58 byte casting with peer.Decode, which adds peer-ID validation and rejects empty or malformed IDs. The change adds no credentials, secrets, cryptographic weakening, logging, or manifest injection. JSON and multiaddr parsing remain structured, and the removed base58 dependency is unused in the changed tree.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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