Skip to content

chore(orderbook): grow an ask only from shares the seller holds - #1450

Merged
MicBun merged 1 commit into
mainfrom
chore/change-ask-holdings
Oct 23, 2026
Merged

MicBun merged 1 commit into
mainfrom
chore/change-ask-holdings

Conversation

@MicBun

@MicBun MicBun commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • change_ask now rejects growing an ask when the seller has no holdings row for that outcome, the same way it already rejects growing an ask past the shares the seller holds.
  • The holdings check was one condition, $held_amount IS NULL OR $held_amount < $additional_shares. With no holdings row, $held_amount is NULL, and the engine evaluates TRUE OR NULL to NULL, so for that seller the check did not raise its error. It is now two checks, IS NULL and then <, which is how place_sell_order already checks holdings.
  • Callers see the same "Insufficient shares in holdings" message as before.

How I checked

  • New tests/streams/order_book/change_ask_holdings_test.go covers growing an ask right after a split, growing it back to full size after a partial fill (the seller then re-quotes what it holds and the market settles), and every branch of the holdings check: too few held, more than enough, exactly enough and nothing held.
  • TestChangeOrder, TestMatchingEngine, TestSettlement, TestPlaceSplitLimitOrder, TestValidateMarketCollateral and TestOrderEvents pass.

Summary by CodeRabbit

  • Bug Fixes
    • Ask increases that require shares now report whether holdings are missing or insufficient. Missing holdings are shown as zero shares, while insufficient holdings display the recorded balance, making the amount available clearer.

@MicBun MicBun self-assigned this Oct 23, 2026
@holdex

holdex Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
MicBun 2h ✅ Submitted Oct 23, 2026, 4:40 PM

Submit or update total time with:

@holdex pr submit-time 2h

Add time on top of previous submission with:

@holdex pr add-time 1h30m

See available commands to help comply with our Guidelines.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fccdfce3-f9d6-4b4d-9513-72ccc6ad872f
📥 Commits

Reviewing files that changed from the base of the PR and between 793a8db and fb3ba9e.

📒 Files selected for processing: 2
  • internal/migrations/032-order-book-actions.sql
  • tests/streams/order_book/change_ask_holdings_test.go
+---------------------------------------------------+
| Great artists ship, but not before a code review. |
+---------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MicBun

MicBun commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@holdex pr submit-time 2h

@MicBun
MicBun merged commit ca0d812 into main Oct 23, 2026
10 of 11 checks passed
@MicBun
MicBun deleted the chore/change-ask-holdings branch October 23, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant