feat: versioned VK registry for circuit upgrades without invalidating existing proofs - #111
Open
Topmatrixmor2014 wants to merge 17 commits into
Open
feat: versioned VK registry for circuit upgrades without invalidating existing proofs#111Topmatrixmor2014 wants to merge 17 commits into
Topmatrixmor2014 wants to merge 17 commits into
Conversation
… existing proofs ## Summary Implements VK versioning across CredentialVerifier and ProofRegistry contracts, supporting circuit upgrades while keeping old proofs valid (issue ToluLabs#85). ## Changes ### CredentialVerifier (contracts/credential_verifier/) - DataKey::Vk now keyed by (credential_type, version: u32) instead of just credential_type - Added DataKey::LatestVersion(Symbol) to track the highest registered version per type - Added DataKey::DeprecatedVersion(Symbol, u32) to mark versions as no longer accepted - Added Error::VersionDeprecated for rejecting submissions against deprecated versions - set_vk now takes a version: u32 parameter and auto-updates LatestVersion - verify_proof now accepts vk_version: Option<u32> (None = use latest) - Added deprecate_version(credential_type, version) admin-only function - Added get_latest_version(credential_type) -> u32 view function - 14 tests pass including 3 new tests: old_proof_valid_after_upgrade, deprecated_version_rejected, get_latest_version_tracks_highest ### ProofRegistry (contracts/proof_registry/) - VerifierInterface::verify_proof updated with vk_version: Option<u32> - ProofRecord gains vk_version: u32 field (0 = latest at submission time) - ProofSubmission gains vk_version: Option<u32> field - submit_proof signature: added vk_version: Option<u32> before expiry - submit_proofs_batch: each ProofSubmission carries its own vk_version - 23 tests pass ### GatedPool (contracts/gated_pool/) - Updated test harness for new set_vk and submit_proof signatures ### Frontend (frontend/) - contracts.ts: submitProof and submitProofsBatch accept optional vkVersion param - proof-registry package: ProofRecord and ProofSubmission types include vk_version - Typecheck passes with no errors ## Testing - cargo test: 43/43 tests pass (credential_verifier: 14, proof_registry: 23, gated_pool: 3, issuer_registry: 3) - pnpm tsc --noEmit: no TypeScript errors Closes ToluLabs#85
|
@Topmatrixmor2014 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Collaborator
|
@Topmatrixmor2014 resolve conflict and address greptile comments |
|
@Topmatrixmor2014 is attempting to deploy a commit to the psalmuel01's projects Team on Vercel. A member of the Team first needs to authorize it. |
DeprecatedVersion was written to persistent storage without extend_ttl, unlike DataKey::Vk and DataKey::LatestVersion. When the entry expires, verify_proof treats the version as active again via .unwrap_or(false), silently lifting the deprecation with no admin intervention. This fix extends the TTL to 180 days (matching VK storage), preventing the deprecation flag from expiring and re-enabling deprecated versions.
nativeToScVal expects a plain JS number for { type: "u32" } and a bigint
for { type: "u64" }. Passing BigInt(vkVersion) for u32 may cause the SDK
to throw a type error at runtime. Fix both submitProof and submitProofsBatch.
…ng to latest Version 0 is the ProofRecord sentinel meaning "no version" and no VK may be registered at version 0. Previously Some(v) if v > 0 would fall through on Some(0) to the wildcard arm, silently redirecting to the latest version. This made it impossible to distinguish an explicit Some(0) submission from one that used the vk_version: 0 sentinel. Now Some(0) panics with VkNotSet — version 0 is reserved.
Collaborator
|
@Topmatrixmor2014 contract ci is failing |
…sses - CredentialVerifier::set_vk now accepts a version param, writes DataKey::Vk(type, version), and auto-advances LatestVersion (never regresses on out-of-order re-registration). Rejects version 0. - Add deprecate_version (admin-only, validates VK exists) and get_latest_version view function to CredentialVerifier. - ProofRegistry::ProofRecord gains vk_version: u32 (0 = latest-at- submission sentinel); submit_proof and submit_proofs_batch persist the effective version instead of discarding it. - Update all contract tests to the versioned set_vk/submit_proof arity and add 3 versioning tests (upgrade path, old-proof validity, deprecation rejection). - Update deploy/benchmark scripts to pass --version 1 / --vk_version null. Fixes: contract compile error (DataKey::Vk arity) that broke the Contract tests & build and Reproducible Build CI jobs on PR ToluLabs#111.
Topmatrixmor2014
force-pushed
the
feat/vk-versioning
branch
from
August 3, 2026 13:29
f03c31a to
cb32cc7
Compare
…ation Completes the versioned-VK migration in ProofRegistry: submit_proofs now passes each submission's vk_version to verify_proof, submit_aggregate_proof resolves the latest aggregate VK (None), and store_claim records the latest-at-submission sentinel (0). Updates proof_registry and credential_verifier tests to the versioned set_vk(type, version, vk) / verify_proof(type, proof, inputs, vk_version) signatures and refreshes the affected test snapshots (including the stale submit_proofs_batch name). Fixes the contract build and cargo test failures that were breaking CI.
The committed range Prover.toml carried a stale commitment that failed
Poseidon2::hash([value, salt]) in-circuit, so nargo execute could never
solve a witness and fixtures/range/{vk,proof} were never generated (only a
4-byte placeholder public_inputs was committed).
Recompute the commitment for value=40000, salt=2024, re-sign it with the
deterministic demo issuer key, and stage the real vk, proof and
public_inputs. verifies_range now runs against a genuine UltraHonk proof.
submit_proof now takes (holder, issuer_id, credential_type, proof, public_inputs, vk_version, expiry). submitProof was sending 6 positional args, so expiry was deserialized as the VK version and every single-proof submission would fail. Add an optional vkVersion param (void when omitted -> latest) and place it before expiry, mirroring the batch path.
Topmatrixmor2014
force-pushed
the
feat/vk-versioning
branch
from
August 3, 2026 14:32
cb32cc7 to
a772a7f
Compare
The LatestVersion pointer's extend_ttl was gated behind `version > current`, so re-registering the current latest VK (e.g. to re-extend its TTL in a long-lived deployment) left the pointer untouched. After 180 days without a new circuit version the pointer expires and every verify_proof(..., None) — the default submission path — panics with VkNotSet. Move the extend_ttl out of the guard so any successful registration refreshes the pointer TTL, and add a regression test that decays the pointer into its bump threshold and re-registers the same version to prove the full 180-day TTL is restored.
Addresses the remaining Greptile P1: set_vk silently overwrote an existing VK at the same (credential_type, version), which would invalidate every proof already verified and cached against the original bytes. - set_vk now panics with VkAlreadySet when a VK already exists at the version, enforcing the PR's core invariant that VKs are immutable per version (register a new version to upgrade the circuit). - Add admin-only refresh_latest_version_ttl(credential_type) so the LatestVersion pointer TTL can be re-extended in long-lived deployments that register no new circuit versions — the refresh path the overwrite guard would otherwise remove, complementing the unconditional extend_ttl from the previous commit. - Tests: overwrite rejection keeps old proofs verifiable, refresh restores the full 180-day TTL, out-of-order registration never regresses latest (now via a gap version), refresh panics VkNotSet with no VK registered. All 79 contract tests pass; WASM release build compiles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #85
Implements VK versioning across the
CredentialVerifierandProofRegistrycontracts so that circuits can be upgraded (e.g., constraint optimisation, new public inputs) without invalidating existing on-chain proofs. Previouslyset_vkoverwrote the single VK per credential type, which would break every cached proof on upgrade. Now VKs are stored per(credential_type, version)and old proofs remain valid indefinitely.Design
(credential_type, version: u32)instead of justcredential_typeverify_proofacceptsvk_version: Option<u32>—Nonedefaults to latestset_vk(credential_type, version, vk)— auto-updates latest-version pointerdeprecate_version(credential_type, version)— admin-only, blocks new submissionsget_latest_version(credential_type) -> u32— view functionProofRecordgainsvk_version: u32(0 = latest at submission time)ProofSubmissiongainsvk_version: Option<u32>submit_proof/submit_proofs_batchcarry optionalvk_versionthrough to verifiersubmitProof()/submitProofsBatch()accept optionalvkVersion?: numberProofRecord,ProofSubmission) includevk_versionHow it works
set_vk("kyc", 1, vk_v1)— VK stored at(kyc, 1),LatestVersion(kyc) = 1vk_version = None→ verifier resolves to version 1set_vk("kyc", 2, vk_v2)— VK stored at(kyc, 2),LatestVersion(kyc) = 2(kyc, 1)was never deleteddeprecate_version("kyc", 1)— new submissions against v1 are rejected, but existing cached proofs in ProofRegistry remain readableFiles changed
Contracts
contracts/credential_verifier/src/lib.rs— Versioned VK storage, deprecation,get_latest_versioncontracts/credential_verifier/src/test.rs— Updated all tests + 3 new versioning testscontracts/proof_registry/src/lib.rs—vk_versioninProofRecord,ProofSubmission,submit_proofcontracts/proof_registry/src/test.rs— Updated all test calls and structscontracts/gated_pool/src/test.rs— Updatedset_vk/submit_proofcallsFrontend
frontend/lib/contracts.ts—vkVersionparam insubmitProof/submitProofsBatchfrontend/packages/proof-registry/src/index.ts— UpdatedProofRecordandProofSubmissiontypesTesting
Acceptance criteria (from #85)
get_latest_versionview function workscargo testpassesBackward compatibility
This is a breaking change to the contract interfaces — all callers must be updated to pass the new
vk_version/versionparameters. The frontend SDK and all internal tests have been updated accordingly. Deployed contracts will need to be re-deployed.Closes #85
Greptile Summary
This PR implements VK versioning across
CredentialVerifierandProofRegistry, storing VKs per(credential_type, version)and preserving old VKs after circuit upgrades so cached proofs remain valid indefinitely. Previous single-VK overwrites are replaced with an immutable-once-set model with explicit deprecation support and arefresh_latest_version_ttladmin utility.CredentialVerifiergains versionedset_vk,deprecate_version,get_latest_version, andrefresh_latest_version_ttl; all prior feedback around TTL management has been addressed.ProofRecordgainsvk_version: u32, the batch and single-proof paths both forward the version to the verifier, andgated_pooltests were updated with the new argument signatures.migrate_recordhas no branch for 5-field intermediate records, silently leaving such holders with permanently unreadable proofs; and theContractSpecblobs infrontend/packages/proof-registry/src/index.tswere not regenerated.Confidence Score: 3/5
Not safe to merge: two unresolved defects from the previous review round remain in the diff.
The contract logic in CredentialVerifier is solid and all prior inline feedback was addressed. Two defects persist: migrate_record has no handler for the 5-field intermediate schema leaving holders permanently blocked, and the ContractSpec blobs were not regenerated so the SDK Client silently drops vk_version from every submit_proof and batch submission.
Files Needing Attention: contracts/proof_registry/src/lib.rs (migrate_record missing 5-field branch) and frontend/packages/proof-registry/src/index.ts (stale ContractSpec blobs)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Admin participant CredentialVerifier participant ProofRegistry participant Holder Admin->>CredentialVerifier: set_vk(kyc, 1, vk_v1) Note over CredentialVerifier: Vk(kyc,1)=vk_v1, LatestVersion(kyc)=1 Holder->>ProofRegistry: submit_proof(kyc, proof, pi, None, expiry) ProofRegistry->>CredentialVerifier: verify_proof(kyc, proof, pi, None) Note over CredentialVerifier: Resolves None to version 1 CredentialVerifier-->>ProofRegistry: true Note over ProofRegistry: ProofRecord stored with vk_version=0 Admin->>CredentialVerifier: set_vk(kyc, 2, vk_v2) Note over CredentialVerifier: Vk(kyc,2)=vk_v2, LatestVersion(kyc)=2 Note over ProofRegistry: Old proof (vk_version=0) still readable via is_verified Admin->>CredentialVerifier: deprecate_version(kyc, 1) Note over CredentialVerifier: DeprecatedVersion(kyc,1)=true Holder->>ProofRegistry: submit_proof(kyc, proof, pi, Some(1), expiry) ProofRegistry->>CredentialVerifier: verify_proof(kyc, proof, pi, Some(1)) CredentialVerifier-->>ProofRegistry: panic VersionDeprecatedComments Outside Diff (1)
contracts/proof_registry/src/lib.rs, line 711-734 (link)migrate_recordsilently skips 5-field recordsThe function handles
len == 4(pre-issuerlegacy) and treatslen == 6as already-current, but has no branch forlen == 5. Any deployment that shipped the 5-field schema (fields:verified_at,expiry,threshold,revoked,issuer) — i.e., the version that addedissuerbut notvk_version— produces records withlen == 5. Whenmigrate_recordis called on such a record it silently exits, leaving the stored map untouched. Any subsequent read of that record via the new 6-fieldProofRecorddeserializer will panic with a shape mismatch, permanently blocking that holder's proof from being verified.A 5-field intermediate type (mirroring the 5-field layout, plus
vk_version: 0) should be added to handle this branch, matching the same pattern used for the 4-fieldLegacyProofRecord.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (15): Last reviewed commit: "fix(credential_verifier): refresh VK blo..." | Re-trigger Greptile