feat(compiler): implement explicit PV11 compiler target (ADR-031) - #93
Conversation
satran004
left a comment
There was a problem hiding this comment.
Review — ADR-031 explicit PV11 compiler target
Verdict: approve with notes — ready to merge. No blockers. The implementation matches ADR-031 D1–D10, the fail-closed contract holds through every emission route I could find, and the PR's central promise (no byte changes) was verified independently rather than taken from the description.
What was verified
ADR conformance (D1–D10)
- D1/D3/D4:
CompilerTargetis the full language/protocol/UPLC triple;CompilerTargetRegistry.SUPPORTEDis an exactSet.of(PLUTUS_V3_PV11); profile-ID lookup is case-sensitive; noLATEST. PV10 / V1 / V2 / UPLC 1.0.0 / PV12 all →JULC0031, never fallback (CompilerTargetRegistryTest). - D2:
julc-compilernowapi project(':julc-vm')and reusesProtocolFeatureRegistry; no parallel builtin table in the compiler. - D5:
CompilationContextis resolved once inbeginCompilation()before classpath library scanning and reachesLibraryCompiler,LibraryMethodRegistry,PirGenerator,UplcGenerator,UplcOptimizer, program construction andCompileResult.compilationSnapshotsOptionsAndResolvesTargetExactlyOnceproves mid-compile mutation ofCompilerOptionscannot leak in. - D6:
LAST_RELEASED_PV11_BUILTIN_TAGis gone;generateBuiltinasksfeatureProfile().isBuiltinAvailable(fun). Grep for raw>= 11/PlutusTarget.CURRENTin compiler passes is clean (two leftovers noted in F4, both envelope-type-only). - D7: all
BUILTIN_{1,2,3}_ARGtable entries,insertCoin,JulcArray.fromListand the three Array type-methods declareLoweringRequirements;LoweringRequirementsTestpins all 15 batch-6 routes. - D8:
UplcTargetValidatorruns after the optimizer on every entry path, includingcompilePirToProgramand source-map builds; the optimizer reports applied pass names forJULC0035.foldBinaryOprefuses to fold an unavailable builtin (optimizer containment). - D9/D10:
CompileResultgainstargetwith explicit 6/5/3/2-arg compatibility constructors (binary-compatible; no record-deconstruction consumers in repo). Catalog entriesJULC0031–0036added in bothDiagnosticCodesanddiagnostics.json.
Tests run fresh
./gradlew :julc-vm:test :julc-compiler:test --tests '*CompilerTarget*' --tests '*UplcTargetValidator*' --tests '*LoweringRequirements*' --tests '*Pv11CompilerContract*' :julc-testkit:test→ 88 + 24 + 190 tests, 0 failures.
Byte-stability vs main — verified directly. No existing test pins a real compiled-script hash, and CompilerTargetPropagationTest compares default-vs-explicit both on the branch. So origin/main was checked out in a worktree, the 10 real validators in julc-plugin-test/src/main/plutus/ + julc-playground/src/main/resources/examples/ were compiled on both trees with a throwaway probe, and FLAT length + sha256 diffed: 10/10 identical (MintingPolicy, SimpleSpending, VestingValidator, AuthorizedMintingPolicy, MultiSigValidator, TimelockValidator, TokenGateValidator, TypedMintingPolicy, TypedMultiSigValidator, TypedVestingValidator).
Findings
F1 (medium, compatibility) — testkit String-source entry points now hard-require a target-aware VM provider.
ValidatorTest.evaluate/assertValidates/assertRejects(String, …) (ValidatorTest.java:126), ContractTest.evaluate, JulcEval and MethodEvaluator.evaluateRaw now call vm.evaluate(program, target, …). Provider priority is Truffle 200 > Java 100 > Scalus 50, so mixed classpaths are fine — but a consumer with only julc-vm-scalus (the setup main's README recommended until this PR) gets a raw UnsupportedOperationException from the SPI default (JulcVmProvider.java:53), not a JULC* diagnostic with remediation. ADR-sanctioned direction, and the external julc-examples / julc-helloworld repos are already on vm-java, so nothing known breaks — but this needs (a) an actionable message ("add testRuntimeOnly julc-vm-java"), consistent with D10's own standard, and (b) an explicit release-note entry. The PR body's "fails closed for uncertified explicit-target evaluation" undersells it.
F2 (medium-low, observable semantics) — Those same paths previously used the language-only path, which on the Java VM defaults to PV10 (JulcVm.java:133-137 javadoc). They now evaluate under exact PV11 (semantics variant E, PV11 decode limits). An improvement, but a semantic change to existing test-suite behaviour; belongs in release notes.
F3 (low, docs/code mismatch) — julc-examples/README.md:35-38 now prescribes compileWithDetails + evaluate(compiledResult, …), but no example was migrated (VestingValidatorTest.java:99-132, the README's "simplest starting point", still uses compile / evaluate(source, …)). Also, the only public way to get a plain CompileResult from ValidatorTest is the heavier PIR/UPLC-capturing compileWithDetails; the new compileResult(String, StdlibLookup) is private. Suggest making it public.
F4 (low, invariant hygiene) — PlutusTarget.CURRENT survives in core as a parallel "current" alias feeding textEnvelopeType() (ScriptPurposeMetadata.java:13, julc-cli/.../ConvertCommand.java:42). Cannot drift with one target, but it is exactly the shape ADR-031 argues against; follow-up: derive envelope type from CompileResult.target().ledgerTarget().ledgerLanguage().
F5 (info)
compileWithSiblingswith a parent-less path (JulcCompiler.java:714-720) no longer classpath-scans@OnchainLibrarysources (old code delegated tocompile(String)). Edge case; arguably more consistent now.TypeMethodRegistry.dispatch(context, …)accepts the context but handlers never receive it — plumbing without a consumer until a second target exists.SubsetValidatoris not context-aware (D5 lists it); nothing feature-bearing lives there today.julc-annotation-processor/build.gradle:16still has Scalus-only test runtime; works (its tests don't hit target-aware paths) but is inconsistent with the repo-wide move to vm-java.
Recommended before/at merge
- Decide whether F1 gets a catalog diagnostic in this PR or as an immediate follow-up.
- Release notes should state F1 + F2 and the
CompileResultrecord-component change (ADR "Binary compatibility" section requires it be documented). - Optional:
UplcTargetValidatorTest.rejectsFutureBuiltinSurvivingAnOptimizerPassvalidates a hand-built program, not an actual optimizer-introduced term — fine today since no pass introduces builtins, but ADR-032 passes should add a real one.
Review performed with Claude Code (Fable 5).
Summary
Implements ADR-031 and closes ADR-029 Phase 4 for the deliberately narrow V3/PV11 compiler profile.
Compatibility and correctness
Milestones
Each implementation milestone was developed and tested on a separate branch, reviewed, and merged into this integration branch.
Verification
./gradlew build --no-daemon --console=plain— BUILD SUCCESSFUL (213 tasks)Closes #76
Related to #65