OpenZeppelin / OpenZeppelin/compact-contracts

dev: compile integration artifacts into their own directory

Open
#758 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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.jsoncompile:integration outputs become artifacts/integration/**/
  • the three test/integration/fixtures/*.ts imports move under artifacts/integration/
  • contracts/test-utils/harness/LiveSimulatorBackend.ts:116 hardcodes contracts/artifacts/<name>/ for ZK key loading, so the root has to come from SimulatorConfig instead
  • emptyKeyArtifacts's variadic sourceRoots, INTEGRATION_MOCKS (scripts/live/paths.ts), and the #integration conditional in ArtifactCompiler exist 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 are ComposedTokens and SharedInitCollision, 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.