Skip to content

Conversation

@Siddharth2207
Copy link
Contributor

@Siddharth2207 Siddharth2207 commented Aug 28, 2025

Motivation

GME feed

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features

    • Added support for four additional USD equity price feeds: GME, MSTR, BRK‑B, and SPLG (available via existing price APIs).
  • Tests

    • Added mappings and assertions validating the new feeds and ID resolution.
    • Standardized price-freshness tests to 24‑hour max-age and updated expected values (including GME).
  • Chores

    • Updated test fork/block baseline for environment consistency.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 28, 2025

Walkthrough

Adds four equity USD price feeds (GME, MSTR, BRK‑B, SPLG) to LibPyth via new ID and IntOrAString symbol constants, extends symbol→ID mapping logic, updates tests to cover constants, mappings and price freshness checks, and updates the test fork block constant.

Changes

Cohort / File(s) Summary
Core library updates
src/lib/pyth/LibPyth.sol
Added constants for four equity feeds: PRICE_FEED_ID_EQUITY_US_GME_USD, PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_GME_USD, and equivalents for MSTR, BRK_B, SPLG; extended getPriceFeedId(IntOrAString) to recognize these symbols and return their IDs; added inline slither-disable comments.
Tests: constants coverage
test/src/lib/pyth/LibPyth.constants.t.sol
Added assertions validating the new IntOrAString symbol constants for GME, MSTR, BRK‑B, and SPLG against IntOrAString.fromString2.
Tests: feed ID mappings
test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol
Added known mapping tests for Equity.US.GME/USD, Equity.US.MSTR/USD, Equity.US.BRK-B/USD, Equity.US.SPLG/USD; updated unknown-mappings exclusion so these are not treated as unknown.
Tests: price freshness checks
test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol
Added/updated price checks including Equity.US.GME/USD; normalized some maxAge values to 24h and adjusted expected prices in base-fork tests; added an Arbitrum-path GME check with maxAge 8000h.
Test fork configuration
test/lib/LibFork.sol
Updated FORK_BLOCK_BASE constant value from 31309429 to 36353737.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Contract/Test
    participant LibPyth as LibPyth.getPriceFeedId
    participant IntOrAString as IntOrAString

    Note over Caller,LibPyth: Resolve price feed ID from symbol
    Caller->>LibPyth: getPriceFeedId(symbol: IntOrAString)
    LibPyth->>IntOrAString: inspect symbol representation
    IntOrAString-->>LibPyth: return symbol identifier
    alt symbol matches known equity (e.g., GME/MSTR/BRK-B/SPLG)
        LibPyth-->>Caller: return corresponding PRICE_FEED_ID_*
    else unknown symbol
        LibPyth-->>Caller: revert (unknown feed)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • 2025 06 10 literal intorastring #1 — Adds IntOrAString-based feed-symbol constants and corresponding getPriceFeedId mappings; directly related to symbol→ID mapping and IntOrAString handling.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly identifies the four equity symbols that are the core of this changeset and directly relates to the addition of those price feeds, so it accurately reflects the main change. Although phrased as a noun phrase rather than a verb-led sentence, it remains concise and unambiguous to a reviewer scanning the history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-08-28-gme-feed

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 38c98bc and 0306d05.

