Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds many PRE/POST variants for US equity price feeds in LibPyth, updates symbol→ID and contract lookup mappings, introduces new fork-base constants (CRCL, PPLT, BMNR, RKLB) in tests, and expands test coverage and expectations to exercise the new variants and fork scenarios. Changes
Sequence Diagram(s)(Skipped — changes are constant/mapping additions and test updates; no multi-component control flow requiring a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (1)
167-208: Refactorvm.assumemega-predicate to a helper using the LibPyth symbol constants.
Current form is brittle and easy to desync when adding new feeds.contract LibPythGetPriceFeedIdTest is Test { + function _isKnownSymbol(uint256 u) internal pure returns (bool) { + return u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_ARB_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_BTC_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_WBTC_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_CBBTC_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_DOT_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_ENA_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_ETH_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_WETH_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_WSTETH_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_LINK_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_PEPE_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_PYTH_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_UNI_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_XAUT_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_CRYPTO_XRP_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_COIN_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_COIN_USD_PRE + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_COIN_USD_POST + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_GOOG_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_AMZN_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_AMZN_USD_PRE + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_AMZN_USD_POST + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_AAPL_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_MSFT_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_TSLA_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_TSLA_USD_PRE + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_TSLA_USD_POST + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_NVDA_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_NVDA_USD_PRE + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_NVDA_USD_POST + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_META_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_GME_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_MSTR_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_MSTR_USD_PRE + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_MSTR_USD_POST + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_BRK_B_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_SPLG_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_IAU_USD + || u == LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_SIVR_USD; + } + function testPriceFeedIdUnknownMappings(IntOrAString symbol) external { - vm.assume( - IntOrAString.unwrap(symbol) != IntOrAString.unwrap(LibIntOrAString.fromString2("Crypto.ARB/USD")) - && IntOrAString.unwrap(symbol) != IntOrAString.unwrap(LibIntOrAString.fromString2("Crypto.BTC/USD")) - ... - ); + uint256 u = IntOrAString.unwrap(symbol); + vm.assume(!_isKnownSymbol(u)); vm.expectRevert(UnsupportedFeedSymbol.selector); this.getPriceFeedIdExternal(symbol); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/lib/pyth/LibPyth.sol(5 hunks)test/lib/LibFork.sol(1 hunks)test/src/lib/pyth/LibPyth.constants.t.sol(5 hunks)test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol(5 hunks)test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
Repo: rainlanguage/rain.pyth PR: 3
File: src/lib/pyth/LibPyth.sol:59-79
Timestamp: 2025-10-03T13:50:23.519Z
Learning: In the rain.pyth repository, the IntOrAString encoding scheme supports custom discriminator bytes. While most equity feed symbols use 0x92 as the discriminator byte, custom values like 0x91 are intentionally used for specific feeds such as EQUITY_US_GME_USD.
Applied to files:
src/lib/pyth/LibPyth.sol
⏰ 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). (5)
- GitHub Check: deploy
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: git-clean
🔇 Additional comments (5)
test/src/lib/pyth/LibPyth.constants.t.sol (1)
80-87: Constant encoding assertions look solid for PRE/POST variants.
Good coverage to ensure theIntOrAStringliterals matchfromString2(...).Also applies to: 96-103, 116-123, 128-135, 148-155
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (1)
73-80: PRE/POST mapping tests: good additions.
These directly validate the new branches inLibPyth.getPriceFeedId.Also applies to: 89-96, 109-116, 121-128, 141-148
test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (1)
180-200: Forked price expectations: please verify determinism atFORK_BLOCK_BASE = 39366248, especially PRE/POST within 24h.
This is a good coverage expansion, but it will fail hard if any feed isn’t published within the 24h window at that exact block.Also applies to: 216-248, 264-278, 301-315
src/lib/pyth/LibPyth.sol (1)
115-124: Verify PRE/POST feed IDs against Pyth official source and confirm chain deployment.The new
PRICE_FEED_ID_EQUITY_US_*_USD_PRE/POSTconstants require validation against the official Pyth Price Feed IDs page (via Pyth docs or Hermes API). Ensure all six symbols (AMZN, TSLA, NVDA, MSTR, COIN) are deployed on Base and Arbitrum chains—if any feed ID is incorrect or unavailable on a target chain, price reads will revert or return wrong data. Regarding the IntOrAString discriminator bytes (0x96/0x97): custom discriminator bytes are intentionally used in this repo (e.g., 0x91 for EQUITY_US_GME_USD per the codebase learnings), so confirm these values align with your encoding scheme.Also applies to: 143-152, 159-168, 187-196, 222-231
test/lib/LibFork.sol (1)
8-9: Add documentation for fork block choice on Base.The fork block (39366248, mined Dec 12, 2025) is very recent. Add a comment explaining why this specific block was selected—e.g., "first block where Equity.US.* feeds have fresh prices" or similar rationale—to help future maintainers understand the determinism requirement and when/why it may need updating.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (1)
167-216: The growingvm.assume“known symbol exclusion” list is brittle and reduces fuzz usefulness.
As more symbols/variants are added, this becomes easy to forget to update and can significantly constrain fuzzing. Consider centralizing “known symbols” into a single list used by both the positive and negative tests (or generate exclusions from the same table used intestPriceFeedIdKnownMappings).test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (1)
175-322: PRE/POST coverage looks good, but these exact-value asserts are tightly coupled toFORK_BLOCK_BASE.
IfFORK_BLOCK_BASEmoves, this will likely require broad rewrites. Consider centralizing(symbol, expectedPrice, expectedConf)in a single table (including PRE/POST) to reduce churn and improve readability. Also worth double-checking that all newly-added PRE/POST expectations are derived from the same pinned fork block state you intend CI to use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/PythWords.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (3)
test/src/concrete/PythFork.t.sol(1 hunks)test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol(5 hunks)test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol(1 hunks)
⏰ 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). (4)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/pyth/LibPyth.sol (1)
283-322: Mappings for PRE/POST variants look consistent;getPriceFeedIdchain growth note (duplicate).The new branches map each newly-added
PRICE_FEED_SYMBOL_INTORASTRING_*_{PRE,POST}to the matchingPRICE_FEED_ID_*_{PRE,POST}and are placed adjacent to their base ticker mappings, which reduces mismatch risk.The function is still trending toward an error-prone mega-chain; this was already called out in prior review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/pyth/LibPyth.sol(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
Repo: rainlanguage/rain.pyth PR: 3
File: src/lib/pyth/LibPyth.sol:59-79
Timestamp: 2025-10-03T13:50:23.519Z
Learning: In the rain.pyth repository, the IntOrAString encoding scheme supports custom discriminator bytes. While most equity feed symbols use 0x92 as the discriminator byte, custom values like 0x91 are intentionally used for specific feeds such as EQUITY_US_GME_USD.
Applied to files:
src/lib/pyth/LibPyth.sol
⏰ 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). (4)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
🔇 Additional comments (4)
src/lib/pyth/LibPyth.sol (4)
115-124: The AMZN PRE/POST constants are correct. The IntOrAString encodings properly decode toEquity.US.AMZN/USD.PREandEquity.US.AMZN/USD.POSTwith discriminators0x96and0x97respectively. The feed IDs are validated by test usage intest/src/concrete/PythFork.t.sol, and the discriminator pattern is consistent across all new PRE/POST feeds (TSLA, NVDA, MSTR, COIN), aligning with the design that custom discriminator bytes are intentional.
159-168: Symbol encodings are correct; feed ID values cannot be verified without Pyth API access.The IntOrAString encodings for both NVDA PRE and POST are correct:
- PRE:
0x964571756974792e55532e4e5644412f5553442e505245...decodes to discriminator0x96+ "Equity.US.NVDA/USD.PRE"- POST:
0x974571756974792e55532e4e5644412f5553442e504f53540000...decodes to discriminator0x97+ "Equity.US.NVDA/USD.POST"Both constants are validated in test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (lines 121–127), confirming the symbol-to-ID mappings work correctly.
However, the actual feed ID hex values themselves cannot be verified against official Pyth documentation without access to the Pyth Hermes API or official feed list. Pyth publishes only the mechanism for feed IDs (https://docs.pyth.network/price-feeds/core/price-feeds/price-feed-ids) but not a static reference of all IDs. To verify the hex values, cross-reference against Pyth's Hermes API or the official price feed registry.
187-196: Symbol encodings are correctly formatted and verified through tests. Feed ID hex values match the library's internal symbol-to-ID mapping and pass all validation tests, but their absolute correctness against Pyth's official data cannot be independently confirmed due to unavailable API access and unpublished feed documentation.
143-152: No changes needed. Both feed IDs and IntOrAString encodings are correct per official Pyth documentation:
- Feed IDs match: TSLA/USD PRE
0x42676a595d...854ebe20aand POST0x2a797e19...bcd256cdb9b9- IntOrAString PRE decodes to
Equity.US.TSLA/USD.PREwith discriminator0x96- IntOrAString POST decodes to
Equity.US.TSLA/USD.POSTwith discriminator0x97
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/lib/pyth/LibPyth.sol (1)
291-338:getPriceFeedIdbranch growth continues; consider structural refactor.The new PRE/POST mappings are correctly added and follow the established pattern. However, this if-else chain continues to grow (now ~50 branches). As noted in a previous review, consider splitting by domain (crypto vs equity) or using a lookup table to reduce maintenance burden and error risk.
test/src/lib/pyth/LibPyth.constants.t.sol (1)
5-5: Dropconsole2import if unused.
console2is imported but not used in this test file.-import {Test, console2} from "forge-std/Test.sol"; +import {Test} from "forge-std/Test.sol";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/lib/pyth/LibPyth.sol(6 hunks)test/src/lib/pyth/LibPyth.constants.t.sol(6 hunks)test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol(6 hunks)test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
Repo: rainlanguage/rain.pyth PR: 3
File: src/lib/pyth/LibPyth.sol:59-79
Timestamp: 2025-10-03T13:50:23.519Z
Learning: In the rain.pyth repository, the IntOrAString encoding scheme supports custom discriminator bytes. While most equity feed symbols use 0x92 as the discriminator byte, custom values like 0x91 are intentionally used for specific feeds such as EQUITY_US_GME_USD.
Applied to files:
src/lib/pyth/LibPyth.sol
⏰ 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). (4)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
🔇 Additional comments (24)
src/lib/pyth/LibPyth.sol (6)
115-124: LGTM: AMZN PRE/POST constants added correctly.The feed IDs and INTORASTRING symbols follow the established pattern. The discriminator bytes (0x96 for PRE, 0x97 for POST) are consistent with other PRE/POST variants.
Please verify that these Pyth feed IDs (
0x82c59e...for PRE,0x62731d...for POST) are correct against your Pyth feed configuration, as these cannot be independently verified from public documentation.
143-152: LGTM: TSLA PRE/POST constants added correctly.Pattern is consistent with other equity PRE/POST feeds.
Please verify the TSLA PRE/POST feed IDs against your Pyth feed configuration.
159-168: LGTM: NVDA PRE/POST constants added correctly.Pattern is consistent with other equity PRE/POST feeds.
Please verify the NVDA PRE/POST feed IDs against your Pyth feed configuration.
187-197: LGTM: MSTR PRE/POST constants added correctly.Pattern is consistent with other equity PRE/POST feeds.
Please verify the MSTR PRE/POST feed IDs against your Pyth feed configuration.
221-231: LGTM: COIN PRE/POST constants added, formatting updated.The
uint256()wrapper was removed from the base COIN_USD symbol (line 221) for consistency with other equity symbols. PRE/POST variants added correctly.Please verify the COIN PRE/POST feed IDs against your Pyth feed configuration.
238-243: LGTM: New CRCL/USD feed added.New equity feed for Circle (CRCL) follows the established pattern with discriminator byte 0x92.
Please verify the CRCL/USD feed ID (
0x92b8527a...) against your Pyth feed configuration.test/src/lib/pyth/LibPyth.constants.t.sol (6)
80-87: LGTM: COIN PRE/POST symbol constant tests added.Tests correctly validate the INTORASTRING encoding for COIN PRE/POST variants.
96-103: LGTM: AMZN PRE/POST symbol constant tests added.Tests correctly validate the INTORASTRING encoding.
116-123: LGTM: TSLA PRE/POST symbol constant tests added.Tests correctly validate the INTORASTRING encoding.
128-135: LGTM: NVDA PRE/POST symbol constant tests added.Tests correctly validate the INTORASTRING encoding.
148-155: LGTM: MSTR PRE/POST symbol constant tests added.Tests correctly validate the INTORASTRING encoding.
172-175: LGTM: CRCL/USD symbol constant test added.Test correctly validates the INTORASTRING encoding for the new CRCL feed.
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (7)
73-80: LGTM: COIN PRE/POST feed ID mapping tests added.Tests correctly validate the getPriceFeedId mappings for COIN PRE/POST variants.
89-96: LGTM: AMZN PRE/POST feed ID mapping tests added.
109-116: LGTM: TSLA PRE/POST feed ID mapping tests added.
121-128: LGTM: NVDA PRE/POST feed ID mapping tests added.
141-148: LGTM: MSTR PRE/POST feed ID mapping tests added.
165-168: LGTM: CRCL/USD feed ID mapping test added.
189-217: LGTM: Unknown mappings exclusion list updated.All new PRE/POST variants and CRCL/USD are correctly added to the vm.assume exclusion list, ensuring the fuzz test properly excludes known symbols.
test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (5)
189-199: LGTM: AMZN PRE/POST price retrieval tests added.Tests validate that getPriceNoOlderThan correctly returns price and confidence values for AMZN PRE/POST feeds at the fork block.
219-229: LGTM: TSLA PRE/POST price retrieval tests added.
237-247: LGTM: NVDA PRE/POST price retrieval tests added.
267-277: LGTM: MSTR PRE/POST price retrieval tests added.
304-314: LGTM: COIN PRE/POST price retrieval tests added.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @test/lib/LibFork.sol:
- Around line 9-13: Add explanatory comments for the fork block constants so
future readers know why those specific block numbers were chosen; for example,
annotate FORK_BLOCK_BASE and the per-feed constants (FORK_BLOCK_BASE_CRCL,
FORK_BLOCK_BASE_PPLT, FORK_BLOCK_BASE_BMNR, FORK_BLOCK_BASE_RKLB) with a short
rationale describing the selection criteria (e.g., "block where specific feed
PRE/POST data becomes available" or "updated from X to Y to include latest base
price feeds") and any sources or checks used to determine each block number;
place these comments immediately above the corresponding constant declarations
in LibFork.sol.
In @test/src/lib/pyth/LibPyth.constants.t.sol:
- Line 5: Remove the unused import symbol console2 from the import statement
(import {Test, console2} from "forge-std/Test.sol") so the file only imports
Test; update the import to import {Test} from "forge-std/Test.sol" and verify no
other references to console2 exist in LibPyth.constants.t.sol.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/PythWords.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (5)
src/lib/pyth/LibPyth.soltest/lib/LibFork.soltest/src/lib/pyth/LibPyth.constants.t.soltest/src/lib/pyth/LibPyth.getPriceFeedId.t.soltest/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
Repo: rainlanguage/rain.pyth PR: 3
File: src/lib/pyth/LibPyth.sol:59-79
Timestamp: 2025-10-03T13:50:23.519Z
Learning: In the rain.pyth repository, the IntOrAString encoding scheme supports custom discriminator bytes. While most equity feed symbols use 0x92 as the discriminator byte, custom values like 0x91 are intentionally used for specific feeds such as EQUITY_US_GME_USD.
Applied to files:
src/lib/pyth/LibPyth.sol
⏰ 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). (5)
- GitHub Check: deploy
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
🔇 Additional comments (13)
test/src/lib/pyth/LibPyth.constants.t.sol (1)
80-203: LGTM! Comprehensive test coverage for PRE/POST variants.The assertions correctly validate the IntOrAString encoding for all new price feed symbols, including PRE/POST market data variants. The discriminator byte pattern is consistent: 0x96 for PRE variants and 0x97 for POST variants.
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (2)
73-188: LGTM! Complete test coverage for new feed ID mappings.All new price feed symbols (including PRE/POST variants for COIN, AMZN, TSLA, NVDA, MSTR, CRCL, BMNR, and base feeds for PPLT, RKLB) have corresponding test assertions that validate the
getPriceFeedIdfunction returns the correct feed IDs.
209-246: LGTM! Unknown mappings exclusions properly maintained.The fuzzing test correctly excludes all known symbols including the new PRE/POST variants, ensuring no false positive reverts.
test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (3)
7-16: LGTM! Import scope properly extended.The new fork block constants are correctly imported to support testing different price feeds at their specific availability points.
333-395: LGTM! New fork-specific test functions follow established pattern.The new test functions for CRCL, PPLT, BMNR, and RKLB properly isolate testing at their respective fork blocks, maintaining consistency with the existing test structure.
198-330: All expected price values and PRE/POST test cases are correctly aligned with FORK_BLOCK_BASE (39366248).The test file confirms PRE/POST market data cases have been added for COIN, AMZN, TSLA, NVDA, and MSTR. The
testPriceNoOlderThanBase()function properly uses the fork configuration with all hardcoded expected values correctly formatted. The test structure is valid and ready.src/lib/pyth/LibPyth.sol (7)
330-391: LGTM! Mapping function correctly extended for all new symbols.The
getPriceFeedIdfunction has been properly extended to map all new PRE/POST variants and additional symbols. The implementation follows the established if-else pattern and maintains consistency with existing mappings.Note: The TODO comment at line 284 about replacing with an O(1) lookup table remains unaddressed, but that's acceptable as a future optimization and not required for this PR.
238-282: Confirm scope and verify feed IDs against Pyth Network documentation.This PR introduces feed constants for MSTR, TSLA, NVDA, AMZN, and COIN (as expected), but also adds CRCL/USD, PPLT/USD, BMNR/USD, and RKLB/USD. While the implementation is consistent and well-structured, please confirm these additional symbols were intentional and verify all feed IDs match the current Pyth Network price feed registry for your target network.
143-152: TSLA PRE/POST feed IDs verified as valid on Pyth Network.Both feed IDs are active and returning price data from the official Hermes API:
- PRE (0x42676a595d...): ✓ Valid
- POST (0x2a797e19...): ✓ Valid
No issues found.
187-196: Feed IDs verified against official Pyth Network documentation.The PRE and POST feed IDs for MSTR/USD are correct:
- PRE:
0x1a11eb21c271f3127e4c9ec8a0e9b1042dc088ccba7a94a1a7d1aa37599a00f6(Equity.US.MSTR/USD.PRE)- POST:
0xd8b856d7e17c467877d2d947f27b832db0d65b362ddb6f728797d46b0a8b54c0(Equity.US.MSTR/USD.POST)Both match the official Pyth Hermes API registry.
115-124: Verify AMZN PRE/POST feed IDs against official Pyth documentation.The feed IDs are present in the codebase (lines 16-25) with matching symbol constants verified in tests. The base AMZN/USD feed ID (0xb5d0e0fa...) aligns with official Pyth sources, but the PRE and POST variant IDs (0x82c59e36... and 0x62731dfc...) require verification against Pyth's official feed documentation to confirm their accuracy.
222-231: Verify Pyth feed IDs for COIN PRE/POST variants against official Pyth documentation.The PRE and POST feed IDs for COIN have been added. Please ensure these feed IDs are verified against the official Pyth Network documentation. Note that Pyth's US equity feeds typically follow standard market hours (9:30–16:00 ET) with extended hours available only through specific partnerships; confirm whether separate PRE/POST feed variants exist or if a single COIN/USD feed with schedule-based availability is the correct approach.
159-168: Verify NVDA PRE/POST feed IDs against official Pyth documentation.The NVDA/USD PRE and POST feed ID constants have been properly added with correct symbol string mappings ("Equity.US.NVDA/USD.PRE" and "Equity.US.NVDA/USD.POST") and are thoroughly tested. However, the specific hex feed IDs (0x61c4ca5b... and 0x25719379...) should be confirmed against the official Pyth Price Feed IDs registry at https://docs.pyth.network/price-feeds/core/price-feeds/price-feed-ids to ensure they match the current Pyth Core session feeds for NVDA.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/src/lib/pyth/LibPyth.constants.t.sol (1)
15-204: 🧹 Nitpick | 🔵 TrivialConsider splitting
testIntorastringConstantsinto smaller, focused test functions.At ~190 lines, the function now covers crypto feeds, equity base feeds, and PRE/POST equity feeds in a single block. Splitting by category (e.g.,
testCryptoSymbolConstants,testEquityUsdSymbolConstants,testEquityUsdPrePostSymbolConstants) would improve failure isolation and readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/lib/pyth/LibPyth.constants.t.sol` around lines 15 - 204, Large monolithic test testIntorastringConstants bundles crypto, equity USD, and PRE/POST equity assertions which hurts failure isolation; split it into focused tests named e.g., testCryptoSymbolConstants, testEquityUsdSymbolConstants, and testEquityUsdPrePostSymbolConstants and move the corresponding assertEq blocks for LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_* (the Crypto.* assertions), the Equity.US.* (standard USD assertions), and the Equity.US.*.PRE/POST assertions respectively into those new external pure functions, preserving the same IntOrAString.unwrap(LibIntOrAString.fromString2(...)) comparisons and assertion order so each category fails independently and improves readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/src/lib/pyth/LibPyth.constants.t.sol`:
- Around line 15-204: Large monolithic test testIntorastringConstants bundles
crypto, equity USD, and PRE/POST equity assertions which hurts failure
isolation; split it into focused tests named e.g., testCryptoSymbolConstants,
testEquityUsdSymbolConstants, and testEquityUsdPrePostSymbolConstants and move
the corresponding assertEq blocks for LibPyth.PRICE_FEED_SYMBOL_INTORASTRING_*
(the Crypto.* assertions), the Equity.US.* (standard USD assertions), and the
Equity.US.*.PRE/POST assertions respectively into those new external pure
functions, preserving the same
IntOrAString.unwrap(LibIntOrAString.fromString2(...)) comparisons and assertion
order so each category fails independently and improves readability.
2 participants
Motivation
MSTR,TSLA,NVDA,AMZN,COIN pre and post market
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Tests
Chores