Enforce warrant issuance vesting references#250
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 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 (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Tightens cap-table invariants so WarrantIssuance.vesting_terms_id (when present) must resolve to an existing VestingTerms record after each UpdateCapTable batch, preventing dangling vesting-term references created via create/edit or via deleting referenced vesting terms.
Changes:
- Adds optional-reference validation for
WarrantIssuance.vesting_terms_idin codegen configuration. - Adds a post-batch invariant (
validateWarrantIssuanceReferences) to reject any resulting cap table state with orphaned warrant vesting-term references. - Updates and adds DAML script tests covering missing references, same-batch creation, bad edits, and blocked deletes.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Test/daml/OpenCapTable/TestWarrantIssuanceReferences.daml | New focused tests for warrant vesting-terms reference validity across create/edit/delete. |
| Test/daml/OpenCapTable/TestWarrantIssuance.daml | Updates existing “optionals” test to create referenced VestingTerms in the same batch. |
| scripts/codegen/templates/CapTable.daml.template | Adds post-batch validateWarrantIssuanceReferences invariant and wires it into UpdateCapTable. |
| scripts/codegen/captable-config.yaml | Adds vesting_terms_id?:vesting_terms validation for WarrantIssuance (and reformats one existing list). |
| dars/OpenCapTable-v34/0.0.2/OpenCapTable-v34.dar | Regenerated DAR (Git LFS pointer updated). |
| dars/dars.lock | Updates DAR hash/size metadata to match regenerated artifact. |
![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: 256903ec0f
ℹ️ 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".
| [stakeholder_id, 'stock_class_id?:stock_classes', 'stock_plan_id?:stock_plans', 'vesting_terms_id?:vesting_terms'] | ||
| ConvertibleIssuance: [stakeholder_id] | ||
| WarrantIssuance: [stakeholder_id] | ||
| WarrantIssuance: [stakeholder_id, 'vesting_terms_id?:vesting_terms'] |
There was a problem hiding this comment.
When an existing warrant is edited to set vesting_terms_id to a VestingTerms created in the same UpdateCapTable call, this new validation makes the generated edit branch assert against the maps before creates are applied. Regular edits are folded before creates in scripts/codegen/templates/CapTable.daml.template lines 472-505, so the new post-batch validateWarrantIssuanceReferences would accept the final state but this early edit validation rejects it. Consider relying on the final invariant for this reference or otherwise validating warrant edits after same-batch creates are present.
Useful? React with 👍 / 👎.
| "sha256": "c89b11662c56c40d0ff7e8bfc03e9f4b863e84ad7b5711a338070a30ed84c08d", | ||
| "size": 2510816, |
There was a problem hiding this comment.
This changes the recorded bytes for the already-backed-up OpenCapTable-v34/0.0.2 DAR while OpenCapTable-v34/daml.yaml still declares version 0.0.2. The repo's backup flow treats existing lock keys as immutable (backup-dar refuses to overwrite them), so consumers or upgrade checks that refer to the previous 0.0.2 package can no longer reproduce that exact archive from dars/; for this contract change, bump the package patch and add a new backup instead of replacing the existing 0.0.2 entry.
Useful? React with 👍 / 👎.
|
Superseded by #251, which consolidates the reference-integrity work into one PR. Before closing this stack PR, I rechecked the live review comments and folded the actionable improvements into #251: StockPlan create-tier ordering, document related-object error messages, stock-class conversion source IDs, warrant conversion validation across all OcfAnyConversionRight variants, WarrantIssuance same-batch vesting-term edits, and immutable DAR backup versioning. |
Summary
Enforce the OCF
WarrantIssuance.vesting_terms_idrelationship inside the contract by validating that any populated warrant vesting terms reference resolves to an existingVestingTermsrecord after each cap table update batch.This closes the dangling-reference path for creates, edits, and deletes: a warrant can still omit
vesting_terms_id, but if it declares one, the target vesting terms must exist in the resulting cap table state.OCF requirement
WarrantIssuance.schema.jsondefinesvesting_terms_idas the identifier of theVestingTermsto which the security is subject.VestingTerms.schema.jsondefines the referenced object family.Changes
WarrantIssuanceoptional reference validation forvesting_terms_id?:vesting_terms.validateWarrantIssuanceReferencesinvariant so deleting referenced vesting terms is rejected.Validation
npm run builddpm test --show-coverage --color --coverage-ignore-choice 'splice-amulet:.*'npm run lint:damlnpm run verify-dars -- --updatenpm run check-upgrade-compatnpm run codegennpm run verify-packageNote
Medium Risk
Tightens cap-table invariants on warrant updates; batches that previously accepted invalid or orphan vesting references will now fail at exercise time.
Overview
Warrant issuances now must resolve optional
vesting_terms_idto an existingVestingTermsrecord, matching OCF and the pattern already used for stock and equity comp issuances.Codegen adds
vesting_terms_id?:vesting_termson create and a post-batchvalidateWarrantIssuanceReferencespass so dangling IDs fail on create, edit, and when deleting vesting terms still referenced by a warrant. The optional-fields warrant test createsVestingTermsin the same batch; TestWarrantIssuanceReferences covers missing reference, same-batch success, bad edit, and blocked delete. The v34 DAR backup anddars.lockhash are refreshed.Reviewed by Cursor Bugbot for commit 256903e. Bugbot is set up for automated code reviews on this repo. Configure here.