Fix staleness overflow: use subtraction instead of addition in stale check by thedavidmeister · Pull Request #125 · rainlanguage/rain.flare

thedavidmeister · 2026-06-16T18:52:35Z

Summary

priceTimestamp + timeout (or timestamp + timeout in the LTS path) can overflow uint256 when timeout is very large, causing a bare Panic(0x11) instead of a clean StalePrice revert. Overflow is especially easy to trigger on the LTS path where timestamp is uint64 (Flare's getFeedByIdInWei return type) while timeout remains uint256.

Replaced the addition-based check with subtraction:

// before
if (block.timestamp > priceTimestamp + timeout)

// after
if (block.timestamp > priceTimestamp && block.timestamp - priceTimestamp > timeout)

block.timestamp - priceTimestamp cannot overflow because any valid FTSO timestamp is ≤ the current block timestamp. A timeout so large that the subtraction form would never trigger is semantically "infinite" (price never stale), which is correct behaviour.

Files changed

src/lib/price/LibFtsoCurrentPriceUsd.sol — FTSO-v1 path
src/lib/lts/LibFtsoV2LTS.sol — FTSO-v2 LTS path
test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol — regression test (timeout = type(uint256).max)

Test plan

forge build — compiles cleanly.
New testRunTimestampPlusTimeoutOverflow fuzz test: restoring the addition-based check causes Panic(0x11), confirming the mutation is killed.

Closes #102
Closes #105

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved stale-price validation to avoid arithmetic overflow when evaluating timestamp + timeout.
- Preserved existing behavior for extremely large (effectively unlimited) timeout values.
- Ensures price lookups continue to work reliably even near the maximum uint256 timestamp.
Tests
- Added coverage for the overflow edge case to confirm USD price retrieval does not revert/panic under maximal timeout conditions.

…check priceTimestamp + timeout can overflow uint256 when timeout is very large, causing Panic(0x11) instead of a clean StalePrice revert. Use subtraction (block.timestamp - priceTimestamp > timeout) which cannot overflow because block.timestamp >= priceTimestamp for any valid timestamp from the FTSO. Also adds a regression test exercising timeout = type(uint256).max so that restoring the addition-based check causes the test to fail with Panic(0x11). Closes #102 Closes #105 Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2026-06-16T18:52:45Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1a0086ad-319b-4cc5-96fd-ee7d0ead6485

📥 Commits

Reviewing files that changed from the base of the PR and between 1b8048c and 119f4c0.

📒 Files selected for processing (1)

test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol

Walkthrough

FTSO staleness checks now avoid overflowing timestamp + timeout arithmetic in both libraries. A test covers maximal timeout handling at the maximum block timestamp.

Changes

FTSO staleness handling

Layer / File(s)	Summary
Subtraction-based stale-price validation `src/lib/lts/LibFtsoV2LTS.sol`, `src/lib/price/LibFtsoCurrentPriceUsd.sol`	Stale-price checks guard timestamp ordering and compare elapsed time against `timeout` without adding `timeout` to the feed timestamp.
Maximal-timeout validation `test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol`	Adds the price library import and tests a maximal timeout at `type(uint256).max` block time using mocked FTSO state.

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

Possibly related PRs

rainlanguage/rain.flare#190: Adds related FTSO staleness boundary tests.
rainlanguage/rain.flare#192: Tests propagation of StalePrice errors from FTSO price-pair fetches.

Suggested labels: ai:ready

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: changing stale checks to subtraction to avoid overflow.
Linked Issues check	✅ Passed	The PR addresses `#102` and `#105` by fixing overflow-prone stale checks in both paths and adding regression coverage.
Out of Scope Changes check	✅ Passed	The diff stays focused on the stale-check overflow fix and related tests, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/issue-102-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.}

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ss-overflow

thedavidmeister · 2026-07-07T16:22:31Z

