-
Notifications
You must be signed in to change notification settings - Fork 15
Description
Admin will cause incomplete event data for off-chain indexers affecting external applications as they update prices to zero without including those assets in the event emission.
Summary
The choice to skip zero-price assets in publish_update_event is a mistake as it prevents off-chain indexers from distinguishing between "asset not updated" and "asset updated to zero", causing data inconsistency for external applications relying on event logs.
Root Cause
In oracle/src/events.rs:14, the event publishing logic skips assets with zero prices:
pub fn publish_update_event(e: &Env, updates: &Vec<i128>, all_assets: &Vec<Asset>, timestamp: u64) {
let mut event_updates = Vec::new(&e);
for (index, asset) in all_assets.iter().enumerate() {
let price = updates.get(index as u32).unwrap_or_default();
if price == 0 {
continue; //skip zero prices // ⚠️ Causes data loss for indexers
}
let symbol = match asset { /* ... */ };
event_updates.push_back((symbol, price));
}
e.events().publish_event(&UpdateEvent { timestamp, update_data: event_updates });
}This design assumes zero prices are invalid, but legitimate scenarios exist where assets have zero prices (data source failures, new asset initialization pending data availability).
Internal Pre-conditions
- Admin needs to call
set_price()to set at least one asset price to be exactly0 - Other assets need to have non-zero prices in the same update batch
External Pre-conditions
- Off-chain indexer needs to be listening to
UpdateEventemissions - External application needs to rely on events for price tracking (rather than querying on-chain storage directly)
Attack Path
- Admin calls
set_prices([BTC=105_0000000, ETH=0, SOL=28_0000000], timestamp)to update prices where ETH data source has failed - The contract stores all three prices correctly in on-chain storage (including ETH=0)
publish_update_event()is called internally- The function iterates through assets and skips ETH due to
if price == 0 { continue } - The emitted event contains only
[(BTC, 105_0000000), (SOL, 28_0000000)]without ETH - Off-chain indexer receives the event and incorrectly infers "ETH was not updated, keep previous value"
- External applications display stale ETH price (e.g., 50_0000000 from previous update)
- Actual on-chain state shows ETH=0, creating a discrepancy between on-chain truth and off-chain indexes
Impact
Off-chain applications suffer data inconsistency, displaying incorrect prices when assets are updated to zero. The impact is limited to information display as:
- On-chain contract state remains correct
- Direct contract queries return accurate zero prices
- No funds are at risk
PoC
Add this test to oracle/src/tests/events_test.rs:
#[cfg(test)]
mod tests {
use super::*;
use soroban_sdk::{Env, vec};
#[test]
fn test_zero_price_excluded_from_event() {
let env = Env::default();
// Setup: Three assets with one zero price
let all_assets = vec![&env, Asset::BTC, Asset::ETH, Asset::SOL];
let updates = vec![&env, 105_0000000_i128, 0_i128, 28_0000000_i128];
let timestamp = 2000u64;
// Publish event
publish_update_event(&env, &updates, &all_assets, timestamp);
// Check emitted events
let events = env.events().all();
let last_event = events.last().unwrap();
let update_event: UpdateEvent = last_event.data.try_into().unwrap();
// Current behavior: Only 2 assets in event (BTC, SOL)
assert_eq!(update_event.update_data.len(), 2); // ❌ Should be 3
// Expected behavior: All 3 assets should be included
// assert_eq!(update_event.update_data.len(), 3);
// assert_eq!(update_event.update_data.get(1).unwrap(), (Asset::ETH.to_symbol(), 0));
}
#[test]
fn test_on_chain_storage_includes_zero() {
let env = Env::default();
let contract = setup_contract(&env);
// Admin sets ETH price to 0
contract.set_price(&Asset::ETH, 0, 2000);
// On-chain query returns zero (correct)
let price_data = contract.price(&Asset::ETH, 2000);
assert_eq!(price_data.unwrap().price, 0); // ✅ On-chain correct
// But event didn't include ETH (data loss for indexers)
}
}Run with: cargo test test_zero_price_excluded_from_event
Mitigation
Remove the zero-price skip logic to include all asset updates in events:
pub fn publish_update_event(e: &Env, updates: &Vec<i128>, all_assets: &Vec<Asset>, timestamp: u64) {
let mut event_updates = Vec::new(&e);
for (index, asset) in all_assets.iter().enumerate() {
let price = updates.get(index as u32).unwrap_or_default();
- if price == 0 {
- continue; //skip zero prices
- }
let symbol = match asset { /* ... */ };
event_updates.push_back((symbol, price)); // Include all prices, even zeros
}
e.events().publish_event(&UpdateEvent { timestamp, update_data: event_updates });
}Benefits:
- Complete event data for off-chain indexers
- Clear distinction between "not updated" vs "updated to zero"
- Minimal cost increase (~32 bytes per zero-price asset, approximately $0.00003)
Alternative: Add a separate field to explicitly list zero-price assets:
pub struct UpdateEvent {
pub timestamp: u64,
pub update_data: Vec<(Val, i128)>, // Non-zero prices
pub zero_price_assets: Vec<Val>, // Assets with zero prices
}This preserves backward compatibility but increases implementation complexity.