Skip to content

chore: remove panic on overflow in release builds - #7559

Merged
LesnyRumcajs merged 1 commit into
mainfrom
dont-panic-fail-on-overflow
Aug 27, 2026
Merged

chore: remove panic on overflow in release builds#7559
LesnyRumcajs merged 1 commit into
mainfrom
dont-panic-fail-on-overflow

Conversation

@LesnyRumcajs

@LesnyRumcajs LesnyRumcajs commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary of changes

Changes introduced in this pull request:

  • we already fixed most prominent bugs with overflows, see fix: limit possible underflows/overflows #7459
  • this is a non-negligible perf cost in some hot loops (mentioned briefly in https://mara.nl/atomics/)
  • allowing overflows is the default anyway, both in Rust and other languages like Go
  • we do panic on abort (AFAIR due to some WASM things) so this completely obliterates the node in case it happens
  • even if we fix all possible under/overflows in Forest, our dependencies can still have them; most are pretty fast with fixes and releases, but, e.g., rust-libp2p had a last release... over a year ago. And we cannot do anything with its pinned versions (except for forking or checking every packet somehow ourselves).
  • Some CI checks could still benefit from this, though, to be tackled via Allow overflow checks in CI on release builds #7560

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • This pull request is based on an issue that a maintainer has accepted (see Before Opening a Pull Request).
  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

  • Chores
    • Simplified release build configuration while retaining thin link-time optimization.

@LesnyRumcajs
LesnyRumcajs requested a review from a team as a code owner August 27, 2026 09:46
@LesnyRumcajs
LesnyRumcajs requested review from akaladarshi and sudo-shashank and removed request for a team August 27, 2026 09:46
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 83221df7-dc0d-45a4-870b-982d85fb4f03

📥 Commits

Reviewing files that changed from the base of the PR and between 03126dc and f37e43a.

📒 Files selected for processing (1)
  • Cargo.toml
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus (manual)
💤 Files with no reviewable changes (1)
  • Cargo.toml

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


Walkthrough

The release profile removes explicit overflow-check settings, including the quinn-proto package override. Thin LTO remains enabled.

Changes

Release profile

Layer / File(s) Summary
Remove overflow-check overrides
Cargo.toml
The release profile no longer enables overflow checks explicitly. The quinn-proto package-specific override is removed. Thin LTO remains configured.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to f37e4

This is a narrowly scoped build-configuration change, and no actionable merge-blocking risk remains at the current head after normal checks and review.

Suggested reviewers: akaladarshi, sudo-shashank

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: release builds no longer panic on integer overflow.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dont-panic-fail-on-overflow
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch dont-panic-fail-on-overflow

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

@LesnyRumcajs
LesnyRumcajs enabled auto-merge August 27, 2026 10:22
@LesnyRumcajs
LesnyRumcajs added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 1afd7c8 Aug 27, 2026
34 checks passed
@LesnyRumcajs
LesnyRumcajs deleted the dont-panic-fail-on-overflow branch August 27, 2026 10:40
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.

3 participants