Skip to content

Add ecNeg for JubJub point negation#269

Open
adamreynolds-io wants to merge 5 commits intoLFDT-Minokawa:mainfrom
adamreynolds-io:adam-ec-neg
Open

Add ecNeg for JubJub point negation#269
adamreynolds-io wants to merge 5 commits intoLFDT-Minokawa:mainfrom
adamreynolds-io:adam-ec-neg

Conversation

@adamreynolds-io
Copy link
Copy Markdown
Contributor

@adamreynolds-io adamreynolds-io commented Mar 26, 2026

Summary

  • Add ecNeg / ec_neg as a native function for JubJub point negation
  • Compose from existing ZKIR instructions (encodenegdecode) — no proof system changes needed
  • Runtime uses pure field math: neg(x, y) = (-x mod p, y)

Closes #166

Note: This PR was AI-assisted using Claude Code (Claude Opus 4.6). All changes were reviewed and verified by a human contributor.

Design Decisions

The following assumptions were made during implementation and should be validated by reviewers:

  1. Scope: ecNeg only, no ecSub — Users can compose subtraction themselves via ecAdd(a, ecNeg(b)). Adding a separate ecSub convenience function was considered but deferred to keep the change minimal. If a stdlib-level ecSub circuit is desired, it can be added as a follow-up without any native/ZKIR changes.

  2. ZKIR approach: compose existing instructions, not a new Lzkir instruction — The proof system (midnight-zk) has EccInstructions::negate() but it's not exposed as a standalone ZKIRv3 instruction in midnight-ledger/zkir-v3/src/ir.rs. Rather than adding a new instruction to the ledger (which would require cross-repo changes), we compose from three existing instructions: encode (extract x, y) → neg (negate x) → decode (reconstruct point). This costs 3 ZKIR instructions per negation. If a dedicated EcNeg instruction is later added to ir.rs, the compiler can be updated to emit a single instruction — same semantics, fewer constraints.

  3. Runtime: pure field math, no ocrt delegation — The onchain-runtime-v3 does not export ecNeg. Rather than blocking on an ocrt update, the runtime implements negation as (-x mod p, y) directly in TypeScript. This is consistent with how jubjubPointX, jubjubPointY, and constructJubjubPoint are implemented — simple coordinate operations use pure TS, while complex curve arithmetic (ecAdd, ecMul) delegates to ocrt. JosephDenman's curves-proposal branch designs a CurveOps interface with neg() that can wrap this in the future.

Changes

