feat(rtds): chainlink twap price streams - #105
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
7fa9510 to
eee742f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit eee742f. Configure here.
| pub fn unsubscribe_chainlink_twap_prices( | ||
| &self, | ||
| twap_window: impl Into<Option<ChainlinkTwapWindow>>, | ||
| ) -> Result<()> { |
There was a problem hiding this comment.
Awkward unsubscribe None API
Low Severity
unsubscribe_chainlink_twap_prices takes impl Into<Option<ChainlinkTwapWindow>> while its docs say to pass None for all windows. Bare None does not infer here, unlike sibling helpers such as unsubscribe_comments(Option<...>), so the documented call shape is awkward to use.
Reviewed by Cursor Bugbot for commit eee742f. Configure here.
There was a problem hiding this comment.
Bare None does infer here. These would all be valid calls:
client.unsubscribe_chainlink_twap_prices(ChainlinkTwapWindow::SixtySeconds)?;
client.unsubscribe_chainlink_twap_prices(ChainlinkTwapWindow::ThirtySeconds)?;
client.unsubscribe_chainlink_twap_prices(None)?;
|
Thanks for adding this — PMKit is consuming the 60-second Chainlink TWAP topic and this typed API is preferable to its temporary I validated this commit cherry-picked on top of #110: 167 library tests and all 45 WebSocket tests pass together. Two small blockers for our use:
Happy to retain and consume this PR once the exact value is exposed. |
|
Follow-up: I combined #105 with #110 on fork branch |
2 participants
Pretty self explanatory, adds support for the RTDS Chainlink TWAP Prices.
The 5m and 15m crypto markets recently started using this as their resolution source.
Pretty much just mirrored the existing chailink price stuff, with some minor refactoring (some constants for topics etc) along the way. Also added the explicit captures to RTDS streams (
+ use<S>etc) like I did with clob ws streams so consumers can actually do stuff with them like merging, mapping, filtering etc (see this PR in the old SDK repo for more info on that).I know this PR won't ever get merged because it seems you guys have stopped merging external PRs (maybe because there are too many LLM PRs these days? maybe because you're working on a new rust SDK that replaces this one again? idk), but opening it anyway so others can merge into their forks if they want.
Tested it out on some actual feeds and it works:
Note
Low Risk
Additive RTDS client API and serialization tests; no auth, trading, or data-handling changes.
Overview
Adds RTDS support for Chainlink TWAP price feeds (
crypto_prices_twap_{thirty,sixty}), matching the existing Chainlink spot stream.subscribe_chainlink_twap_pricestakes an optional symbol and aChainlinkTwapWindow. Unsubscribe can target one window or all. Filters still serialize as escaped JSON strings for Chainlink topics.Also adds precise
+ use<>/+ use<S>captures on RTDS subscribe methods so returned streams can be composed. New types (ChainlinkTwapPrice,ChainlinkTwapWindow) are re-exported.Reviewed by Cursor Bugbot for commit eee742f. Bugbot is set up for automated code reviews on this repo. Configure here.