OpenZeppelin / OpenZeppelin/compact-contracts
dev: compile integration artifacts into their own directory
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 55
- Forks
- 29
- Avg merge
- 5d 7h
- Merged PRs (30d)
- 25
Description
Raised by @andrew-fleming in #717.
Problem
compile:integration writes into the same contracts/artifacts/ tree as src, and the compiler names each artifact directory after the source basename. Two sources sharing a basename silently overwrite each other, last compile wins.
Already happened: ConfidentialFungibleTokenPublicSupply.compact existed under both src/token/extensions and test/integration/_mocks. It surfaced as a wrong-contract arity error, invisible to lint, typecheck, and the truncated-key scan, and was worked around by renaming the mock (f453342d).
Proposed
Give the mocks their own output tree. The compiler already supports it:
-"compile:integration": "compact-compiler --src test/integration/_mocks"
+"compile:integration": "compact-compiler --src test/integration/_mocks --out artifacts/integration"
Then:
turbo.json—compile:integrationoutputs becomeartifacts/integration/**/- the three
test/integration/fixtures/*.tsimports move underartifacts/integration/ contracts/test-utils/harness/LiveSimulatorBackend.ts:116hardcodescontracts/artifacts/<name>/for ZK key loading, so the root has to come fromSimulatorConfiginsteademptyKeyArtifacts's variadicsourceRoots,INTEGRATION_MOCKS(scripts/live/paths.ts), and the#integrationconditional inArtifactCompilerexist only because the trees are shared, and can go
Caveats
- Not net-simpler: it swaps a multi-source-root scan for a multi-artifact-root lookup. The win is that the filesystem enforces the split rather than a naming convention nothing checks.
- Does not fix the turbo output overlap. All nine compile tasks declare
outputs: ["artifacts/**/"], so the shared-tree hazard behind #675 is repo-wide. - Keep the
Composed…mock name. Siblings areComposedTokensandSharedInitCollision, so the prefix is the convention for these mocks, not a workaround for the clash. - Touches the live deploy path, so it needs a live run to verify. That is why it stayed out of #717.
Cheaper interim
A scripts unit test asserting no basename appears under both src/**/*.compact and test/integration/_mocks/*.compact catches the collision in CI with no live-path change. Worth doing whether or not the move happens.
Refs #716.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the compile:integration script and turbo.json, then inspect the three test/integration/fixtures/*.ts imports and contracts/test-utils/harness/LiveSimulatorBackend.ts:116. Trace SimulatorConfig, emptyKeyArtifacts, INTEGRATION_MOCKS in scripts/live/paths.ts, and the #integration conditional in ArtifactCompiler. Done means integration artifacts use artifacts/integration, live key loading still works, obsolete shared-tree plumbing is removed, and a live run verifies the deploy path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- build-system, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100