Skip to content

test(pair): cover StalePrice and PriceNotFinalized from both fetch legs#192

Open
thedavidmeister wants to merge 2 commits into
mainfrom
2026-06-30-issue-58-pair-revert-paths
Open

test(pair): cover StalePrice and PriceNotFinalized from both fetch legs#192
thedavidmeister wants to merge 2 commits into
mainfrom
2026-06-30-issue-58-pair-revert-paths

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Closes #58

Summary

The pair op (LibOpFtsoCurrentPricePair) fetches symbolB (denominator) first, then symbolA (numerator), via two calls to LibOpFtsoCurrentPriceUsd.run. Issue #58 noted that only InactiveFtso was exercised; StalePrice, PriceNotFinalized, and DecimalsTooLarge paths had no tests.

This PR adds four revert-path tests:

  • testRunPairStalePriceFirstLeg — symbolB stale; only FTSO_B mocked (symbolA never reached)
  • testRunPairStalePriceSecondLeg — symbolB fresh, symbolA stale; both FTSOs mocked
  • testRunPairNotFinalFirstLeg — symbolB NOT_FINALIZED; only FTSO_B mocked
  • testRunPairNotFinalSecondLeg — symbolB finalized, symbolA NOT_FINALIZED; both FTSOs mocked

Key insight for first-leg tests: because the pair op's trampoline fetches symbolB first, any error on symbolB means symbolA is never queried. First-leg tests use mockRegistry(1) and omit all FTSO_A mocks — adding vm.expectCall for FTSO_A would create an unsatisfied expectation and cause spurious failures.

DecimalsTooLarge is not included here: the pair op delegates decimal handling to LibDecimalFloat, and the existing testRunPairZeroQuoteReverts / testRunPairZeroBaseIsZero already cover division-by-zero; a DecimalsTooLarge path would require fabricating an impossible decimal count from the mock FTSO, which is out of scope.

Test plan

  • All 14 LibOpFtsoCurrentPricePairTest tests pass locally
  • CI rainix-sol / test / test green (note: pre-existing fork tests using the public Ankr RPC are intermittently rate-limited on CI — not caused by this PR)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added coverage for paired price lookups to ensure revert reasons are preserved correctly.
    • Verified behavior when either leg is stale or not finalized, including both first- and second-leg failure paths.
    • Improved confidence that the app surfaces the correct error when one side of a paired price check cannot be used.

Adds four revert-path tests to LibOpFtsoCurrentPricePairTest.  The pair
op fetches symbolB (denominator) first then symbolA (numerator), so
first-leg tests only mock FTSO_B with mockRegistry(1) — FTSO_A is never
touched and must not have vm.expectCall set up.

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

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ed16171f-1540-407e-aced-cdf627a01fad

📥 Commits

Reviewing files that changed from the base of the PR and between cb20d03 and 68acfc2.

📒 Files selected for processing (1)
  • test/src/lib/op/LibOpFtsoCurrentPricePair.t.sol

Walkthrough

Four new test functions are added to LibOpFtsoCurrentPricePair.t.sol, covering StalePrice and PriceNotFinalized revert propagation through both the first leg (symbolB) and second leg (symbolA) of the pair execution flow. The StalePrice and PriceNotFinalized error types are also added to the existing import.

Changes

Pair op revert propagation tests

Layer / File(s) Summary
Error imports and StalePrice tests
test/src/lib/op/LibOpFtsoCurrentPricePair.t.sol
Import expanded to include StalePrice and PriceNotFinalized. testRunPairStalePriceFirstLeg mocks only symbolB as stale and asserts revert; testRunPairStalePriceSecondLeg mocks symbolB as fresh and symbolA as stale and asserts revert.
PriceNotFinalized tests for both legs
test/src/lib/op/LibOpFtsoCurrentPricePair.t.sol
testRunPairNotFinalFirstLeg sets symbolB finalization to NOT_FINALIZED without mocking current price and asserts revert; testRunPairNotFinalSecondLeg mocks symbolB as finalized and sets symbolA to NOT_FINALIZED and asserts revert.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR covers StalePrice and PriceNotFinalized on both legs, but it omits the required DecimalsTooLarge test from #58. Add the DecimalsTooLarge case for LibOpFtsoCurrentPricePair.run() and assert the expected selector, alongside the existing revert-path tests.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly names the pair-op revert-path coverage added for StalePrice and PriceNotFinalized on both fetch legs.
Out of Scope Changes check ✅ Passed The changes stay focused on pair-op revert-path test coverage and do not introduce unrelated code 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 2026-06-30-issue-58-pair-revert-paths

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.

Formatting: trailing double-space before // and multi-line vm.expectRevert.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

[F11] [MEDIUM] Pair op only tests InactiveFtso; StalePrice, PriceNotFinalized and DecimalsTooLarge paths are not exercised through run()

1 participant