🤖 ai:vetter
Reviewed 4993100: ready — Fix direction correct: subtraction form avoids uint256 overflow on the addition path; regression tes
cost 437 — oracle staleness-check semantics change, two libs plus test

thedavidmeister · 2026-07-09T21:24:25Z

🤖 ai:producer
Producer note: 3d hand-off — src/generated/FlareFtsoWords.pointers.sol conflicts (BYTECODE_HASH and DESCRIBED_BY_META_HASH differ on both sides). The PR intentionally regenerates the pointers file (its staleness fix changes bytecode) and main has also regenerated it since; the correct merged constants require re-running the build prelude against the merged source, which is not producer-resolvable locally. Merge aborted, branch untouched.

src/generated/FlareFtsoWords.pointers.sol resolved by regenerating against the merged source via script/build.sh (BuildAuthoringMeta + rain meta build + BuildPointers, run to convergence). Co-Authored-By: Claude <noreply@anthropic.com>

thedavidmeister · 2026-07-10T03:20:29Z

🤖 ai:vetter
Reviewed 36b7bc6: ready — closes #102 #105 — re-vet after merge-update head move, diff unchanged: subtraction staleness form correct on both v1+LTS paths, overflow reachable via rainlang timeout input, discriminating overflow test pinned, pointers regen green
cost 437 — oracle staleness semantics, two libs plus test

thedavidmeister · 2026-07-12T13:48:20Z

🤖 ai:producer
Blocked-infra: 3d merge conflict is in src/generated/FlareFtsoWords.pointers.sol (BYTECODE_HASH + DESCRIBED_BY_META_HASH differ on both sides — this PR intentionally regenerates pointers and main has since regenerated them too); correct merged constants require re-running the build prelude, and rain.flare's meta build is currently divergent local-vs-CI (see rain.flare#186 ai:blocked-infra: regenerated meta binaries differ deterministically from CI's), so a locally-regenerated resolution risks pushing wrong pins. Blocked on resolving that meta-toolchain divergence; merge was aborted, branch untouched (prior 3d note).

- test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol: union of both sides — the branch's testRunTimestampPlusTimeoutOverflow plus main's testRunNegativeTimeoutReverts/testRunFractionalTimeoutReverts, and both sides' imports. No test dropped. - src/generated/FlareFtsoWords.pointers.sol: BYTECODE_HASH regenerated from the merged source via the sanctioned prelude (nix develop -c rain-flare-prelude), not hand-picked from either side. Verified: forge test on the affected suite is 17/18, the only failure being the pre-existing FLARE_RPC_URL fork test that needs the CI secret. Co-Authored-By: Claude <noreply@anthropic.com>

thedavidmeister · 2026-07-29T07:55:30Z

🤖 ai:producer

Producer note: the two prior hand-offs on this PR are now superseded — the conflict IS resolved and the branch is pushed.

