Skip to content

refactor(contracts): move witnesses into per-module test/witnesses/#528

Merged
0xisk merged 2 commits into
mainfrom
chore/move-witneeses
May 29, 2026
Merged

refactor(contracts): move witnesses into per-module test/witnesses/#528
0xisk merged 2 commits into
mainfrom
chore/move-witneeses

Conversation

@0xisk
Copy link
Copy Markdown
Member

@0xisk 0xisk commented May 28, 2026

Types of changes

What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an `` in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices 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

    • Added guidance clarifying that witness implementations are test-only artifacts and not part of the public API; highlighted their security-critical nature and production usage requirements.
  • Chores

    • Reorganized witness files into test directories for improved repository structure.
    • Updated all related references throughout the codebase.

Review Change Stack

0xisk added 2 commits May 28, 2026 15:04
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.
@0xisk 0xisk requested review from a team as code owners May 28, 2026 13:07
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4548d70d-80c5-4e54-b50d-55ed832f57c1

📥 Commits

Reviewing files that changed from the base of the PR and between ed68e80 and d3dadbd.

📒 Files selected for processing (33)
  • README.md
  • contracts/README.md
  • contracts/src/access/test/ShieldedAccessControl.test.ts
  • contracts/src/access/test/ZOwnablePK.test.ts
  • contracts/src/access/test/simulators/AccessControlSimulator.ts
  • contracts/src/access/test/simulators/OwnableSimulator.ts
  • contracts/src/access/test/simulators/ShieldedAccessControlSimulator.ts
  • contracts/src/access/test/simulators/ZOwnablePKSimulator.ts
  • contracts/src/access/test/witnesses/AccessControlWitnesses.test.ts
  • contracts/src/access/test/witnesses/AccessControlWitnesses.ts
  • contracts/src/access/test/witnesses/OwnableWitnesses.test.ts
  • contracts/src/access/test/witnesses/OwnableWitnesses.ts
  • contracts/src/access/test/witnesses/ShieldedAccessControlWitnesses.ts
  • contracts/src/access/test/witnesses/ZOwnablePKWitnesses.test.ts
  • contracts/src/access/test/witnesses/ZOwnablePKWitnesses.ts
  • contracts/src/archive/test/simulators/ShieldedTokenSimulator.ts
  • contracts/src/archive/test/witnesses/ShieldedTokenWitnesses.ts
  • contracts/src/security/test/simulators/InitializableSimulator.ts
  • contracts/src/security/test/simulators/PausableSimulator.ts
  • contracts/src/security/test/witnesses/InitializableWitnesses.ts
  • contracts/src/security/test/witnesses/PausableWitnesses.ts
  • contracts/src/token/test/simulators/FungibleTokenSimulator.ts
  • contracts/src/token/test/simulators/MultiTokenSimulator.ts
  • contracts/src/token/test/simulators/NonFungibleTokenSimulator.ts
  • contracts/src/token/test/witnesses/FungibleTokenWitnesses.test.ts
  • contracts/src/token/test/witnesses/FungibleTokenWitnesses.ts
  • contracts/src/token/test/witnesses/MultiTokenWitnesses.test.ts
  • contracts/src/token/test/witnesses/MultiTokenWitnesses.ts
  • contracts/src/token/test/witnesses/NonFungibleTokenWitnesses.test.ts
  • contracts/src/token/test/witnesses/NonFungibleTokenWitnesses.ts
  • contracts/src/utils/test/simulators/UtilsSimulator.ts
  • contracts/src/utils/test/witnesses/UtilsWitnesses.ts
  • contracts/tsconfig.json

Walkthrough

Witness 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 src/<module>/test/witnesses/ as the canonical witness location. No witness logic or exported interfaces change.

Changes

Witness Directory Reorganization

Layer / File(s) Summary
Documentation and build configuration
README.md, contracts/README.md, contracts/tsconfig.json
README adds a warning that witnesses are test-only artifacts not included in the published package; module directory layout diagram updated to show test/witnesses/ nesting; TypeScript include pattern changed from src/**/witnesses/**/*.ts to src/**/test/witnesses/**/*.ts.
Access module witness reorganization
contracts/src/access/test/witnesses/*, contracts/src/access/test/ShieldedAccessControl.test.ts, contracts/src/access/test/ZOwnablePK.test.ts, contracts/src/access/test/simulators/*
All four access-module witness files (AccessControl, Ownable, ShieldedAccessControl, ZOwnablePK) have header paths updated; test*.ts and simulator import paths changed from ../witnesses/ to ./witnesses/ for test files and ../witnesses/ for simulators.
Archive module witness reorganization
contracts/src/archive/test/witnesses/ShieldedTokenWitnesses.ts, contracts/src/archive/test/simulators/ShieldedTokenSimulator.ts
Witness header path updated; simulator import path adjusted to ../witnesses/.
Security module witness reorganization
contracts/src/security/test/witnesses/InitializableWitnesses.ts, contracts/src/security/test/witnesses/PausableWitnesses.ts, contracts/src/security/test/simulators/InitializableSimulator.ts, contracts/src/security/test/simulators/PausableSimulator.ts
Both witness header paths updated; both simulator imports adjusted to ../witnesses/.
Token module witness reorganization
contracts/src/token/test/witnesses/*, contracts/src/token/test/simulators/*
All three token-module witness files (Fungible, Multi, NonFungible) have header paths updated; witness test imports changed to ./ paths, simulator imports changed to ../witnesses/.
Utils module witness reorganization
contracts/src/utils/test/witnesses/UtilsWitnesses.ts, contracts/src/utils/test/simulators/UtilsSimulator.ts
Witness header path updated; simulator import path adjusted to ../witnesses/.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🐰 Witnesses tucked in test-time nooks,
No longer loose in module books,
Each import path now points just right,
To /test/witnesses/ in plain sight!
Security-first, as the docs now say—
Audit your own witnesses, come what may! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring: relocating witness files from module roots to per-module test/witnesses/ directories, which is the core structural change throughout the pull request.
Linked Issues check ✅ Passed The PR successfully addresses issue #390 by repositioning witnesses under test/ directories and adding documentation clarifying they are test-only artifacts not part of the published package, fulfilling the objective to remove them from the packaging process.
Out of Scope Changes check ✅ Passed All changes are scoped to witness relocation, import path updates, documentation reflecting new structure, and tsconfig alignment. No unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/move-witneeses

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

Copy link
Copy Markdown
Contributor

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I just left a question

Comment thread README.md
Comment thread contracts/src/utils/test/witnesses/UtilsWitnesses.ts
@0xisk 0xisk merged commit 719f29b into main May 29, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove witness files from packaging process

2 participants