File Change
compiler/midnight-natives.ss Native declaration: (JubjubPoint) -> JubjubPoint
compiler/standard-library-aliases.ss ec_negecNeg alias
compiler/zkir-v3-passes.ss ZKIRv3: compose encodenegdecode
compiler/zkir-passes.ss ZKIRv2: negate x, bind y unchanged
runtime/src/built-ins.ts ecNeg() — pure field math
runtime/test/stdlib.test.ts 2 tests: correctness + identity point
compiler/test.ss ZKIRv3 inline test expectation
doc/api/CompactStandardLibrary/exports.md API docs for ecNeg
doc/api/CompactStandardLibrary/README.md Added to function index
runtime/README.md Added to built-ins list
CHANGELOG.md Unreleased entry
examples/** ecNeg/ec_neg usage in 5 example contracts

Test plan

  • Compiler unit tests: 168/168 print-zkir-v3, 162/162 print-zkir
  • Runtime tests: 26/26 (2 new ecNeg tests)
  • Zero regressions across all compiler passes
  • E2E tests (require nix CI environment)

🤖 Generated with Claude Code

@adamreynolds-io adamreynolds-io requested review from a team as code owners March 26, 2026 12:39
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

Plugin Test Summary

 1 files   3 suites   1s ⏱️
21 tests 21 ✅ 0 💤 0 ❌
23 runs  23 ✅ 0 💤 0 ❌

Results for commit 8e35a42.

♻️ This comment has been updated with latest results.

Implement elliptic curve point negation as a new native function in the
Compact standard library. On the JubJub twisted Edwards curve, negation
of (x, y) is (-x, y).

Compiler: compose from existing ZKIR instructions (encode → neg → decode)
rather than adding a new instruction to the proof system. In ZKIRv2, the
point is already flattened to (x, y) fields, so we negate x and bind y.

Runtime: pure field math (-x mod p, y), consistent with the existing
pattern where simple coordinate operations (jubjubPointX, jubjubPointY,
constructJubjubPoint) don't delegate to onchain-runtime.

Closes LFDT-Minokawa#166

Signed-off-by: Adam Reynolds <adam.reynolds@shielded.io>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update the standard library API docs, runtime README, and changelog
for the new ecNeg (ec_neg) function.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Adam Reynolds <adam.reynolds@shielded.io>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

Compactc E2E Test Summary

    1 files  ±    0      1 suites   - 47   5m 10s ⏱️ + 2m 41s
2 828 tests +2 361  2 828 ✅ +2 367  0 💤  - 6  0 ❌ ±0 
2 828 runs  +2 350  2 828 ✅ +2 356  0 💤  - 6  0 ❌ ±0 

Results for commit 8e35a42. ± Comparison against base commit 90d3f41.

This pull request removes 467 and adds 2828 tests. Note that renamed tests count towards both.
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[MFG-413] should compile and not thro…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-12371] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-15405] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-15733] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-15826] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-16040] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-16059] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-16447] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-16853] should compile and not thr…'
src/tests/bugs/compiler.bugs.e2e.test.ts ‑ [Bugs] Compiler > '[PM-16999] should return an error if …'
…
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_0.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_10.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_100.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1000.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1001.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1002.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1003.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1004.compact'
src/tests/extracted.e2e.test.ts ‑ [E2E] Extracted unit tests for compiler > should be able to compile extracted contract: '/home/runner/work/_temp/nix-shell.Ti2rTj/temp-test-QyLBAB/contract_1005.compact'
…

♻️ This comment has been updated with latest results.

Add ec_neg/ecNeg calls to the standard library example contracts
alongside the existing ecAdd, ecMul, and ecMulGenerator usage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Adam Reynolds <adam.reynolds@shielded.io>

(declare-native-entry circuit ecNeg
"__compactRuntime.ecNeg"
([a (TypeRef JubjubPoint) (discloses "the negation of")])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To match the style of the existing disclosure failure messages, this should be: "the elliptic curve negation of".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

(degrade_to_transient . degradeToTransient)
(upgrade_from_transient . upgradeFromTransient)
(ec_add . ecAdd)
(ec_neg . ecNeg)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's no reason for this, remove it.

This support allows a programmer to write ec_neg instead of ecNeg and then run compact fix to change it to ecNeg. But nobody would do that in new code, they'd just write the real name. Fixups are intended for existing code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

compiler/test.ss Outdated
'(
"import CompactStandardLibrary;"
""
"ledger forceField: Field; circuit forceProof(): [] { forceField = 7; }"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This complicated setup doesn't really make a difference here. I just write:

ledger impure: Boolean;

export circuit test(...): ... {
  impure = true;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switched to the simpler ledger impure: Boolean; impure = true; pattern.

const test_21 = upgradeFromTransient(1);
const test_22 = default<JubjubPoint>;
const test_23 = ecAdd(default<JubjubPoint>, default<JubjubPoint>);
const test_23b = ecNeg(default<JubjubPoint>);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Get rid of this line (see below).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

const test_21 = upgradeFromTransient(1);
const test_22 = default<JubjubPoint>;
const test_23 = ecAdd(default<JubjubPoint>, default<JubjubPoint>);
const test_23b = ecNeg(default<JubjubPoint>);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know why the tests are structured this way, but this duplicates the coverage in external/examples.compact and isn't necessary here.

This test is supposedly testing that the output of compact fix can be compiled, and nobody will have code using ec_neg at all, nor will they have code using ecNeg(default<NativePoint>). So I'd get rid of it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

test('elliptic curve negation', () => {
const g = compactRuntime.ecMulGenerator(1n);
const neg_g = compactRuntime.ecNeg(g);
// g + neg(g) should equal the identity point (0, 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm clearly out of my area of expertise, but doesn't g - g equal zero (not the identity)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On twisted Edwards curves the identity element is (0, 1), which is what ecAdd(g, ecNeg(g)) returns. The test confirms this. "Zero" and "identity" are the same thing here — the additive identity of the group.

}

/**
* The Compact builtin `ec_neg` function
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, it's really unfortunate that the comment uses the old spelling for all these. I'd change this to ecNeg to stop contributing to the problem :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@kmillikin kmillikin changed the title Add ecNeg (ec_neg) for JubJub point negation Add ecNeg for JubJub point negation Mar 26, 2026
const g = compactRuntime.ecMulGenerator(1n);
const neg_g = compactRuntime.ecNeg(g);
// neg negates x, preserves y
expect(neg_g.x).not.toEqual(g.x);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a stronger statement to be made here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point — we can assert the exact negation rather than just "not equal":

const FIELD_MODULUS = 0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001n;
expect(neg_g.x).toEqual(FIELD_MODULUS - g.x);

The double-negation and identity tests already cover the algebraic properties, but checking the exact field arithmetic makes this less fragile. Will update.

- Fix instruction order in ZKIRv3 (cons* accumulates reversed list)
- Fix disclosure message to "the elliptic curve negation of"
- Remove ec_neg alias (no fixup support needed for new functions)
- Simplify test boilerplate (ledger impure: Boolean pattern)
- Bump toolchain to 0.30.105, language to 0.22.102
- Fix CHANGELOG format to match project convention
- Remove unnecessary ecNeg from camelCase examples
- Fix runtime test assertions (neg negates x, preserves y)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Adam Reynolds <adam.reynolds@shielded.io>
Assert neg_g.x equals FIELD_MODULUS - g.x instead of just checking
inequality, per gilescope's review feedback.

Signed-off-by: Adam Reynolds <adam.reynolds@shielded.io>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@adamreynolds-io
Copy link
Copy Markdown
Contributor Author

Pushed a fix strengthening the ecNeg test assertion per @gilescope's feedback — now checks exact field arithmetic (FIELD_MODULUS - g.x) instead of just inequality.

All review comments have been addressed. The ZKIRv2 and print-typescript tests Kevin requested are already in the previous push. The Release build check failure is expected (requires run-release-workflow label).

@kmillikin ready for re-review when you get a chance — the outstanding threads are all either outdated or the (-x, y) vs (x, -y) discussion where I provided empirical verification.

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.

CLONE - Add negation of JubJub points to compact

3 participants