Enforce issuer authorized share ceiling#243
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ing' into codex/ocf-stock-issuance-reference-integrity
…ce-integrity' into codex/ocf-stock-plan-class-reference-integrity
…ence-integrity' into codex/ocf-issuer-adjustment-reference-integrity
…rence-integrity' into codex/ocf-issuer-aggregate-share-ceiling
There was a problem hiding this comment.
Pull request overview
Adds a final-state cap-table invariant ensuring that when an issuer has a finite authorized-share ceiling, the sum of effective stock-class authorized shares cannot exceed it (and any unlimited stock class is rejected under a finite issuer cap). This integrates into the UpdateCapTable batch flow so the invariant is evaluated once after deletes/edits/issuer edit/creates have all been applied.
Changes:
- Implement issuer-vs-stock-class authorized share ceiling validation in the generated
CapTablelogic (final-state check inUpdateCapTable). - Add a dedicated
TestIssuerAuthorizedSharestest module covering issuer/stock-class authorization interactions and edge cases (including unlimited stock classes under finite issuer caps). - Adjust test fixtures to use a higher default issuer authorization and make the “unlimited stock class issuance” test explicitly use an uncapped issuer.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Test/daml/OpenCapTable/TestStockClassAuthorizedShares.daml | Makes the unlimited-stock-class issuance test explicitly operate under an uncapped issuer by editing issuer authorization to None. |
| Test/daml/OpenCapTable/TestIssuerAuthorizedShares.daml | Adds new test coverage for issuer authorized-share ceiling enforcement across creates/edits/deletes/adjustments. |
| Test/daml/OpenCapTable/TestHelpers.daml | Introduces defaultIssuerWithAuthorizedShares to reduce issuer-edit boilerplate in tests. |
| Test/daml/OpenCapTable/Setup.daml | Raises the default test issuer authorization ceiling to avoid unrelated failures from the new invariant. |
| scripts/codegen/templates/CapTable.daml.template | Adds issuer authorized-share limit computation, stock-class authorization total computation, and final-state invariant validation in UpdateCapTable. |
| dars/OpenCapTable-v34/0.0.2/OpenCapTable-v34.dar | Updates the compiled DAR artifact to include the new invariant and related changes. |
| dars/dars.lock | Updates locked DAR hash/size metadata to match the regenerated artifact. |
| (\totalOpt stockClassCid -> do | ||
| stockClass <- fetch stockClassCid | ||
| stockClassLimitOpt <- currentStockClassAuthorizedLimit maps stockClass.stock_class_data.id None None None | ||
| pure (addOptionalAuthorizedShares totalOpt stockClassLimitOpt)) |
![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: 14f3e5b5c6
ℹ️ 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".
| validateIssuerAuthorizedShares : CapTableMaps -> ContractId Issuer -> Update () | ||
| validateIssuerAuthorizedShares maps issuerCid = do | ||
| issuerLimitOpt <- currentIssuerAuthorizedLimit maps issuerCid | ||
| stockClassTotalOpt <- currentStockClassAuthorizedTotal maps |
There was a problem hiding this comment.
When the issuer has no finite cap (None/enum initial shares and no issuer adjustment), this still fetches every stock class and, for each one, scans all stock-class authorization adjustments before the None -> pure () branch returns. On large uncapped cap tables, unrelated updates pay O(stock classes × adjustments) ledger reads and can hit transaction limits even though there is no issuer ceiling to enforce; compute currentStockClassAuthorizedTotal only inside the Some issuerLimit branch.
Useful? React with 👍 / 👎.
| foldlA | ||
| (\totalOpt stockClassCid -> do | ||
| stockClass <- fetch stockClassCid | ||
| stockClassLimitOpt <- currentStockClassAuthorizedLimit maps stockClass.stock_class_data.id None None None |
There was a problem hiding this comment.
When the issuer has a finite cap, every UpdateCapTable folds over all stock classes and this call scans all stock-class authorization adjustments again for each class. A cap table with N stock classes and M authorization adjustments therefore performs O(N×M) contract fetches even for an unrelated update, which can make otherwise valid transactions exceed ledger limits; build the latest adjustment per class in one pass and sum from that index instead.
Useful? React with 👍 / 👎.
| None -> | ||
| assertMsg "Issuer authorized shares exceeded: stock class authorization is unlimited but issuer authorization is finite" False |
There was a problem hiding this comment.
For a finite issuer, this branch rejects any stock class whose effective authorization is None, but authorizedSharesLimit returns None for both OcfAuthorizedSharesUnlimited and OcfAuthorizedSharesNotApplicable. That means a valid OCF stock class using NOT_APPLICABLE is treated as if it authorizes unlimited shares and blocks all future updates under a finite issuer cap; distinguish the enum variants so only UNLIMITED triggers this failure.
Useful? React with 👍 / 👎.
HardlyDifficult
changed the base branch from
codex/ocf-issuer-adjustment-reference-integrity
to
codex/ocf-reference-integrity
|
Retargeted this non-reference aggregate-ceiling PR onto #251 so it stays stacked above the consolidated reference-integrity branch after the old reference-check stack was closed. |
|
Superseded by the combined authorized-share ceiling PR: #238.\n\nI folded this issuer aggregate ceiling work into #238 because both PRs enforce the same OCF authorized-share ceiling requirement. While consolidating, I also incorporated the review feedback from this PR: issuer validation now short-circuits when uncapped, stock-class authorized adjustments are indexed once for aggregate validation, already-fetched stock-class data is reused through the final-state path, and NOT_APPLICABLE is distinguished from UNLIMITED under a finite issuer cap. The combined PR includes the issuer-level tests from this PR plus an explicit NOT_APPLICABLE regression test. |
Summary
Stacked on #241.
This adds a final-state cap-table invariant: when the issuer has a finite authorized-share limit, the sum of effective stock-class authorized shares cannot exceed it.
The effective issuer limit is:
IssuerAuthorizedSharesAdjustment.new_shares_authorized, when present;Issuer.initial_shares_authorized, when finite;The effective stock-class total is the sum of each stock class's current authorization, using the latest
StockClassAuthorizedSharesAdjustment.new_shares_authorizedwhen present, otherwiseStockClass.initial_shares_authorized. If any stock class is unlimited, a finite issuer cap rejects the final cap table.The check runs once after deletes, edits, issuer edit, and creates have all been processed, so a single batch can correct the issuer max and add/raise stock-class authorization together while still rejecting invalid final state.
OCF documentation
Issuer.initial_shares_authorized: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/Issuer.schema.json#L60-L64StockClass.initial_shares_authorized: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/StockClass.schema.json#L28-L31IssuerAuthorizedSharesAdjustment.new_shares_authorized: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/adjustment/IssuerAuthorizedSharesAdjustment.schema.json#L26-L29StockClassAuthorizedSharesAdjustment.new_shares_authorized: https://github.com/Open-Cap-Table-Coalition/Open-Cap-Format-OCF/blob/main/schema/objects/transactions/adjustment/StockClassAuthorizedSharesAdjustment.schema.json#L26-L29Tests
Added
TestIssuerAuthorizedSharescoverage for:Also updated the default test issuer authorization to a valid high ceiling and made the existing unlimited-stock-class issuance test explicitly use an uncapped issuer.
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="/opt/homebrew/opt/openjdk@17/bin:$HOME/.dpm/bin:$PATH" dpm test --show-coverage --color --coverage-ignore-choice 'splice-amulet:.*'Note
Medium Risk
Changes cap-table mutation rules for authorized shares across issuer edits, adjustments, and stock classes; incorrect logic could block valid updates or allow invalid totals, but behavior is covered by new Daml script tests.
Overview
When the issuer has a finite authorized-share limit,
UpdateCapTablenow rejects the final cap table if the sum of each stock class’s effective authorization exceeds that limit. Effective issuer cap uses the latestIssuerAuthorizedSharesAdjustmentwhen present, otherwise finiteIssuer.initial_shares_authorized; missing or unlimited issuer authorization stays uncapped. Stock-class totals use each class’s latest adjustment or initial authorization, and any unlimited class fails against a finite issuer cap.The check runs once at the end of a batch (after deletes, edits, issuer edit, and creates), so a single update can raise the issuer ceiling and add or increase classes in the same transaction while still blocking invalid end state.
OpenCapTable-v34 is bumped to 0.0.4 with updated DAR artifacts. Tests add
TestIssuerAuthorizedSharesfor create/edit/adjustment/delete edge cases; default setup uses a high issuer ceiling anddefaultIssuerWithAuthorizedSharesfor targeted limits. The unlimited stock-class issuance test explicitly sets an uncapped issuer.Reviewed by Cursor Bugbot for commit 84cce5c. Bugbot is set up for automated code reviews on this repo. Configure here.