-
Notifications
You must be signed in to change notification settings - Fork 6
feat/finishing rfq #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat/finishing rfq #193
Conversation
8ball030
commented
Jan 23, 2026
- chore:removed-dead-code
- feat:e2e-for-quoting
- feat:added-rfq-demo
- feat:added-simple-rfq-quoter
| while True: | ||
| portfolio_delta: Decimal | None = None | ||
| while True: | ||
| try: | ||
| portfolio_delta = self.hedging_queue.get_nowait() | ||
| except asyncio.QueueEmpty: | ||
| break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment explaining the drainage pattern:
# Drain queue to get most recent delta (we only care about latest state, not history)
while True:
try:
portfolio_delta = self.hedging_queue.get_nowait()
except asyncio.QueueEmpty:
breakThis clarifies why you're discarding intermediate values.
| async def calculate_delta_from_quote( | ||
| self, quote: QuoteResultSchema | RFQResultPublicSchema | ||
| ) -> tuple[Decimal, bool]: | ||
| """Calculates the hedge amount needed to neutralize delta exposure from the quote.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring should clarify what happens on error:
"""
Calculates the hedge amount needed to neutralize delta exposure from the quote.
Returns:
tuple[Decimal, bool]: (total_delta, is_error)
- total_delta: Calculated delta exposure (partial sum if error occurred)
- is_error: True if option pricing data was unavailable for any leg
"""This makes the return semantics explicit.
| async with self.quoting_lock: | ||
| if quote.status == Status.expired and quote.rfq_id in self.quotes: | ||
| del self.quotes[quote.rfq_id] | ||
| self.logger.info(f" ✗ Our quote {quote.quote_id} expired Better luck next time!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing punctuation:
self.logger.info(f" ✗ Our quote {quote.quote_id} expired. Better luck next time!")
| # we are the SELLER of the quote so we are in efffect taking the opposite side of the leg direction | ||
| # we therefore subtract the delta for buy legs and add for sell legs | ||
| if leg.direction == Direction.sell: | ||
| total_delta += leg_delta | ||
| else: | ||
| total_delta -= leg_delta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "efffect" → "effect"
Also, this logic is critical but might be confusing. Consider expanding:
# As the quoter (seller), we take the opposite side of each leg's direction:
# - If leg.direction is SELL, the taker buys from us → we are short → negative delta for us
# - If leg.direction is BUY, the taker sells to us → we are long → positive delta for us
# But since we're calculating what WE need to hedge, we invert:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the quoter is selling the quote and leg.direction == Direction.sell, that means the taker wants to sell that leg, so the quoter buys it, gaining positive delta. But your code adds leg_delta in this case.
This might be a bug or I'm misunderstanding the perspective. Can you clarify what leg.direction represents in the RFQ context? Is it the taker's desired direction or the quoter's position?
| self.quotes.pop(quote.rfq_id, None) | ||
| self.logger.info(f" ✗ Our quote {quote.quote_id} expired. Better luck next time!") | ||
|
|
||
| async def run(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it appears such that this, unambiguously, requires a match statement! And full pattern matching if the Enum! I like good enough, but this isn't there yet! We shall exhaustively match quote.status, even if we merely log all of those of any another designation than Status.filled!
|
|
||
| all_tickers = {} | ||
| for expiry in {instrument.option_details.expiry for instrument in instruments}: | ||
| expiry_date = datetime.fromtimestamp(expiry, tz=timezone.utc).strftime("%Y%m%d") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fucking ffabulous, now we call get tickers on every one of the 500 markets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was done to future proof the example, as get_ticker is deprecated.
|
https://github.com/8ball030/derive_client/pull/201/changes |