Enforce stock plan reserved share ceilings#252
Conversation
📝 WalkthroughWalkthroughThis PR adds reserved-share pool ceiling validation to the CapTable DAML template, bumps OpenCapTable-v34 to 0.0.5, and adds/updates DAML tests for stock plan pool adjustments, return-to-pool behavior, and issuance allocation limits. ChangesStock plan pool ceiling validation
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@copilot review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87737fa232
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case issuance.issuance_data.stock_plan_id of | ||
| Some planId -> | ||
| if planId == stockPlanId | ||
| then pure (total + issuance.issuance_data.quantity) |
There was a problem hiding this comment.
When an EquityCompensationIssuance has a negative quantity (currently not rejected by validateEquityCompensationIssuanceOcfData), adding it here reduces the computed allocation for the stock plan. That lets a batch pair an oversized positive plan issuance with a negative equity-compensation issuance for the same plan and still pass the new allocatedQuantity <= reservedShares check, bypassing the reserved-share ceiling this change is trying to enforce.
Useful? React with 👍 / 👎.
| assertMsg | ||
| ("Stock plan reserved shares exceeded for " <> stockPlan.plan_data.id <> ": allocated " <> show allocatedQuantity <> " exceeds reserved " <> show reservedShares) | ||
| (allocatedQuantity <= reservedShares)) | ||
| (Map.values maps.stock_plans) |
There was a problem hiding this comment.
When a batch deletes a StockPlan that is still referenced by StockIssuance.stock_plan_id, this loop only visits surviving stock plans, so the deleted plan id is never passed to stockPlanNetAllocatedQuantity and its allocations disappear from the reserved-share check. Unlike equity compensation, stock issuances are not revalidated after deletes, so this leaves orphaned plan-backed stock and bypasses the new pool ceiling entirely.
Useful? React with 👍 / 👎.
| case issuance.issuance_data.stock_plan_id of | ||
| Some planId -> | ||
| if planId == stockPlanId | ||
| then pure (total + issuance.issuance_data.quantity) |
There was a problem hiding this comment.
When a plan-backed stock issuance is followed by a StockClassSplit for its class, this adds the original issuance quantity rather than the split-adjusted current quantity already computed for authorized-share checks. A 100-share plan issuance from a 100-share pool followed by a 2-for-1 split still contributes only 100 here, so the cap table can end with 200 plan-backed stock shares while passing the new reserved-pool ceiling unless a separate return/issuance changes the original quantity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements a final-state invariant in UpdateCapTable to ensure plan-backed issuances (stock + equity compensation), net of valid returns-to-pool, never exceed a stock plan’s effective reserved share pool (latest pool adjustment by date, same-date ties choosing the lower reserve; otherwise the plan’s initial reserve). This tightens correctness around stock plan capacity accounting and return-to-pool integrity.
Changes:
- Add stock plan pool ceiling validation (
validateStockPlanPoolCeilings) to the cap table update flow, including stricter return-to-pool constraints. - Add a comprehensive
TestStockPlanReservedSharessuite and update existing return-to-pool smoke tests to use actual plan-backed securities. - Bump
OpenCapTable-v34to0.0.5and update test DAR/data dependencies and lockfiles accordingly.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Test/daml/OpenCapTable/TestStockPlanReturnToPool.daml | Updates return-to-pool smoke tests to create real plan-backed issuances before returning shares. |
| Test/daml/OpenCapTable/TestStockPlanReservedShares.daml | Adds new test coverage for reserved-share ceiling enforcement, adjustments precedence, and return-to-pool constraints. |
| Test/daml.yaml | Updates test data-dependency DAR to OpenCapTable-v34-0.0.5. |
| scripts/codegen/templates/CapTable.daml.template | Adds pool adjustment precedence/indexing plus final-state stock plan pool ceiling and return-to-pool validations, wired into UpdateCapTable. |
| OpenCapTable-v34/daml.yaml | Bumps package version from 0.0.4 to 0.0.5. |
| dars/OpenCapTable-v34/0.0.5/OpenCapTable-v34.dar | Adds the new immutable DAR artifact (LFS pointer). |
| dars/dars.lock | Records the new DAR’s sha/size/sdkVersion/upload timestamp. |
| stockPlanNetAllocatedQuantity : CapTableMaps -> Text -> Update Decimal | ||
| stockPlanNetAllocatedQuantity maps stockPlanId = do | ||
| stockAllocation <- stockIssuancePlanAllocation maps stockPlanId | ||
| equityCompensationAllocation <- equityCompensationPlanAllocation maps stockPlanId | ||
| returnedQuantity <- stockPlanReturnedQuantity maps stockPlanId | ||
| pure (stockAllocation + equityCompensationAllocation - returnedQuantity) |
| stockPlanReturnedQuantityForSecurity : CapTableMaps -> Text -> Update Decimal | ||
| stockPlanReturnedQuantityForSecurity maps securityId = | ||
| foldlA | ||
| (\total returnCid -> do | ||
| returnToPool <- fetch returnCid | ||
| if returnToPool.return_data.security_id == securityId | ||
| then pure (total + returnToPool.return_data.quantity) | ||
| else pure total) | ||
| 0.0 | ||
| (Map.values maps.stock_plan_return_to_pools) |
…nto codex/ocf-stock-plan-pool-ceiling-v2 # Conflicts: # scripts/codegen/templates/CapTable.daml.template
…nto codex/ocf-stock-plan-pool-ceiling-v2
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b7debe1. Configure here.
|
@copilot review |
Addressed with commit |
…nto codex/ocf-stock-plan-pool-ceiling-v2
…nto codex/ocf-stock-plan-pool-ceiling-v2
|
Closed as superseded by #238. The stock plan reserved-share ceiling work is now folded into the authorized/reserved share ceiling category branch. I carried over the actionable review feedback before closing this PR:
Local validation on #238: |
Summary
Stacked on #251.
This adds a final-state stock-plan pool invariant: securities issued from a stock plan cannot leave the plan with more net allocated shares than its effective reserved pool.
The effective reserved pool is:
StockPlanPoolAdjustment.shares_reserved, when present;StockPlan.initial_shares_reserved.The net allocated amount is stock issuances plus equity-compensation issuances tied to the plan, minus explicit
StockPlanReturnToPoolquantities. Returns are also validated so they cannot manufacture capacity: each return must reference a stock or equity-compensation security from the same plan, and cumulative returns for a security cannot exceed that security's original allocation.The check runs after deletes, edits, issuer edit, and creates have all been processed, so a single batch can raise the pool or return shares and issue from the corrected pool while still rejecting invalid final state.
OCF documentation
StockPlan.initial_shares_reserved: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/StockPlan.schema.jsonStockPlanPoolAdjustment.shares_reserved: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/adjustment/StockPlanPoolAdjustment.schema.jsonStockPlanReturnToPool: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/return_to_pool/StockPlanReturnToPool.schema.jsonStockIssuance.stock_plan_idandquantity: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/issuance/StockIssuance.schema.jsonEquityCompensationIssuance.stock_plan_idandquantity: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/issuance/versions/EquityCompensationIssuance.v1.schema.jsonTests
Added
TestStockPlanReservedSharescoverage for:Updated existing return-to-pool smoke tests to create real plan-backed securities before returning them to pool.
Validation
NODE_PATH=/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules PATH="/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules/.bin:$HOME/.dpm/bin:$PATH" npm run buildTest/:JAVA_HOME=/opt/homebrew/opt/openjdk@17 PATH="/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules/.bin:$HOME/.dpm/bin:$PATH" dpm test --show-coverage --color --coverage-ignore-choice 'splice-amulet:.*'NODE_PATH=/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules PATH="/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules/.bin:$HOME/.dpm/bin:$PATH" npm run lint:damlNODE_PATH=/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules PATH="/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules/.bin:$HOME/.dpm/bin:$PATH" npm run verify-darsJAVA_HOME=/opt/homebrew/opt/openjdk@17 NODE_PATH=/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules PATH="/Users/fairnick/Documents/code/fairmint/open-captable-protocol-daml/node_modules/.bin:/opt/homebrew/opt/openjdk@17/bin:$HOME/.dpm/bin:$PATH" npm run check-upgrade-compatNote
Medium Risk
Changes core cap-table mutation validation for equity plan pools and returns, which can block previously accepted
UpdateCapTablebatches; scope is bounded to stock-plan accounting with broad script coverage.Overview
Cap table
UpdateCapTablenow rejects invalid final state where net plan allocations exceed the effective reserved pool (latestStockPlanPoolAdjustment.shares_reserved, elseStockPlan.initial_shares_reserved). Net allocation sums stock and equity-compensation issuances tied to the plan and subtractsStockPlanReturnToPoolamounts.Return-to-pool rules are tightened: returns must reference a plan-backed security on the same plan, and cumulative returns per security cannot exceed that security’s issuance quantity. Pool adjustments use same-date precedence where the lower
shares_reservedwins (mirroring other adjustment types). Validation runs after the full batch (creates, edits, deletes), so a single update can raise the pool or return shares and then issue from the corrected ceiling.OpenCapTable-v34 is bumped to 0.0.5 with an updated packaged DAR. Tests add
TestStockPlanReservedSharesfor over-issuance, same-batch adjustments, return reuse/limits, and edits/deletes that would shrink the pool below outstanding allocations; existing return-to-pool smoke tests now create real plan-backed securities first.Reviewed by Cursor Bugbot for commit c21a636. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests