Reject lossy generated DAML decoding#431
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 |
…ossless-daml-decoding
…n-readers' into codex/lossless-daml-decoding
|
@coderabbitai pause |
✅ Action performedReviews paused. |
…ssless-daml-decoding # Conflicts: # src/functions/OpenCapTable/capTable/damlEntityData.ts
…ssless-daml-decoding
…ossless-daml-decoding # Conflicts: # src/functions/OpenCapTable/capTable/acceptanceContractData.ts # src/functions/OpenCapTable/capTable/cancellationContractData.ts # src/functions/OpenCapTable/capTable/damlCodecLosslessness.ts # src/functions/OpenCapTable/capTable/damlEntityData.ts # test/functions/cancellationReaders.test.ts
…ossless-daml-decoding
…n-readers' into codex/lossless-daml-decoding
|
Reconciled the final #430 parent and tightened this PR at exact head Improvements after independent audit:
Validation on |
…ssless-daml-decoding
|
Final stack reconciliation complete at exact head e079e00, incorporating #430 at 1abca30. The stacked delta remains test-only: test/functions/damlEntityDataLosslessness.test.ts is the sole diff from the updated parent, with no production-code delta. Full validation passed on canton-node-sdk 0.0.227: build, declaration checks, exact public config, source typecheck, ESLint, Prettier, focused losslessness suite (1 suite / 8 tests), and full Jest coverage (92 suites / 4,810 tests). The temporary node_modules symlink was removed; the worktree is clean. The PR remains draft and unassigned while CI runs. |
|
@copilot review Please review exact head e079e00. Copilot is the first requested reviewer for this material revision. The stacked delta against codex/decoder-backed-cancellation-readers is exactly one losslessness test file: test/functions/damlEntityDataLosslessness.test.ts. Local full validation passed 92 suites / 4,810 tests. Exact build-and-test and full QuickStart are SUCCESS, GitHub reports CLEAN/MERGEABLE, the PR is unassigned, and there are zero unresolved review threads. |
Reviewed exact head |
…n-readers' into codex/lossless-daml-decoding
|
Propagated the corrected #426→#430 ancestry into this lossless-decoding proof branch at exact head 8b04e25. Ancestry: #430 head 206699f and #426 fix d999c29 are ancestors of this head. Stack invariant reverified against updated #430:
Validation on @fairmint/canton-node-sdk 0.0.227:
The merge was conflict-free. The temporary dependency symlink was removed and the worktree is clean. This PR remains draft and unassigned; no reviewer was requested. |
…ssless-daml-decoding
|
Propagated exact #430 head The stacked delta remains exactly |
…n-readers' into codex/lossless-daml-decoding
|
Propagated exact #430 head bbae333 with a normal merge commit. New exact #431 head: 6719439. The stacked delta remains exactly test/functions/damlEntityDataLosslessness.test.ts with unchanged patch ID 25fd9fa6; no source or other test file leaked into the slice. Validation on @fairmint/canton-node-sdk@0.0.227 is fully green: acceptance readers 85/85; focused losslessness 8/8; build; declarations; exact public config; source typecheck; lint; formatting; and test:ci with 92 suites / 4,824 tests. The temporary dependency symlink was removed and the worktree is clean. PR remains draft/unassigned; no reviewer requested. |
…ssless-daml-decoding
|
Material parent propagation complete at exact head e2e40e8, incorporating #430 at 2c33641. Patch stability is exact: the sole stacked delta remains A test/functions/damlEntityDataLosslessness.test.ts, with no production-code delta. Full validation passed on canton-node-sdk 0.0.227: build, declarations, exact public config, explicit typecheck, ESLint, Prettier, focused losslessness suite (1 suite / 8 tests), and full test:ci (92 suites / 4,836 tests). The temporary node_modules symlink was removed and the worktree is clean. The PR remains draft and unassigned while CI runs. |
|
Fresh lossless-decoder audit hardening is pushed at exact head The audit reproduced a global identity-trust bypass: once any object had decoded successfully, later same- or cross-codec calls skipped decoding entirely, so deleting required fields or reusing the object with another codec could pass. The global WeakSet/recursive marking is removed. A current-call-only Regressions cover required-field deletion, nested mutation and exact paths, current-call handoff, repeated same-codec calls, cross-codec reuse, clones, and public reader behavior. Direct proof reports deletion=decode, cross=decode, firstCalls=2, secondCalls=1. Numeric/Time string preservation remains intentionally lossless at this layer; native converters enforce semantic validity. Validation: focused 5 suites / 431 tests; full 92 suites / 4,841 tests; build; typecheck; declarations; exact public-config; lint; format. The PR remains draft/unassigned; no parent merge or reviews were requested. |
…ssless-daml-decoding
|
Corrected parent stack propagated through PR #431 at exact head 40428ed. Lossless generated-DAML validation keeps its decoded-value handoff confined to the current synchronous call: there is no global decoded-object WeakSet or identity cache, every later decode runs the selected codec again, and mutation/cross-codec/cloned-value diagnostics retain exact paths. Acceptance, cancellation, strict-map, exact wrapper/template/contract-ID, sorter, and single-fetch parent invariants remain intact. I also moved the public-boundary losslessness fixture onto the current canton-node-sdk client shape required by the corrected parent. Local validation: focused losslessness/acceptance/cancellation/dispatcher/single-contract/map/sorter suite 8 suites / 579 tests; full coverage suite 97 suites / 5,661 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. Per propagation scope, no work was performed on #432 or later. |
|
@copilot review Please review exact current head |
Reviewed exact head |
|
@cursor review Please review exact current 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 40428ed. Configure here.
…ssless-daml-decoding
|
Parent propagation is complete at exact head 855d5c1 via a normal conflict-free merge of #430 4647d3d. The non-overlapping parent and child layers remain byte-identical. Validation passes: 15 focused suites / 1,198 tests, full 97 suites / 5,666 tests, typecheck, lint, formatting, declarations, exact-public/cardinality, and package-consumer gates. Independent semantic inspection found no issue. Keeping this PR in draft until fresh remote CI is green. |
…n-readers' into codex/lossless-daml-decoding # Conflicts: # src/functions/OpenCapTable/capTable/acceptanceContractData.ts # src/functions/OpenCapTable/capTable/damlEntityData.ts # src/functions/OpenCapTable/capTable/damlToOcf.ts # src/utils/generatedDamlValidation.ts # test/utils/generatedDamlValidation.test.ts
|
Decoder-ownership remediation and the refreshed #430 parent are pushed at exact head |
Why this belongs in the SDK
DAML is authoritative for domain correctness: quantities, cardinality, identifier relationships, pricing rules, vesting graphs, and other business invariants should be accepted or rejected by the contract.
This PR addresses a different boundary. After contract data crosses into JavaScript, DAML cannot guarantee that the generated decoder:
The intended check is therefore structural fidelity: snapshot the untrusted input once, give decoder and encoder isolated clones, decode exactly once, and compare independent source/decoded/re-encoded evidence. It does not decide whether the decoded contract data is semantically valid.
Scope after the DAML-validation pivot
Keep:
Remove:
Current stack status
The current branch is stacked on #430 and still contains inherited semantic validation. It must not merge as-is. The useful #431 transport/ownership slice will be extracted onto a clean base after the approved pivot PRs land, and will receive fresh exact-head validation and review before any merge decision.
Existing branch validation
npm run test:ci— 99 suites / 5,794 tests passed