fix(core, proto): use i128 for price instead of u128 #1991
Merged
Conversation
This file contains 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
github-actions
bot
added
proto
SuperFluffy
approved these changes
Feb 20, 2025
| assert_eq!(expected, actual); | ||
| } | ||
| #[test] | ||
| fn i128_roundtrips_work() { |
There was a problem hiding this comment.
This should contain the case of i128::MIN, -1, and other distinct negative i128 values.
| #[test] | ||
| fn can_parse_negative_price() { | ||
| let price = "-1".parse::<Price>().unwrap(); | ||
| assert_eq!(price.get(), -1); |
There was a problem hiding this comment.
IMO compare on Price::new(-1) instead.
noot
changed the title
@SuperFluffy no, there shouldn't be any case where prices are turned into a u128, the |
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.
Summary
it's theoretically possible for the price of an asset to become negative, although rare. in the case where a price became negative, the oracle would be unable to decode the price from the sidecar.
Background
resolves issue 3.2 "Price can become negative" in the zellic audit of the sequencer-side oracle implementation.
Changes
Int128proto type, similar to our existing primitiveUint128Pricein core to bei128internally, instead ofu128Testing
unit tests