Skip to content

Commit

Permalink
fix: field divison / journal comparisions (AztecProtocol#4489)
Browse files Browse the repository at this point in the history
Stacked on
`fc/02-07-chore-avm_add/improve_tests_for_AvmContext_tagged_memory_etc`
  • Loading branch information
Maddiaa0 authored Feb 7, 2024
1 parent 99c3fba commit 15c06c5
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 11 deletions.
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

0 comments on commit 15c06c5

Please sign in to comment.