Validate acceptance reader payloads#429
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ed-acceptance-readers
…into codex/decoder-backed-acceptance-readers
|
@coderabbitai pause |
✅ Action performedReviews paused. |
…coder-backed-acceptance-readers # Conflicts: # src/functions/OpenCapTable/convertibleAcceptance/getConvertibleAcceptanceAsOcf.ts # src/functions/OpenCapTable/equityCompensationAcceptance/getEquityCompensationAcceptanceAsOcf.ts # src/functions/OpenCapTable/stockAcceptance/getStockAcceptanceAsOcf.ts # src/functions/OpenCapTable/warrantAcceptance/getWarrantAcceptanceAsOcf.ts
…coder-backed-acceptance-readers
Validate all acceptance create arguments as exact, lossless generated template wrappers and preserve bounded ledger diagnostics.
|
Prepared and independently audited exact head |
|
@copilot review Please perform a formal review of exact current head |
Formal review completed on head |
|
cursor review Copilot completed a no-findings formal review of exact current head |
Formal review completed for 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 24e71ee. Configure here.
…coder-backed-acceptance-readers
|
@copilot review Please review exact current head |
Formal review of exact head |
|
@cursor review Please review exact current head |
There was a problem hiding this comment.
Pull request overview
This PR tightens ledger-boundary parsing for the four acceptance readers (stock, convertible, equity-compensation, warrant) by decoding the full generated DAML create-argument wrapper, validating template identity up-front, and reusing the canonical DAML→OCF converters to ensure exact, non-any result types.
Changes:
- Enforce
expectedTemplateIdin acceptance readers and decode acceptance payloads via generated DAML decoders (full wrapper, not justacceptance_data). - Route acceptance entity decoding through a shared
acceptanceContractDatapath and reusedaml*AcceptanceToNativeconverters. - Add runtime reader tests plus compile-time (source and dist) type-contract tests ensuring exact/non-cross-assignable acceptance result families.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/types/acceptanceReaders.types.ts | Source type-level contracts proving exact acceptance reader result types (no any, not cross-assignable). |
| test/functions/acceptanceReaders.test.ts | Parameterized runtime tests for decoding, malformed payloads, hostile JSON shapes, and wrong-template/package cases. |
| test/declarations/acceptanceReaders.types.ts | Built-declaration type-level contracts mirroring the source contracts for dist output. |
| test/batch/damlToOcfDispatcher.test.ts | Regression ensuring generic acceptance reads enforce the full generated wrapper (incl. context). |
| src/functions/OpenCapTable/warrantAcceptance/getWarrantAcceptanceAsOcf.ts | Switch to template-id enforcement + shared decode path + canonical DAML→native conversion. |
| src/functions/OpenCapTable/stockAcceptance/getStockAcceptanceAsOcf.ts | Same as above for stock acceptance. |
| src/functions/OpenCapTable/equityCompensationAcceptance/getEquityCompensationAcceptanceAsOcf.ts | Same as above for equity-compensation acceptance. |
| src/functions/OpenCapTable/convertibleAcceptance/getConvertibleAcceptanceAsOcf.ts | Same as above for convertible acceptance. |
| src/functions/OpenCapTable/capTable/damlEntityData.ts | Route acceptance entity types through the new acceptance wrapper decoder. |
| src/functions/OpenCapTable/capTable/acceptanceContractData.ts | New shared decoder enforcing safe JSON preflight + lossless round-trip for acceptance create arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed Copilot’s single-invocation test finding in 63f009f. The extra-field loop now captures one rejection from each reader invocation, then asserts both the OcpParseError runtime type and the complete structured lossy-decode diagnostics on that same value. Validation:
The PR remains draft and unassigned while exact-head CI reruns. |
…coder-backed-acceptance-readers
|
Material parent propagation complete at exact head 2ad6848, incorporating #428 at 5394ea3. The single-invocation acceptance failure test fix is byte-for-byte stable: test/functions/acceptanceReaders.test.ts retains its exact pre-merge blob. Source typecheck and the focused acceptance-reader suite passed (1 suite / 85 tests) on canton-node-sdk 0.0.227. The PR remains draft and unassigned while CI runs. |
|
Fresh acceptance-reader audit hardening is pushed at exact head All four acceptance families now reject zero-length generated Dedicated and generic success/failure tests now assert exactly one ledger fetch. Malformed-date and wrong-template cases capture one rejection rather than invoking readers twice, and the preserved extra-field test also proves single invocation. Contract-ID correlation is intentionally parent-resolved by #427 and must remain intact during restacking. Validation: focused reader/converter/dispatcher 236 tests; full 90 suites / 4,713 tests; build; typecheck; declarations; exact public-config; lint; format; diff check. No other blocker was found in #429’s own delta. The PR remains draft/unassigned; no parent merge or reviews were requested. |
…coder-backed-acceptance-readers # Conflicts: # src/functions/OpenCapTable/capTable/damlEntityData.ts
|
Corrected parent stack propagated through PR #429 at exact head 21f8572. The acceptance-specific full-wrapper decoder remains correlated across all four families, identifier validation remains symmetric with writers, and direct/generic reads retain one ledger fetch plus exact template and contract-ID correlation. I also updated one stale generic-reader fixture so it reaches the intended wrapper-decoder assertion under the stricter parent contract-ID gate. Local validation: focused acceptance/dispatcher/single-contract/generated-boundary/sorter suite 6 suites / 396 tests; full coverage suite 95 suites / 5,521 tests; clean build and declaration checks; exact public-config checks; built cardinality; package-consumer declarations; TypeScript; ESLint; Prettier; diff checks; canonical inventory 55 variants / 0 ingestion aliases (56 reachable objects, 160 resources). The worktree is clean and local/remote heads match. PR remains draft, unassigned, with no review requests while fresh CI runs. |
|
@copilot review Please review exact current head |
Formal review completed for exact head I also verified current workflow state for this head on |
|
@cursor review Please review exact current 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 21f8572. Configure here.
…coder-backed-acceptance-readers
|
Parent propagation is complete at exact head 0f8d00b via a normal conflict-free merge of #428 c45265e. The parent and child file sets do not overlap and both layers remain byte-identical. Validation passes: 13 focused suites / 1,012 tests, full 95 suites / 5,526 tests, typecheck, lint, formatting, declarations, exact-public/cardinality, and package-consumer gates. Independent inspection found no issue. Keeping this PR in draft until fresh remote CI is green. |
…into codex/decoder-backed-acceptance-readers # Conflicts: # src/functions/OpenCapTable/capTable/damlEntityData.ts
|
Restacked onto audited #428 and pushed at exact head |
Summary
issuerandsystem_operatoras Canton Party identifiers with exact diagnostic pathsidandsecurity_idvalues to match the pinned DAML rulesanyin both source and built declarationsValidation
npm run test:ci— 97 suites / 5,612 tests passednpm run test:declarationsnpm run test:package-consumernpm run test:exact-public-confignpm run typechecknpm run buildnpm run lintnpm run format060cbf3cfc6005308ab75e7ac54fb5495e159affStack