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.

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

Improve CircuitValue #269

Merged
merged 10 commits into from
Jul 7, 2022
7 changes: 0 additions & 7 deletions src/examples/ex04_signed_message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ class Transaction extends CircuitValue {
@prop sender: PublicKey;
@prop receiver: PublicKey;
@prop amount: Field;

constructor(sender: PublicKey, receiver: PublicKey, amount: Field) {
super();
this.sender = sender;
this.receiver = receiver;
this.amount = amount;
}
}

/* Exercise 4:
Expand Down
5 changes: 0 additions & 5 deletions src/examples/rollup/merkle_proof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,6 @@ export function MerkleAccumulatorFactory<A extends CircuitValue>(
export abstract class IndexedAccumulator<A> extends CircuitValue {
@prop commitment: Field;

constructor(commitment: Field) {
super();
this.commitment = commitment;
}

abstract set(index: Index, a: A): void;

abstract get(index: Index): A;
Expand Down
35 changes: 0 additions & 35 deletions src/examples/rollup/wip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,63 +44,28 @@ class RollupAccount extends CircuitValue {
@prop balance: UInt64;
@prop nonce: UInt32;
@prop publicKey: PublicKey;

constructor(balance: UInt64, nonce: UInt32, publicKey: PublicKey) {
super();
this.balance = balance;
this.nonce = nonce;
this.publicKey = publicKey;
}
}

class RollupTransaction extends CircuitValue {
@prop amount: UInt64;
@prop nonce: UInt32;
@prop sender: PublicKey;
@prop receiver: PublicKey;

constructor(
amount: UInt64,
nonce: UInt32,
sender: PublicKey,
receiver: PublicKey
) {
super();
this.amount = amount;
this.nonce = nonce;
this.sender = sender;
this.receiver = receiver;
}
}

class RollupDeposit extends CircuitValue {
@prop publicKey: PublicKey;
@prop amount: UInt64;
constructor(publicKey: PublicKey, amount: UInt64) {
super();
this.publicKey = publicKey;
this.amount = amount;
}
}

class RollupState extends CircuitValue {
@prop pendingDepositsCommitment: Field;
@prop accountDbCommitment: Field;
constructor(p: Field, c: Field) {
super();
this.pendingDepositsCommitment = p;
this.accountDbCommitment = c;
}
}

class RollupStateTransition extends CircuitValue {
@prop source: RollupState;
@prop target: RollupState;
constructor(source: RollupState, target: RollupState) {
super();
this.source = source;
this.target = target;
}
}

// a recursive proof system is kind of like an "enum"
Expand Down
7 changes: 0 additions & 7 deletions src/examples/schnorr_sign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ class Witness extends CircuitValue {
@prop signature: Signature;
@prop acc: Group;
@prop r: Scalar;

constructor(sig: Signature, acc: Group, r: Scalar) {
super();
this.signature = sig;
this.acc = acc;
this.r = r;
}
}

// Public input:
Expand Down
10 changes: 5 additions & 5 deletions src/examples/sudoku/sudoku-zkapp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ export { deploy, submitSolution, getZkappState };
class Sudoku extends CircuitValue {
@matrixProp(Field, 9, 9) value: Field[][];

constructor(value: number[][]) {
super();
this.value = value.map((row) => row.map(Field));
static from(value: number[][]) {
return new Sudoku(value.map((row) => row.map(Field)));
}

hash() {
Expand Down Expand Up @@ -84,6 +83,7 @@ class SudokuZkapp extends SmartContract {

// finally, we check that the sudoku is the one that was originally deployed
let sudokuHash = this.sudokuHash.get(); // get the hash from the blockchain
this.sudokuHash.assertEquals(sudokuHash);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a fix that's unrelated to the rest of the PR

sudokuInstance.hash().assertEquals(sudokuHash);

// all checks passed => the sudoku is solved!
Expand All @@ -105,7 +105,7 @@ async function deploy(sudoku: number[][]) {
let tx = await Mina.transaction(account1, () => {
Party.fundNewAccount(account1);
let zkapp = new SudokuZkapp(zkappAddress);
let sudokuInstance = new Sudoku(sudoku);
let sudokuInstance = Sudoku.from(sudoku);
zkapp.deploy({ zkappKey });
zkapp.setPermissions({
...Permissions.default(),
Expand All @@ -120,7 +120,7 @@ async function deploy(sudoku: number[][]) {
async function submitSolution(sudoku: number[][], solution: number[][]) {
let tx = await Mina.transaction(account1, () => {
let zkapp = new SudokuZkapp(zkappAddress);
zkapp.submitSolution(new Sudoku(sudoku), new Sudoku(solution));
zkapp.submitSolution(Sudoku.from(sudoku), Sudoku.from(solution));
zkapp.sign(zkappKey);
});
await tx.send().wait();
Expand Down
81 changes: 55 additions & 26 deletions src/lib/circuit_value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,36 @@ export {
circuitValue,
};

type Constructor<T> = { new (...args: any[]): T };
type AnyConstructor = new (...args: any) => any;

// TODO: Synthesize the constructor if possible (bkase)
//
abstract class CircuitValue {
constructor(...props: any[]) {
Copy link
Contributor Author

@mitschabaude mitschabaude Jun 30, 2022

Choose a reason for hiding this comment

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

fun fact: this function was written verbatim, completely correctly, by github copilot on the first try!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

That's pretty amazing!

Copy link
Contributor

Choose a reason for hiding this comment

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

That is really cool! I'm going to try out copilot.

const fields = (this.constructor as any).prototype._fields;
if (fields === undefined || fields === null) {
return;
}

if (props.length !== fields.length) {
throw Error(
`${this.constructor.name} constructor called with ${props.length} arguments, but expected ${fields.length}`
);
}

for (let i = 0; i < fields.length; ++i) {
const [key, propType] = fields[i];
(this as any)[key] = props[i];
}
}

static sizeInFields(): number {
const fields: [string, any][] = (this as any).prototype._fields;
return fields.reduce((acc, [_, typ]) => acc + typ.sizeInFields(), 0);
}

static toFields<T>(this: Constructor<T>, v: T): Field[] {
static toFields<T extends AnyConstructor>(
this: T,
v: InstanceType<T>
): Field[] {
const res: Field[] = [];
const fields = (this as any).prototype._fields;
if (fields === undefined || fields === null) {
Expand All @@ -48,15 +67,18 @@ abstract class CircuitValue {
return (this.constructor as any).toJSON(this);
}

equals(this: this, x: this): Bool {
equals(x: this): Bool {
return Circuit.equal(this, x);
}

assertEquals(this: this, x: typeof this): void {
assertEquals(x: this): void {
Circuit.assertEqual(this, x);
}

static ofFields<T>(this: Constructor<T>, xs: Field[]): T {
static ofFields<T extends AnyConstructor>(
this: T,
xs: Field[]
): InstanceType<T> {
const fields = (this as any).prototype._fields;
let offset = 0;
const props: any[] = [];
Expand All @@ -70,14 +92,33 @@ abstract class CircuitValue {
return new this(...props);
}

static check: (x: any) => void;
static check<T extends AnyConstructor>(this: T, v: InstanceType<T>) {
const fields = (this as any).prototype._fields;
if (fields === undefined || fields === null) {
return;
}

for (let i = 0; i < fields.length; ++i) {
const [key, propType] = fields[i];
const value = (v as any)[key];
if (propType.check === undefined)
throw Error('bug: circuit value without .check()');
propType.check(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this error get thrown if a CircuitValue class does not define a check function for itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no! it would get thrown if one of our primitives was missing check -- which isn't the case --, or maybe if a developer uses something which isn't a CircuitValue,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but probably won't get thrown at all :D

}
}

static toConstant<T>(this: Constructor<T>, t: T): T {
static toConstant<T extends AnyConstructor>(
this: T,
t: InstanceType<T>
): InstanceType<T> {
const xs: Field[] = (this as any).toFields(t);
return (this as any).ofFields(xs.map((x) => x.toConstant()));
}

static toJSON<T>(this: Constructor<T>, v: T): JSONValue {
static toJSON<T extends AnyConstructor>(
this: T,
v: InstanceType<T>
): JSONValue {
const res: { [key: string]: JSONValue } = {};
if ((this as any).prototype._fields !== undefined) {
const fields: [string, any][] = (this as any).prototype._fields;
Expand All @@ -88,7 +129,10 @@ abstract class CircuitValue {
return res;
}

static fromJSON<T>(this: Constructor<T>, value: JSONValue): T | null {
static fromJSON<T extends AnyConstructor>(
this: T,
value: JSONValue
): InstanceType<T> | null {
const props: any[] = [];
const fields: [string, any][] = (this as any).prototype._fields;

Expand Down Expand Up @@ -117,21 +161,6 @@ abstract class CircuitValue {
}
}

(CircuitValue as any).check = function (v: any) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was able to remove this hack because I got the types working on CircuitValue

const fields = (this as any).prototype._fields;
if (fields === undefined || fields === null) {
return;
}

for (let i = 0; i < fields.length; ++i) {
const [key, propType] = fields[i];
const value = (v as any)[key];
if (propType.check === undefined)
throw Error('bug: circuit value without .check()');
propType.check(value);
}
};

function prop(this: any, target: any, key: string) {
const fieldType = Reflect.getMetadata('design:type', target, key);
if (!target.hasOwnProperty('_fields')) {
Expand Down
14 changes: 1 addition & 13 deletions src/lib/int.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ class UInt64 extends CircuitValue implements Types.UInt64 {
static NUM_BITS = 64;
_type?: 'UInt64';

constructor(value: Field) {
super();
this.value = value;
}

static get zero() {
return new UInt64(Field.zero);
}
Expand Down Expand Up @@ -182,11 +177,6 @@ class UInt32 extends CircuitValue implements Types.UInt32 {
static NUM_BITS = 32;
_type?: 'UInt32';

constructor(value: Field) {
super();
this.value = value;
}

static get zero(): UInt32 {
return new UInt32(Field.zero);
}
Expand Down Expand Up @@ -364,9 +354,7 @@ class Int64 extends CircuitValue implements BalanceChange {
// Overall, I think the existing implementation is the optimal one.

constructor(magnitude: UInt64, sgn = Field.one) {
super();
this.magnitude = magnitude;
this.sgn = sgn;
super(magnitude, sgn);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this constructor because it adds something nice -- a default value for one of the arguments

}

private static fromFieldUnchecked(x: Field) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/party.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ class Party {
? Circuit.witness(UInt32, () => account.nonce)
: account.nonce;
} else {
nonce = Circuit.witness(UInt32, () => {
nonce = Circuit.witness(UInt32, (): UInt32 => {
throw Error('this should never happen');
});
}
Expand Down
16 changes: 0 additions & 16 deletions src/lib/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ import { Poseidon } from './hash';
export class PrivateKey extends CircuitValue {
@prop s: Scalar;

constructor(s: Scalar) {
super();
this.s = s;
}

/**
* You can use this method to generate a private key. You can then obtain
* the associated public key via [[toPublicKey]]. And generate signatures
Expand Down Expand Up @@ -59,11 +54,6 @@ export class PrivateKey extends CircuitValue {
export class PublicKey extends CircuitValue {
@prop g: Group;

constructor(g: Group) {
super();
this.g = g;
}

static fromPrivateKey(p: PrivateKey): PublicKey {
return p.toPublicKey();
}
Expand Down Expand Up @@ -94,12 +84,6 @@ export class Signature extends CircuitValue {
@prop r: Field;
@prop s: Scalar;

constructor(r: Field, s: Scalar) {
super();
this.r = r;
this.s = s;
}

static create(privKey: PrivateKey, msg: Field[]): Signature {
const { g: publicKey } = PublicKey.fromPrivateKey(privKey);
const d = privKey.s;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe('Circuit String', () => {
expect(() => {
Circuit.runAndCheck(() => {
const str = Circuit.witness(CircuitString, () => {
return new CircuitString([
return CircuitString.fromCharacters([
new Character(Field(100)),
new Character(Field(10000)),
new Character(Field(100)),
Expand Down
Loading