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

fix: field divison / journal comparisions #4489

Merged
merged 13 commits into from
Feb 7, 2024
121 changes: 120 additions & 1 deletion yarn-project/foundation/src/fields/fields.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { GrumpkinScalar } from './fields.js';
import { Fr, GrumpkinScalar } from './fields.js';

describe('GrumpkinScalar Serialization', () => {
// Test case for GrumpkinScalar.fromHighLow
Expand Down Expand Up @@ -53,3 +53,122 @@ describe('GrumpkinScalar Serialization', () => {
expect(deserialized).toEqual(original);
});
});

describe('Bn254 arithmetic', () => {
describe('Addition', () => {
it('Low Boundary', () => {
// 0 + -1 = -1
const a = Fr.ZERO;
const b = new Fr(Fr.MODULUS - 1n);
const expected = new Fr(Fr.MODULUS - 1n);

const actual = a.add(b);
expect(actual).toEqual(expected);
});

it('High Boundary', () => {
// -1 + 1 = 0
const a = new Fr(Fr.MODULUS - 1n);
const b = new Fr(1);
const expected = Fr.ZERO;

const actual = a.add(b);
expect(actual).toEqual(expected);
});

it('Performs addition correctly', () => {
const a = new Fr(2);
const b = new Fr(3);
const expected = new Fr(5);

const actual = a.add(b);
expect(actual).toEqual(expected);
});
});

describe('Subtraction', () => {
it('Low Boundary', () => {
// 0 - 1 = -1
const a = new Fr(0);
const b = new Fr(1);
const expected = new Fr(Fr.MODULUS - 1n);

const actual = a.sub(b);
expect(actual).toEqual(expected);
});

it('High Bonudary', () => {
// -1 - (-1) = 0
const a = new Fr(Fr.MODULUS - 1n);
const b = new Fr(Fr.MODULUS - 1n);

const actual = a.sub(b);
expect(actual).toEqual(Fr.ZERO);
});

it('Performs subtraction correctly', () => {
const a = new Fr(10);
const b = new Fr(5);
const expected = new Fr(5);

const actual = a.sub(b);
expect(actual).toEqual(expected);
});
});

describe('Multiplication', () => {
it('Identity', () => {
const a = new Fr(Fr.MODULUS - 1n);
const b = new Fr(1);
const expected = new Fr(Fr.MODULUS - 1n);

const actual = a.mul(b);
expect(actual).toEqual(expected);
});

it('Performs multiplication correctly', () => {
const a = new Fr(2);
const b = new Fr(3);
const expected = new Fr(6);

const actual = a.mul(b);
expect(actual).toEqual(expected);
});

it('High Boundary', () => {
const a = new Fr(Fr.MODULUS - 1n);
const b = new Fr(Fr.MODULUS / 2n);
const expected = new Fr(10944121435919637611123202872628637544274182200208017171849102093287904247809n);

const actual = a.mul(b);
expect(actual).toEqual(expected);
});
});

describe('Division', () => {
it('Should succeed when mod inverse is -ve', () => {
const a = new Fr(2);
const b = new Fr(3);

const actual = a.div(b);
expect(actual.mul(b)).toEqual(a);
});

it('Should succeed when mod inverse is +ve', () => {
const a = new Fr(10);
const b = new Fr(5);
const expected = new Fr(2);

const actual = a.div(b);
expect(actual.mul(b)).toEqual(a);
expect(actual).toEqual(expected);
});

it('Should not allow a division by 0', () => {
const a = new Fr(10);
const b = Fr.ZERO;

expect(() => a.div(b)).toThrowError();
});
});
});
4 changes: 2 additions & 2 deletions yarn-project/foundation/src/fields/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ function modInverse(b: bigint) {
if (gcd != 1n) {
throw Error('Inverse does not exist');
}
// Add modulus to ensure positive
return new Fr(x + Fr.MODULUS);
// Add modulus if -ve to ensure positive
return new Fr(x > 0 ? x : x + Fr.MODULUS);
}

/**
Expand Down
10 changes: 6 additions & 4 deletions yarn-project/simulator/src/avm/avm_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ describe('Avm Context', () => {
pc: 0,
}),
);
// FIXME: I can't get this to work.
// expect(newContext.worldState).toEqual(context.worldState.fork());

// We stringify to remove circular references (parentJournal)
expect(JSON.stringify(newContext.worldState)).toEqual(JSON.stringify(context.worldState.fork()));
});

it('New static call should fork context correctly', () => {
Expand All @@ -49,7 +50,8 @@ describe('Avm Context', () => {
pc: 0,
}),
);
// FIXME: I can't get this to work.
// expect(newContext.worldState).toEqual(context.worldState.fork());

// We stringify to remove circular references (parentJournal)
expect(JSON.stringify(newContext.worldState)).toEqual(JSON.stringify(context.worldState.fork()));
});
});
3 changes: 1 addition & 2 deletions yarn-project/simulator/src/avm/avm_memory_types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ describe('Field', () => {
expect(result).toStrictEqual(new Field(50n));
});

// FIXME: field division is wrong
it.skip(`Should divide two Fields correctly`, () => {
it(`Should divide two Fields correctly`, () => {
const field1 = new Field(10);
const field2 = new Field(5);
const result = field1.div(field2);
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,7 @@ describe('Arithmetic Instructions', () => {
expect(inst.serialize()).toEqual(buf);
});

// FIXME: field division is wrong
it.skip('Should perform field division', async () => {
it('Should perform field division', async () => {
const a = new Field(10n);
const b = new Field(5n);

Expand Down
Loading