add OrbOracle adapter - #26
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesOracle scaling and adapter integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant OrbOracleToOracleAdapter
participant OrbOracleInterface
Caller->>OrbOracleToOracleAdapter: readValue()
OrbOracleToOracleAdapter->>OrbOracleInterface: readValue()
OrbOracleInterface-->>OrbOracleToOracleAdapter: signed value
OrbOracleToOracleAdapter-->>Caller: scaled WAD value
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/oracles/OrbOracleToOracleAdapter.sol (1)
54-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_scaleToWadis duplicated fromChainlinkToOracleAdapter.The same pure function exists verbatim in
src/oracles/ChainlinkToOracleAdapter.sol(lines 54–62). Extract it to a shared library to avoid divergence and ensure bug fixes apply everywhere.♻️ Suggested shared library
// src/oracles/OracleScalingLib.sol library OracleScalingLib { function scaleToWad(uint256 value, uint8 valueDecimals) internal pure returns (uint256) { if (valueDecimals == 18) { return value; } if (valueDecimals < 18) { return value * (10 ** (18 - uint256(valueDecimals))); } return value / (10 ** (uint256(valueDecimals) - 18)); } }Then in both adapters:
+import {OracleScalingLib} from "./OracleScalingLib.sol"; ... - function _scaleToWad(uint256 value, uint8 decimals) internal pure returns (uint256) { - if (decimals == 18) { - return value; - } - if (decimals < 18) { - return value * (10 ** (18 - uint256(decimals))); - } - return value / (10 ** (uint256(decimals) - 18)); - } + using OracleScalingLib for uint256;And update call sites to
value.scaleToWad(valueDecimals)orOracleScalingLib.scaleToWad(value, valueDecimals).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/oracles/OrbOracleToOracleAdapter.sol` around lines 54 - 62, Extract the duplicated scaling logic from _scaleToWad in OrbOracleToOracleAdapter and its counterpart in ChainlinkToOracleAdapter into a shared OracleScalingLib.scaleToWad function. Import the library in both adapters, replace all existing _scaleToWad call sites with the shared function, and remove the duplicated internal helper methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/oracles/OrbOracleToOracleAdapter.sol`:
- Around line 54-62: Extract the duplicated scaling logic from _scaleToWad in
OrbOracleToOracleAdapter and its counterpart in ChainlinkToOracleAdapter into a
shared OracleScalingLib.scaleToWad function. Import the library in both
adapters, replace all existing _scaleToWad call sites with the shared function,
and remove the duplicated internal helper methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55912c4e-e06a-4e7f-b4d9-d52fa80c4843
📒 Files selected for processing (2)
src/oracles/OrbOracleToOracleAdapter.soltest/OrbOracleAdapter.t.sol
d7fb1f4 to
c4b56d8
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
| function description() external view returns (string memory); | ||
| } | ||
|
|
||
| contract OrbOracleToOracleAdapter is IOracle { |
There was a problem hiding this comment.
We shouldn't need an adapter for orb oracles. Instead, by having Gluon and Orb agreeing on the oracle interface, Gluon should be able to use Orb oracles directly.
It seems that there are two disagreements that are forcing you to implement this adapter:
1 - Orb is returning int256 but Gluon is expecting uint256.
2 - Orb has a configurable number of decimals, but Gluon is expecting 18 decimals.
To solve 2, let's make Orb always use 18 decimals too.
To solve 2, let's make Orb always return uint256.
Addressed Issues:
N/A
Screenshots/Recordings:
N/A. This PR only adds an oracle adapter and tests.
Additional Notes:
This PR adds
OrbOracleToOracleAdapter, which wraps an OrbOracle feed and exposes it through Gluon'sIOracleinterface.Changes:
OrbOracleToOracleAdapterreadValue()to OrbOraclereadValue()readMaxValue()andreadMinValue()using the configuredsampleSizelastUpdated()to OrbOraclelastSubmissionTime()description()to OrbOracledescription()The adapter exposes the timestamp through
lastUpdated(), and the consumer can decide how to handle stale values as discussed earlier.Local checks:
forge fmtforge buildforge testCurrent result:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: ChatGPT for planning and review. I reviewed the code myself and tested it locally with Foundry.
Checklist
Summary by CodeRabbit