Enforce OCF reference integrity#268
Conversation
…ing' into codex/ocf-reference-integrity # Conflicts: # scripts/codegen/templates/CapTable.daml.template # scripts/codegen/templates/loops/edit-case.daml
…ing' into codex/ocf-reference-integrity # Conflicts: # scripts/codegen/generate-captable.ts
…ing' into codex/ocf-reference-integrity
📝 WalkthroughWalkthroughOpenCapTable-v34 is upgraded to package version 0.0.6. Generated CapTable validation now covers cross-object, optional, issuer, document, conversion, vesting, and same-batch references. New and updated DAML scripts exercise creation, editing, deletion, and prerequisite-reference scenarios. ChangesReference validation and generated CapTable flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TestScript
participant UpdateCapTable
participant CapTableMaps
participant ReferenceValidators
TestScript->>UpdateCapTable: submit creates, edits, and deletes
UpdateCapTable->>CapTableMaps: apply pending mutations
UpdateCapTable->>ReferenceValidators: validate final references
ReferenceValidators-->>UpdateCapTable: success or failure
UpdateCapTable-->>TestScript: updated cap table or rejected submission
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6529ed33b8
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR centralizes post-batch referential-integrity enforcement in UpdateCapTable by validating object references against the final cap table state (after deletes/edits/creates), and updates tests/fixtures and package artifacts to match the stricter invariants.
Changes:
- Adds final-state reference validation passes for documents’
related_objects, conversion targets, valuations, stock plans, and issuance prerequisite references. - Updates codegen templates/config to support final-state validation patterns (incl. optional/array ref specs and issuer-id checks).
- Adds/updates Daml Script tests and bumps
OpenCapTable-v34to0.0.6with new DAR + lock updates.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Test/daml/OpenCapTable/TestWarrantIssuanceReferences.daml | New tests for warrant vesting-terms reference integrity across create/edit/delete batches. |
| Test/daml/OpenCapTable/TestWarrantIssuance.daml | Updates warrant issuance fixture to create referenced vesting terms. |
| Test/daml/OpenCapTable/TestWarrantConversionRightReferences.daml | New tests for warrant conversion-target references (including nested triggers). |
| Test/daml/OpenCapTable/TestVestingStart.daml | Removes placeholder equity-comp references to avoid failing new reference checks. |
| Test/daml/OpenCapTable/TestVestingEvent.daml | Removes placeholder equity-comp references to avoid failing new reference checks. |
| Test/daml/OpenCapTable/TestVestingAcceleration.daml | Removes placeholder equity-comp references to avoid failing new reference checks. |
| Test/daml/OpenCapTable/TestValuationReferences.daml | New tests for valuation stock-class reference integrity. |
| Test/daml/OpenCapTable/TestStockRetraction.daml | Removes placeholder stock legend IDs from stock issuance fixtures. |
| Test/daml/OpenCapTable/TestStockPlanReferences.daml | New tests for stock plan stock-class reference integrity. |
| Test/daml/OpenCapTable/TestStockIssuanceReferences.daml | New tests for stock issuance prerequisite references (legend/plan/vesting/class). |
| Test/daml/OpenCapTable/TestStockIssuance.daml | Updates stock issuance test to create referenced legend/plan/vesting terms. |
| Test/daml/OpenCapTable/TestStockClassConversionRightReferences.daml | New tests for stock-class conversion target references. |
| Test/daml/OpenCapTable/TestStockClass.daml | Refactors common stock-class fixture and ensures prerequisite creation in tests. |
| Test/daml/OpenCapTable/TestStockCancellation.daml | Removes placeholder stock legend IDs from issuance fixtures. |
| Test/daml/OpenCapTable/TestStockAcceptance.daml | Removes placeholder stock legend IDs from issuance fixtures. |
| Test/daml/OpenCapTable/TestIssuerAuthorizedSharesAdjustment.daml | Adds helper + tests for issuer-id reference validation on adjustments. |
| Test/daml/OpenCapTable/TestHelpers.daml | Adds default legend/vesting-term helpers and stops using placeholder legend IDs. |
| Test/daml/OpenCapTable/TestEquityCompensationTransfer.daml | Removes placeholder equity-comp stock-class reference. |
| Test/daml/OpenCapTable/TestEquityCompensationRetraction.daml | Removes placeholder equity-comp stock-class reference. |
| Test/daml/OpenCapTable/TestEquityCompensationIssuanceReferences.daml | New tests for equity-comp issuance prerequisite references (class/plan/vesting). |
| Test/daml/OpenCapTable/TestEquityCompensationIssuance.daml | Updates equity-comp issuance test to create referenced class/plan/vesting terms. |
| Test/daml/OpenCapTable/TestEquityCompensationCancellation.daml | Removes placeholder equity-comp stock-class reference. |
| Test/daml/OpenCapTable/TestEquityCompensationAcceptance.daml | Removes placeholder equity-comp stock-class reference. |
| Test/daml/OpenCapTable/TestDocument.daml | Adds document related-object reference tests and fixes issuer reference to real issuer id. |
| Test/daml/OpenCapTable/TestConvertibleConversionRightReferences.daml | New tests for convertible conversion target references. |
| Test/daml/OpenCapTable/TestConversionMechanisms.daml | Ensures prerequisite stock class exists where conversion logic requires it. |
| Test/daml.yaml | Updates Test package to depend on OpenCapTable-v34-0.0.6.dar. |
| scripts/codegen/templates/loops/edit-case.daml | Adds optional-ref validation and issuer-id validation hooks; extends edit return tuple. |
| scripts/codegen/templates/loops/create-case.daml | Adds optional-ref validation and issuer-id validation hooks for creates. |
| scripts/codegen/templates/CapTable.daml.template | Adds final-state reference validation helpers and runs them post-batch. |
| scripts/codegen/generate-captable.ts | Enhances codegen to support optional validations + object-type constructor mapping. |
| scripts/codegen/captable-config.yaml | Retiers StockPlan/plan adjustments and shifts some validations to final-state checks. |
| OpenCapTable-v34/daml.yaml | Bumps package version to 0.0.6. |
| dars/OpenCapTable-v34/0.0.6/OpenCapTable-v34.dar | Adds new DAR artifact (LFS pointer) for 0.0.6. |
| dars/dars.lock | Registers new DAR hash/size metadata for 0.0.6. |
…ing' into codex/ocf-reference-integrity-category
…ing' into codex/ocf-reference-integrity-category
…ing' into codex/ocf-reference-integrity-category
…ing' into codex/ocf-reference-integrity-category # Conflicts: # Test/daml/OpenCapTable/TestConvertibleConversion.daml
|
Restacked onto the final #238 head at The inherited authorized-ceiling conversion tests now materialize every referenced result and balance security, so they exercise both categories together instead of failing early on missing references. The full suite passes with 496 scenarios, and all local gates pass: build, ESLint (0 errors; 2 pre-existing warnings), DAML lint, format, pinned schema parity, DAR verification, package/import verification, Refreshed DAR: SHA-256 |
…ing' into codex/ocf-reference-integrity-category
|
Propagated the latest #238 negative equity-exercise fix through this branch. Current head is |
|
@copilot review Please review exact head |
|
@copilot review exact head |
|
@codex review current head |
|
cursor review current head |
|
@coderabbitai resume @coderabbitai review current head |
|
✅ Action performedReviews resumed. Review finished.
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 88ae4aa. Configure here.
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Exact head |
Reviewed exact head |
Preliminary OCF validation-policy auditPolicy: OCF Validation Policy Important correction from the current audit: internal-reference validity is part of the OCF data model even though the official JSON Schema validator does not currently check it. Appears supported
Needs verification
Proposed next step |
Summary
Groups the OCF reference-integrity enforcement into one category PR, superseding the standalone reference PRs #251, #255, and #256.
UpdateCapTablenow validates final-state object references after deletes, edits, issuer edits, and creates, including:related_objectsreferences, including issuer references and plan-security aliasesvesting_condition_idreferences against the referenced security's VestingTerms, including trigger-type compatibilityThe branch is restacked on #238, so it includes the authorized/reserved ceiling base and uses OpenCapTable-v34
0.0.6.OCF requirement references
Schema references are pinned to Open-Cap-Format-OCF commit
dd84bdb6bafb63681c77166f315cea5a2078a4c8.Document.related_objectsidentifies objects related to a document.StockIssuance.stakeholder_id,stock_class_id,stock_legend_ids,stock_plan_id, andvesting_terms_ididentify prerequisite cap-table objects for issued stock.EquityCompensationIssuance.stock_plan_id,stock_class_id,stakeholder_id, andvesting_terms_ididentify prerequisite cap-table objects for equity compensation.StockPlan.stock_class_idsidentifies stock classes covered by a plan.Valuation.stock_class_ididentifies the valued stock class.StockClass.conversion_rights,WarrantIssuance.exercise_triggers, andConvertibleIssuance.conversion_triggerscarry conversion target references.Transfer.balance_security_idandresulting_security_idsidentify the remainder and newly created securities for stock, warrant, convertible, and equity-compensation transfers.Cancellation.balance_security_id,Repurchase.balance_security_id, and stock/convertible conversionbalance_security_idfields identify remainder securities for partial lifecycle events.Conversion.resulting_security_ids,Exercise.resulting_security_ids, andRelease.resulting_security_idsidentify newly issued securities produced by those transactions.Reissuance.resulting_security_idsandConsolidation.resulting_security_ididentify replacement stock securities.VestingStart.v1.vesting_condition_idis required and documented as a reference to aVestingConditionin the security's VestingTerms whose trigger type should beVESTING_START_DATE.VestingEvent.v1.vesting_condition_idis required and documented as a reference to aVestingConditionin the security's VestingTerms whose trigger type should beVESTING_EVENT.Validation
npm run buildnpm test(497 scenarios)npm run lint(0 errors; 2 pre-existing warnings inscripts/remove-participant-dar.ts)npm run lint:damlnpm run formatnpm run check:schema-gaps -- --ocf-schema-dir /tmp/open-cap-format-ocf-schema-check/schemanpm run verify-darsnpm run verify-packagenpm run check-upgrade-compat(0.0.5 -> 0.0.6)git diff --checkOpenCapTable-v34-0.0.6.dar: SHA-256f321638da45f5ad87e3f91c8701eaf6ac2b846bcef7be1e8b9221144972e3997, 3,070,303 bytesNote
High Risk
Tightens validation on all cap-table batch updates (references, deletes, and lifecycle securities), so previously accepted
UpdateCapTablepayloads may now reject; contract upgrade is 0.0.5 → 0.0.6.Overview
OpenCapTable-v34 is released as 0.0.6 (Test package retargets the new DAR). The visible diff is dominated by Daml Script work that locks in OCF reference integrity on batched
UpdateCapTableupdates: missing balance and resulting securities, wrong security types, dangling deletes, and bad capitalization / conversion / document links are expected to fail, while references created or fixed in the same batch should succeed.New focused suites cover balance-security successors, resulting-security rules, document and stock-plan links, stock-class and convertible conversion targets, equity-compensation prerequisites, and “final state” checks (e.g. trigger IDs, reissuance split transactions, stakeholder delete ordering).
TestHelpersgains successor issuance builders (stockSuccessorIssuance,convertibleSuccessorIssuance, etc.), empty default stock legends, and shared stock plan / vesting / legend fixtures so existing transfer, conversion, exercise, release, and authorized-shares tests materialize successor issuances instead of relying on implicit placeholders.Reviewed by Cursor Bugbot for commit 88ae4aa. Bugbot is set up for automated code reviews on this repo. Configure here.