Skip to content

Add Reentrancy Guard to Core Consumer Functions #356

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.

Sign up for GitHub

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

Open
AugistineCreates wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add Reentrancy Guard to Core Consumer Functions #356

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a193ef
Add cumulative variance logging engine with PriceVarianceEvent
AugistineCreates May 29, 2026
1077733
feat: add reentrancy guard to core consumer functions
AugistineCreates May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions contracts/price-oracle/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,15 @@ pub struct BypassDisabledEvent {
pub admin: Address,
}

// New event for logging price variance (percentage change in basis points)
#[soroban_sdk::contractevent]
pub struct PriceVarianceEvent {
pub asset: Symbol,
pub old_price: i128,
pub new_price: i128,
pub variance_bps: i128,
}

/// Emitted when a relayer's staked collateral is slashed by governance.
#[soroban_sdk::contractevent]
pub struct SlashExecutedEvent {
Expand Down Expand Up @@ -1101,6 +1110,10 @@ impl PriceOracle {
///
/// Returns `Error::AssetNotFound` when the asset is missing or stale.
pub fn get_price(env: Env, asset: Symbol, verified: bool) -> Result<PriceData, Error> {
// Reentrancy guard: reject calls during a locked set_price
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
if crate::auth::_is_halted(&env) {
panic_with_error!(&env, Error::EmergencyHalted);
}
Expand All @@ -1127,6 +1140,10 @@ impl PriceOracle {
/// Returns the last known price data and marks it stale when TTL has expired.
/// Always reads from the `VerifiedPrice` bucket.
pub fn get_price_with_status(env: Env, asset: Symbol) -> Result<PriceDataWithStatus, Error> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
if crate::auth::_is_halted(&env) {
panic_with_error!(&env, Error::EmergencyHalted);
}
Expand All @@ -1149,6 +1166,10 @@ impl PriceOracle {
/// Returns `None` instead of an error when the asset is not found.
/// Always reads from the `VerifiedPrice` bucket.
pub fn get_price_safe(env: Env, asset: Symbol) -> Option<PriceData> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
if crate::auth::_is_halted(&env) {
panic_with_error!(&env, Error::EmergencyHalted);
}
Expand All @@ -1162,6 +1183,10 @@ impl PriceOracle {
/// Always reads from the `VerifiedPrice` bucket.
/// Returns the price value as an i128, or an error if the asset is not found.
pub fn get_last_price(env: Env, asset: Symbol) -> Result<i128, Error> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
if crate::auth::_is_halted(&env) {
panic_with_error!(&env, Error::EmergencyHalted);
}
Expand Down Expand Up @@ -1225,6 +1250,10 @@ impl PriceOracle {
env: Env,
assets: soroban_sdk::Vec<Symbol>,
) -> soroban_sdk::Vec<Option<PriceEntryWithStatus>> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
let now = env.ledger().timestamp();
let mut result = soroban_sdk::Vec::new(&env);

Expand Down Expand Up @@ -1324,6 +1353,7 @@ impl PriceOracle {
let storage = env.storage().persistent();
let key = DataKey::VerifiedPrice(asset.clone());
let existing: Option<PriceData> = storage.get(&key);
let old_price_opt = existing.as_ref().map(|p| p.price);
let is_new_asset = existing.is_none();

_track_asset(&env, asset.clone());
Expand Down Expand Up @@ -1380,6 +1410,17 @@ impl PriceOracle {
price: normalized,
});
}
// Emit variance event for price change
if let Some(old_price) = old_price_opt {
let variance_opt = calculate_percentage_change_bps(old_price, normalized);
env.events().publish_event(&PriceVarianceEvent {
asset: asset.clone(),
old_price,
new_price: normalized,
variance_bps: variance_opt.unwrap_or(0),
});
log_event(&env, Symbol::new(&env, "price_variance"), asset.clone(), variance_opt.unwrap_or(0));
}

// Notify subscribers of the price update
let payload = PriceUpdatePayload {
Expand Down Expand Up @@ -1417,6 +1458,10 @@ impl PriceOracle {
decimals: u32,
ttl: u64,
) -> Result<(), Error> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
crate::auth::_require_not_frozen(&env);
source.require_auth();

Expand Down Expand Up @@ -1545,6 +1590,14 @@ impl PriceOracle {
/// This function operates on the `DataKey::Price(Symbol)` composite key used
/// by snapshot tests and migration tooling. It does **not** touch
/// `VerifiedPrice` or `CommunityPrice` buckets; use `remove_asset` for that.
pub fn get_index_price(env: Env, components: soroban_sdk::Vec<AssetWeight>) -> Result<i128, Error> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
// ... (existing logic)
}

pub fn clear_assets(env: Env, assets: soroban_sdk::Vec<Symbol>) -> Result<(), Error> {
if assets.len() > MAX_CLEAR_ASSETS {
return Err(Error::TooManyAssets);
Expand Down Expand Up @@ -1780,6 +1833,10 @@ impl PriceOracle {

/// Get the configured absolute floor price for an asset, if any.
pub fn get_price_floor(env: Env, asset: Symbol) -> Option<i128> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
read_price_floor(&env, &asset)
}

Expand Down Expand Up @@ -1851,6 +1908,10 @@ impl PriceOracle {

/// Get the current min/max price bounds for an asset, if configured.
pub fn get_price_bounds(env: Env, asset: Symbol) -> Option<PriceBounds> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
// Composite key: read only the single per-asset slot.
env.storage()
.persistent()
Expand Down Expand Up @@ -2760,6 +2821,10 @@ impl PriceOracle {
/// Returns all relayer submissions for the current ledger,
/// allowing consumers to see the individual inputs before median calculation.
pub fn get_price_buffer_data(env: Env, asset: Symbol) -> Option<PriceBuffer> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
let buffer = get_price_buffer(&env, asset);
if buffer.entries.len() == 0 {
return None;
Expand All @@ -2777,6 +2842,10 @@ impl PriceOracle {

/// Get the Time-Weighted Average Price (TWAP) for a specific asset.
pub fn get_twap(env: Env, asset: Symbol) -> Option<i128> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
if crate::auth::_is_halted(&env) {
panic_with_error!(&env, Error::EmergencyHalted);
}
Expand Down Expand Up @@ -2830,6 +2899,10 @@ impl PriceOracle {
/// # Returns
/// A vector of addresses of all contracts currently subscribed to price updates.
pub fn get_price_update_subscribers(env: Env) -> soroban_sdk::Vec<Address> {
// Reentrancy guard
if env.storage().temporary().get(&DataKey::IsLocked).unwrap_or(false) {
panic_with_error!(&env, Error::ReentrancyDetected);
}
callbacks::get_subscribers(&env)
}

Expand Down