Skip to content

fix: treat priceTimestamp+timeout overflow as stale, not a panic - #147

Closed
thedavidmeister wants to merge 4 commits into
mainfrom
fix/issue-105-staleness-overflow
Closed

fix: treat priceTimestamp+timeout overflow as stale, not a panic#147
thedavidmeister wants to merge 4 commits into
mainfrom
fix/issue-105-staleness-overflow

Conversation

@thedavidmeister

Copy link
Copy Markdown
Collaborator

priceTimestamp + timeout in LibFtsoCurrentPriceUsd.ftsoCurrentPriceUsd
uses checked arithmetic (Solidity 0.8 default). A misbehaving or compromised
FTSO that reports a near-type(uint256).max timestamp causes Panic(0x11)
instead of the intended StalePrice domain error, making the failure harder to
diagnose.

The fix uses unchecked arithmetic and checks deadline < priceTimestamp
as the overflow sentinel, converting the overflow into a clean StalePrice
revert that callers already handle.

A new testRunStaleOverflow fuzz test covers the overflow boundary
(priceTimestamp > type(uint256).max - timeout) and verifies StalePrice
is emitted rather than a panic.

Closes #105

Co-Authored-By: Claude noreply@anthropic.com

priceTimestamp + timeout in checked arithmetic reverts with Panic(0x11)
when a misbehaving FTSO reports a near-uint256-max timestamp; the callers
receive an opaque panic instead of the intended StalePrice domain error.

Use unchecked arithmetic and check deadline < priceTimestamp to detect
the overflow, reverting cleanly with StalePrice in that case.

Adds testRunStaleOverflow fuzz test to cover the overflow boundary.

Closes #105

Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thedavidmeister, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 55 minutes and 6 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2d80734-fb51-4d38-afdc-3886aa54daab

📥 Commits

Reviewing files that changed from the base of the PR and between cb20d03 and 3a6f24e.

⛔ Files ignored due to path filters (1)
  • src/generated/FlareFtsoWords.pointers.sol is excluded by !**/generated/**
📒 Files selected for processing (2)
  • src/lib/price/LibFtsoCurrentPriceUsd.sol
  • test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-105-staleness-overflow

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

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

thedavidmeister and others added 3 commits June 16, 2026 22:21
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e unchecked [3b-attempt]

slither-disable-next-line only suppresses the line it precedes (unchecked {),
not the block.timestamp comparison inside the unchecked block. Switch to
disable-start/disable-end so the timestamp detector is suppressed for the
entire unchecked region.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister added the ai:ready AI vetter: passes review, ready for human decision label Jul 6, 2026
@thedavidmeister

thedavidmeister commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 ai:vetter
Reviewed 3a6f24e: ready — Unchecked overflow sentinel (deadline < priceTimestamp) is correct; fuzz test covers overflow bounda
cost 467 — unchecked overflow sentinel in oracle staleness check

@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #125 (kept). #125 fixes the same overflow via subtraction (block.timestamp > ts && block.timestamp - ts > timeout) across BOTH LibFtsoCurrentPriceUsd.sol (#105) and LibFtsoV2LTS.sol (#102), treating a large timeout or a future/large priceTimestamp as "never stale" (staleness is only about being too old). This PR reverts StalePrice on overflow — which mislabels a large timeout (a valid "never stale" config per #102) as stale. Keeping the single #125.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:ready AI vetter: passes review, ready for human decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[F54] [LOW] Stale-price boundary not exercised at uint256 overflow of priceTimestamp + timeout

1 participant