Skip to content

Commit b280a79

Browse files
authored
fix: ensure to_bytes returns the canonical decomposition (#6084)
# Description ## Problem\* Resolves #5846 ## Summary\* Enforce canonical decomposition for to_bytes ## Additional Context I also added from_bytes methods since they are often requested ## Documentation\* I updated the comments, I assume the doc is auto generated but I am not sure. Let me know if I need to modify it manually. Check one: - [ ] No documentation needed. - [X] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
1 parent 79f8954 commit b280a79

File tree

3 files changed

+84
-25
lines changed
  • noir_stdlib/src/field
  • test_programs/execution_success

3 files changed

+84
-25
lines changed

noir_stdlib/src/field/mod.nr

Lines changed: 81 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub mod bn254;
22
use bn254::lt as bn254_lt;
3+
use crate::runtime::is_unconstrained;
34

45
impl Field {
56
/// Asserts that `self` can be represented in `bit_size` bits.
@@ -49,39 +50,71 @@ impl Field {
4950
pub fn to_be_bits<let N: u32>(self: Self) -> [u1; N] {}
5051
// docs:end:to_be_bits
5152

52-
/// Decomposes `self` into its little endian byte decomposition as a `[u8]` slice of length `byte_size`.
53-
/// This slice will be zero padded should not all bytes be necessary to represent `self`.
53+
/// Decomposes `self` into its little endian byte decomposition as a `[u8;N]` array
54+
/// This array will be zero padded should not all bytes be necessary to represent `self`.
5455
///
5556
/// # Failures
56-
/// Causes a constraint failure for `Field` values exceeding `2^{8*byte_size}` as the resulting slice will not
57-
/// be able to represent the original `Field`.
57+
/// The length N of the array must be big enough to contain all the bytes of the 'self',
58+
/// and no more than the number of bytes required to represent the field modulus
5859
///
5960
/// # Safety
60-
/// Values of `byte_size` equal to or greater than the number of bytes necessary to represent the `Field` modulus
61-
/// (e.g. 32 for the BN254 field) allow for multiple byte decompositions. This is due to how the `Field` will
62-
/// wrap around due to overflow when verifying the decomposition.
61+
/// The result is ensured to be the canonical decomposition of the field element
6362
// docs:start:to_le_bytes
6463
pub fn to_le_bytes<let N: u32>(self: Self) -> [u8; N] {
65-
self.to_le_radix(256)
64+
// docs:end:to_le_bytes
65+
// Compute the byte decomposition
66+
let bytes = self.to_le_radix(256);
67+
68+
if !is_unconstrained() {
69+
// Ensure that the byte decomposition does not overflow the modulus
70+
let p = modulus_le_bytes();
71+
assert(bytes.len() <= p.len());
72+
let mut ok = bytes.len() != p.len();
73+
for i in 0..N {
74+
if !ok {
75+
if (bytes[N - 1 - i] != p[N - 1 - i]) {
76+
assert(bytes[N - 1 - i] < p[N - 1 - i]);
77+
ok = true;
78+
}
79+
}
80+
}
81+
assert(ok);
82+
}
83+
bytes
6684
}
67-
// docs:end:to_le_bytes
6885

69-
/// Decomposes `self` into its big endian byte decomposition as a `[u8]` slice of length `byte_size`.
70-
/// This slice will be zero padded should not all bytes be necessary to represent `self`.
86+
/// Decomposes `self` into its big endian byte decomposition as a `[u8;N]` array of length required to represent the field modulus
87+
/// This array will be zero padded should not all bytes be necessary to represent `self`.
7188
///
7289
/// # Failures
73-
/// Causes a constraint failure for `Field` values exceeding `2^{8*byte_size}` as the resulting slice will not
74-
/// be able to represent the original `Field`.
90+
/// The length N of the array must be big enough to contain all the bytes of the 'self',
91+
/// and no more than the number of bytes required to represent the field modulus
7592
///
7693
/// # Safety
77-
/// Values of `byte_size` equal to or greater than the number of bytes necessary to represent the `Field` modulus
78-
/// (e.g. 32 for the BN254 field) allow for multiple byte decompositions. This is due to how the `Field` will
79-
/// wrap around due to overflow when verifying the decomposition.
94+
/// The result is ensured to be the canonical decomposition of the field element
8095
// docs:start:to_be_bytes
8196
pub fn to_be_bytes<let N: u32>(self: Self) -> [u8; N] {
82-
self.to_be_radix(256)
97+
// docs:end:to_be_bytes
98+
// Compute the byte decomposition
99+
let bytes = self.to_be_radix(256);
100+
101+
if !is_unconstrained() {
102+
// Ensure that the byte decomposition does not overflow the modulus
103+
let p = modulus_be_bytes();
104+
assert(bytes.len() <= p.len());
105+
let mut ok = bytes.len() != p.len();
106+
for i in 0..N {
107+
if !ok {
108+
if (bytes[i] != p[i]) {
109+
assert(bytes[i] < p[i]);
110+
ok = true;
111+
}
112+
}
113+
}
114+
assert(ok);
115+
}
116+
bytes
83117
}
84-
// docs:end:to_be_bytes
85118

86119
// docs:start:to_le_radix
87120
pub fn to_le_radix<let N: u32>(self: Self, radix: u32) -> [u8; N] {
@@ -130,6 +163,32 @@ impl Field {
130163
lt_fallback(self, another)
131164
}
132165
}
166+
167+
/// Convert a little endian byte array to a field element.
168+
/// If the provided byte array overflows the field modulus then the Field will silently wrap around.
169+
pub fn from_le_bytes<let N: u32>(bytes: [u8; N]) -> Field {
170+
let mut v = 1;
171+
let mut result = 0;
172+
173+
for i in 0..N {
174+
result += (bytes[i] as Field) * v;
175+
v = v * 256;
176+
}
177+
result
178+
}
179+
180+
/// Convert a big endian byte array to a field element.
181+
/// If the provided byte array overflows the field modulus then the Field will silently wrap around.
182+
pub fn from_be_bytes<let N: u32>(bytes: [u8; N]) -> Field {
183+
let mut v = 1;
184+
let mut result = 0;
185+
186+
for i in 0..N {
187+
result += (bytes[N-1-i] as Field) * v;
188+
v = v * 256;
189+
}
190+
result
191+
}
133192
}
134193

135194
#[builtin(modulus_num_bits)]
@@ -207,6 +266,7 @@ mod tests {
207266
let field = 2;
208267
let bits: [u8; 8] = field.to_be_bytes();
209268
assert_eq(bits, [0, 0, 0, 0, 0, 0, 0, 2]);
269+
assert_eq(Field::from_be_bytes::<8>(bits), field);
210270
}
211271
// docs:end:to_be_bytes_example
212272

@@ -216,6 +276,7 @@ mod tests {
216276
let field = 2;
217277
let bits: [u8; 8] = field.to_le_bytes();
218278
assert_eq(bits, [2, 0, 0, 0, 0, 0, 0, 0]);
279+
assert_eq(Field::from_le_bytes::<8>(bits), field);
219280
}
220281
// docs:end:to_le_bytes_example
221282

@@ -225,6 +286,7 @@ mod tests {
225286
let field = 2;
226287
let bits: [u8; 8] = field.to_be_radix(256);
227288
assert_eq(bits, [0, 0, 0, 0, 0, 0, 0, 2]);
289+
assert_eq(Field::from_be_bytes::<8>(bits), field);
228290
}
229291
// docs:end:to_be_radix_example
230292

@@ -234,6 +296,7 @@ mod tests {
234296
let field = 2;
235297
let bits: [u8; 8] = field.to_le_radix(256);
236298
assert_eq(bits, [2, 0, 0, 0, 0, 0, 0, 0]);
299+
assert_eq(Field::from_le_bytes::<8>(bits), field);
237300
}
238301
// docs:end:to_le_radix_example
239302
}

test_programs/execution_success/to_be_bytes/src/main.nr

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ fn main(x: Field) -> pub [u8; 31] {
88
if (bytes[30] != 60) | (bytes[29] != 33) | (bytes[28] != 31) {
99
assert(false);
1010
}
11+
assert(Field::from_be_bytes::<31>(bytes) == x);
1112
bytes
1213
}

test_programs/execution_success/to_le_bytes/src/main.nr

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,12 @@ fn main(x: Field, cond: bool) -> pub [u8; 31] {
22
// The result of this byte array will be little-endian
33
let byte_array: [u8; 31] = x.to_le_bytes();
44
assert(byte_array.len() == 31);
5-
6-
let mut bytes = [0; 31];
7-
for i in 0..31 {
8-
bytes[i] = byte_array[i];
9-
}
10-
5+
assert(Field::from_le_bytes::<31>(byte_array) == x);
116
if cond {
127
// We've set x = "2040124" so we shouldn't be able to represent this as a single byte.
138
let bad_byte_array: [u8; 1] = x.to_le_bytes();
149
assert_eq(bad_byte_array.len(), 1);
1510
}
1611

17-
bytes
12+
byte_array
1813
}

0 commit comments

Comments
 (0)