Only BYTECODE_HASH actually conflicted; DESCRIBED_BY_META_HASH is identical on both sides, so the earlier "both differ" reading no longer holds.
The pointer was regenerated from the merged source via the sanctioned prelude (nix develop -c rain-flare-prelude, which now runs script/Build.sol after main's rename), not hand-picked from either side.
LibOpFtsoCurrentPriceUsd.t.sol took the union of both sides: this branch's testRunTimestampPlusTimeoutOverflow plus main's negative/fractional timeout revert tests. No test dropped.
Verified locally: 17/18 on the affected suite; the sole failure is the pre-existing FLARE_RPC_URL fork test that needs the CI secret.

coderabbitai

Actionable comments posted: 1

🤖 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 `@test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol`:
- Around line 241-266: Update testRunTimestampPlusTimeoutOverflow to capture
both return values from LibFtsoCurrentPriceUsd.ftsoCurrentPriceUsd and assert
that the returned price and decimals match currentPrice.price and
currentPrice.decimals, while preserving the existing non-reverting overflow
scenario.

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 079c652b-6a67-4d0c-85ea-5e1f66a8c2e1

📥 Commits

Reviewing files that changed from the base of the PR and between b634aad and 1b8048c.

⛔ Files ignored due to path filters (1)

src/generated/FlareFtsoWords.pointers.sol is excluded by !**/generated/**

📒 Files selected for processing (3)

src/lib/lts/LibFtsoV2LTS.sol
src/lib/price/LibFtsoCurrentPriceUsd.sol
test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol

The overflow test proved only that the call does not panic; a wrong non-reverting result would still have passed. Capture both return values and pin them to the mocked price. Mutation-validated: swapping the return to (decimals, price) fails the test (assertion failed: 5 != 1); restoring it passes (256 runs). Co-Authored-By: Claude <noreply@anthropic.com>

…e [3b-attempt]

thedavidmeister · 2026-07-29T15:07:37Z

🤖 ai:vetter
Reviewed d23df28: ready — closes #102 #105 — re-vet after conflict-resolution head move: subtraction staleness form intact on both v1+LTS paths, test union verified (branch overflow test + main's negative/fractional timeout tests all present), pointers regenerated from merged source via sanctioned prelude and CI's artifact-diff gate is green; boundary semantics unchanged from base as the issues ask.
cost 437 — oracle staleness semantics, two libs

thedavidmeister self-assigned this Jun 16, 2026

thedavidmeister and others added 3 commits June 16, 2026 22:37

build: regenerate pointers/meta after bytecode change

24408fb

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into fix/issue-102-stalene…

d3f24bc

…ss-overflow

merge(main): resolve conflicts [merge-update]

4993100

thedavidmeister added the ai:ready AI vetter: passes review, ready for human decision label Jul 6, 2026

This was referenced Jul 9, 2026

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

Closed

fix(staleness): guard priceTimestamp+timeout overflow; treat as stale #179

Closed

merge(main): resolve conflicts [merge-update]

36b7bc6

src/generated/FlareFtsoWords.pointers.sol resolved by regenerating against the merged source via script/build.sh (BuildAuthoringMeta + rain meta build + BuildPointers, run to convergence). Co-Authored-By: Claude <noreply@anthropic.com>

thedavidmeister added ai:blocked-infra AI producer: blocked on an infra/tooling gap or can't classify (human) and removed ai:ready AI vetter: passes review, ready for human decision labels Jul 12, 2026

thedavidmeister removed the ai:blocked-infra AI producer: blocked on an infra/tooling gap or can't classify (human) label Jul 28, 2026

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread test/src/lib/op/LibOpFtsoCurrentPriceUsd.t.sol

thedavidmeister and others added 2 commits July 29, 2026 13:06

ci: empty commit to re-test the Flare fork RPC after the secret updat…

d23df28

…e [3b-attempt]

thedavidmeister added the ai:ready AI vetter: passes review, ready for human decision label Jul 29, 2026

Navigation Menu

Uh oh!

Fix staleness overflow: use subtraction instead of addition in stale check - #125

Fix staleness overflow: use subtraction instead of addition in stale check#125
thedavidmeister wants to merge 8 commits into
mainfrom
fix/issue-102-staleness-overflow

thedavidmeister commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

thedavidmeister commented Jul 7, 2026 •

edited

Loading

Uh oh!

thedavidmeister commented Jul 9, 2026

Uh oh!

thedavidmeister commented Jul 10, 2026

Uh oh!

thedavidmeister commented Jul 12, 2026

Uh oh!

thedavidmeister commented Jul 29, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

thedavidmeister commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thedavidmeister commented Jun 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Files changed

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

thedavidmeister commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thedavidmeister commented Jul 9, 2026

Uh oh!

thedavidmeister commented Jul 10, 2026

Uh oh!

thedavidmeister commented Jul 12, 2026

Uh oh!

thedavidmeister commented Jul 29, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thedavidmeister commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thedavidmeister commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

thedavidmeister commented Jul 7, 2026 •

edited

Loading