Skip to content

Commit 3c5bf50

Browse files
spalladinospalladino
authored
fix: Check mod in field construction and remove buffer representation (#19198)
Ensures that the field modulus is not exceeded during field construction, and removes the duplicate representation for fields. Supersedes #19090
2 parents 331b458 + 2f80d28 commit 3c5bf50

File tree

12 files changed

+143
-104
lines changed

12 files changed

+143
-104
lines changed

‎yarn-project/cli/src/cmds/validator_keys/valkeys.test.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ describe('validator keys utilities', () => {
383383
count: 2,
384384
publisherCount: 1,
385385
mnemonic: TEST_MNEMONIC,
386-
feeRecipient: ('0x' + '44'.repeat(32)) as unknown as AztecAddress,
386+
feeRecipient: ('0x' + '04'.repeat(32)) as unknown as AztecAddress,
387387
},
388388
log,
389389
);
@@ -409,7 +409,7 @@ describe('validator keys utilities', () => {
409409
publisherCount: 0,
410410
// no mnemonic on purpose
411411
remoteSigner: 'http://localhost:9000',
412-
feeRecipient: ('0x' + '55'.repeat(32)) as unknown as AztecAddress,
412+
feeRecipient: ('0x' + '05'.repeat(32)) as unknown as AztecAddress,
413413
},
414414
log,
415415
),
@@ -429,7 +429,7 @@ describe('validator keys utilities', () => {
429429
mnemonic: TEST_MNEMONIC,
430430
password: '',
431431
encryptedKeystoreDir: tmp,
432-
feeRecipient: ('0x' + '77'.repeat(32)) as unknown as AztecAddress,
432+
feeRecipient: ('0x' + '07'.repeat(32)) as unknown as AztecAddress,
433433
},
434434
log,
435435
);
@@ -459,7 +459,7 @@ describe('validator keys utilities', () => {
459459
mnemonic: TEST_MNEMONIC,
460460
password,
461461
encryptedKeystoreDir: tmp,
462-
feeRecipient: ('0x' + 'ee'.repeat(32)) as unknown as AztecAddress,
462+
feeRecipient: ('0x' + '0e'.repeat(32)) as unknown as AztecAddress,
463463
},
464464
log,
465465
);
@@ -507,7 +507,7 @@ describe('validator keys utilities', () => {
507507
count: 1,
508508
publishers: [publisherKey],
509509
mnemonic: TEST_MNEMONIC,
510-
feeRecipient: ('0x' + 'ff'.repeat(32)) as unknown as AztecAddress,
510+
feeRecipient: ('0x' + '0f'.repeat(32)) as unknown as AztecAddress,
511511
},
512512
log,
513513
);
@@ -533,7 +533,7 @@ describe('validator keys utilities', () => {
533533
count: 2,
534534
publishers: [publisherKeyNoPrefix],
535535
mnemonic: TEST_MNEMONIC,
536-
feeRecipient: ('0x' + 'dd'.repeat(32)) as unknown as AztecAddress,
536+
feeRecipient: ('0x' + '0d'.repeat(32)) as unknown as AztecAddress,
537537
},
538538
log,
539539
);
@@ -559,7 +559,7 @@ describe('validator keys utilities', () => {
559559
count: 1,
560560
publishers: [invalidPublisherKey],
561561
mnemonic: TEST_MNEMONIC,
562-
feeRecipient: ('0x' + 'cc'.repeat(32)) as unknown as AztecAddress,
562+
feeRecipient: ('0x' + '0c'.repeat(32)) as unknown as AztecAddress,
563563
},
564564
log,
565565
),
@@ -580,7 +580,7 @@ describe('validator keys utilities', () => {
580580
count: 1,
581581
publishers: [publisherKey1, publisherKey2],
582582
mnemonic: TEST_MNEMONIC,
583-
feeRecipient: ('0x' + 'ee'.repeat(32)) as unknown as AztecAddress,
583+
feeRecipient: ('0x' + '0e'.repeat(32)) as unknown as AztecAddress,
584584
},
585585
log,
586586
);
@@ -597,10 +597,10 @@ describe('validator keys utilities', () => {
597597
describe('materialization helpers (invoked directly)', () => {
598598
it('replaces plaintext keys with file references', async () => {
599599
const validators = [
600-
{ attester: '0x' + 'aa'.repeat(32), feeRecipient: ('0x' + '99'.repeat(32)) as unknown as AztecAddress },
600+
{ attester: '0x' + '0a'.repeat(32), feeRecipient: ('0x' + '09'.repeat(32)) as unknown as AztecAddress },
601601
{
602-
attester: { eth: '0x' + 'bb'.repeat(32), bls: '0x' + 'cc'.repeat(32) },
603-
feeRecipient: ('0x' + '88'.repeat(32)) as unknown as AztecAddress,
602+
attester: { eth: '0x' + '0b'.repeat(32), bls: '0x' + '0c'.repeat(32) },
603+
feeRecipient: ('0x' + '08'.repeat(32)) as unknown as AztecAddress,
604604
},
605605
] as any;
606606
const dirA = mkdtempSync(join(tmpdir(), 'aztec-mat-a-'));
@@ -622,8 +622,8 @@ describe('validator keys utilities', () => {
622622
schemaVersion: 1,
623623
validators: [
624624
{
625-
attester: '0x' + 'aa'.repeat(32),
626-
feeRecipient: ('0x' + '66'.repeat(32)) as unknown as AztecAddress,
625+
attester: '0x' + '0a'.repeat(32),
626+
feeRecipient: ('0x' + '06'.repeat(32)) as unknown as AztecAddress,
627627
},
628628
],
629629
} as any;
@@ -637,7 +637,7 @@ describe('validator keys utilities', () => {
637637
dataDir: tmp,
638638
count: 2,
639639
mnemonic: TEST_MNEMONIC,
640-
feeRecipient: ('0x' + '66'.repeat(32)) as unknown as AztecAddress,
640+
feeRecipient: ('0x' + '06'.repeat(32)) as unknown as AztecAddress,
641641
},
642642
log,
643643
);
@@ -703,7 +703,7 @@ describe('validator keys utilities', () => {
703703
file: 'staker-test.json',
704704
count: 1,
705705
mnemonic: TEST_MNEMONIC,
706-
feeRecipient: ('0x' + '44'.repeat(32)) as unknown as AztecAddress,
706+
feeRecipient: ('0x' + '04'.repeat(32)) as unknown as AztecAddress,
707707
stakerOutput: true,
708708
// Missing gseAddress
709709
l1RpcUrls: ['http://localhost:8545'],

‎yarn-project/foundation/src/curves/bn254/field.test.ts‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,72 @@ describe('Fr Serialization', () => {
77
const obtained = Fr.schema.parse(string);
88
expect(obtained).toEqual(original);
99
});
10+
});
11+
12+
describe('Fr Modulus Validation', () => {
13+
it('throws when constructing from bigint >= modulus', () => {
14+
expect(() => new Fr(Fr.MODULUS)).toThrow('greater or equal to field modulus');
15+
expect(() => new Fr(Fr.MODULUS + 1n)).toThrow('greater or equal to field modulus');
16+
});
17+
18+
it('throws when constructing from negative bigint', () => {
19+
expect(() => new Fr(-1n)).toThrow('is negative');
20+
});
21+
22+
it('throws when constructing from Buffer with value >= modulus', () => {
23+
const buf = Buffer.from(Fr.MODULUS.toString(16).padStart(64, '0'), 'hex');
24+
expect(() => new Fr(buf)).toThrow('greater or equal to field modulus');
25+
});
1026

11-
it('should validate hex strings', () => {
27+
it('throws when using fromBuffer with value >= modulus', () => {
28+
const buf = Buffer.from(Fr.MODULUS.toString(16).padStart(64, '0'), 'hex');
29+
expect(() => Fr.fromBuffer(buf)).toThrow('greater or equal to field modulus');
30+
});
31+
32+
it('throws when using fromString with numeric string >= modulus', () => {
33+
expect(() => Fr.fromString(Fr.MODULUS.toString())).toThrow('greater or equal to field modulus');
34+
});
35+
36+
it('throws when using fromHexString with value >= modulus', () => {
1237
expect(() => Fr.fromHexString(Fr.MODULUS.toString(16))).toThrow('greater or equal to field modulus');
1338
});
39+
40+
it('accepts MAX_FIELD_VALUE (modulus - 1)', () => {
41+
expect(() => new Fr(Fr.MODULUS - 1n)).not.toThrow();
42+
});
43+
});
44+
45+
describe('Fq Modulus Validation', () => {
46+
it('throws when constructing from bigint >= modulus', () => {
47+
expect(() => new Fq(Fq.MODULUS)).toThrow('greater or equal to field modulus');
48+
expect(() => new Fq(Fq.MODULUS + 1n)).toThrow('greater or equal to field modulus');
49+
});
50+
51+
it('throws when constructing from negative bigint', () => {
52+
expect(() => new Fq(-1n)).toThrow('is negative');
53+
});
54+
55+
it('throws when constructing from Buffer with value >= modulus', () => {
56+
const buf = Buffer.from(Fq.MODULUS.toString(16).padStart(64, '0'), 'hex');
57+
expect(() => new Fq(buf)).toThrow('greater or equal to field modulus');
58+
});
59+
60+
it('throws when using fromBuffer with value >= modulus', () => {
61+
const buf = Buffer.from(Fq.MODULUS.toString(16).padStart(64, '0'), 'hex');
62+
expect(() => Fq.fromBuffer(buf)).toThrow('greater or equal to field modulus');
63+
});
64+
65+
it('throws when using fromString with numeric string >= modulus', () => {
66+
expect(() => Fq.fromString(Fq.MODULUS.toString())).toThrow('greater or equal to field modulus');
67+
});
68+
69+
it('throws when using fromHexString with value >= modulus', () => {
70+
expect(() => Fq.fromHexString(Fq.MODULUS.toString(16))).toThrow('greater or equal to field modulus');
71+
});
72+
73+
it('accepts modulus - 1', () => {
74+
expect(() => new Fq(Fq.MODULUS - 1n)).not.toThrow();
75+
});
1476
});
1577

1678
describe('Bn254 arithmetic', () => {

‎yarn-project/foundation/src/curves/bn254/field.ts‎

Lines changed: 23 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import { hexSchemaFor } from '../../schemas/utils.js';
88
import { BufferReader } from '../../serialize/buffer_reader.js';
99
import { TypeRegistry } from '../../serialize/type_registry.js';
1010

11-
const ZERO_BUFFER = Buffer.alloc(32);
12-
1311
/* eslint-disable @typescript-eslint/no-unsafe-declaration-merging */
1412

1513
/**
@@ -25,14 +23,12 @@ type DerivedField<T extends BaseField> = {
2523

2624
/**
2725
* Base field class.
28-
* Conversions from Buffer to BigInt and vice-versa are not cheap.
29-
* We allow construction with either form and lazily convert to other as needed.
30-
* We only check we are within the field modulus when initializing with bigint.
26+
* Uses bigint as the internal representation.
27+
* Buffers are generated on demand from the bigint value.
3128
*/
3229
abstract class BaseField {
3330
static SIZE_IN_BYTES = 32;
34-
private asBuffer?: Buffer;
35-
private asBigInt?: bigint;
31+
private readonly asBigInt: bigint;
3632

3733
/**
3834
* Return bigint representation.
@@ -52,74 +48,60 @@ abstract class BaseField {
5248
if (value.length > BaseField.SIZE_IN_BYTES) {
5349
throw new Error(`Value length ${value.length} exceeds ${BaseField.SIZE_IN_BYTES}`);
5450
}
55-
this.asBuffer =
56-
value.length === BaseField.SIZE_IN_BYTES
57-
? value
58-
: Buffer.concat([Buffer.alloc(BaseField.SIZE_IN_BYTES - value.length), value]);
51+
this.asBigInt = toBigIntBE(value);
5952
} else if (typeof value === 'bigint' || typeof value === 'number' || typeof value === 'boolean') {
6053
this.asBigInt = BigInt(value);
61-
if (this.asBigInt >= this.modulus()) {
62-
throw new Error(`Value 0x${this.asBigInt.toString(16)} is greater or equal to field modulus.`);
63-
} else if (this.asBigInt < 0n) {
64-
throw new Error(`Value 0x${this.asBigInt.toString(16)} is negative.`);
65-
}
6654
} else if (value instanceof BaseField) {
67-
this.asBuffer = value.asBuffer;
6855
this.asBigInt = value.asBigInt;
6956
} else {
7057
throw new Error(`Type '${typeof value}' with value '${value}' passed to BaseField ctor.`);
7158
}
59+
60+
if (this.asBigInt < 0n) {
61+
throw new Error(`Value 0x${this.asBigInt.toString(16)} is negative.`);
62+
} else if (this.asBigInt >= this.modulus()) {
63+
throw new Error(`Value 0x${this.asBigInt.toString(16)} is greater or equal to field modulus.`);
64+
}
7265
}
7366

7467
protected abstract modulus(): bigint;
7568

7669
/**
77-
* We return a copy of the Buffer to ensure this remains immutable.
70+
* Converts the bigint to a Buffer.
7871
*/
7972
toBuffer(): Buffer {
80-
if (!this.asBuffer) {
81-
this.asBuffer = toBufferBE(this.asBigInt!, 32);
82-
}
83-
return Buffer.from(this.asBuffer);
73+
return toBufferBE(this.asBigInt, 32);
8474
}
8575

8676
toString(): `0x${string}` {
87-
return `0x${this.toBuffer().toString('hex')}`;
77+
return `0x${this.asBigInt.toString(16).padStart(64, '0')}`;
8878
}
8979

9080
toBigInt(): bigint {
91-
if (this.asBigInt === undefined) {
92-
this.asBigInt = toBigIntBE(this.asBuffer!);
93-
if (this.asBigInt >= this.modulus()) {
94-
throw new Error(`Value 0x${this.asBigInt.toString(16)} is greater or equal to field modulus.`);
95-
}
96-
}
9781
return this.asBigInt;
9882
}
9983

10084
toBool(): boolean {
101-
return Boolean(this.toBigInt());
85+
return this.asBigInt !== 0n;
10286
}
10387

10488
/**
10589
* Converts this field to a number.
10690
* Throws if the underlying value is greater than MAX_SAFE_INTEGER.
10791
*/
10892
toNumber(): number {
109-
const value = this.toBigInt();
110-
if (value > Number.MAX_SAFE_INTEGER) {
111-
throw new Error(`Value ${value.toString(16)} greater than than max safe integer`);
93+
if (this.asBigInt > Number.MAX_SAFE_INTEGER) {
94+
throw new Error(`Value ${this.asBigInt.toString(16)} greater than than max safe integer`);
11295
}
113-
return Number(value);
96+
return Number(this.asBigInt);
11497
}
11598

11699
/**
117100
* Converts this field to a number.
118101
* May cause loss of precision if the underlying value is greater than MAX_SAFE_INTEGER.
119102
*/
120103
toNumberUnsafe(): number {
121-
const value = this.toBigInt();
122-
return Number(value);
104+
return Number(this.asBigInt);
123105
}
124106

125107
toShortString(): string {
@@ -128,21 +110,20 @@ abstract class BaseField {
128110
}
129111

130112
equals(rhs: BaseField): boolean {
131-
return this.toBuffer().equals(rhs.toBuffer());
113+
return this.asBigInt === rhs.asBigInt;
132114
}
133115

134116
lt(rhs: BaseField): boolean {
135-
return this.toBigInt() < rhs.toBigInt();
117+
return this.asBigInt < rhs.asBigInt;
136118
}
137119

138120
cmp(rhs: BaseField): -1 | 0 | 1 {
139-
const lhsBigInt = this.toBigInt();
140-
const rhsBigInt = rhs.toBigInt();
141-
return lhsBigInt === rhsBigInt ? 0 : lhsBigInt < rhsBigInt ? -1 : 1;
121+
const rhsBigInt = rhs.asBigInt;
122+
return this.asBigInt === rhsBigInt ? 0 : this.asBigInt < rhsBigInt ? -1 : 1;
142123
}
143124

144125
isZero(): boolean {
145-
return this.toBuffer().equals(ZERO_BUFFER);
126+
return this.asBigInt === 0n;
146127
}
147128

148129
isEmpty(): boolean {

0 commit comments

Comments
 (0)