refactor(contracts): move witnesses into per-module test/witnesses/#528
Conversation
Witnesses are TypeScript stubs consumed only by simulators and tests (no production code imports them). Placing them next to `.compact` sources at the module root made them look like part of the public surface. They now live at `src/<module>/test/witnesses/`, sibling to `simulators/` and `mocks/`. - Move 11 witness sources and 6 collocated unit tests via `git mv`. - Rewrite 13 simulator imports, 2 top-level access test imports, and the 6 witness-test self-imports. - Narrow `tsconfig.json` include to the new path. - Update `contracts/README.md` layout diagram and the header path comment in each moved witness file. - Fix a pre-existing 3-up artifacts path in the access and token witness unit tests (silent at compile time because of `import type`). Verified: `yarn types` clean, `yarn test` 16 files / 895 tests pass.
Adds a callout in the root README, beside the existing experimental-code warning, flagging that witness implementations in this repo exist solely to drive Compact circuits during off-chain tests. Witnesses are security-critical and a buggy or malicious one can leak private state, so consumers building on this library must author and audit their own for production use.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (33)
WalkthroughWitness files are reorganized from module-level directories into test-scoped subdirectories. All 31 import paths, file header comments, and TypeScript configuration patterns are updated to reflect ChangesWitness Directory Reorganization
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Actionable comments posted: 0 |
andrew-fleming
left a comment
There was a problem hiding this comment.
LGTM! I just left a question
Types of changes
What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an `` in the boxes that apply
Fixes #390
PR Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...
Summary by CodeRabbit
Documentation
Chores