Price Feed Resilience: Add support for Pyth oracles - #193
Conversation
…ound tests, add checks on function call return values
📝 WalkthroughWalkthroughAdds IPyth interface and SafeCast helper; implements a new PythPriceFeeds submodule that queries and validates Pyth feed prices (single and two-feed div/mul flows); provides MockPyth and extensive Forge tests; wires PythPriceFeeds into an existing fork test for WETH/ETH-USD. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PythPriceFeeds
participant IPyth as PythContract
participant Validator as _validatePriceFeedResult
Client->>PythPriceFeeds: getOneFeedPrice(outputDecimals, params)
PythPriceFeeds->>PythPriceFeeds: decode params & validate inputs
PythPriceFeeds->>IPyth: staticcall getPriceNoOlderThan(priceId, age)
IPyth-->>PythPriceFeeds: returns Price(price, conf, expo, publishTime) / revert
PythPriceFeeds->>Validator: validate price, conf, publishTime
Validator-->>PythPriceFeeds: ok / revert
PythPriceFeeds->>PythPriceFeeds: enforce expo ≤ 0, scale/round to outputDecimals
PythPriceFeeds-->>Client: return scaled uint256 price
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 9. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/test/modules/PRICE/OlympusPricev1_2Fork.t.sol (1)
189-190: Address the TODO comment before merging.The TODO indicates pending cleanup to remove the Chainlink mock price feed. Consider tracking this in an issue if it's intentionally deferred.
Would you like me to open an issue to track the removal of the Chainlink mock price feed?
src/test/mocks/MockPyth.sol (1)
37-40: Potential underflow in stale price check.If
block.timestamp < age, the subtractionblock.timestamp - agewill underflow in Solidity 0.8+, causing a revert. While unlikely in practice (age is typically small), this differs from Pyth's actual behavior which handles this edge case.Consider adding a guard or noting this limitation:
♻️ Suggested fix
// Check if price is stale - if (priceData.publishTime < block.timestamp - age) { + if (age > block.timestamp || priceData.publishTime < block.timestamp - age) { revert StalePrice(); }src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceDiv.t.sol (1)
1149-1178: Consider extending fuzz range for outputDecimals.The test bounds
outputDecimals_to[18, 36]. While this avoids precision loss withEXPO_3 = -18, consider adding a separate test withoutputDecimals < 18using feeds with smaller absolute exponents to ensure the scaling logic works in both directions.src/modules/PRICE/submodules/feeds/PythPriceFeeds.sol (1)
396-411: Consider extracting shared parameter validation.The parameter validation logic for
TwoFeedParamsis duplicated betweengetTwoFeedPriceDiv(lines 339-352) andgetTwoFeedPriceMul(lines 398-411). Consider extracting to a private helper function.♻️ Suggested refactor
+ /// @notice Validates TwoFeedParams + function _validateTwoFeedParams(TwoFeedParams memory params) internal pure { + if (params.firstPyth == address(0)) revert Pyth_ParamsPythInvalid(0, params.firstPyth); + if (params.firstPriceFeedId == bytes32(0)) + revert Pyth_ParamsPriceFeedIdInvalid(1, params.firstPriceFeedId); + if (params.firstUpdateThreshold == 0) + revert Pyth_ParamsUpdateThresholdInvalid(2, params.firstUpdateThreshold); + if (params.firstMaxConfidence == 0) + revert Pyth_ParamsMaxConfidenceInvalid(3, params.firstMaxConfidence); + if (params.secondPyth == address(0)) revert Pyth_ParamsPythInvalid(4, params.secondPyth); + if (params.secondPriceFeedId == bytes32(0)) + revert Pyth_ParamsPriceFeedIdInvalid(5, params.secondPriceFeedId); + if (params.secondUpdateThreshold == 0) + revert Pyth_ParamsUpdateThresholdInvalid(6, params.secondUpdateThreshold); + if (params.secondMaxConfidence == 0) + revert Pyth_ParamsMaxConfidenceInvalid(7, params.secondMaxConfidence); + }Then replace the inline checks in both functions with
_validateTwoFeedParams(params);src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (1)
493-515: Test name may be slightly misleading.The test
test_getOneFeedPrice_confidenceEqualsMaximumtests withCONF_1 = 1000000which converts to 1e16, butMAX_CONFIDENCE = 2e16. This is actually testing confidence below maximum, not equal to maximum. Consider renaming or adjusting the test to truly test the boundary case.💡 Suggested fix
To truly test confidence at the maximum boundary, set confidence to exactly
2e6(which converts to2e16in output decimals):function test_getOneFeedPrice_confidenceEqualsMaximum() public { - // Confidence interval equals maximum threshold - // CONF_1 = 1000000 with expo=-8 converts to 1e16 in output decimals - // MAX_CONFIDENCE = 2e16, so CONF_1 should pass - // This test verifies that a confidence that is below the maximum passes - pyth.setPrice(PRICE_ID_1, PRICE_1, CONF_1, EXPO_1, block.timestamp); + // Set confidence exactly at maximum (2e6 in Pyth scale = 2e16 in output decimals) + uint64 exactMaxConf = 2e6; + pyth.setPrice(PRICE_ID_1, PRICE_1, exactMaxConf, EXPO_1, block.timestamp);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/interfaces/IPyth.solsrc/libraries/SafeCast.solsrc/modules/PRICE/submodules/feeds/PythPriceFeeds.solsrc/test/mocks/MockPyth.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/PythPriceFeedsTest.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceDiv.t.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceMul.t.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/supportsInterface.t.solsrc/test/modules/PRICE/OlympusPricev1_2Fork.t.sol
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
🔇 Additional comments (34)
src/libraries/SafeCast.sol (1)
28-33: LGTM!The
encodeUInt64function follows the established pattern of other safe cast functions in the library. The overflow check and revert behavior are consistent with the existing implementations.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/supportsInterface.t.sol (1)
12-37: LGTM!The test coverage for
supportsInterfaceis comprehensive, covering both supported interfaces (IERC165, IVersioned) and verifying that unsupported interfaces return false. The test structure is clean and follows established patterns.src/test/modules/PRICE/OlympusPricev1_2Fork.t.sol (2)
244-281: LGTM!The WETH asset configuration with Pyth feed is well-structured. The 24-hour update threshold and $10 max confidence (10e18 in 18 decimals) are reasonable values for ETH/USD price feed tolerance. The comment at line 268 correctly notes that the call will revert if the Pyth feed fails.
57-62: PYTH mainnet address is correct. The address0x4305FB66699C3B2702D4d05CF36551390A4c69C6matches the official Pyth Network deployment on Ethereum mainnet per Pyth Developer Docs.src/test/mocks/MockPyth.sol (1)
7-43: LGTM overall!The MockPyth implementation correctly replicates Pyth's interface and error semantics. The
setPricefunction provides flexibility for test scenarios, andgetPriceNoOlderThanappropriately validates price existence and staleness.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/PythPriceFeedsTest.sol (2)
60-92: LGTM!The test setup is well-organized with clear documentation of the test price values and their decimal representations. The timestamp warp and price feed initialization correctly establish the boundary conditions for staleness testing.
97-127: LGTM!The helper functions for encoding one-feed and two-feed parameters provide a clean abstraction for test cases. The encoding matches the expected parameter structure of
PythPriceFeeds.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceMul.t.sol (6)
15-44: LGTM!The success test case correctly validates the multiplication logic:
(first_price * second_price) / 10^outputDecimals. The expected calculation is well-documented in comments.
46-228: LGTM!Comprehensive parameter validation tests covering all edge cases for zero/invalid values across both feeds. The error expectations correctly match the parameter indices in the encoded params structure.
284-368: LGTM!The price feed not found and stale price tests correctly validate error conditions. The fuzz bounds for publish time properly test the stale price boundary condition.
370-650: LGTM!Thorough testing of first feed exponent handling scenarios. The tests correctly verify:
- Positive exponents trigger
Pyth_ExponentPositiveerror- Negative/zero exponents correctly scale prices to output decimals
- Confidence thresholds are properly validated in Pyth's scale
682-967: LGTM!Second feed exponent tests mirror the first feed tests, ensuring symmetric coverage. The calculations and expected values are correctly documented.
999-1027: LGTM!The output decimals fuzz test provides good coverage for different decimal configurations. The bounds [18, 36] are appropriate to avoid underflow in exponent calculations, and the max confidence scaling correctly adapts to the output decimals.
src/interfaces/IPyth.sol (1)
13-18: ThepublishTimefield in thePricestruct is correctly declared asuint256, matching Pyth Network's official interface.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceDiv.t.sol (7)
1-17: LGTM - Test file setup and imports are correct.The test contract properly extends
PythPriceFeedsTestand imports necessary dependencies includingFullMathfor precise calculations andMockPythfor simulating price feed behavior.
22-41: LGTM - Success test correctly validates two-feed division.The test properly sets up two feeds with different exponents and validates the division calculation using
mulDivfor precision.
43-65: LGTM - Edge case for zero denominator handled correctly.The test properly validates that when the denominator price rounds to zero due to exponent scaling, the function returns 0 without reverting, matching the implementation's early exit behavior.
67-249: LGTM - Comprehensive parameter validation tests.All eight parameters of
TwoFeedParamsare tested for zero/invalid values with correct error selectors and parameter indices (0-3 for first feed, 4-7 for second feed).
251-341: LGTM - Feed validation tests cover invalid prices and missing feeds.The fuzz tests properly bound prices to invalid ranges (≤0), and the price feed not found tests correctly expect
MockPyth.PriceFeedNotFoundfor unregistered feed IDs.
343-389: LGTM - Stale price tests correctly validate freshness boundaries.The fuzz tests properly bound publish times to stale values and expect
MockPyth.StalePriceerrors for both feeds.
391-536: LGTM - First feed exponent and confidence tests are thorough.The tests correctly validate confidence interval calculations across different exponent scenarios including negative, zero, and positive exponents. The boundary cases for confidence at/below maximum are properly tested.
src/modules/PRICE/submodules/feeds/PythPriceFeeds.sol (8)
1-18: LGTM - License, pragma, and imports are appropriate.The contract uses AGPL-3.0 license, requires Solidity ≥0.8.15 for overflow protection, and imports necessary interfaces and libraries.
25-141: LGTM - Well-structured parameter definitions and comprehensive error handling.The struct definitions properly separate single and dual feed configurations. Error declarations include parameter indices for easier debugging and appropriate types for all fields.
173-202: LGTM - Validation logic is correct and complete.The function properly validates positive price, publish time freshness, and confidence bounds. The
purevisibility is appropriate since all data comes from parameters.
213-284: LGTM - Feed price retrieval is well-implemented with proper error handling.The function correctly:
- Uses
staticcallfor view-safe external calls- Validates return data length before decoding
- Rejects positive exponents to prevent precision loss
- Converts confidence and price between Pyth scale and output decimals scale
- Uses
FullMath.mulDivfor overflow-safe calculations
295-319: LGTM - Single feed price function is clean and correct.Parameter validation covers all four fields with appropriate error indices. The unused first parameter follows the expected interface pattern.
332-378: LGTM - Two-feed division correctly handles zero denominator.The function properly validates all parameters, handles the zero denominator edge case gracefully, and uses
mulDivfor precision-safe division with proper scaling.
391-433: LGTM - Two-feed multiplication is correctly implemented.The function properly validates parameters and uses
mulDivto compute the product while maintaining the correct decimal scale.
441-444: LGTM - Standard ERC165 interface support.The implementation correctly reports support for
IERC165andIVersionedinterfaces.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (5)
1-14: LGTM - Test file imports and structure are correct.The test contract properly extends
PythPriceFeedsTestand imports necessary dependencies.
18-100: LGTM - Success and parameter validation tests are comprehensive.All four parameters are tested for zero/invalid values with correct error selectors and parameter indices.
102-231: LGTM - Feed validation tests cover all error paths.Tests properly validate:
- Invalid Pyth contract detection (no code at address)
- Wrong return data length detection
- Error bubbling from underlying Pyth calls
- Invalid price values (≤0)
- Missing price feeds
- Stale price data
252-491: LGTM - Exponent handling tests are thorough.Tests cover all exponent scenarios including negative, zero, positive (revert), equal to negative output decimals, and very negative values. Confidence interval calculations in expected errors are correctly computed.
591-614: LGTM - Output decimals fuzz test correctly handles variable precision.The test appropriately bounds output decimals to avoid precision loss and correctly scales the expected values based on the exponent difference.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (1)
489-512: Consider adding explicit boundary test for confidence equals maximum.The fuzz test
test_confidenceBelowMaximumbounds confidence to[1, 2e6], which could occasionally hit the exact boundary. However, the two-feed tests have explicitconfidenceEqualsMaximumtests while this file lacks one. Adding an explicit boundary test liketest_confidenceEqualsMaximum(similar to Line 445-471 ingetTwoFeedPriceDiv.t.sol) would strengthen boundary condition coverage.🔧 Suggested test to add
// given the confidence interval is at the maximum (boundary case) // [X] it correctly converts the price function test_confidenceEqualsMaximum() public { // expo = -8, outputDecimals = 18 // confidenceExponent = 18 + (-8) = 10 // maxConfidenceInPythScale = maxConfidence / 10^10 = 2e16 / 1e10 = 2e6 // Test with confidence exactly at the maximum (2e6) uint64 priceConfidence_ = 2e6; pyth.setPrice(PRICE_ID_1, PRICE_1, priceConfidence_, EXPO_1, block.timestamp); bytes memory params = encodeOneFeedParams( address(pyth), PRICE_ID_1, UPDATE_THRESHOLD, MAX_CONFIDENCE ); uint256 priceInt = pythSubmodule.getOneFeedPrice(address(0), PRICE_DECIMALS, params); assertEq( priceInt, EXPECTED_PRICE_1_18_DEC, "Price should match expected when confidence equals maximum" ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceDiv.t.solsrc/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceMul.t.sol
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cursor Bugbot
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
🔇 Additional comments (13)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceDiv.t.sol (4)
1-16: LGTM - Well-structured test file with comprehensive coverage.The test contract properly extends
PythPriceFeedsTestand usesFullMathfor precise arithmetic operations. The test organization follows a clear pattern covering success cases, parameter validation, and edge cases.
251-276: Good fuzzing coverage for invalid price scenarios.The fuzz test properly bounds the price to
type(int64).minto0inclusive, ensuring all non-positive values trigger thePyth_FeedPriceInvaliderror. This catches both zero and negative prices.
1143-1171: Comprehensive output decimals fuzzing with proper scaling.The test correctly scales
MAX_CONFIDENCEproportionally when changing output decimals, and properly calculates expected values using the formulaprice * 10^(outputDecimals - expo). The bound of[18, 36]prevents overflow while testing meaningful ranges.
43-65: The test correctly validates intentional behavior. ThegetTwoFeedPriceDivfunction explicitly returns 0 when the denominator price is zero, as documented in the implementation with comments stating "The PRICE module will handle the zero value." This is a deliberate architectural design where zero handling is delegated to the PRICE module, not a hidden edge case.src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getTwoFeedPriceMul.t.sol (4)
1-44: LGTM - Multiplication test correctly calculates expected result.The success test properly computes the expected result as
(first * second) / 10^outputDecimals:
- First feed:
1234567890000000000(1.23456789 in 18 decimals)- Second feed:
500000000- Result:
1234567890000000000 * 500000000 / 10^18 = 617283945000The
mulDivusage with denominator10^PRICE_DECIMALScorrectly implements this formula.
46-228: Parameter validation tests are thorough and correct.All eight parameters (2 feeds × 4 params each) have validation tests with correct error selector indices matching their positions in the encoded params. This ensures comprehensive input validation coverage.
556-583: LGTM - Correctly tests precision loss when product is very small.When
first feed = 1e9(expo=-18) andsecond feed = 500000000, the product divided by10^18rounds down to 0. The expected value calculation1e9 * 500000000 / 10^18 = 0is correct, and this edge case is important for understanding precision limitations.
1023-1115: Good coverage of precision loss scenarios.The tests comprehensively cover scenarios where:
- Price < 1 in the scaled representation loses precision (Line 1026)
- Price rounds down to zero entirely (Line 1059)
- Price >= 1 but fractional part is lost (Line 1087)
This helps users understand the precision characteristics of the multiplication operation.
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (5)
1-34: LGTM - Clean test structure with proper success case verification.The success test correctly validates that price
123456789withexpo=-8converts to1234567890000000000in 18 decimals (multiplied by10^10).
124-159: Excellent test for return data validation.This test verifies the implementation correctly rejects malformed return data by mocking a successful call that returns only 64 bytes instead of the expected 128 bytes (
_PRICE_DATA_SIZE). This protects against oracle implementations returning truncated or malformed data.The
vm.clearMockedCalls()cleanup at Line 158 is good practice.
161-193: Good test for error propagation.Using
vm.mockCallRevertto verify that custom errors from the Pyth contract bubble up correctly is important for debugging in production. This ensures callers receive meaningful error information rather than generic failures.
563-585: Smart fuzzing bounds prevent false negatives from precision loss.The bound
[8, 36]foroutputDecimals_ensures the exponent difference is always non-negative (sinceexpo=-8), so precision loss doesn't interfere with the fuzz test's purpose of verifying decimal conversion correctness. The precision loss scenarios are tested separately in dedicated tests below.
587-651: Thorough precision loss test coverage.The three precision loss tests comprehensively cover:
- Price < 1:
23456789(0.23456789) →234567in 6 decimals- Price rounds to 0:
89(0.00000089) →0in 6 decimals- Price >= 1:
123456789(1.23456789) →1234567in 6 decimalsThese tests document the expected precision behavior for downstream consumers.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| ); | ||
|
|
||
| return feedPrice; | ||
| } |
There was a problem hiding this comment.
Missing outputDecimals bounds validation causes unclear overflow panics
Low Severity
The PythPriceFeeds functions (getOneFeedPrice, getTwoFeedPriceDiv, getTwoFeedPriceMul) don't validate that outputDecimals_ is within bounds before computing 10 ** uint256(outputDecimals_). The comparable ChainlinkPriceFeeds implementation validates outputDecimals_ <= BASE_10_MAX_EXPONENT (50) and reverts with a clear error if exceeded. Without this check, extreme outputDecimals_ values (> ~77) cause the exponentiation to overflow uint256, resulting in a generic panic instead of a descriptive error message. This creates inconsistent error handling between the two price feed implementations.
Additional Locations (2)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (2)
124-193: Mock lifecycle: consider clearing mocks consistently aftermockCallReverttoo.
You clear intest_returnDataLengthInvalid_revertsbut not aftertest_pythRevertsWithData_bubblesUpRevert; depending on forge-std behavior, this can reduce any chance of cross-test interference.Proposed diff
function test_pythRevertsWithData_bubblesUpRevert() public { @@ pythSubmodule.getOneFeedPrice(address(0), PRICE_DECIMALS, params); + + vm.clearMockedCalls(); }
252-285: Hardcoded “derived” confidence thresholds are a bit brittle.
Values like2e6,2e16,2e18,10are correct per the comments, but will silently desync ifMAX_CONFIDENCE,PRICE_DECIMALS, or expo conventions change; consider a small helper to compute “maxConfInPythScale” in one place.Also applies to: 333-366, 386-469, 489-538
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cursor Bugbot
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
🔇 Additional comments (3)
src/test/modules/PRICE.v2/submodules/feeds/PythPriceFeeds/getOneFeedPrice.t.sol (3)
1-34: Nice, tight “happy path” coverage;viewis appropriate here.
This sets the baseline expected scaling behavior clearly.
36-123: Param validation + “non-IPyth” negative path looks solid.
Minor: using an EOA-like address (address(0x1234)) can be slightly less explicit than deploying a dummy contract that intentionally fails interface checks, but it’s still fine if the module treats “no code” as invalid.
588-676: Output-decimals coverage is strong (precision loss + round-to-zero cases are especially valuable).
The max-confidence rescaling in params matches the intent of testing non-18 decimal outputs.
1 participant
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Introduces Pyth oracle support and plugs it into the PRICE module with strict validation.
IPythinterface andPythPriceFeedssubmodule (PRICE.PYTH) exposinggetOneFeedPrice,getTwoFeedPriceDiv, andgetTwoFeedPriceMulupdateThreshold, confidence bound (scaled across decimals), non-positiveexpo, and return-size checks; uses low-levelstaticcallSafeCast.encodeUInt64used for confidence scalingMockPythand comprehensive unit tests covering params, errors, exponent/rounding, division/multiplication, andsupportsInterfacePythPriceFeedsalongside Chainlink, configures WETH with real ETH/USD Pyth feedWritten by Cursor Bugbot for commit 89121b0. This will update automatically on new commits. Configure here.