-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test RYUSD Share Price Oracle Permission #276
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce new constants related to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (1)
src/cellars.rs (1)
Line range hint
69-386
: Consider adding tests for the new TEST_RYUSD functionalityWhile the changes for TEST_RYUSD_ORACLE and CELLAR_TEST_RYUSD have been implemented correctly, it would be beneficial to add corresponding unit tests to ensure the new functionality works as expected.
Consider adding the following tests to the existing test module:
- A test case for validating the TEST_RYUSD_ORACLE in the
test_validate_oracle
function.- A test case for the CELLAR_TEST_RYUSD constant to ensure it's recognized as a valid cellar ID.
These additions will help maintain the robustness of the codebase and catch any potential issues early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/cellars.rs (3 hunks)
🔇 Additional comments not posted (2)
src/cellars.rs (2)
239-239
: LGTM: New constant CELLAR_TEST_RYUSD added correctlyThe new constant CELLAR_TEST_RYUSD is added correctly, following the existing naming convention and structure for cellar constants.
381-386
: LGTM: Oracle validation for CELLAR_TEST_RYUSD added correctlyThe new condition for validating oracles associated with CELLAR_TEST_RYUSD has been implemented correctly. It follows the existing pattern and uses the appropriate constants.
src/cellars.rs
Outdated
pub const TEST_RYUSD_ORACLE: (U256, &str) = ( | ||
// need the real id | ||
U256([2, 0, 0, 0]), | ||
"ddf603866d6d8d207c6200552655df1ebde5a641", | ||
); | ||
|
||
pub const ALLOWED_TEST_RYUSD_PRICE_ORACLES: [(U256, &str); 1] = [TEST_RYUSD_ORACLE]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Placeholder Registry ID for TEST_RYUSD_ORACLE Still Present
The registry ID for TEST_RYUSD_ORACLE remains as a placeholder value (U256([2, 0, 0, 0])). Please update it with the correct registry ID before deploying to production.
🔗 Analysis chain
Update the placeholder registry ID for TEST_RYUSD_ORACLE
The registry ID for TEST_RYUSD_ORACLE is currently set to a placeholder value (U256([2, 0, 0, 0])). Please update this with the correct registry ID before deploying to production.
To ensure the correct registry ID is used, you can run the following command:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the registry ID for TEST_RYUSD_ORACLE
# Expected: The correct registry ID should be displayed
rg --type rust 'pub const TEST_RYUSD_ORACLE: \(U256, &str\) = \(' src/cellars.rs -A 3
Length of output: 229
Summary by CodeRabbit
New Features
Bug Fixes