Skip to content

Handle non-stale Pyth errors in multi feed oracle#235

Open
TUPM96 wants to merge 1 commit into
ST0x-Technology:mainfrom
TUPM96:codex/multipyth-rethrow-non-stale-34
Open

Handle non-stale Pyth errors in multi feed oracle#235
TUPM96 wants to merge 1 commit into
ST0x-Technology:mainfrom
TUPM96:codex/multipyth-rethrow-non-stale-34

Conversation

@TUPM96

@TUPM96 TUPM96 commented May 25, 2026

Copy link
Copy Markdown

Summary

  • Treat only the canonical PythErrors.StalePrice() revert as a stale-feed fallback in MultiPythOracleAdapter.
  • Revert with the original Pyth error data for non-staleness failures such as PriceFeedNotFound(), preserving the operational signal instead of masking it as AllFeedsStale().
  • Update the stale-feed fuzz mock to use the real Pyth stale selector and add regression coverage for bubbling a non-stale Pyth revert.

Fixes #34.

Validation

  • forge fmt --check src/concrete/oracle/MultiPythOracleAdapter.sol test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol
  • forge build
  • forge test --match-path test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol -vvv -> 3 passed
  • RPC_URL_BASE_FORK=https://mainnet.base.org forge test --match-path 'test/src/concrete/oracle/MultiPythOracleAdapter*.t.sol' -vv -> 38 passed
  • RPC_URL_BASE_FORK=https://mainnet.base.org FORK_BLOCK_BASE=38996123 forge test --no-match-contract ProdForkTest -> 122 passed

Note: full forge test currently has unrelated ProdForkTest failures against production fork assumptions; the suite above covers all non-production-fork tests plus the complete MultiPythOracleAdapter tests.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced Oracle adapter to intelligently handle price feed errors. Stale price data now triggers automatic fallback to the next available feed instead of causing transaction failure. Other error types continue to propagate appropriately.
  • Tests

    • Added test coverage for non-stale price feed error handling.

Review Change Stack

Copilot AI review requested due to automatic review settings May 25, 2026 09:30
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@TUPM96, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 56 minutes and 49 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5fad9492-854b-4797-802c-07934f2fcfa9

📥 Commits

Reviewing files that changed from the base of the PR and between 7a69052 and 3f70805.

📒 Files selected for processing (2)
  • src/concrete/oracle/MultiPythOracleAdapter.sol
  • test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol
📝 Walkthrough

Walkthrough

This PR implements selective Pyth error handling in MultiPythOracleAdapter._tryGetPrice by inspecting revert selectors: StalePrice allows loop fallthrough, while other errors (PriceFeedNotFound, OOG, future contract reverts) are rethrowed. Tests verify both behaviors.

Changes

Selector-based Pyth error handling

Layer / File(s) Summary
Selector-based error handling in _tryGetPrice
src/concrete/oracle/MultiPythOracleAdapter.sol
Imports PythErrors and decodes caught revert data to extract the 4-byte selector. Only PythErrors.StalePrice.selector returns (false, price) for loop fallthrough; all other errors rethrow the original revert data.
Test coverage for stale and non-stale Pyth errors
test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol
Adds PythErrors import, updates _mockStaleFeed helper to revert with StalePrice selector, and adds testNonStalePythRevertBubbles test to verify latestAnswer() bubbles up PriceFeedNotFound instead of masking it.

Possibly related issues

  • #34 — This PR directly implements the proposed fix for the security finding about try/catch swallowing all Pyth errors including misconfiguration and out-of-gas reverts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through error states,
Catching stale, but never gates,
Other reverts must make their way,
Shining bright in morning's day! 🐰✨

🚥 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 clearly and specifically describes the main change: handling non-stale Pyth errors in the multi-feed oracle adapter.
Linked Issues check ✅ Passed The PR fully implements issue #34: decoding revert selectors, treating only StalePrice as fallthrough, rethrowing other Pyth errors, and adding test coverage for non-stale errors.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #34: selective error handling in _tryGetPrice, mock stale behavior, and regression test for non-stale error propagation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates MultiPythOracleAdapter to treat Pyth “stale price” as a soft-failure while bubbling all other Pyth reverts unchanged, with accompanying test coverage.

Changes:

  • Bubble non-stale Pyth errors while continuing to suppress StalePrice as a (false, price) result.
  • Update stale-feed mocking to revert with PythErrors.StalePrice.
  • Add a test ensuring non-stale Pyth reverts (e.g., PriceFeedNotFound) bubble through latestAnswer().

Reviewed changes

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

File Description
test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol Updates stale mocking to use PythErrors and adds a revert-bubbling test.
src/concrete/oracle/MultiPythOracleAdapter.sol Adds selective handling for StalePrice and bubbles other revert payloads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +179 to +185
bytes4 selector;
assembly ("memory-safe") {
selector := mload(add(err, 0x20))
}
if (selector == PythErrors.StalePrice.selector) {
return (false, price);
}
Comment on lines +179 to +182
bytes4 selector;
assembly ("memory-safe") {
selector := mload(add(err, 0x20))
}
abi.encodeWithSelector(PythErrors.PriceFeedNotFound.selector)
);

vm.expectRevert(abi.encodeWithSelector(PythErrors.PriceFeedNotFound.selector));
@TUPM96 TUPM96 force-pushed the codex/multipyth-rethrow-non-stale-34 branch from 7a69052 to 3f70805 Compare May 25, 2026 09:33
@TUPM96

TUPM96 commented May 25, 2026

Copy link
Copy Markdown
Author

Pushed follow-up commit 3f70805 addressing Copilot review:

  • Added an err.length >= 4 guard before reading the revert selector.
  • Factored selector matching into _isStalePrice.
  • Updated the non-stale bubbling assertion to use selector-only expectRevert.

Revalidated:

  • forge fmt --check src/concrete/oracle/MultiPythOracleAdapter.sol test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol
  • forge build
  • forge test --match-path test/src/concrete/oracle/MultiPythOracleAdapter.fuzz.t.sol -vvv -> 3 passed
  • RPC_URL_BASE_FORK=https://mainnet.base.org forge test --match-path 'test/src/concrete/oracle/MultiPythOracleAdapter*.t.sol' -vv -> 38 passed
  • RPC_URL_BASE_FORK=https://mainnet.base.org FORK_BLOCK_BASE=38996123 forge test --no-match-contract ProdForkTest -> 122 passed

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.

[A09-1] [MEDIUM] try/catch in MultiPythOracleAdapter._tryGetPrice swallows all errors, masking non-staleness reverts

2 participants