Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add POD uniqueness module #1884

Merged
merged 9 commits into from
Sep 19, 2024
Merged

Add POD uniqueness module #1884

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7ddf10f
Add uniqueness module
ax0 Sep 12, 2024
b3722e5
Test
ax0 Sep 16, 2024
24c37da
Lint
ax0 Sep 16, 2024
50e4ffe
Tests
ax0 Sep 16, 2024
cfa1f33
Code review
ax0 Sep 17, 2024
5daa712
Code review
ax0 Sep 19, 2024
2117cec
npm update
ax0 Sep 19, 2024
9b7aafe
Merge branch 'main' into gpc-uniqueness
ax0 Sep 19, 2024
0ab9d5a
Merge branch 'main' into gpc-uniqueness
ax0 Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions packages/lib/gpc/src/gpcChecks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import {
import { Identity as IdentityV4 } from "@semaphore-protocol/core";
import { Identity } from "@semaphore-protocol/identity";
import JSONBig from "json-bigint";
import _ from "lodash";
import isEqual from "lodash/isEqual";
import uniq from "lodash/uniq";
import {
GPCBoundConfig,
GPCIdentifier,
Expand Down Expand Up @@ -635,12 +636,12 @@ export function checkProofPODUniquenessInputsForConfig(
const contentIDs = Object.values(proofInputs.pods).map(
(pod) => pod.contentID
);
const uniqueContentIDs = _.uniq(contentIDs);
const podsAreUnique = _.isEqual(contentIDs, uniqueContentIDs);
const uniqueContentIDs = uniq(contentIDs);
const podsAreUnique = isEqual(contentIDs, uniqueContentIDs);

if (!podsAreUnique) {
throw new Error(
"Proof configuration specifies that the PODs should be unique, but they aren't."
"Proof configuration specifies that the PODs should have unique content IDs, but they don't."
);
}
}
Expand Down Expand Up @@ -727,7 +728,7 @@ export function checkProofListMembershipInputsForConfig(
for (const element of inputList) {
const elementWidth = widthOfEntryOrTuple(element);

if (!_.isEqual(elementWidth, comparisonWidth)) {
if (!isEqual(elementWidth, comparisonWidth)) {
throw new TypeError(
`Membership list ${listIdentifier} in input contains element of width ${elementWidth} while comparison value with identifier ${JSON.stringify(
comparisonId
Expand All @@ -740,7 +741,7 @@ export function checkProofListMembershipInputsForConfig(
// hashes as this reflects how the values will be treated in the
// circuit.
const isComparisonValueInList = inputList.find((element) =>
_.isEqual(
isEqual(
applyOrMap(podValueHash, element),
applyOrMap(podValueHash, comparisonValue)
)
Expand Down Expand Up @@ -785,7 +786,7 @@ export function checkInputListNamesForConfig(
);
const inputListNames = new Set(listNames);

if (!_.isEqual(configListNames, inputListNames)) {
if (!isEqual(configListNames, inputListNames)) {
throw new Error(
`Config and input list mismatch.` +
` Configuration expects lists ${JSON.stringify(
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/gpc/src/gpcCompile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,9 +728,9 @@ function compileProofEntry(
export function compileProofPODUniqueness(proofConfig: {
uniquePODs?: boolean;
}): {
uniquenessModuleIsEnabled: CircuitSignal;
requireUniqueContentIDs: CircuitSignal;
} {
return { uniquenessModuleIsEnabled: BigInt(proofConfig.uniquePODs ?? false) };
return { requireUniqueContentIDs: BigInt(proofConfig.uniquePODs ?? false) };
}

function compileProofVirtualEntry<
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/gpc/src/gpcTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ export type GPCProofConfig = {
pods: Record<PODName, GPCProofObjectConfig>;

/**
* Indicates whether the configured PODs are unique. If this is true, it
* enables the POD uniqueness module on the circuit level.
* Indicates whether the configured PODs should have unique content IDs.
* If this is true, it enables the POD uniqueness module on the circuit level.
*/
uniquePODs?: boolean;

Expand Down
2 changes: 1 addition & 1 deletion packages/lib/gpc/test/gpc.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ describe("gpc library (Compiled test artifacts) should work", async function ()
);
},
"Error",
"Proof configuration specifies that the PODs should be unique, but they aren't."
"Proof configuration specifies that the PODs should have unique content IDs, but they don't."
);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/lib/gpc/test/gpcCompile.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,13 @@ describe("Semaphore V4 owner module compilation for verification should work", (
describe("POD uniqueness module compilation for proving and verification should work", () => {
it("should work as expected for a proof configuration with POD uniqueness enabled", () => {
expect(compileProofPODUniqueness({ uniquePODs: true })).to.deep.equal({
uniquenessModuleIsEnabled: 1n
requireUniqueContentIDs: 1n
});
});
it("should work as expected for a proof configuration with POD uniqueness disabled", () => {
for (const config of [{}, { uniquePODs: false }]) {
expect(compileProofPODUniqueness(config)).to.deep.equal({
uniquenessModuleIsEnabled: 0n
requireUniqueContentIDs: 0n
});
}
});
Expand Down
12 changes: 6 additions & 6 deletions packages/lib/gpcircuits/circuits.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
},
Expand Down Expand Up @@ -73,7 +73,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
},
Expand Down Expand Up @@ -112,7 +112,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
},
Expand Down Expand Up @@ -151,7 +151,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
},
Expand Down Expand Up @@ -190,7 +190,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
},
Expand Down Expand Up @@ -229,7 +229,7 @@
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
]
}
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/gpcircuits/circuits/gpc-util.circom
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ template MaybeInputSelector (N) {
/**
* Left-rotates elements of a given array by I positions.
*/
template Rotl(I,N) {
template ArrayRotl(I,N) {
signal input in[N];
signal output out[N];

Expand All @@ -111,7 +111,7 @@ template Take(I,N) {
/**
* Adds a field element to all elements of a given array.
*/
template Add(N) {
template ArrayAddScalar(N) {
signal input element;
signal input in[N];
signal output out[N];
Expand Down
26 changes: 13 additions & 13 deletions packages/lib/gpcircuits/circuits/list-membership.circom
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ template ListMembershipModule(
signal output isMember;

isMember <== IsZero()(
UnnormalisedListNonMembership(MAX_LIST_ELEMENTS, MAX_LIST_ELEMENTS)(
NotEqualsAny(MAX_LIST_ELEMENTS, MAX_LIST_ELEMENTS)(
comparisonValue,
validValues
)
Expand All @@ -34,31 +34,31 @@ template ListMembershipModule(

/**
* Helper template returning a non-zero field element if the given
* value is not an element of the given list restricted to the first
* `NUM_LIST_ELEMENTS` elements and zero otherwise. This is done by
* subtracting the value from all elements of the list and folding it
* by means of field multiplication.
* value is not equal to any element of the given list restricted to
* the first `NUM_LIST_ELEMENTS` elements and zero otherwise. This is
* done by subtracting the value from all elements of the list and
* folding it by means of field multiplication.
*/
template UnnormalisedListNonMembership(NUM_LIST_ELEMENTS, MAX_LIST_ELEMENTS) {
template NotEqualsAny(NUM_LIST_ELEMENTS, MAX_LIST_ELEMENTS) {
// Value to be checked.
signal input comparisonValue;

// List of admissible values.
signal input validValues[MAX_LIST_ELEMENTS];
// List of values to check against for equality.
signal input values[MAX_LIST_ELEMENTS];

// Indicator of whether the value is not an element of the list of
// admissible values, viz. a non-zero field element iff the value
// is a non-member.
signal output isNotMember;
signal output isNotEqual;

if (NUM_LIST_ELEMENTS == 0) {
isNotMember <== 1;
isNotEqual <== 1;
} else {
isNotMember <== MultiAND(NUM_LIST_ELEMENTS)(
Add(NUM_LIST_ELEMENTS)(
isNotEqual <== MultiAND(NUM_LIST_ELEMENTS)(
ArrayAddScalar(NUM_LIST_ELEMENTS)(
-comparisonValue,
Take(NUM_LIST_ELEMENTS, MAX_LIST_ELEMENTS)(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems out-of-place for this as a general list-non-membership template. The need to take only the head of the list is motivated only by the uniqueness use case, so maybe Take should happen at that level, on the input array?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is how I initially did it, but Circom doesn't like that approach:

thread 'main' panicked at compiler/src/intermediate_representation/translate.rs:959:25:
internal error: entered unreachable code: On development: Circom compiler does not accept for now the assignment of arrays of unknown sizes during the execution of loops

Copy link
Collaborator

Choose a reason for hiding this comment

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

Where's the unknown size? I'd expect all the array lenghts to be known (based on template args) at the point of use. I'd have to see the specific code to comment on it, though, and I'm fully willing to believe that circom is broken.

Copy link
Collaborator Author

@ax0 ax0 Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

All of the array sizes depend only on the template parameter and loop index. I assume there is a bug in the loop + anonymous component unrolling code. What I did was replace the list non-membership and uniqueness module templates with the following:

template NotEqualsAny(MAX_LIST_ELEMENTS) {
    signal input comparisonValue; 

    signal input values[MAX_LIST_ELEMENTS];

    signal output isNotEqual;

    if (MAX_LIST_ELEMENTS == 0) {
        isNotEqual <== 1;
    } else {
        isNotEqual <== MultiAND(MAX_LIST_ELEMENTS)(
            ArrayAddScalar(MAX_LIST_ELEMENTS)(
                -comparisonValue,
                values
            )
        );
    }
}

template UniquenessModule(
    NUM_LIST_ELEMENTS
) {
    signal input values[NUM_LIST_ELEMENTS];

    signal output valuesAreUnique;

    signal noDupsAfter[NUM_LIST_ELEMENTS];

    for(var i = 0; i < NUM_LIST_ELEMENTS; i++) {
        var j = i+1;
        noDupsAfter[i] <==
            NotEqualsAny(NUM_LIST_ELEMENTS - j)(
                values[i],
                Take(NUM_LIST_ELEMENTS - j, NUM_LIST_ELEMENTS)(
                    ArrayRotl(j, NUM_LIST_ELEMENTS)(values)
                )
            );
    }

    valuesAreUnique <== NOT()(
        IsZero()(
            MultiAND(NUM_LIST_ELEMENTS)(
                noDupsAfter
            )
        )
    );
}

My original version used Drop instead of Take and Rotl (which is what I believe you suggested in your other comment) but yielded the same result.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. Seems like a circom limitation keeps us from sizing arrays based on loop variables, but not if they're inside of templates. It's unintuitive to me that a loop variable can be a template argument (as demonstrated in our use of Take and Rotl) but can't be an array size. Take does return an array of a size which depends on its template argument, so it shouldn't matter if it's directly in the loop or in a template instantiated in the loop.

Would it help if you combined Take and Rotl into a single Slice template which returned array elements from [I, I+N)? Or would that make the situation worse? I don't really understand what circom's limitation is, so it's hard to know what to suggest. If there's no obvious solution I'm okay with accepting what works, given the final number of constraints is the same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried doing this with a single drop-like template but ran into the same error, which is what led me to the current approach.

validValues
values
)
)
);
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/gpcircuits/circuits/proto-pod-gpc.circom
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,11 +373,11 @@ template ProtoPODGPC (
*/

// Boolean indicating whether the uniqueness module is enabled.
signal input uniquenessModuleIsEnabled;
signal input requireUniqueContentIDs;
signal podsAreUnique <== UniquenessModule(MAX_OBJECTS)(
objectContentID
);
uniquenessModuleIsEnabled * (1 - podsAreUnique) === 0;
requireUniqueContentIDs * (1 - podsAreUnique) === 0;

/*
* 1 GlobalModule with its inputs & outputs.
Expand Down
16 changes: 8 additions & 8 deletions packages/lib/gpcircuits/circuits/uniqueness.circom
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,26 @@ template UniquenessModule(
signal output valuesAreUnique;

// Array of field elements indicating whether the corresponding
ax0 marked this conversation as resolved.
Show resolved Hide resolved
// element of `values` is unique, i.e. isUnique[i] = 0 iff
// values[i] is not unique.
signal isUnique[NUM_LIST_ELEMENTS];
// element of `values` has no duplicates following it in `values`,
// i.e. noDupsAfter[i] = 0 iff this is the case.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be != 0 rather than = 0? Otherwise I'd reverse the naming of the variable so it indicates what's true, not what's false.

signal noDupsAfter[NUM_LIST_ELEMENTS];

// Loop through and check whether the ith element of `values` is
// not an element of `values` with the first i+1 elements removed.
for(var i = 0; i < NUM_LIST_ELEMENTS; i++) {
var j = i+1;
isUnique[i] <==
UnnormalisedListNonMembership(NUM_LIST_ELEMENTS - j, NUM_LIST_ELEMENTS)(
noDupsAfter[i] <==
NotEqualsAny(NUM_LIST_ELEMENTS - j, NUM_LIST_ELEMENTS)(
values[i],
Rotl(j, NUM_LIST_ELEMENTS)(values)
ArrayRotl(j, NUM_LIST_ELEMENTS)(values)
);
}

// All values are unique iff all elements of `isUnique` are non-zero.
// All values are unique iff all elements of `noDupsAfter` are non-zero.
valuesAreUnique <== NOT()(
IsZero()(
MultiAND(NUM_LIST_ELEMENTS)(
ax0 marked this conversation as resolved.
Show resolved Hide resolved
isUnique
noDupsAfter
)
)
);
Expand Down
12 changes: 6 additions & 6 deletions packages/lib/gpcircuits/src/proto-pod-gpc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export type ProtoPODGPCInputs = {
/*PUB*/ listValidValues: CircuitSignal /*MAX_LISTS*/[] /*MAX_LIST_ENTRIES*/[];

// POD uniqueness module (1)
/*PUB*/ uniquenessModuleIsEnabled: CircuitSignal;
/*PUB*/ requireUniqueContentIDs: CircuitSignal;

// Global module (1)
/*PUB*/ globalWatermark: CircuitSignal;
Expand Down Expand Up @@ -110,7 +110,7 @@ export type ProtoPODGPCInputNamesType = [
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
];

Expand Down Expand Up @@ -157,7 +157,7 @@ export type ProtoPODGPCPublicInputs = {
/*PUB*/ listValidValues: CircuitSignal /*MAX_LISTS*/[] /*MAX_LIST_ENTRIES*/[];

// POD uniqueness module (1)
/*PUB*/ uniquenessModuleIsEnabled: CircuitSignal;
/*PUB*/ requireUniqueContentIDs: CircuitSignal;

// Global module (1)
/*PUB*/ globalWatermark: CircuitSignal;
Expand Down Expand Up @@ -186,7 +186,7 @@ export const PROTO_POD_GPC_PUBLIC_INPUT_NAMES = [
"listComparisonValueIndex",
"listContainsComparisonValue",
"listValidValues",
"uniquenessModuleIsEnabled",
"requireUniqueContentIDs",
"globalWatermark"
];

Expand Down Expand Up @@ -455,7 +455,7 @@ export class ProtoPODGPC {
listComparisonValueIndex: allInputs.listComparisonValueIndex,
listContainsComparisonValue: allInputs.listContainsComparisonValue,
listValidValues: allInputs.listValidValues,
uniquenessModuleIsEnabled: allInputs.uniquenessModuleIsEnabled,
requireUniqueContentIDs: allInputs.requireUniqueContentIDs,
globalWatermark: allInputs.globalWatermark
};
}
Expand Down Expand Up @@ -529,7 +529,7 @@ export class ProtoPODGPC {
...inputs.listComparisonValueIndex,
inputs.listContainsComparisonValue,
...inputs.listValidValues.flat(),
inputs.uniquenessModuleIsEnabled,
inputs.requireUniqueContentIDs,
inputs.globalWatermark
].map(BigInt);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/gpcircuits/src/uniqueness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { CircuitSignal } from "./types";

export type UniquenessModuleInputs = {
values: CircuitSignal[];
values: CircuitSignal /*NUM_LIST_ELEMENTS*/[];
};

export type UniquenessModuleInputNamesType = ["values"];
Expand Down
5 changes: 5 additions & 0 deletions packages/lib/gpcircuits/src/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ export function zeroResidueMod(x: CircuitSignal, n: bigint): bigint {
* Creates dummy signals for unused object slots in ProtoPODGPC via dummy
ax0 marked this conversation as resolved.
Show resolved Hide resolved
* content IDs in the form of POD string hashes of the message `unused POD ${n}`
* as well as corresponding signatures.
*
* Note that these do not arise from actual PODs but they present valid content
* IDs since they will pass the necessary signature checks in the
* ProtoPODGPC. This is sufficient because they do not have any entries
* associated with them.
*/
export function dummyObjectSignals(
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we ever care enough about performance, we could pre-calculate several of these and store them in a static array.

numObjects: number
Expand Down
Loading
Loading