Enforce OCF schema field constraints - #269
Conversation
…ing' into codex/ocf-reference-integrity # Conflicts: # scripts/codegen/templates/CapTable.daml.template # scripts/codegen/templates/loops/edit-case.daml
…nto codex/ocf-stock-plan-pool-ceiling-v2 # Conflicts: # scripts/codegen/templates/CapTable.daml.template
…ing' into codex/ocf-reference-integrity # Conflicts: # scripts/codegen/generate-captable.ts
…nto codex/ocf-stock-plan-pool-ceiling-v2
…ing' into codex/ocf-reference-integrity
…nto codex/ocf-stock-plan-pool-ceiling-v2
![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: 6a7b63ff5e
ℹ️ 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".
|
Addressed both exact-head Codex findings in
Local validation: |
|
@copilot review exact head |
|
@codex review exact head |
|
@cursor review exact head |
Reviewed exact head |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 c184a4e. Configure here.
|
Follow-up |
|
@copilot review exact head |
|
@codex review exact head |
|
@cursor review exact head |
Reviewed exact head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Test/daml/OpenCapTable/TestConversionMechanisms.daml`:
- Around line 153-158: Add positive test cases to the conversion-mechanism cases
around OcfConvMechPpsBased, covering discount-disabled PPS mechanisms with
exactly one of discount_percentage or discount_amount populated. Preserve the
existing invalid and supported cases, and ensure the tests verify both
schema-valid single-field forms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd5d222f-7ae4-4cd0-879d-569d742253ee
⛔ Files ignored due to path filters (1)
dars/dars.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
OpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/ConvertibleConversion.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/EquityCompensationExercise.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/EquityCompensationRelease.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/StockConversion.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/StockIssuance.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/StockReissuance.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/WarrantExercise.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/OCF/WarrantIssuance.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/Types/Conversion.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/Types/Monetary.damlOpenCapTable-v34/daml/Fairmint/OpenCapTable/Types/Stock.damlTest/daml/OpenCapTable/TestConversionMechanisms.damlTest/daml/OpenCapTable/TestCountrySubdivisionCode.damlTest/daml/OpenCapTable/TestIssuer.damlTest/daml/OpenCapTable/TestStockClass.damlTest/daml/OpenCapTable/TestUniqueSecurityArrays.damldars/OpenCapTable-v34/0.0.3/OpenCapTable-v34.dar
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 3b74ab2. Configure here.
|
Addressed the opportunistic positive-test gap in |
|
@copilot review exact head |
|
@codex review exact head |
|
@cursor review exact head |
Reviewed exact head |
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 ba66040. Configure here.
SDK integration blocker discoveredThe contract rule is correct, but current
This means a replay can reject a valid non-1:1 right or appear green after silently deleting a valid 1:1 right. That violates the purpose of this validation framework, so this PR is not final-review ready even if the current snapshots pass. Required dependency: lossless stock-class and warrant ratio round-trip support in |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
Read-only database impact confirmationThe SDK issue is exercised by current sync-enabled data, so it is not theoretical:
Current SDK normalization deletes every one of these rights before contract submission because it treats a single 1:1 right as equivalent to no right. Therefore a green replay on the current SDK would be a false positive for these 19 objects, and the three dev Queries were read-only ( |
|
Narrow SDK dependency opened as draft: Fairmint/ocp-canton-sdk#447 It targets SDK |
Final exact-head replay results (
|
Summary
Standalone OCF schema/field validation on
main. This consolidates the field-format work previously split across #257 through #267, without carrying their former stacked parents.The standalone scope enforces constraints directly representable in OpenCapTable-v34:
path/urimutual exclusionMonetaryvalidation for optional stockcost_basisand warrantexercise_priceCap-table-wide share-number lineage/overlap enforcement is intentionally excluded here because the old implementation depended on unmerged balance/ceiling engines. That behavior should be evaluated separately instead of importing predecessor validators into this field-constraint PR.
OCF basis
Primary sources include Document, StockIssuance, Monetary, CurrencyCode, Phone, CountrySubdivisionCode, and the conversion-right/mechanism schemas under
schema/types.Standalone state
main6a7b63ff5ec3b633a07204cf4a186d7b61d175920.0.3onlyLocal validation
npm run build✅npm run check:dar-version-policy -- --base origin/main✅npm run verify-dars✅git diff --check✅npm testcould not start locally because this Mac has no Java runtime; CI rerun and database replays remain pending.Note
Medium Risk
Validation changes can reject previously accepted cap-table payloads (conversion rights, documents with both path and uri, duplicate security IDs, phone/currency/subdivision formats). Scope is contract
ensurelogic, not auth or payments.Overview
Tightens OCF field validation across OpenCapTable-v34
ensureclauses so payloads must match schema-shaped rules before contracts are created.Security ID arrays on transfers, consolidations, conversions, exercises, releases, and reissuances now use shared helpers for non-empty (where required) and unique elements; duplicate
resulting_security_idsor consolidationsecurity_idsare rejected.Documents must have exactly one of
pathoruri(mutual exclusion), not both or neither. Conversion triggers delegate tovalidateOcfConversionTriggerFieldsso each trigger type only allows the optional fields OCF defines (dates, conditions, elective ranges).Conversion mechanisms are stricter: convertible PPS/valuation/ratio variants are rejected as unsupported; stock-class conversion rights only accept the ratio mechanism with required
conversion_price,ratio, androunding_typeand no extra optional fields; warrant valuation types must matchCAP/FIXED/ACTUAL; share-price discounts whendiscountis false may still carry reference percent/amount.Formats: ISO uppercase 3-letter currency codes, space-separated international phone patterns (with extensions), and 1–3 character country subdivision codes on issuer/address fields. Stock issuance
share_numbers_issuedmust not overlap and must sum toquantitywhen present; optionalcost_basisand warrantexercise_pricegetMonetaryvalidation.Adds focused DAML script tests and refreshes the OpenCapTable-v34 0.0.3 DAR artifact.
Reviewed by Cursor Bugbot for commit ba66040. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit