fix(utils): use Go 1.20+ unsafe.String in UnsafeCastToString and add upfront overflow check in CalculateMaxSize

[yappermoar-boop]

Updated UnsafeCastToString in pkg/utils/bytes.go to use standard Go 1.20+ unsafe.String(unsafe.SliceData(byteSlice), len(byteSlice)) with zero-length check, replacing the legacy slice header cast. Also added upfront overflow guard in CalculateMaxSize before int64 addition.

Summary by CodeRabbit

• Bug Fixes
  • Improved internal byte conversion reliability.
  • Enhanced maximum-size calculations to handle overhead and integer limits more safely.
  • Reduced the risk of incorrect results caused by overflow or floating-point rounding.

[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: e5caa016-1764-480b-9f25-f4e3c145f4b9

📥 Commits

Reviewing files that changed from the base of the PR and between d67d9e8 and d079ca6.

📒 Files selected for processing (1)

• pkg/utils/bytes.go

💤 Files with no reviewable changes (1)

• pkg/utils/bytes.go

---

📝 Walkthrough

Walkthrough

The utility updates replace slice-header reinterpretation with unsafe.String in UnsafeCastToString. CalculateMaxSize now computes overhead with integer arithmetic and checks for int overflow before addition.

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

Suggested reviewers: alexanderbez, swarna1101

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title uses the required type and domain format, but it is 111 characters and exceeds the 72-character limit.	Shorten the title to 72 characters or fewer while preserving the main change.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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	The merge-base diff changes only pkg/utils/bytes.go and pkg/utils/calc.go, and both edits directly implement the stated unsafe string and integer overflow updates; no unrelated or dependency files...
Behavior Safety	✅ Passed	unsafe.String permits a nil pointer with zero length, and focused CalculateMaxSize tests cover normal, invalid, and overflow inputs; no unsafe semantic break is introduced.
Over-Engineering	✅ Passed	The PR changes only existing utility implementations. The diff adds no caches, helper layers, signature changes, or implementation-detail tests.
Security	✅ Passed	The diff only modernizes the byte-to-string cast and adds integer overflow validation; it introduces no injection path, credentials, weak crypto, or secret logging.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/utils/bytes.go`:
- Around line 18-21: Verify the repository targets Go 1.20 or newer, then add
focused tests for the byte-slice conversion function in pkg/utils/bytes.go
covering nil input, non-nil empty slices, and ordinary byte content. Assert only
the resulting string values; do not test pointer layout or aliasing.

In `@pkg/utils/calc.go`:
- Around line 15-20: Update the overflow validation in the calculation function
around maxAllowed and finalVal to compare the computed increase against
maxAllowed-src, allowing src = maxAllowed - maxAllowed/5 + 1 when the resulting
value fits. Perform this validation before adding to src, preserve rejection
when the increase would exceed the remaining capacity, and add a regression test
covering this boundary.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0ae61ca1-313d-4820-8e94-a973df452833

📥 Commits

Reviewing files that changed from the base of the PR and between 7936160 and eb518ac.

📒 Files selected for processing (2)

• pkg/utils/bytes.go
• pkg/utils/calc.go

[swarna1101]

Thanks for the PR - happy to check the UnsafeCastToString to Go 1.20+ unsafe.String.

For CalculateMaxSize, the upfront MaxInt - MaxInt/5 guard is redundant with the existing post-check and still relies on float math. Could you replace that part with integer-based overflow checking (overhead := src / 5 + pre-add bounds check)? That gives us the safety improvement without the extra guard.

Once updated, we will check

[yappermoar-boop]

Hi @swarna1101,
Thank you for the review and suggestions!
I have updated the PR based on your feedback:

1. CalculateMaxSize Refactoring: Replaced the float64 math and redundant guard in pkg/utils/calc.go with pure integer-based overhead calculation (overhead := src / 5) and a pre-add bounds check (src > maxAllowed - overhead).
2. Unit Tests for UnsafeCastToString: Added focused tests in pkg/utils/bytes_test.go covering nil slice, non-nil empty slice ([]byte{}), and ordinary byte contents.
3. Boundary Tests: Updated pkg/utils/calc_peers_test.go with boundary tests for maxAllowed - maxAllowed/5 and math.MaxInt overflow rejections.
   I've pushed the updated commit to the branch. Thank you!

[swarna1101]

Thanks for updating CalculateMaxSize to integer math.

Please slim the rest: drop bytes_test.go and the MaxInt boundary test (not needed for real call paths). For UnsafeCastToString, unsafe.String(unsafe.SliceData(...), len(...)) alone is enough, the empty-slice branch isn’t required. That should also clear the revive lint failure.

[yappermoar-boop]

Done! I have simplified the PR as requested:

1. UnsafeCastToString: Simplified to directly return unsafe.String(unsafe.SliceData(byteSlice), len(byteSlice)) without the empty-slice branch (which also clears the revive linter failure).
2. Removed Extra Tests: Dropped bytes_test.go and removed the MaxInt boundary test from calc_peers_test.go.
   I've force-pushed the updated commit to the branch. Thank you!

[Copilot]

Pull request overview

This PR modernizes and hardens two small helpers in pkg/utils. It replaces the legacy unsafe.Pointer slice-header cast in UnsafeCastToString with the standard Go 1.20+ unsafe.String/unsafe.SliceData API, and rewrites the overflow guard in CalculateMaxSize to use pure integer arithmetic (checking before the addition) instead of a float-based post-addition check. These utilities support message-ID hashing (consensus layer) and max message-size calculation (mump2p node setup).

Changes:

• Switch UnsafeCastToString to the idiomatic unsafe.String(unsafe.SliceData(...), len(...)) form.
• Rewrite CalculateMaxSize overflow detection with an upfront integer check (src > math.MaxInt - src/5) and integer overhead (src/5) instead of floating-point math.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pkg/utils/bytes.go	Replaces legacy pointer-cast with Go 1.20+ unsafe.String; behaviorally equivalent and zero-length safe.
pkg/utils/calc.go	Adds upfront integer overflow guard and integer-based overhead, avoiding float rounding before int64 addition.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[swarna1101]

LGTM

Dismissing — want to slim the PR further before merge