fix: improve price feed error handling and add edge-case tests#535
Open
leventfhxy-sudo wants to merge 4 commits into
Open
fix: improve price feed error handling and add edge-case tests#535leventfhxy-sudo wants to merge 4 commits into
leventfhxy-sudo wants to merge 4 commits into
Conversation
|
Someone is attempting to deploy a commit to the Ayomide Adeniran's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves oracle/price-feed error reporting and arithmetic safety, and updates contract logic and tests accordingly.
Changes:
- Added new contract error codes for price availability/confidence and arithmetic overflow.
- Hardened oracle and storage arithmetic (checked ops) and improved error specificity.
- Updated contract flows and added tests for new edge cases (price missing, low confidence, overflow).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/synthetic-assets/src/types.rs | Introduces new error variants/codes for price-feed and arithmetic failures. |
| contracts/synthetic-assets/src/test.rs | Adds edge-case tests for price feed adapter behavior and arithmetic overflow paths. |
| contracts/synthetic-assets/src/storage.rs | Makes position counter increment fallible (overflow-safe) and distinguishes missing price from invalid price. |
| contracts/synthetic-assets/src/oracle.rs | Adds checked arithmetic for staleness/deviation checks and returns more precise oracle errors. |
| contracts/synthetic-assets/src/lib.rs | Propagates the new fallible counter increment and adjusts price checks on burns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+126
| #[should_panic(expected = "Error(Contract, #17)")] | ||
| fn test_initialize_invalid_collateral_ratio() { | ||
| let (_env, client, admin, oracle, collateral_token) = setup_env(); | ||
|
|
||
| // Additional tests for Price Feed Adapter edge cases and errors | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "Error(Contract, #24)")] | ||
| fn test_storage_price_not_available() { | ||
| let (env, _client, _admin, _oracle, _collateral) = setup_env(); | ||
| let symbol = soroban_sdk::Symbol::new(&env, "MISSING"); | ||
|
|
||
| // Directly call storage getter which should return Err(Error::PriceNotAvailable) | ||
| crate::storage::get_price(&env, &symbol).unwrap(); | ||
| } | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "Error(Contract, #25)")] | ||
| fn test_update_price_low_confidence() { | ||
| let (env, client, _admin, _oracle, _collateral) = setup_contract(); | ||
| let symbol = soroban_sdk::Symbol::new(&env, "sUSD"); | ||
|
|
||
| client.register_synthetic_asset(&symbol, &String::from_str(&env, "Synthetic USD"), &8u32, &100000000i128); | ||
|
|
||
| // Low confidence below MIN_CONFIDENCE should be rejected with LowConfidence | ||
| client.update_price(&symbol, &105000000i128, &10u32); | ||
| } | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "Error(Contract, #6)")] | ||
| fn test_update_price_confidence_too_high_invalid() { |
| return Err(Error::StalePrice); | ||
| } | ||
| } | ||
| None => return Err(Error::StalePrice), |
Comment on lines
81
to
84
| pub fn calculate_price_deviation(old_price: i128, new_price: i128) -> u32 { | ||
| if old_price == 0 { | ||
| return 0; | ||
| if old_price <= 0 { | ||
| return u32::MAX; | ||
| } |
Comment on lines
115
to
121
| pub fn is_price_valid_deviation(old_price: i128, new_price: i128, max_deviation: u32) -> bool { | ||
| calculate_price_deviation(old_price, new_price) <= max_deviation | ||
| let dev = calculate_price_deviation(old_price, new_price); | ||
| if dev == u32::MAX { | ||
| return false; | ||
| } | ||
| dev <= max_deviation | ||
| } |
Comment on lines
+262
to
+263
| set_collateral_position(&env, position_id, &position); | ||
| increment_position_counter(&env, 1); | ||
| increment_position_counter(&env, 1)?; |
Comment on lines
+128
to
+133
| #[should_panic(expected = "Error(Contract, #24)")] | ||
| fn test_storage_price_not_available() { | ||
| let (env, _client, _admin, _oracle, _collateral) = setup_env(); | ||
| let symbol = Symbol::new(&env, "MISSING"); | ||
|
|
||
| crate::storage::get_price(&env, &symbol).unwrap(); |
Comment on lines
+193
to
+198
| #[should_panic(expected = "Error(Contract, #26)")] | ||
| fn test_increment_position_counter_overflow() { | ||
| let (env, _client, _admin, _oracle, _collateral) = setup_env(); | ||
|
|
||
| crate::storage::set_position_counter(&env, u64::MAX); | ||
| crate::storage::increment_position_counter(&env, 1).unwrap(); |
Comment on lines
488
to
491
| set_trading_position(&env, position_id, &position); | ||
| increment_position_counter(&env, 1); | ||
| increment_position_counter(&env, 1)?; | ||
|
|
||
| Ok(position_id) |
Comment on lines
89
to
108
| let diff = if new_price >= old_price { | ||
| match new_price.checked_sub(old_price) { | ||
| Some(v) => v, | ||
| None => return Err(Error::Overflow), | ||
| } | ||
| } else { | ||
| old_price - new_price | ||
| match old_price.checked_sub(new_price) { | ||
| Some(v) => v, | ||
| None => return Err(Error::Overflow), | ||
| } | ||
| }; | ||
|
|
||
| // Multiply then divide with checked ops to avoid overflow | ||
| let scaled = match diff.checked_mul(10000) { | ||
| Some(v) => match v.checked_div(old_price) { | ||
| Some(d) => d, | ||
| None => return Err(Error::Overflow), | ||
| }, | ||
| None => return Err(Error::Overflow), | ||
| }; |
added 2 commits
May 26, 2026 22:57
… before persisting trading position
|
true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
name: Pull Request
about: Submit changes to Soroban Playground
title: "Fix: Improve Price Feed Adapter error handling"
labels: "bug"
assignees: ""
What does this PR do?
Enhances error handling and edge-case management in the Price Feed Adapter: fixes inaccurate error types, adds stale/invalid price checks, prevents arithmetic overflow, improves deviation validation, and adds unit tests for the new cases.
Related Issue
Closes
Type of Change
🗂️ Affected Area(s)
How Has This Been Tested?
Rust tests were not run locally due to missing Cargo/Rust toolchain. Unit tests were added to cover PriceNotAvailable, LowConfidence, Overflow, StalePrice, InvalidPrice, and deviation overflow paths.
npm test)Screenshots / Demo (if applicable)
Before
N/A
After
N/A
Checklist
Related Issues or References
N/A
Additional Context
ABI/Storage/Event compatibility preserved. Changes are limited to Price Feed Adapter error handling and edge-case management.