From 15c06c5281ef1905690d20d9b2759059d1a41961 Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 7 Feb 2024 19:29:31 +0000 Subject: [PATCH] fix: field divison / journal comparisions (#4489) Stacked on `fc/02-07-chore-avm_add/improve_tests_for_AvmContext_tagged_memory_etc` --- .../foundation/src/fields/fields.test.ts | 121 +++++++++++++++++- yarn-project/foundation/src/fields/fields.ts | 4 +- .../simulator/src/avm/avm_context.test.ts | 10 +- .../src/avm/avm_memory_types.test.ts | 3 +- .../src/avm/opcodes/arithmetic.test.ts | 3 +- 5 files changed, 130 insertions(+), 11 deletions(-) diff --git a/yarn-project/foundation/src/fields/fields.test.ts b/yarn-project/foundation/src/fields/fields.test.ts index 02f08f0e319..c2673ff86e9 100644 --- a/yarn-project/foundation/src/fields/fields.test.ts +++ b/yarn-project/foundation/src/fields/fields.test.ts @@ -1,4 +1,4 @@ -import { GrumpkinScalar } from './fields.js'; +import { Fr, GrumpkinScalar } from './fields.js'; describe('GrumpkinScalar Serialization', () => { // Test case for GrumpkinScalar.fromHighLow @@ -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(); + }); + }); +}); diff --git a/yarn-project/foundation/src/fields/fields.ts b/yarn-project/foundation/src/fields/fields.ts index 28e0a64dad8..1f23f5473f5 100644 --- a/yarn-project/foundation/src/fields/fields.ts +++ b/yarn-project/foundation/src/fields/fields.ts @@ -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); } /** diff --git a/yarn-project/simulator/src/avm/avm_context.test.ts b/yarn-project/simulator/src/avm/avm_context.test.ts index f54fe99a358..240de5eb14f 100644 --- a/yarn-project/simulator/src/avm/avm_context.test.ts +++ b/yarn-project/simulator/src/avm/avm_context.test.ts @@ -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', () => { @@ -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())); }); }); diff --git a/yarn-project/simulator/src/avm/avm_memory_types.test.ts b/yarn-project/simulator/src/avm/avm_memory_types.test.ts index c5701c54d93..9aeccaa73b9 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.test.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.test.ts @@ -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); diff --git a/yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts b/yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts index 1475ef1a7c4..e08fe4bcb8e 100644 --- a/yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts @@ -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);