feat: stylus parse price feed updates fxn #2840
Conversation
…g vector indices - Modified parse_price_feed_updates_internal to build a BTreeMap internally mapping price IDs to PriceInfoReturn - Updated parse_price_feed_updates_with_config to return a vector with matching indices to input price_ids - Added price_ids parameter to parse_price_feed_updates_internal function - Maintained existing error handling and validation logic Co-Authored-By: [email protected] <[email protected]>
…y ordering - parse_price_feed_updates_internal now returns Vec<([u8; 32], PriceInfoReturn)> without taking price_ids parameter - parse_price_feed_updates_with_config converts internal result to BTreeMap for efficient lookup and returns ordered vector - This enables update_price_feeds functions to use the dictionary directly as requested Co-Authored-By: [email protected] <[email protected]>
…eed data - Extract price data from price_feed_message instead of reading from storage - Fixes test failures where price data wasn't being processed correctly - Maintains separation of concerns between parsing and storage updates Co-Authored-By: [email protected] <[email protected]>
- Add validation to check if publish times are within [min_allowed_publish_time, max_allowed_publish_time] range - Return PythReceiverError::PriceFeedNotFoundWithinRange (error code 16) when publish times are outside allowed range - Fix type casting from u64 to i64 for proper comparison with price_feed_message.publish_time Co-Authored-By: [email protected] <[email protected]>
…es_internal - Add uniqueness checking logic when check_uniqueness parameter is true - Require minAllowedPublishTime > prevPublishTime for uniqueness validation - Return PythReceiverError::PriceFeedNotFoundWithinRange when uniqueness check fails - Retrieve previous publish time from latest_price_info storage Co-Authored-By: [email protected] <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| _unique: bool, | ||
| ) -> Result<Vec<([u8; 32], PriceInfoReturn)>, PythReceiverError> { | ||
| let price_pairs = self.parse_price_feed_updates_internal( | ||
| update_data, | ||
| min_publish_time, | ||
| max_publish_time, | ||
| false, // check_uniqueness |
There was a problem hiding this comment.
why not pass unique to parse_price_feed_updates_internal?
| let price_info_return = ( | ||
| U64::from(publish_time), | ||
| I32::from_be_bytes(price_feed_message.exponent.to_be_bytes()), | ||
| I64::from_be_bytes(price_feed_message.price.to_be_bytes()), | ||
| U64::from(price_feed_message.conf), | ||
| I64::from_be_bytes(price_feed_message.ema_price.to_be_bytes()), | ||
| U64::from(price_feed_message.ema_conf), | ||
| ); |
There was a problem hiding this comment.
Noticed you're turning the signed ints to bytes and back again. There should be from/try_from impls for them as well, any reason you're going to bytes and back?
There was a problem hiding this comment.
the alloy_primitives crate doesn't have a normal From<> for the I32 type, so this was the cleanest workaround I could think of.
There was a problem hiding this comment.
There is a try_from which seems to work in my brief testing, would try that out. Also, it looks like the from and from_be_bytes methods panic if the conversion fails. I think it would be better to use try_from and gracefully catch and propagate any errors. (Can do this in a future PR)
There was a problem hiding this comment.
Ok will do. Will add this into the next PR that I'll send out in a second
| fn get_total_fee(&self, total_num_updates: u64) -> U256 { | ||
| U256::from(total_num_updates).saturating_mul(self.single_update_fee_in_wei.get()) | ||
| + self.transaction_fee_in_wei.get() | ||
| } |
There was a problem hiding this comment.
We should just have get_update_fee(&self, update_data: Vec<Vec<u8>>) since the old version that just takes the update data length is deprecated
There was a problem hiding this comment.
Yeah, I went ahead and compressed the two functions. This one was just a helper function for the public facing get_update_fee function though that took the whole update data.
| } | ||
|
|
||
| #[motsu::test] | ||
| fn test_multiple_updates_same_id_updates_latest( |
There was a problem hiding this comment.
The next PR in the line completely cleaned up the test suite and added more, so if it's okay can we circle back to reviewing the tests on that PR?
| &mut self, | ||
| update_data: Vec<u8>, | ||
| ) -> Result<(), PythReceiverError> { | ||
| _price_ids: Vec<[u8; 32]>, |
There was a problem hiding this comment.
Looks like price_ids is unused here because we don't pass it in parse_price_feed_updates_with_config -- we parse and update all the data, and then filter them using price_ids. There was a subtle griefing attack possibility with this behavior in the Pulse use case, but I don't think that's really a factor here and there are other mitigations (check_update_data_is_minimal.)
We can remove the _price_ids arg here.
2 participants
Summary
Implemented the parse price feed updates function for the stylus contract, and restructured the architecture of the update functions to deal with that.
Rationale
parsePriceFeedUpdates is a necessary component of the Pythnet API.
How has this been tested?
I have test cases validating these methods.