📒 Files selected for processing (1)
  • src/lib/pyth/LibPyth.sol (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.pyth#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
🔇 Additional comments (2)
src/lib/pyth/LibPyth.sol (2)

119-126: LGTM!

The new mapping branches correctly extend the symbol→ID lookup for GME, MSTR, BRK-B, and SPLG. The implementation follows the existing pattern and maintains consistency with the current linear search approach.


66-126: Well-structured additions.

The new equity feed constants and mappings are cleanly integrated, following established patterns throughout. The custom discriminator bytes (0x91, 0x92, 0x93) correctly encode the respective string lengths per the IntOrAString scheme, and the test coverage mentioned in the PR summary validates the implementation.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 451a8cc and ba514e6.

📒 Files selected for processing (4)
  • src/lib/pyth/LibPyth.sol (2 hunks)
  • test/src/lib/pyth/LibPyth.constants.t.sol (1 hunks)
  • test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (2 hunks)
  • test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (2 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-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
🔇 Additional comments (6)
src/lib/pyth/LibPyth.sol (1)

93-94: Branch is unreachable until the GME IntOrAString constant is fixed.

Once the 0x91→0x92 fix is applied, this case will match correctly. Please re-run tests after the fix.

test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (2)

44-47: LGTM: Adds positive mapping test for GME.

Covers the new symbol-to-feed mapping as expected.


59-60: LGTM: Updates unknown-set exclusion for GME.

Prevents false negatives in the revert test.

test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (2)

30-30: LGTM: Arbitrum GME price check added.

Ensure FORK_BLOCK_ARBITRUM remains pinned; otherwise this assertion will drift if fork constants change.


39-39: LGTM: Base GME price check added.

Same note: keep FORK_BLOCK_BASE pinned to avoid brittleness.

test/src/lib/pyth/LibPyth.constants.t.sol (1)

44-47: LGTM: Constant self-check for GME symbol.

This will catch the 0x91 vs 0x92 discriminator bug—good addition.

@Siddharth2207 Siddharth2207 self-assigned this Aug 28, 2025
@Siddharth2207 Siddharth2207 changed the title GME feed GME, MSTR, SPLG, BRK-B feed Oct 3, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/lib/pyth/LibPyth.sol (1)

59-63: Critical issue from previous review not addressed: Fix GME IntOrAString discriminator and capitalization.

The GME constant still has the incorrect IntOrAString discriminator (0x91 instead of 0x92), which will cause getPriceFeedId to fail matching this symbol. Additionally, the brand capitalization is incorrect.

Apply this diff to fix both issues:

-    /// Gamestop
+    /// GameStop
     bytes32 constant PRICE_FEED_ID_EQUITY_US_GME_USD =
         0x6f9cd89ef1b7fd39f667101a91ad578b6c6ace4579d5f7f285a4b06aa4504be6;
     uint256 constant PRICE_FEED_SYMBOL_INTORASTRING_EQUITY_US_GME_USD =
-        0x914571756974792e55532e474d452f5553440000000000000000000000000000;
+        0x924571756974792e55532e474d452f5553440000000000000000000000000000;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba514e6 and 9b26960.

📒 Files selected for processing (5)
  • src/lib/pyth/LibPyth.sol (2 hunks)
  • test/lib/LibFork.sol (1 hunks)
  • test/src/lib/pyth/LibPyth.constants.t.sol (1 hunks)
  • test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (2 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: rainix (ubuntu-latest, rainix-sol-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
  • GitHub Check: git-clean
🔇 Additional comments (9)
test/lib/LibFork.sol (1)

9-9: LGTM! Fork block updated for recent base chain state.

The fork block constant has been updated to reflect a more recent base chain state, which is necessary to test the newly added price feeds with current on-chain data.

test/src/lib/pyth/LibPyth.constants.t.sol (1)

44-59: LGTM! Test coverage expanded for new equity feeds.

The new assertions correctly validate the IntOrAString encoding for GME, MSTR, BRK-B, and SPLG feed symbols. The test pattern is consistent with existing feed constant validations.

test/src/lib/pyth/LibPyth.getPriceFeedId.t.sol (2)

44-59: LGTM! Known mappings test extended for new feeds.

The new assertions correctly validate that getPriceFeedId returns the expected price feed IDs for GME, MSTR, BRK-B, and SPLG symbols. The test pattern is consistent with existing feed mappings.


71-74: LGTM! Fuzzing exclusions updated for new known feeds.

The fuzzing test correctly excludes the newly added feed symbols (GME, MSTR, BRK-B, SPLG) from being treated as unknown, ensuring the test only validates that truly unsupported symbols trigger the expected revert.

test/src/lib/pyth/LibPyth.getPriceNoOlderThan.t.sol (2)

30-31: LGTM! GME price check added to Arbitrum test.

The new check validates GME price retrieval on the Arbitrum fork, consistent with the existing test pattern. The 8000-hour maxAge matches the META feed check tolerance.


35-46: LGTM! Base test updated with uniform maxAge and new feeds.

The test now uses a consistent 24-hour maxAge for all feeds and includes the newly added GME, MSTR, BRK-B, and SPLG price checks. The expected prices align with the updated fork block constant.

src/lib/pyth/LibPyth.sol (3)

64-68: LGTM! MSTR constants correctly defined.

The MSTR price feed ID and IntOrAString symbol encoding are correctly defined with the proper 0x92 discriminator.


74-78: LGTM! SPLG constants correctly defined.

The SPLG price feed ID and IntOrAString symbol encoding are correctly defined with the proper 0x92 discriminator.


108-115: LGTM! getPriceFeedId mappings correctly structured.

The mapping logic for the new feed symbols (GME, MSTR, BRK-B, SPLG) follows the established pattern correctly. Once the IntOrAString discriminator issues in the constant definitions are fixed, these mappings will function as intended.

@hardyjosh hardyjosh self-requested a review October 3, 2025 14:00
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
src/lib/pyth/LibPyth.sol (1)

66-66: Fix brand capitalization.

The comment should read "GameStop" not "Gamestop" to match the official brand capitalization.

Apply this diff:

-    /// Gamestop
+    /// GameStop
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 703386b and cb15211.

📒 Files selected for processing (1)
  • src/lib/pyth/LibPyth.sol (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T13:50:23.519Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.pyth#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
🔇 Additional comments (5)
src/lib/pyth/LibPyth.sol (5)

67-71: LGTM!

The GME price feed constants are correctly defined. The custom discriminator byte (0x91) is intentional per the IntOrAString encoding scheme used in this repository.

Based on learnings.


73-77: LGTM!

The MSTR price feed constants are correctly defined with the appropriate discriminator byte (0x92).


79-83: LGTM!

The BRK-B price feed constants are correctly defined. The custom discriminator byte (0x93) is intentional for encoding the 19-byte string "Equity.US.BRK-B/USD".

Based on learnings.


85-89: LGTM!

The SPLG price feed constants are correctly defined with the appropriate discriminator byte (0x92).


119-126: LGTM!

The new feed mappings in getPriceFeedId are correctly implemented and follow the established pattern. Each new equity feed (GME, MSTR, BRK-B, SPLG) is properly mapped from its IntOrAString symbol to its corresponding bytes32 feed ID.

thedavidmeister and others added 5 commits October 3, 2025 19:54
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@thedavidmeister thedavidmeister merged commit 8564c61 into main Oct 3, 2025
5 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 29, 2025
4 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 6, 2025
4 tasks
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.

4 participants