Integrate Chainlink OracleManager for token price feeds and USD normalization in Jobs contract. Add OracleManager and IOracleManager contracts, update Job struct for usd_budget, wire conversions in job creation, payout, refund. Robust fallback/stale handling. [core oracle integration]#45
Integrate Chainlink OracleManager for token price feeds and USD normalization in Jobs contract. Add OracleManager and IOracleManager contracts, update Job struct for usd_budget, wire conversions in job creation, payout, refund. Robust fallback/stale handling. [core oracle integration]#45Cybermaxi7 merged 1 commit intoStarkHive:mainfrom
Conversation
…lization in Jobs contract. Add OracleManager and IOracleManager contracts, update Job struct for usd_budget, wire conversions in job creation, payout, refund. Robust fallback/stale handling. [core oracle integration]
WalkthroughThis change introduces an OracleManager contract for modular, on-chain price feeds using Chainlink oracles with fallback mechanisms. The Jobs contract is updated to convert token budgets to normalized USD values via OracleManager during job creation, approval, and cancellation. An interface for OracleManager is added, and the Job struct now stores a USD budget. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JobsContract
participant OracleManager
participant ChainlinkOracle
User->>JobsContract: create_job(token, budget, ...)
JobsContract->>OracleManager: to_usd(token, budget)
OracleManager->>ChainlinkOracle: get_price(token)
ChainlinkOracle-->>OracleManager: price, decimals, valid
alt Price valid & not stale
OracleManager-->>JobsContract: usd_value, Ok
else Price invalid or stale
OracleManager-->>JobsContract: fallback_usd_value, Fallback/Stale
end
JobsContract-->>User: job_id (with usd_budget stored)
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
src/contracts/Jobs.cairo (3)
172-172: Implement conversion event emissionThe TODO indicates missing event emission for USD conversions, which is important for transparency and frontend synchronization.
Would you like me to help implement the ConversionPerformed event and its emission logic?
443-446: Consider using or removing the computed USD valueThe
usd_valueis computed but never used. If you only need to validate price availability, consider adding a validation-only method to OracleManager.If the USD value isn't needed, you could add a simpler validation method to the OracleManager interface:
fn validate_price(self: @TContractState, token: felt252) -> OracleStatus;Alternatively, consider emitting the USD value in the proposed conversion event for tracking purposes.
136-141: Refactor duplicated oracle validation logicThe pattern of converting token to symbol, calling OracleManager, and validating status is repeated across multiple functions.
Consider extracting this into a helper function:
fn validate_and_convert_to_usd( self: @ContractState, token: ContractAddress, amount: u256 ) -> u256 { let token_symbol: felt252 = token.into(); // Or use proper mapping let oracle_manager_addr = self.oracle_manager.read(); let oracle_manager = IOracleManagerTrait { contract_address: oracle_manager_addr }; let (usd_value, status) = oracle_manager.to_usd(token_symbol, amount); assert(status == OracleStatus::Ok || status == OracleStatus::Fallback, 'Invalid price'); usd_value }Also applies to: 440-445, 466-471
src/contracts/OracleManager.cairo (3)
70-75: Track the Chainlink integration TODO.The stub function needs to be replaced with actual Chainlink oracle integration.
Would you like me to open an issue to track the Chainlink oracle integration implementation?
51-58: Add event emission for Chainlink feed updates.For consistency with
set_fallback_price, emit an event when Chainlink feeds are updated.#[external] fn set_chainlink_feed(ref self: ContractState, token: felt252, feed: ContractAddress, decimals: u8) { let admin = self.admin.read(); assert(get_caller_address() == admin, 'only_admin'); self.chainlink_feeds.write(token, feed); self.decimals.write(token, decimals); + emit PriceFeedUpdated(token, 0, decimals, 'chainlink_feed_set'); }
60-68: Consider adding price bounds validation for fallback prices.The admin can set arbitrary fallback prices without validation, which could be misused.
Consider adding reasonable bounds checking:
#[external] fn set_fallback_price(ref self: ContractState, token: felt252, price: u256) { let admin = self.admin.read(); assert(get_caller_address() == admin, 'only_admin'); + // Example: Ensure price is within reasonable bounds (e.g., $0.01 to $1M with 18 decimals) + assert(price >= 10000000000000000 && price <= 1000000000000000000000000, 'price_out_of_bounds'); self.fallback_prices.write(token, price); self.last_update.write(token, get_block_timestamp()); emit FallbackPriceUpdated(token, price, admin); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/base/types.cairo(1 hunks)src/contracts/Jobs.cairo(7 hunks)src/contracts/OracleManager.cairo(1 hunks)src/interfaces/IOracleManager.cairo(1 hunks)
🔇 Additional comments (3)
src/base/types.cairo (1)
55-55: LGTM!The addition of the
usd_budgetfield is properly implemented and documented.src/interfaces/IOracleManager.cairo (1)
1-24: Well-structured interface design!The IOracleManager interface is properly defined with comprehensive functions for price feed management and clear status handling through the OracleStatus enum.
src/contracts/OracleManager.cairo (1)
111-128:Details
✅ Verification successful
Update function signature to support mutable operations.
This function calls
get_pricewhich needs to write to storage, so it must take a mutable reference.#[external] -fn to_usd(self: @ContractState, token: felt252, amount: u256) -> (u256, OracleStatus) { +fn to_usd(ref self: ContractState, token: felt252, amount: u256) -> (u256, OracleStatus) { let (price, decimals, status) = self.get_price(token);Additionally, verify the power function syntax is correct for Cairo.
🏁 Script executed:
#!/bin/bash # Description: Check if the power function syntax is correct in Cairo by searching for similar usage patterns # Search for power function usage in Cairo files rg -A 2 -B 2 '\.pow\(' --type cairo || rg -A 2 -B 2 '\.pow\(' -g '*.cairo' # Also search for u256 power operations specifically ast-grep --pattern 'u256.pow($_)'Length of output: 800
To confirm whether
to_usdneeds a mutableself, let’s inspect the signature and implementation ofget_price:
🏁 Script executed:
#!/bin/bash # Find get_price definition in Cairo contracts rg -R "fn get_price" -g "*.cairo" -A 5 -B 2Length of output: 450
🏁 Script executed:
#!/bin/bash # Display the top of OracleManager to inspect imports and type definitions head -n 20 src/contracts/OracleManager.cairo # Locate the get_price function signature and receiver type rg "fn get_price" -g "*.cairo" -A5 -B2Length of output: 2101
No mutable receiver needed for
to_usd
- The implementation of
get_priceonly reads from storage (uses.read), so leavingto_usd(self: @ContractState, …)immutable is correct.- The
u256.pow(u8)method is part of Cairo’su256utility and the syntax10u256.pow(…)is valid.No changes required.
| Applicant, ApplicationStatus, ExperienceLevel, Job, JobCategory, JobDuration, Status, | ||
| }; | ||
| use starkhive_contract::interfaces::IJobs::IJobs; | ||
| use starkhive_contract::interfaces::IOracleManager::IOracleManagerTrait; |
There was a problem hiding this comment.
Add missing OracleStatus import
The code uses OracleStatus enum on lines 141, 444, and 470, but it's not imported. This will cause a compilation error.
-use starkhive_contract::interfaces::IOracleManager::IOracleManagerTrait;
+use starkhive_contract::interfaces::IOracleManager::{IOracleManagerTrait, OracleStatus};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use starkhive_contract::interfaces::IOracleManager::IOracleManagerTrait; | |
| use starkhive_contract::interfaces::IOracleManager::{IOracleManagerTrait, OracleStatus}; |
🤖 Prompt for AI Agents
In src/contracts/Jobs.cairo at line 10, the OracleStatus enum is used later in
the file but not imported, causing compilation errors. Add an import statement
for OracleStatus from its defining module alongside the existing imports to
ensure it is recognized and the code compiles successfully.
| fn set_oracle_manager(ref self: ContractState, oracle_manager: OracleManagerAddress) { | ||
| // TODO: add admin check | ||
| self.oracle_manager.write(oracle_manager); | ||
| } |
There was a problem hiding this comment.
Critical: Implement access control for set_oracle_manager
The function lacks access control, allowing anyone to change the oracle manager address. This is a critical security vulnerability that could lead to price manipulation.
Implement admin access control:
fn set_oracle_manager(ref self: ContractState, oracle_manager: OracleManagerAddress) {
- // TODO: add admin check
+ let caller = get_caller_address();
+ assert(caller == self.admin.read(), 'Unauthorized: admin only');
self.oracle_manager.write(oracle_manager);
}Note: You'll need to add an admin storage variable and initialization logic if not already present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn set_oracle_manager(ref self: ContractState, oracle_manager: OracleManagerAddress) { | |
| // TODO: add admin check | |
| self.oracle_manager.write(oracle_manager); | |
| } | |
| fn set_oracle_manager(ref self: ContractState, oracle_manager: OracleManagerAddress) { | |
| let caller = get_caller_address(); | |
| assert(caller == self.admin.read(), 'Unauthorized: admin only'); | |
| self.oracle_manager.write(oracle_manager); | |
| } |
🤖 Prompt for AI Agents
In src/contracts/Jobs.cairo around lines 115 to 118, the set_oracle_manager
function lacks access control, allowing any caller to change the oracle manager
address. To fix this, add an admin storage variable to the contract state and
initialize it appropriately. Then, implement a check in set_oracle_manager to
verify that only the admin can call this function before updating the
oracle_manager. This will enforce proper access control and prevent unauthorized
changes.
| location: felt252, | ||
| ) -> u256 { | ||
| // Normalize token address to symbol for OracleManager (assume token symbol is passed as felt252 for now) | ||
| let token_symbol: felt252 = token.into(); // TODO: Replace with actual mapping/token registry if needed |
There was a problem hiding this comment.
Fix incorrect token symbol conversion
Converting a contract address to felt252 doesn't produce a valid token symbol. This will cause oracle lookups to fail.
Consider implementing a token registry mapping or passing the token symbol as a parameter:
fn create_job(
ref self: ContractState,
token: ContractAddress,
+ token_symbol: felt252,
title: felt252,
// ... other parameters
) -> u256 {
- let token_symbol: felt252 = token.into(); // TODO: Replace with actual mapping/token registry if needed
let oracle_manager_addr = self.oracle_manager.read();Alternatively, implement a token registry that maps addresses to symbols.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let token_symbol: felt252 = token.into(); // TODO: Replace with actual mapping/token registry if needed | |
| fn create_job( | |
| ref self: ContractState, | |
| token: ContractAddress, | |
| token_symbol: felt252, | |
| title: felt252, | |
| // ... other parameters | |
| ) -> u256 { | |
| let oracle_manager_addr = self.oracle_manager.read(); | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In src/contracts/Jobs.cairo at line 136, the current code incorrectly converts a
contract address to a felt252 token symbol, which is invalid and causes oracle
lookup failures. To fix this, replace the direct conversion with a proper token
registry mapping that maps token contract addresses to their corresponding
symbols, or modify the function to accept the token symbol as a parameter
instead of deriving it from the address.
| // --- Read-only: Expose Current Exchange Rate --- | ||
| #[view] | ||
| fn get_exchange_rate(self: @ContractState, token: felt252) -> (u256, u8, OracleStatus) { | ||
| self.get_price(token) | ||
| } |
There was a problem hiding this comment.
Architectural issue: View function calls state-modifying function.
The get_exchange_rate view function calls get_price which modifies storage. This violates the view function contract.
Consider separating the price retrieval logic:
- Create an internal
_get_price_readonlyfunction that doesn't update storage - Use it in view functions like
get_exchange_rate - Keep
get_pricefor external calls that need to update the last price/timestamp
Would you like me to provide a refactored implementation that properly separates read-only and state-modifying operations?
🤖 Prompt for AI Agents
In src/contracts/OracleManager.cairo around lines 130 to 134, the view function
get_exchange_rate calls get_price, which modifies storage, violating the view
function contract. To fix this, create a new internal function
_get_price_readonly that retrieves the price without modifying storage, then
update get_exchange_rate to call _get_price_readonly instead. Keep get_price
unchanged for external calls that update state. This separation ensures view
functions remain read-only.
| use starknet::ContractAddress; | ||
| use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; | ||
| use starknet::{get_block_timestamp}; | ||
| use core::option::OptionTrait; | ||
| use core::array::ArrayTrait; | ||
|
|
There was a problem hiding this comment.
Add missing import for get_caller_address.
The contract uses get_caller_address() in the admin functions but doesn't import it.
Add this import:
use starknet::ContractAddress;
use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess};
-use starknet::{get_block_timestamp};
+use starknet::{get_block_timestamp, get_caller_address};
use core::option::OptionTrait;
use core::array::ArrayTrait;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use starknet::ContractAddress; | |
| use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; | |
| use starknet::{get_block_timestamp}; | |
| use core::option::OptionTrait; | |
| use core::array::ArrayTrait; | |
| use starknet::ContractAddress; | |
| use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; | |
| use starknet::{get_block_timestamp, get_caller_address}; | |
| use core::option::OptionTrait; | |
| use core::array::ArrayTrait; |
🤖 Prompt for AI Agents
In src/contracts/OracleManager.cairo around lines 5 to 10, the function
get_caller_address() is used but not imported. Add an import statement for
get_caller_address from the starknet module alongside the existing imports to
ensure the function is available in the contract.
| #[constructor] | ||
| fn constructor(ref self: ContractState, admin: ContractAddress, stale_threshold: u64) { | ||
| self.admin.write(admin); | ||
| self.stale_threshold.write(stale_threshold); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate admin address in constructor.
Add validation to ensure the admin address is not zero to prevent contract lockout.
#[constructor]
fn constructor(ref self: ContractState, admin: ContractAddress, stale_threshold: u64) {
+ assert(admin != 0.try_into().unwrap(), 'invalid_admin_address');
self.admin.write(admin);
self.stale_threshold.write(stale_threshold);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/contracts/OracleManager.cairo around lines 45 to 49, the constructor does
not validate the admin address, which could lead to contract lockout if a zero
address is provided. Add a check at the start of the constructor to ensure the
admin address is not zero, and revert or raise an error if it is, before writing
the admin value to storage.
| // --- External: Get Price (with Fallback) --- | ||
| #[external] | ||
| fn get_price(self: @ContractState, token: felt252) -> (u256, u8, OracleStatus) { | ||
| let feed = self.chainlink_feeds.read(token); | ||
| let (price, decimals, valid) = get_chainlink_price(feed); | ||
| let now = get_block_timestamp(); | ||
| let mut status = OracleStatus::Missing; | ||
| let mut final_price = price; | ||
| let mut final_decimals = decimals; | ||
|
|
||
| if valid && price > 0 { | ||
| let last = self.last_update.read(token); | ||
| let threshold = self.stale_threshold.read(); | ||
| if now - last <= threshold { | ||
| status = OracleStatus::Ok; | ||
| self.last_price.write(token, price); | ||
| self.last_update.write(token, now); | ||
| emit PriceFeedUpdated(token, price, decimals, 'chainlink'); | ||
| } else { | ||
| status = OracleStatus::Stale; | ||
| } | ||
| } else { | ||
| // Fallback to manual price | ||
| let fallback = self.fallback_prices.read(token); | ||
| if fallback > 0 { | ||
| final_price = fallback; | ||
| final_decimals = 18; | ||
| status = OracleStatus::Fallback; | ||
| emit PriceFeedUpdated(token, fallback, 18, 'fallback'); | ||
| } | ||
| } | ||
| (final_price, final_decimals, status) | ||
| } |
There was a problem hiding this comment.
Fix function signature and staleness check logic.
The function has two critical issues:
- It takes an immutable reference but attempts to write to storage
- The staleness check logic is inverted
Apply this fix:
#[external]
-fn get_price(self: @ContractState, token: felt252) -> (u256, u8, OracleStatus) {
+fn get_price(ref self: ContractState, token: felt252) -> (u256, u8, OracleStatus) {
let feed = self.chainlink_feeds.read(token);
let (price, decimals, valid) = get_chainlink_price(feed);
let now = get_block_timestamp();
let mut status = OracleStatus::Missing;
let mut final_price = price;
let mut final_decimals = decimals;
if valid && price > 0 {
let last = self.last_update.read(token);
let threshold = self.stale_threshold.read();
- if now - last <= threshold {
+ if now - last <= threshold {
status = OracleStatus::Ok;
self.last_price.write(token, price);
self.last_update.write(token, now);
emit PriceFeedUpdated(token, price, decimals, 'chainlink');
} else {
status = OracleStatus::Stale;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // --- External: Get Price (with Fallback) --- | |
| #[external] | |
| fn get_price(self: @ContractState, token: felt252) -> (u256, u8, OracleStatus) { | |
| let feed = self.chainlink_feeds.read(token); | |
| let (price, decimals, valid) = get_chainlink_price(feed); | |
| let now = get_block_timestamp(); | |
| let mut status = OracleStatus::Missing; | |
| let mut final_price = price; | |
| let mut final_decimals = decimals; | |
| if valid && price > 0 { | |
| let last = self.last_update.read(token); | |
| let threshold = self.stale_threshold.read(); | |
| if now - last <= threshold { | |
| status = OracleStatus::Ok; | |
| self.last_price.write(token, price); | |
| self.last_update.write(token, now); | |
| emit PriceFeedUpdated(token, price, decimals, 'chainlink'); | |
| } else { | |
| status = OracleStatus::Stale; | |
| } | |
| } else { | |
| // Fallback to manual price | |
| let fallback = self.fallback_prices.read(token); | |
| if fallback > 0 { | |
| final_price = fallback; | |
| final_decimals = 18; | |
| status = OracleStatus::Fallback; | |
| emit PriceFeedUpdated(token, fallback, 18, 'fallback'); | |
| } | |
| } | |
| (final_price, final_decimals, status) | |
| } | |
| // --- External: Get Price (with Fallback) --- | |
| #[external] | |
| fn get_price(ref self: ContractState, token: felt252) -> (u256, u8, OracleStatus) { | |
| let feed = self.chainlink_feeds.read(token); | |
| let (price, decimals, valid) = get_chainlink_price(feed); | |
| let now = get_block_timestamp(); | |
| let mut status = OracleStatus::Missing; | |
| let mut final_price = price; | |
| let mut final_decimals = decimals; | |
| if valid && price > 0 { | |
| let last = self.last_update.read(token); | |
| let threshold = self.stale_threshold.read(); | |
| if now - last <= threshold { | |
| status = OracleStatus::Ok; | |
| self.last_price.write(token, price); | |
| self.last_update.write(token, now); | |
| emit PriceFeedUpdated(token, price, decimals, 'chainlink'); | |
| } else { | |
| status = OracleStatus::Stale; | |
| } | |
| } else { | |
| // Fallback to manual price | |
| let fallback = self.fallback_prices.read(token); | |
| if fallback > 0 { | |
| final_price = fallback; | |
| final_decimals = 18; | |
| status = OracleStatus::Fallback; | |
| emit PriceFeedUpdated(token, fallback, 18, 'fallback'); | |
| } | |
| } | |
| (final_price, final_decimals, status) | |
| } |
🤖 Prompt for AI Agents
In src/contracts/OracleManager.cairo lines 77 to 109, the get_price function
incorrectly uses an immutable reference for self while writing to storage, and
the staleness check logic is inverted. Change the function signature to take a
mutable reference to self. Then, invert the staleness condition to check if now
- last is greater than the threshold to mark the price as stale. Adjust the
logic so that if the price is stale, status is set to Stale without updating
storage; otherwise, update last_price and last_update and set status to Ok.
1 participant
Integrate Chainlink OracleManager for Token Price Feeds and USD Normalization
Overview
This PR introduces a modular on-chain OracleManager contract and integrates it with the Jobs contract to enable real-time token price feeds (ETH/USD, USDC/USD, STRK/USD) using Chainlink oracles. All job budgets and payments are now normalized to USD (18 decimals), improving accuracy and transparency for users and the frontend.
Key Changes
1. OracleManager Contract
OracleManager.cairo2. IOracleManager Interface
IOracleManager.cairo3. Jobs Contract Integration
usd_budgetfield for normalized USD value.4. Types Update
Jobstruct intypes.cairoupdated to includeusd_budget: u256.Why?
Next Steps
Checklist
Closes #38
Please review and provide feedback!
Summary by CodeRabbit
New Features
Improvements