From d56bf49434e5cc5bbaff9e7d86b8161640bf55c8 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Sep 2024 09:10:09 +0000 Subject: [PATCH 01/36] feat: bytes to fields and back --- .../aztec-nr/aztec/src/utils/byte_array.nr | 21 +++++++++++++++++++ noir-projects/aztec-nr/aztec/src/utils/mod.nr | 1 + 2 files changed, 22 insertions(+) create mode 100644 noir-projects/aztec-nr/aztec/src/utils/byte_array.nr diff --git a/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr b/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr new file mode 100644 index 00000000000..7f32314124b --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr @@ -0,0 +1,21 @@ +// Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. +// This implies that M = ceil(N / 31). +pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { + let mut output = [0; M]; + + for field_index in 0..M { + let mut field_value = 0; + + for i in 0..31 { + let byte_index = field_index * 31 + i; + if byte_index < N { + // Shift the existing value left by 8 bits and add the new byte + field_value = field_value * 256 + input[byte_index] as Field; + } + } + + output[field_index] = field_value; + } + + output +} diff --git a/noir-projects/aztec-nr/aztec/src/utils/mod.nr b/noir-projects/aztec-nr/aztec/src/utils/mod.nr index 2da0e5b639e..dba99592fe5 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/mod.nr @@ -1,3 +1,4 @@ +mod byte_array; mod collapse_array; mod comparison; mod point; From b26fce3007b8f34e86fd1db120b719d7acc8c5a7 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Sep 2024 09:43:45 +0000 Subject: [PATCH 02/36] WIP --- .../aztec-nr/aztec/src/utils/byte_array.nr | 21 ---- .../aztec-nr/aztec/src/utils/bytes.nr | 108 ++++++++++++++++++ noir-projects/aztec-nr/aztec/src/utils/mod.nr | 3 +- 3 files changed, 110 insertions(+), 22 deletions(-) delete mode 100644 noir-projects/aztec-nr/aztec/src/utils/byte_array.nr create mode 100644 noir-projects/aztec-nr/aztec/src/utils/bytes.nr diff --git a/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr b/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr deleted file mode 100644 index 7f32314124b..00000000000 --- a/noir-projects/aztec-nr/aztec/src/utils/byte_array.nr +++ /dev/null @@ -1,21 +0,0 @@ -// Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. -// This implies that M = ceil(N / 31). -pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { - let mut output = [0; M]; - - for field_index in 0..M { - let mut field_value = 0; - - for i in 0..31 { - let byte_index = field_index * 31 + i; - if byte_index < N { - // Shift the existing value left by 8 bits and add the new byte - field_value = field_value * 256 + input[byte_index] as Field; - } - } - - output[field_index] = field_value; - } - - output -} diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr new file mode 100644 index 00000000000..7ac0570c2c1 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -0,0 +1,108 @@ +// Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. +// This implies that M = ceil(N / 31). +pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { + let mut output = [0; M]; + + for field_index in 0..M { + let mut field_value = 0; + + for i in 0..31 { + let byte_index = field_index * 31 + i; + if byte_index < N { + // Shift the existing value left by 8 bits and add the new byte + field_value = field_value * 256 + input[byte_index] as Field; + } + } + + output[field_index] = field_value; + } + + output +} + +// Converts the input array of fields into bytes. Each field of input has to contain only 31 bytes. +pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { + let mut output = [0; N]; + + for field_index in 0..M { + // We convert the field to 31 bytes. Normally a field is 254 bits, but we only used 248 bits + // in `bytes_to_fields` so we can safely convert it to 31 bytes. + let field_value_bytes: [u8; 31] = input[field_index].to_be_bytes(); + + for i in 0..31 { + let byte_index = field_index * 31 + i; + output[byte_index] = field_value_bytes[i]; + } + } + + output +} + +mod test { + use crate::utils::bytes::{bytes_to_fields, fields_to_bytes}; + + #[test] + fn test_bytes_to_1_field() { + let input = [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 + ]; + let output = bytes_to_fields::<31, 1>(input); + + assert_eq(output[0], 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f); + } + + #[test] + fn test_1_field_to_bytes() { + let input = [0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f]; + let output = fields_to_bytes::<31, 1>(input); + + assert_eq( + output, [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 + ] + ); + } + + #[test] + fn test_bytes_to_2_fields() { + let input = [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59 + ]; + let output = bytes_to_fields::<59, 2>(input); + + assert_eq(output[0], 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f); + assert_eq(output[1], 0x202122232425262728292a2b2c2d2e2f303132333435363738393a3b); + } + + #[test] + fn test_2_fields_to_bytes() { + let input = [ + 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f, 0x202122232425262728292a2b2c2d2e2f303132333435363738393a3b + ]; + let output = fields_to_bytes::<62, 2>(input); + + assert_eq( + output, [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 0, 0, 0, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59 + ] + ); + } + + #[test] + fn test_large_random_input_to_fields_and_back() { + let input = [ + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, + 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, + 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, 0x40, + 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50, + 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x60, + 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, 0x70, + 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80 + ]; + let output = bytes_to_fields::<128, 5>(input); + let input_back = fields_to_bytes::<155, 5>(output); + + std::println(input_back); + } +} diff --git a/noir-projects/aztec-nr/aztec/src/utils/mod.nr b/noir-projects/aztec-nr/aztec/src/utils/mod.nr index dba99592fe5..ae469c45465 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/mod.nr @@ -1,4 +1,4 @@ -mod byte_array; +mod bytes; mod collapse_array; mod comparison; mod point; @@ -6,3 +6,4 @@ mod test; mod to_bytes; pub use crate::utils::collapse_array::collapse_array; +pub use crate::utils::bytes::{bytes_to_fields, fields_to_bytes}; From 916c0696c0b012b09daa11d1120793bc68a82c91 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Sep 2024 09:57:03 +0000 Subject: [PATCH 03/36] WIP --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 7ac0570c2c1..818ab664a7b 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -101,8 +101,8 @@ mod test { 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80 ]; let output = bytes_to_fields::<128, 5>(input); - let input_back = fields_to_bytes::<155, 5>(output); + let input_back = fields_to_bytes::<128, 5>(output); - std::println(input_back); + assert_eq(input, input_back); } } From cb6bf5ec29b9aa029fdcbc76068b70194abf22e1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Sep 2024 10:32:09 +0000 Subject: [PATCH 04/36] WIP --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 818ab664a7b..7e5f0d8257b 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -29,9 +29,18 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { // in `bytes_to_fields` so we can safely convert it to 31 bytes. let field_value_bytes: [u8; 31] = input[field_index].to_be_bytes(); + let remaining_bytes = N - field_index * 31; + let mut field_value_bytes_start_index = 0; + if remaining_bytes < 31 { + // If the remaining bytes are less than 31, we only copy the remaining bytes + field_value_bytes_start_index = 31 - remaining_bytes; + } + for i in 0..31 { let byte_index = field_index * 31 + i; - output[byte_index] = field_value_bytes[i]; + if byte_index < N { + output[byte_index] = field_value_bytes[field_value_bytes_start_index + i]; + } } } From d7c59d320da0a80d60e5b60de065ba6ef5bcbcd4 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Sep 2024 10:42:58 +0000 Subject: [PATCH 05/36] WIP --- .../aztec-nr/aztec/src/utils/bytes.nr | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 7e5f0d8257b..b65a4efbb8c 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -1,7 +1,7 @@ // Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. // This implies that M = ceil(N / 31). pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { - let mut output = [0; M]; + let mut dst = [0; M]; for field_index in 0..M { let mut field_value = 0; @@ -14,37 +14,40 @@ pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { } } - output[field_index] = field_value; + dst[field_index] = field_value; } - output + dst } -// Converts the input array of fields into bytes. Each field of input has to contain only 31 bytes. +// Converts an input array of fields into bytes. Each field of input has to contain only 31 bytes. pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { - let mut output = [0; N]; + let mut dst = [0; N]; for field_index in 0..M { // We convert the field to 31 bytes. Normally a field is 254 bits, but we only used 248 bits // in `bytes_to_fields` so we can safely convert it to 31 bytes. - let field_value_bytes: [u8; 31] = input[field_index].to_be_bytes(); + let src: [u8; 31] = input[field_index].to_be_bytes(); + // Since some of the bytes might not be occupied (if the source value requiring less than 31 bytes), + // we have to compute the start index from which to copy. let remaining_bytes = N - field_index * 31; - let mut field_value_bytes_start_index = 0; - if remaining_bytes < 31 { + let src_start_index = if remaining_bytes < 31 { // If the remaining bytes are less than 31, we only copy the remaining bytes - field_value_bytes_start_index = 31 - remaining_bytes; - } + 31 - remaining_bytes + } else { + 0 + }; for i in 0..31 { let byte_index = field_index * 31 + i; if byte_index < N { - output[byte_index] = field_value_bytes[field_value_bytes_start_index + i]; + dst[byte_index] = src[src_start_index + i]; } } } - output + dst } mod test { From c70c8ea8eb8b1705cafc1aa9b478b5a175d86123 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 07:54:28 +0000 Subject: [PATCH 06/36] asserting max value --- .../aztec-nr/aztec/src/utils/bytes.nr | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index b65a4efbb8c..c77e8ee8b6a 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -25,9 +25,13 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { let mut dst = [0; N]; for field_index in 0..M { - // We convert the field to 31 bytes. Normally a field is 254 bits, but we only used 248 bits - // in `bytes_to_fields` so we can safely convert it to 31 bytes. - let src: [u8; 31] = input[field_index].to_be_bytes(); + let field = input[field_index]; + + // We expect that the field contains at most 31 bytes of information. + field.assert_max_bit_size(248); + + // Now we can safely convert the field to 31 bytes. + let src: [u8; 31] = field.to_be_bytes(); // Since some of the bytes might not be occupied (if the source value requiring less than 31 bytes), // we have to compute the start index from which to copy. @@ -117,4 +121,21 @@ mod test { assert_eq(input, input_back); } + + #[test(should_fail_with="call to assert_max_bit_size")] + fn test_fields_to_bytes_value_too_large() { + let input = [2.pow_32(248)]; + let _ignored_result = fields_to_bytes::<31, 1>(input); + } + + #[test] + fn test_fields_to_bytes_max_value() { + let input = [2.pow_32(248) - 1]; + let result = fields_to_bytes::<31, 1>(input); + + // We check that all the bytes were set to max value (255) + for i in 0..31 { + assert_eq(result[i], 255); + } + } } From 57acc4aac053da800cca3269ca17bc7ee69c7882 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 08:02:54 +0000 Subject: [PATCH 07/36] linking issue --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 1 + 1 file changed, 1 insertion(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index c77e8ee8b6a..dca220c7a60 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -21,6 +21,7 @@ pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { } // Converts an input array of fields into bytes. Each field of input has to contain only 31 bytes. +// TODO(#8618): Optimize for public use. pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { let mut dst = [0; N]; From 79b5d14231368052ed274c2637e550eeb6deb24a Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 08:28:07 +0000 Subject: [PATCH 08/36] random input fuzz test --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index dca220c7a60..5d67abe3333 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -106,17 +106,7 @@ mod test { } #[test] - fn test_large_random_input_to_fields_and_back() { - let input = [ - 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, - 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, - 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, - 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, 0x40, - 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50, - 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x60, - 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, 0x70, - 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80 - ]; + fn test_large_random_input_to_fields_and_back(input: [u8; 128]) { let output = bytes_to_fields::<128, 5>(input); let input_back = fields_to_bytes::<128, 5>(output); From a2979c267ad51b78c55ec8bda0483fb4c62f680b Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 09:57:35 +0000 Subject: [PATCH 09/36] test_large_random_input_to_bytes_and_back --- .../aztec-nr/aztec/src/utils/bytes.nr | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 5d67abe3333..757b630db94 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -113,6 +113,29 @@ mod test { assert_eq(input, input_back); } + // I need to get an array of random values lower than 2^248 on input and since there is no u248 type and modulo + // operation is not supported on a Field (to do field % 2^248), I will take multiple smaller values and combine + // them to get a value lower than 2^248. + #[test] + fn test_large_random_input_to_bytes_and_back( + input1: [u64; 5], + input2: [u64; 5], + input3: [u64; 5], + input4: [u32; 5], + input5: [u16; 5], + input6: [u8; 5] + ) { + let mut input = [0; 5]; + for i in 0..5 { + input[i] = (input1[i] as Field * 2.pow_32(184)) + (input2[i] as Field * 2.pow_32(120)) + (input3[i] as Field * 2.pow_32(56)) + (input4[i] as Field * 2.pow_32(24)) + (input5[i] as Field * 2.pow_32(8)) + input6[i] as Field; + } + + let output = fields_to_bytes::<155, 5>(input); + let input_back = bytes_to_fields::<155, 5>(output); + + assert_eq(input, input_back); + } + #[test(should_fail_with="call to assert_max_bit_size")] fn test_fields_to_bytes_value_too_large() { let input = [2.pow_32(248)]; From ffcb5e3b13c2a34cd13a09510c05d9f97bfc71d3 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 10:31:10 +0000 Subject: [PATCH 10/36] test fields partially used --- .../aztec-nr/aztec/src/utils/bytes.nr | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 757b630db94..d43ae3634c4 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -80,6 +80,34 @@ mod test { ); } + #[test] + fn test_3_small_fields_to_bytes() { + let input = [1, 2, 3]; + let output = fields_to_bytes::<93, 3>(input); + + // Each field should occupy 31 bytes with the non-zero value being placed in the last one. + assert_eq( + output, [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3 + ] + ); + } + + #[test] + fn test_3_small_fields_to_less_bytes() { + let input = [1, 2, 3]; + let output = fields_to_bytes::<63, 3>(input); + + // First 2 fields should occupy 31 bytes with the non-zero value being placed in the last one while the last + // field should occupy 1 byte. There is not information destruction here because the last field fits into + // 1 byte. + assert_eq( + output, [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 3 + ] + ); + } + #[test] fn test_bytes_to_2_fields() { let input = [ From e0b574fa9ecceff0c8b4a26b521a7aec92d3d253 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 11:03:02 +0000 Subject: [PATCH 11/36] all non-zero bytes used check --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index d43ae3634c4..3ae11ce82e5 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -44,6 +44,13 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { 0 }; + // Note: I tried combining this check with `assert_max_bit_size` above but `assert_max_bit_size` expects + // the argument to be a constant. Using comptime block to derive the number of bits also does not work + // because comptime is evaluated before generics. + for i in 0..src_start_index { + assert(src[i] == 0, "Field does not fit into remaining bytes"); + } + for i in 0..31 { let byte_index = field_index * 31 + i; if byte_index < N { @@ -164,6 +171,14 @@ mod test { assert_eq(input, input_back); } + #[test(should_fail_with="Field does not fit into remaining bytes")] + fn test_too_few_destination_bytes() { + // We should get an error here because first field gets converted to 31 bytes and the second field needs + // at least 2 bytes but we provide it with 1. + let input = [1, 256]; + let _ignored_result = fields_to_bytes::<32, 2>(input); + } + #[test(should_fail_with="call to assert_max_bit_size")] fn test_fields_to_bytes_value_too_large() { let input = [2.pow_32(248)]; From 30fd9ffc618c18699616bcbb1e781f7b75137ee9 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 18 Sep 2024 11:14:15 +0000 Subject: [PATCH 12/36] enough fields check --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 3ae11ce82e5..1fe15a67410 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -1,6 +1,7 @@ // Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. // This implies that M = ceil(N / 31). pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { + assert(N <= 31 * M, "Bytes do not fit into fields"); let mut dst = [0; M]; for field_index in 0..M { @@ -171,6 +172,13 @@ mod test { assert_eq(input, input_back); } + #[test(should_fail_with="Bytes do not fit into fields")] + fn test_too_few_destination_fields() { + // This should fail because we need 2 fields to store 32 bytes but we only provide 1. + let input = [0 as u8; 32]; + let _ignored_result = bytes_to_fields::<32, 1>(input); + } + #[test(should_fail_with="Field does not fit into remaining bytes")] fn test_too_few_destination_bytes() { // We should get an error here because first field gets converted to 31 bytes and the second field needs From c9025e0bcb01df24a379b10c52672cfbbbb76015 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 17:00:51 +0000 Subject: [PATCH 13/36] fixes after update to new noir --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 1fe15a67410..4d2ca15afd8 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -30,7 +30,7 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { let field = input[field_index]; // We expect that the field contains at most 31 bytes of information. - field.assert_max_bit_size(248); + field.assert_max_bit_size::<248>(); // Now we can safely convert the field to 31 bytes. let src: [u8; 31] = field.to_be_bytes(); From 8d23c1b94720a0c857f4875b6af88dd5395335a1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 17 Oct 2024 10:33:40 +0000 Subject: [PATCH 14/36] static_assert --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 4d2ca15afd8..f10df2ab4f9 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -1,7 +1,9 @@ +use std::static_assert; + // Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. // This implies that M = ceil(N / 31). pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { - assert(N <= 31 * M, "Bytes do not fit into fields"); + static_assert(N <= 31 * M, "Bytes do not fit into fields"); let mut dst = [0; M]; for field_index in 0..M { @@ -172,14 +174,14 @@ mod test { assert_eq(input, input_back); } - #[test(should_fail_with="Bytes do not fit into fields")] + #[test(should_fail_with = "Bytes do not fit into fields")] fn test_too_few_destination_fields() { // This should fail because we need 2 fields to store 32 bytes but we only provide 1. let input = [0 as u8; 32]; let _ignored_result = bytes_to_fields::<32, 1>(input); } - #[test(should_fail_with="Field does not fit into remaining bytes")] + #[test(should_fail_with = "Field does not fit into remaining bytes")] fn test_too_few_destination_bytes() { // We should get an error here because first field gets converted to 31 bytes and the second field needs // at least 2 bytes but we provide it with 1. @@ -187,7 +189,7 @@ mod test { let _ignored_result = fields_to_bytes::<32, 2>(input); } - #[test(should_fail_with="call to assert_max_bit_size")] + #[test(should_fail_with = "call to assert_max_bit_size")] fn test_fields_to_bytes_value_too_large() { let input = [2.pow_32(248)]; let _ignored_result = fields_to_bytes::<31, 1>(input); From 72155ed13f212a03697d8829162ed4b86eaf7f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Thu, 17 Oct 2024 12:36:58 +0200 Subject: [PATCH 15/36] Update noir-projects/aztec-nr/aztec/src/utils/bytes.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index f10df2ab4f9..4f98dcf2392 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -2,6 +2,10 @@ use std::static_assert; // Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. // This implies that M = ceil(N / 31). +// +// Each 31 byte chunk is converted into a Field as if the chunk was the Field's big endian representation. If the last chunk +// is less than 31 bytes long, then only the relevant bytes are conisdered. +// For example, [1, 10, 3] is encoded as [1 * 256^2 + 10 * 256 + 3] pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { static_assert(N <= 31 * M, "Bytes do not fit into fields"); let mut dst = [0; M]; From 47bb1cbbfa76bd1a9c540a5b66f3fe4d6a650af5 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 17 Oct 2024 10:45:55 +0000 Subject: [PATCH 16/36] better index names --- .../aztec-nr/aztec/src/utils/bytes.nr | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 4f98dcf2392..9e7faa0159c 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -2,26 +2,26 @@ use std::static_assert; // Converts the input bytes into an array of fields. A Field is ~254 bits meaning that each field can store 31 bytes. // This implies that M = ceil(N / 31). -// +// // Each 31 byte chunk is converted into a Field as if the chunk was the Field's big endian representation. If the last chunk // is less than 31 bytes long, then only the relevant bytes are conisdered. -// For example, [1, 10, 3] is encoded as [1 * 256^2 + 10 * 256 + 3] +// For example, [1, 10, 3] is encoded as [1 * 256^2 + 10 * 256 + 3] pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { static_assert(N <= 31 * M, "Bytes do not fit into fields"); let mut dst = [0; M]; - for field_index in 0..M { + for dst_index in 0..M { let mut field_value = 0; for i in 0..31 { - let byte_index = field_index * 31 + i; + let byte_index = dst_index * 31 + i; if byte_index < N { // Shift the existing value left by 8 bits and add the new byte field_value = field_value * 256 + input[byte_index] as Field; } } - dst[field_index] = field_value; + dst[dst_index] = field_value; } dst @@ -32,8 +32,8 @@ pub fn bytes_to_fields(input: [u8; N]) -> [Field; M] { pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { let mut dst = [0; N]; - for field_index in 0..M { - let field = input[field_index]; + for src_index in 0..M { + let field = input[src_index]; // We expect that the field contains at most 31 bytes of information. field.assert_max_bit_size::<248>(); @@ -43,7 +43,7 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { // Since some of the bytes might not be occupied (if the source value requiring less than 31 bytes), // we have to compute the start index from which to copy. - let remaining_bytes = N - field_index * 31; + let remaining_bytes = N - src_index * 31; let src_start_index = if remaining_bytes < 31 { // If the remaining bytes are less than 31, we only copy the remaining bytes 31 - remaining_bytes @@ -59,7 +59,7 @@ pub fn fields_to_bytes(input: [Field; M]) -> [u8; N] { } for i in 0..31 { - let byte_index = field_index * 31 + i; + let byte_index = src_index * 31 + i; if byte_index < N { dst[byte_index] = src[src_start_index + i]; } From 3271b22625f7a980eb4c1d6822692210de09c5d1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 18 Oct 2024 08:18:14 +0000 Subject: [PATCH 17/36] updated error message in test --- noir-projects/aztec-nr/aztec/src/utils/bytes.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 9e7faa0159c..58bbd233065 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -178,7 +178,7 @@ mod test { assert_eq(input, input_back); } - #[test(should_fail_with = "Bytes do not fit into fields")] + #[test(should_fail_with = "Argument is false")] fn test_too_few_destination_fields() { // This should fail because we need 2 fields to store 32 bytes but we only provide 1. let input = [0 as u8; 32]; From 32d7f3c2dded472e53c74dcc9eb994ae1d1265ee Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 12:16:10 +0000 Subject: [PATCH 18/36] refactor: updating NFT flows --- .../contracts/nft_contract/src/main.nr | 48 ++++++++++++------- .../src/test/transfer_to_private.nr | 29 ++--------- .../contracts/nft_contract/src/test/utils.nr | 10 +--- 3 files changed, 36 insertions(+), 51 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 6ec923ba8c2..4087433b178 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -134,21 +134,31 @@ contract NFT { public_owners_storage.write(to); } + #[private] + fn transfer_to_private(from: AztecAddress, to: AztecAddress, token_id: Field) { + let nft = NFT::at(context.this_address()); + + let slot_commitment = nft.prepare_transfer_to_private(from, to).call(&mut context); + + // We now transfer the token to this contract so that the `msg_sender` in `finalize_transfer_to_private` call + // below is the token owner. + nft.transfer_in_public(from, context.this_address(), token_id, 0).enqueue(&mut context); + + // At last we finalize the transfer. + nft.finalize_transfer_to_private(slot_commitment, token_id).enqueue(&mut context); + } + /// Prepares a transfer from public balance of `from` to a private balance of `to`. The transfer then needs to be /// finalized by calling `finalize_transfer_to_private`. `transient_storage_slot_randomness` is passed - /// as an argument so that we can derive `transfer_preparer_storage_slot_commitment` off-chain and then pass it + /// as an argument so that we can derive `slot_commitment` off-chain and then pass it /// as an argument to the followup call to `finalize_transfer_to_private`. #[private] - fn prepare_transfer_to_private( - from: AztecAddress, - to: AztecAddress, - transient_storage_slot_randomness: Field - ) { + fn prepare_transfer_to_private(from: AztecAddress, to: AztecAddress) -> Field { let to_keys = get_public_keys(to); let to_npk_m_hash = to_keys.npk_m.hash(); let to_note_slot = storage.private_nfts.at(to).storage_slot; - // We create a partial NFT note hiding point with unpopulated/zero token id for 'to' + // We create a setup payload with unpopulated/zero token id for 'to' // TODO(#7775): Manually fetching the randomness here is not great. If we decide to include randomness in all // notes we could just inject it in macros. let note_randomness = unsafe { @@ -161,18 +171,23 @@ contract NFT { // We make the msg_sender/transfer_preparer part of the slot preimage to ensure he cannot interfere with // non-sender's slots - let transfer_preparer_storage_slot_commitment: Field = pedersen_hash( + let transient_storage_slot_randomness = unsafe { + random() + }; + let slot_commitment: Field = pedersen_hash( [context.msg_sender().to_field(), transient_storage_slot_randomness], TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX ); // Then we hash the transfer preparer storage slot commitment with `from` and use that as the final slot // --> by hashing it with a `from` we ensure that `from` cannot interfere with slots not assigned to him. let slot: Field = pedersen_hash( - [from.to_field(), transfer_preparer_storage_slot_commitment], + [from.to_field(), slot_commitment], TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX ); NFT::at(context.this_address())._store_point_in_transient_storage(note_setup_payload.hiding_point, slot).enqueue(&mut context); + + slot_commitment } #[public] @@ -184,16 +199,12 @@ contract NFT { } /// Finalizes a transfer of NFT with `token_id` from public balance of `from` to a private balance of `to`. - /// The transfer must be prepared by calling `prepare_transfer_to_private` first. - /// The `transfer_preparer_storage_slot_commitment` has to be computed off-chain the same way as was done - /// in the preparation call. + /// The transfer must be prepared by calling `prepare_transfer_to_private` first and the resulting + /// `slot_commitment` must be passed as an argument to this function. #[public] - fn finalize_transfer_to_private( - token_id: Field, - transfer_preparer_storage_slot_commitment: Field - ) { + fn finalize_transfer_to_private(slot_commitment: Field, token_id: Field) { // We don't need to support authwit here because `prepare_transfer_to_private` allows us to set arbitrary - // `from` and `from` will always be the msg sender here. + // `from` and hence the call is authenticated by the loaded hiding point being non-zero. let from = context.msg_sender(); let public_owners_storage = storage.public_owners.at(token_id); assert(public_owners_storage.read().eq(from), "invalid NFT owner"); @@ -201,7 +212,7 @@ contract NFT { // Derive the slot from the transfer preparer storage slot commitment and the `from` address (declared // as `from` in this function) let hiding_point_slot = pedersen_hash( - [from.to_field(), transfer_preparer_storage_slot_commitment], + [from.to_field(), slot_commitment], TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX ); @@ -308,3 +319,4 @@ contract NFT { (owned_nft_ids, page_limit_reached) } } + diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 22106d8810b..5538cf15730 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -26,25 +26,15 @@ unconstrained fn transfer_to_private_to_a_different_account() { let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); let note_randomness = random(); - let transient_storage_slot_randomness = random(); - // Sender will be the msg_sender/transfer_preparer in prepare_transfer_to_private - let transfer_preparer_storage_slot_commitment = pedersen_hash( - [sender.to_field(), transient_storage_slot_randomness], - NFT::TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); // We mock the Oracle to return the note randomness such that later on we can manually add the note let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - env.call_private_void( - NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient, transient_storage_slot_randomness) - ); + let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient)); // Finalize the transfer of the NFT - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. @@ -85,25 +75,14 @@ unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { let correct_sender = AztecAddress::from_field(9); - let transient_storage_slot_randomness = random(); - // Sender will be the msg_sender/transfer_preparer in prepare_transfer_to_private - let transfer_preparer_storage_slot_commitment = pedersen_hash( - [correct_sender.to_field(), transient_storage_slot_randomness], - NFT::TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); - // We prepare the transfer - env.call_private_void( - NFT::at(nft_contract_address).prepare_transfer_to_private(correct_sender, recipient, transient_storage_slot_randomness) - ); + let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(correct_sender, recipient)); // We impersonate incorrect sender and try to finalize the transfer of the NFT. The incorrect sender owns the NFT // but tries to consume a prepared transfer not belonging to him. For this reason the test should fail with // "transfer not prepared". env.impersonate(incorrect_sender); - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); } #[test(should_fail_with = "invalid NFT owner")] diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr index 0dc1e3c73c9..3628c32d0c9 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr @@ -50,21 +50,15 @@ pub unconstrained fn setup_mint_and_transfer_to_private(with_account_contracts: let (env, nft_contract_address, owner, recipient, minted_token_id) = setup_and_mint(with_account_contracts); let note_randomness = random(); - let transient_storage_slot_randomness = random(); - let transfer_preparer_storage_slot_commitment = pedersen_hash( - [owner.to_field(), transient_storage_slot_randomness], - NFT::TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); // We mock the Oracle to return the note randomness such that later on we can manually add the note let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer with user being both the sender and the recipient (classical "shield" flow) - let prepare_transfer_to_private_call_interface = NFT::at(nft_contract_address).prepare_transfer_to_private(owner, owner, transient_storage_slot_randomness); - env.call_private_void(prepare_transfer_to_private_call_interface); + let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(owner, owner)); // Finalize the transfer of the NFT - let finalize_transfer_to_private_call_interface = NFT::at(nft_contract_address).finalize_transfer_to_private(minted_token_id, transfer_preparer_storage_slot_commitment); + let finalize_transfer_to_private_call_interface = NFT::at(nft_contract_address).finalize_transfer_to_private(minted_token_id, slot_commitment); env.call_public(finalize_transfer_to_private_call_interface); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` From 84e2ef0f2cf7fbd2b0c9723aa5e4ef36d74de703 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 13:39:42 +0000 Subject: [PATCH 19/36] WIP --- .../contracts/nft_contract/src/main.nr | 4 ++-- .../nft_contract/src/test/transfer_to_private.nr | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 4087433b178..9e139881ae9 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -145,7 +145,7 @@ contract NFT { nft.transfer_in_public(from, context.this_address(), token_id, 0).enqueue(&mut context); // At last we finalize the transfer. - nft.finalize_transfer_to_private(slot_commitment, token_id).enqueue(&mut context); + nft.finalize_transfer_to_private(token_id, slot_commitment).enqueue(&mut context); } /// Prepares a transfer from public balance of `from` to a private balance of `to`. The transfer then needs to be @@ -202,7 +202,7 @@ contract NFT { /// The transfer must be prepared by calling `prepare_transfer_to_private` first and the resulting /// `slot_commitment` must be passed as an argument to this function. #[public] - fn finalize_transfer_to_private(slot_commitment: Field, token_id: Field) { + fn finalize_transfer_to_private(token_id: Field, slot_commitment: Field) { // We don't need to support authwit here because `prepare_transfer_to_private` allows us to set arbitrary // `from` and hence the call is authenticated by the loaded hiding point being non-zero. let from = context.msg_sender(); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 5538cf15730..6e4390a906e 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -1,7 +1,7 @@ use crate::{test::utils, types::nft_note::NFTNote, NFT}; use dep::aztec::{ - hash::pedersen_hash, keys::getters::get_public_keys, prelude::{AztecAddress, NoteHeader}, - oracle::random::random, protocol_types::storage::map::derive_storage_slot_in_map + keys::getters::get_public_keys, prelude::{AztecAddress, NoteHeader}, oracle::random::random, + protocol_types::storage::map::derive_storage_slot_in_map }; use std::test::OracleMock; @@ -60,12 +60,10 @@ unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer was not prepared so we can use random value for the commitment - let transfer_preparer_storage_slot_commitment = random(); + let slot_commitment = random(); // Try finalizing the transfer without preparing it - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); } #[test(should_fail_with = "transfer not prepared")] @@ -91,11 +89,9 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); // We set random value for the commitment as the NFT owner check is before we use the value - let transfer_preparer_storage_slot_commitment = random(); + let slot_commitment = random(); // Try transferring someone else's public NFT env.impersonate(not_owner); - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); } From 9fc8a5e2351001bcd0ccaa5824c07d7148588aaf Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 15:03:24 +0000 Subject: [PATCH 20/36] x coord as hiding point slot --- .../contracts/nft_contract/src/main.nr | 55 +++++++------------ .../src/test/transfer_to_private.nr | 31 +++-------- 2 files changed, 26 insertions(+), 60 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 9e139881ae9..6899db2c133 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -21,8 +21,6 @@ contract NFT { use std::{embedded_curve_ops::EmbeddedCurvePoint, meta::derive}; use crate::types::nft_note::NFTNote; - global TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX = 3; - // TODO(#8467): Rename this to Transfer - calling this NFTTransfer to avoid export conflict with the Transfer event // in the Token contract. #[event] @@ -138,20 +136,18 @@ contract NFT { fn transfer_to_private(from: AztecAddress, to: AztecAddress, token_id: Field) { let nft = NFT::at(context.this_address()); - let slot_commitment = nft.prepare_transfer_to_private(from, to).call(&mut context); + let hiding_point_slot = nft.prepare_transfer_to_private(from, to).call(&mut context); // We now transfer the token to this contract so that the `msg_sender` in `finalize_transfer_to_private` call // below is the token owner. nft.transfer_in_public(from, context.this_address(), token_id, 0).enqueue(&mut context); // At last we finalize the transfer. - nft.finalize_transfer_to_private(token_id, slot_commitment).enqueue(&mut context); + nft.finalize_transfer_to_private(token_id, hiding_point_slot).enqueue(&mut context); } /// Prepares a transfer from public balance of `from` to a private balance of `to`. The transfer then needs to be - /// finalized by calling `finalize_transfer_to_private`. `transient_storage_slot_randomness` is passed - /// as an argument so that we can derive `slot_commitment` off-chain and then pass it - /// as an argument to the followup call to `finalize_transfer_to_private`. + /// finalized by calling `finalize_transfer_to_private`. Returns a hiding point slot. #[private] fn prepare_transfer_to_private(from: AztecAddress, to: AztecAddress) -> Field { let to_keys = get_public_keys(to); @@ -169,53 +165,40 @@ contract NFT { // We encrypt and emit the partial note log encrypt_and_emit_partial_log(&mut context, note_setup_payload.log_plaintext, to_keys, to); - // We make the msg_sender/transfer_preparer part of the slot preimage to ensure he cannot interfere with - // non-sender's slots - let transient_storage_slot_randomness = unsafe { - random() - }; - let slot_commitment: Field = pedersen_hash( - [context.msg_sender().to_field(), transient_storage_slot_randomness], - TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); - // Then we hash the transfer preparer storage slot commitment with `from` and use that as the final slot - // --> by hashing it with a `from` we ensure that `from` cannot interfere with slots not assigned to him. - let slot: Field = pedersen_hash( - [from.to_field(), slot_commitment], - TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); + NFT::at(context.this_address())._store_point_in_transient_storage(note_setup_payload.hiding_point).enqueue(&mut context); - NFT::at(context.this_address())._store_point_in_transient_storage(note_setup_payload.hiding_point, slot).enqueue(&mut context); + // Using the x-coordinate as a hiding point slot is safe because we have a guarantee that the public functions + // of the transaction are executed right after the private ones and for this reason the protocol guarantees + // that nobody can front-run us in consuming the hiding point. This guarantee would break + // if `finalize_transfer_to_private` was not called in the same transaction. This however is not the flow we + // are currently concerned with. To support the multi-transaction flow we could hash the x-coordinate with + // `from` and then repeat the hashing in `finalize_transfer_to_private`. + let hiding_point_slot = note_setup_payload.hiding_point.x; - slot_commitment + hiding_point_slot } #[public] #[internal] - fn _store_point_in_transient_storage(point: Point, slot: Field) { - // We don't perform check for the overwritten value to be non-zero because the slots are siloed to `to` - // and hence `to` can interfere only with his own execution. + fn _store_point_in_transient_storage(point: Point) { + let slot = point.x; + // We don't perform check for the overwritten value to be non-zero because the slot is implictly siloed to `to` + // (`to_note_slot` is part of the hiding point preimage) and hence `to` can interfere only with his own + // execution. context.storage_write(slot, point); } /// Finalizes a transfer of NFT with `token_id` from public balance of `from` to a private balance of `to`. /// The transfer must be prepared by calling `prepare_transfer_to_private` first and the resulting - /// `slot_commitment` must be passed as an argument to this function. + /// `hiding_point_slot` must be passed as an argument to this function. #[public] - fn finalize_transfer_to_private(token_id: Field, slot_commitment: Field) { + fn finalize_transfer_to_private(token_id: Field, hiding_point_slot: Field) { // We don't need to support authwit here because `prepare_transfer_to_private` allows us to set arbitrary // `from` and hence the call is authenticated by the loaded hiding point being non-zero. let from = context.msg_sender(); let public_owners_storage = storage.public_owners.at(token_id); assert(public_owners_storage.read().eq(from), "invalid NFT owner"); - // Derive the slot from the transfer preparer storage slot commitment and the `from` address (declared - // as `from` in this function) - let hiding_point_slot = pedersen_hash( - [from.to_field(), slot_commitment], - TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX - ); - // Read the hiding point from "transient" storage and check it's not empty to ensure the transfer was prepared let hiding_point: Point = context.storage_read(hiding_point_slot); assert(!is_empty(hiding_point), "transfer not prepared"); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 6e4390a906e..dddfe210e9e 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -31,10 +31,10 @@ unconstrained fn transfer_to_private_to_a_different_account() { let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient)); + let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient)); // Finalize the transfer of the NFT - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. @@ -59,28 +59,11 @@ unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); - // Transfer was not prepared so we can use random value for the commitment - let slot_commitment = random(); + // Transfer was not prepared so we can use random value for the hiding point slot + let hiding_point_slot = random(); // Try finalizing the transfer without preparing it - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); -} - -#[test(should_fail_with = "transfer not prepared")] -unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { - // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, nft_contract_address, incorrect_sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); - - let correct_sender = AztecAddress::from_field(9); - - // We prepare the transfer - let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(correct_sender, recipient)); - - // We impersonate incorrect sender and try to finalize the transfer of the NFT. The incorrect sender owns the NFT - // but tries to consume a prepared transfer not belonging to him. For this reason the test should fail with - // "transfer not prepared". - env.impersonate(incorrect_sender); - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); } #[test(should_fail_with = "invalid NFT owner")] @@ -89,9 +72,9 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); // We set random value for the commitment as the NFT owner check is before we use the value - let slot_commitment = random(); + let hiding_point_slot = random(); // Try transferring someone else's public NFT env.impersonate(not_owner); - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, slot_commitment)); + env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); } From 643c53f44ef9bfc36fd71c1b7e13b866f6b0c5c7 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 15:27:09 +0000 Subject: [PATCH 21/36] WIP --- noir-projects/aztec-nr/aztec/src/prelude.nr | 2 +- .../contracts/nft_contract/src/main.nr | 38 ++++++++++++++----- yarn-project/end-to-end/src/e2e_nft.test.ts | 22 ++--------- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/prelude.nr b/noir-projects/aztec-nr/aztec/src/prelude.nr index 793550c5668..5d4d32954d7 100644 --- a/noir-projects/aztec-nr/aztec/src/prelude.nr +++ b/noir-projects/aztec-nr/aztec/src/prelude.nr @@ -9,7 +9,7 @@ pub use crate::{ public_immutable::PublicImmutable, public_mutable::PublicMutable, private_set::PrivateSet, shared_immutable::SharedImmutable, shared_mutable::SharedMutable, storage::Storable }, - context::{PrivateContext, PackedReturns, FunctionReturns}, + context::{PrivateContext, PackedReturns, FunctionReturns, PublicContext}, note::{ note_header::NoteHeader, note_interface::{NoteInterface, NullifiableNote}, note_getter_options::NoteGetterOptions, note_viewer_options::NoteViewerOptions, diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 6899db2c133..4357d06f6a1 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -10,9 +10,12 @@ contract NFT { use dep::compressed_string::FieldCompressedString; use dep::aztec::{ oracle::random::random, - prelude::{NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateSet, AztecAddress}, + prelude::{ + NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateSet, + AztecAddress, PublicContext + }, encrypted_logs::{encrypted_note_emission::{encode_and_encrypt_note, encrypt_and_emit_partial_log}}, - hash::pedersen_hash, keys::getters::get_public_keys, note::constants::MAX_NOTES_PER_PAGE, + keys::getters::get_public_keys, note::constants::MAX_NOTES_PER_PAGE, protocol_types::traits::is_empty, utils::comparison::Comparator, protocol_types::{point::Point, traits::Serialize}, macros::{storage::storage, events::event, functions::{private, public, view, internal, initializer}} @@ -132,18 +135,18 @@ contract NFT { public_owners_storage.write(to); } + // Transfers token with `token_id` from public balance of message sender to a private balance of `to`. #[private] - fn transfer_to_private(from: AztecAddress, to: AztecAddress, token_id: Field) { + fn transfer_to_private(to: AztecAddress, token_id: Field) { + let from = context.msg_sender(); + let nft = NFT::at(context.this_address()); + // We prepare the transfer. let hiding_point_slot = nft.prepare_transfer_to_private(from, to).call(&mut context); - // We now transfer the token to this contract so that the `msg_sender` in `finalize_transfer_to_private` call - // below is the token owner. - nft.transfer_in_public(from, context.this_address(), token_id, 0).enqueue(&mut context); - // At last we finalize the transfer. - nft.finalize_transfer_to_private(token_id, hiding_point_slot).enqueue(&mut context); + nft._finalize_transfer_to_private_trusted(from, token_id, hiding_point_slot).enqueue(&mut context); } /// Prepares a transfer from public balance of `from` to a private balance of `to`. The transfer then needs to be @@ -193,9 +196,24 @@ contract NFT { /// `hiding_point_slot` must be passed as an argument to this function. #[public] fn finalize_transfer_to_private(token_id: Field, hiding_point_slot: Field) { - // We don't need to support authwit here because `prepare_transfer_to_private` allows us to set arbitrary - // `from` and hence the call is authenticated by the loaded hiding point being non-zero. let from = context.msg_sender(); + _finalize_transfer_to_private(from, token_id, hiding_point_slot, &mut context, storage); + } + + #[public] + #[internal] + fn _finalize_transfer_to_private_trusted(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { + _finalize_transfer_to_private(from, token_id, hiding_point_slot, &mut context, storage); + } + + #[contract_library_method] + fn _finalize_transfer_to_private( + from: AztecAddress, + token_id: Field, + hiding_point_slot: Field, + context: &mut PublicContext, + storage: Storage<&mut PublicContext> + ) { let public_owners_storage = storage.public_owners.at(token_id); assert(public_owners_storage.read().eq(from), "invalid NFT owner"); diff --git a/yarn-project/end-to-end/src/e2e_nft.test.ts b/yarn-project/end-to-end/src/e2e_nft.test.ts index a63049dbf0a..7efbc9aef71 100644 --- a/yarn-project/end-to-end/src/e2e_nft.test.ts +++ b/yarn-project/end-to-end/src/e2e_nft.test.ts @@ -1,5 +1,4 @@ -import { type AccountWallet, AztecAddress, BatchCall, Fr } from '@aztec/aztec.js'; -import { pedersenHash } from '@aztec/foundation/crypto'; +import { type AccountWallet, AztecAddress, Fr } from '@aztec/aztec.js'; import { NFTContract } from '@aztec/noir-contracts.js'; import { jest } from '@jest/globals'; @@ -24,7 +23,6 @@ describe('NFT', () => { // Arbitrary token id const TOKEN_ID = Fr.random().toBigInt(); - const TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX = 3; beforeAll(async () => { let wallets: AccountWallet[]; @@ -63,21 +61,9 @@ describe('NFT', () => { // In a simple "shield" flow the sender and recipient are the same. In the "uniswap swap to private" flow // it would be the uniswap contract. const recipient = user1Wallet.getAddress(); - const sender = recipient; - const transientStorageSlotRandomness = Fr.random(); - const transferPreparerStorageSlotCommitment = pedersenHash( - [user1Wallet.getAddress(), transientStorageSlotRandomness], - TRANSIENT_STORAGE_SLOT_PEDERSEN_INDEX, - ); - - const { debugInfo } = await new BatchCall(user1Wallet, [ - nftContractAsUser1.methods - .prepare_transfer_to_private(sender, recipient, transientStorageSlotRandomness) - .request(), - nftContractAsUser1.methods - .finalize_transfer_to_private(TOKEN_ID, transferPreparerStorageSlotCommitment) - .request(), - ]) + + const { debugInfo } = await nftContractAsUser1.methods + .transfer_to_private(recipient, TOKEN_ID) .send() .wait({ debug: true }); From 4e97c3024034d9751b5d2ba25df0d8a0209389ec Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 15:29:35 +0000 Subject: [PATCH 22/36] WIP --- noir-projects/noir-contracts/contracts/nft_contract/src/main.nr | 2 +- yarn-project/end-to-end/src/e2e_nft.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 4357d06f6a1..3d93127c76b 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -21,7 +21,7 @@ contract NFT { macros::{storage::storage, events::event, functions::{private, public, view, internal, initializer}} }; use dep::authwit::auth::{assert_current_call_valid_authwit, assert_current_call_valid_authwit_public, compute_authwit_nullifier}; - use std::{embedded_curve_ops::EmbeddedCurvePoint, meta::derive}; + use std::meta::derive; use crate::types::nft_note::NFTNote; // TODO(#8467): Rename this to Transfer - calling this NFTTransfer to avoid export conflict with the Transfer event diff --git a/yarn-project/end-to-end/src/e2e_nft.test.ts b/yarn-project/end-to-end/src/e2e_nft.test.ts index 7efbc9aef71..afa41541f15 100644 --- a/yarn-project/end-to-end/src/e2e_nft.test.ts +++ b/yarn-project/end-to-end/src/e2e_nft.test.ts @@ -59,7 +59,7 @@ describe('NFT', () => { const nftContractAsUser1 = await NFTContract.at(nftContractAddress, user1Wallet); // In a simple "shield" flow the sender and recipient are the same. In the "uniswap swap to private" flow - // it would be the uniswap contract. + // the sender would be the uniswap contract. const recipient = user1Wallet.getAddress(); const { debugInfo } = await nftContractAsUser1.methods From 0ce52a6e81218397c5e30a63a96e8d4e20a286e5 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 15:40:06 +0000 Subject: [PATCH 23/36] last touches --- .../nft_contract/src/test/transfer_to_private.nr | 4 +++- .../contracts/nft_contract/src/test/utils.nr | 8 ++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index dddfe210e9e..c275d578126 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -20,8 +20,10 @@ unconstrained fn transfer_to_private_to_self() { utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id); } +/// External orchestration means that the calls to prepare and finalize are not done by the NFT contract. This flow +/// will typically be used by a DEX. #[test] -unconstrained fn transfer_to_private_to_a_different_account() { +unconstrained fn transfer_to_private_external_orchestration() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr index 3628c32d0c9..91b3e16c677 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr @@ -54,12 +54,8 @@ pub unconstrained fn setup_mint_and_transfer_to_private(with_account_contracts: // We mock the Oracle to return the note randomness such that later on we can manually add the note let _ = OracleMock::mock("getRandomField").returns(note_randomness); - // We prepare the transfer with user being both the sender and the recipient (classical "shield" flow) - let slot_commitment: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(owner, owner)); - - // Finalize the transfer of the NFT - let finalize_transfer_to_private_call_interface = NFT::at(nft_contract_address).finalize_transfer_to_private(minted_token_id, slot_commitment); - env.call_public(finalize_transfer_to_private_call_interface); + // We transfer the public NFT to private. + env.call_private_void(NFT::at(nft_contract_address).transfer_to_private(owner, minted_token_id)); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. From a3d63376f45f25fb10f6a83241c88f354bb2370d Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 10 Oct 2024 15:44:35 +0000 Subject: [PATCH 24/36] comment fix --- .../noir-contracts/contracts/nft_contract/src/main.nr | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 3d93127c76b..e34c596e5ef 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -185,9 +185,8 @@ contract NFT { #[internal] fn _store_point_in_transient_storage(point: Point) { let slot = point.x; - // We don't perform check for the overwritten value to be non-zero because the slot is implictly siloed to `to` - // (`to_note_slot` is part of the hiding point preimage) and hence `to` can interfere only with his own - // execution. + // We don't perform a check for the overwritten value to be zero because the slot is the x-coordinate + // of the hiding point and hence we could only overwrite the value in the slot with the same value. context.storage_write(slot, point); } From 7b2aa8e3fb43858ecd0fbef5ef6abb4ceb062bbd Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 10:29:10 +0000 Subject: [PATCH 25/36] WIP --- .../contracts/nft_contract/src/main.nr | 29 ++++++++++--------- .../src/test/transfer_to_private.nr | 6 ++-- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index e34c596e5ef..e804b75e8f3 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -143,16 +143,17 @@ contract NFT { let nft = NFT::at(context.this_address()); // We prepare the transfer. - let hiding_point_slot = nft.prepare_transfer_to_private(from, to).call(&mut context); + let hiding_point_slot = nft.prepare_transfer_to_private(to).call(&mut context); - // At last we finalize the transfer. - nft._finalize_transfer_to_private_trusted(from, token_id, hiding_point_slot).enqueue(&mut context); + // At last we finalize the transfer. Usafe of the `unsafe` method here is safe because we set the `from` + // function argument to a message sender, guaranteeing that he can transfer only his own NFTs. + nft._finalize_transfer_to_private_unsafe(from, token_id, hiding_point_slot).enqueue(&mut context); } - /// Prepares a transfer from public balance of `from` to a private balance of `to`. The transfer then needs to be + /// Prepares a transfer to a private balance of `to`. The transfer then needs to be /// finalized by calling `finalize_transfer_to_private`. Returns a hiding point slot. #[private] - fn prepare_transfer_to_private(from: AztecAddress, to: AztecAddress) -> Field { + fn prepare_transfer_to_private(to: AztecAddress) -> Field { let to_keys = get_public_keys(to); let to_npk_m_hash = to_keys.npk_m.hash(); let to_note_slot = storage.private_nfts.at(to).storage_slot; @@ -168,25 +169,25 @@ contract NFT { // We encrypt and emit the partial note log encrypt_and_emit_partial_log(&mut context, note_setup_payload.log_plaintext, to_keys, to); - NFT::at(context.this_address())._store_point_in_transient_storage(note_setup_payload.hiding_point).enqueue(&mut context); - // Using the x-coordinate as a hiding point slot is safe because we have a guarantee that the public functions // of the transaction are executed right after the private ones and for this reason the protocol guarantees // that nobody can front-run us in consuming the hiding point. This guarantee would break // if `finalize_transfer_to_private` was not called in the same transaction. This however is not the flow we - // are currently concerned with. To support the multi-transaction flow we could hash the x-coordinate with - // `from` and then repeat the hashing in `finalize_transfer_to_private`. + // are currently concerned with. To support the multi-transaction flow we could introduce a `from` function + // argument, hash the x-coordinate with it and then repeat the hashing in `finalize_transfer_to_private`. let hiding_point_slot = note_setup_payload.hiding_point.x; + // We don't need to perform a check that the value overwritten by `_store_point_in_transient_storage_unsafe` + // is zero because the slot is the x-coordinate of the hiding point and hence we could only overwrite + // the value in the slot with the same value. This makes usage of the `unsafe` method safe. + NFT::at(context.this_address())._store_point_in_transient_storage_unsafe(hiding_point_slot, note_setup_payload.hiding_point).enqueue(&mut context); + hiding_point_slot } #[public] #[internal] - fn _store_point_in_transient_storage(point: Point) { - let slot = point.x; - // We don't perform a check for the overwritten value to be zero because the slot is the x-coordinate - // of the hiding point and hence we could only overwrite the value in the slot with the same value. + fn _store_point_in_transient_storage_unsafe(slot: Field, point: Point) { context.storage_write(slot, point); } @@ -201,7 +202,7 @@ contract NFT { #[public] #[internal] - fn _finalize_transfer_to_private_trusted(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { + fn _finalize_transfer_to_private_unsafe(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { _finalize_transfer_to_private(from, token_id, hiding_point_slot, &mut context, storage); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index c275d578126..f0de54ee61f 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -25,7 +25,7 @@ unconstrained fn transfer_to_private_to_self() { #[test] unconstrained fn transfer_to_private_external_orchestration() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); + let (env, nft_contract_address, _, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); let note_randomness = random(); @@ -33,9 +33,9 @@ unconstrained fn transfer_to_private_external_orchestration() { let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient)); + let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(recipient)); - // Finalize the transfer of the NFT + // Finalize the transfer of the NFT (message sender owns the NFT in public) env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` From 12332f6ed6dac8f7663c3578c27f63d77db8a773 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 13:06:45 +0000 Subject: [PATCH 26/36] WIP --- .../contracts/nft_contract/src/main.nr | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index e804b75e8f3..23ee83360f4 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -169,12 +169,17 @@ contract NFT { // We encrypt and emit the partial note log encrypt_and_emit_partial_log(&mut context, note_setup_payload.log_plaintext, to_keys, to); - // Using the x-coordinate as a hiding point slot is safe because we have a guarantee that the public functions - // of the transaction are executed right after the private ones and for this reason the protocol guarantees - // that nobody can front-run us in consuming the hiding point. This guarantee would break - // if `finalize_transfer_to_private` was not called in the same transaction. This however is not the flow we - // are currently concerned with. To support the multi-transaction flow we could introduce a `from` function - // argument, hash the x-coordinate with it and then repeat the hashing in `finalize_transfer_to_private`. + // Using the x-coordinate as a hiding point slot is safe against someone else interfering with it because + // we have a guarantee that the public functions of the transaction are executed right after the private ones + // and for this reason the protocol guarantees that nobody can front-run us in consuming the hiding point. + // This guarantee would break if `finalize_transfer_to_private` was not called in the same transaction. This + // however is not the flow we are currently concerned with. To support the multi-transaction flow we could + // introduce a `from` function argument, hash the x-coordinate with it and then repeat the hashing in + // `finalize_transfer_to_private`. + // + // We can also be sure that the `hiding_point_slot` will not overwrite any other value in the storage because + // in our state variables we derive slots using a different hash function from multi scalar multiplication + // (MSM). let hiding_point_slot = note_setup_payload.hiding_point.x; // We don't need to perform a check that the value overwritten by `_store_point_in_transient_storage_unsafe` From e5f01b83983940dd7f92261c4e468bdac0d6ee27 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 13:48:54 +0000 Subject: [PATCH 27/36] internal func opt. --- .../contracts/nft_contract/src/main.nr | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 23ee83360f4..517f3f9f560 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -143,7 +143,7 @@ contract NFT { let nft = NFT::at(context.this_address()); // We prepare the transfer. - let hiding_point_slot = nft.prepare_transfer_to_private(to).call(&mut context); + let hiding_point_slot = nft._prepare_transfer_to_private(to).call(&mut context); // At last we finalize the transfer. Usafe of the `unsafe` method here is safe because we set the `from` // function argument to a message sender, guaranteeing that he can transfer only his own NFTs. @@ -154,6 +154,17 @@ contract NFT { /// finalized by calling `finalize_transfer_to_private`. Returns a hiding point slot. #[private] fn prepare_transfer_to_private(to: AztecAddress) -> Field { + NFT::at(context.this_address())._prepare_transfer_to_private(to).call(&mut context) + } + + /// This function exists separately from `prepare_transfer_to_private` solely as an optimization as it allows + /// us to have it inlined in the `transfer_to_private` function which results in one less kernel iteration. + /// + /// TODO(#9180): Consider adding macro support for functions callable both as an entrypoint and as an internal + /// function. + #[private] + #[internal] + fn _prepare_transfer_to_private(to: AztecAddress) -> Field { let to_keys = get_public_keys(to); let to_npk_m_hash = to_keys.npk_m.hash(); let to_note_slot = storage.private_nfts.at(to).storage_slot; From 982a4ee69adc77a2ce3d73834ce80f28cb328f6f Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 13:51:47 +0000 Subject: [PATCH 28/36] removing lies --- .../contracts/nft_contract/src/test/transfer_to_private.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index f0de54ee61f..4d1a5c95292 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -56,8 +56,8 @@ unconstrained fn transfer_to_private_external_orchestration() { utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id); } -#[test(should_fail_with = "transfer not prepared")] -unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { +#[test(should_fail_with="transfer not prepared")] +unconstrained fn transfer_to_private_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); From 7b83c802ce9508b73782042dcc7cb86f62c61fb6 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 13:57:12 +0000 Subject: [PATCH 29/36] clarifying test names --- .../nft_contract/src/test/transfer_to_private.nr | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 4d1a5c95292..5fa3a63a7f7 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -5,9 +5,13 @@ use dep::aztec::{ }; use std::test::OracleMock; +/// Internal orchestration means that the calls to `prepare_transfer_to_private` +/// and `finalize_transfer_to_private` are done by the NFT contract itself. +/// In this test's case this is done by the `NFT::transfer_to_private(...)` function called +/// in `utils::setup_mint_and_transfer_to_private`. #[test] -unconstrained fn transfer_to_private_to_self() { - // The transfer to private to self is done in `utils::setup_mint_and_transfer_to_private` and for this reason +unconstrained fn transfer_to_private_internal_orchestration() { + // The transfer to private is done in `utils::setup_mint_and_transfer_to_private` and for this reason // in this test we just call it and check the outcome. // Setup without account contracts. We are not using authwits here, so dummy accounts are enough From d8471bd85f319291cb105b4c8bea4b4c1a1f7f95 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 14:05:43 +0000 Subject: [PATCH 30/36] WIP --- yarn-project/end-to-end/src/e2e_nft.test.ts | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_nft.test.ts b/yarn-project/end-to-end/src/e2e_nft.test.ts index afa41541f15..1f7c5eb3008 100644 --- a/yarn-project/end-to-end/src/e2e_nft.test.ts +++ b/yarn-project/end-to-end/src/e2e_nft.test.ts @@ -58,9 +58,9 @@ describe('NFT', () => { it('transfers to private', async () => { const nftContractAsUser1 = await NFTContract.at(nftContractAddress, user1Wallet); - // In a simple "shield" flow the sender and recipient are the same. In the "uniswap swap to private" flow - // the sender would be the uniswap contract. - const recipient = user1Wallet.getAddress(); + // In a simple "shield" flow the sender and recipient are the same. In the "AMM swap to private" flow + // the sender would be the AMM contract. + const recipient = user2Wallet.getAddress(); const { debugInfo } = await nftContractAsUser1.methods .transfer_to_private(recipient, TOKEN_ID) @@ -80,29 +80,29 @@ describe('NFT', () => { }); it('transfers in private', async () => { - const nftContractAsUser1 = await NFTContract.at(nftContractAddress, user1Wallet); + const nftContractAsUser2 = await NFTContract.at(nftContractAddress, user2Wallet); - await nftContractAsUser1.methods - .transfer_in_private(user1Wallet.getAddress(), user2Wallet.getAddress(), TOKEN_ID, 0) + await nftContractAsUser2.methods + .transfer_in_private(user2Wallet.getAddress(), user1Wallet.getAddress(), TOKEN_ID, 0) .send() .wait(); const user1Nfts = await getPrivateNfts(user1Wallet.getAddress()); - expect(user1Nfts).toEqual([]); + expect(user1Nfts).toEqual([TOKEN_ID]); const user2Nfts = await getPrivateNfts(user2Wallet.getAddress()); - expect(user2Nfts).toEqual([TOKEN_ID]); + expect(user2Nfts).toEqual([]); }); it('transfers to public', async () => { - const nftContractAsUser2 = await NFTContract.at(nftContractAddress, user2Wallet); + const nftContractAsUser1 = await NFTContract.at(nftContractAddress, user1Wallet); - await nftContractAsUser2.methods - .transfer_to_public(user2Wallet.getAddress(), user2Wallet.getAddress(), TOKEN_ID, 0) + await nftContractAsUser1.methods + .transfer_to_public(user1Wallet.getAddress(), user2Wallet.getAddress(), TOKEN_ID, 0) .send() .wait(); - const publicOwnerAfter = await nftContractAsUser2.methods.owner_of(TOKEN_ID).simulate(); + const publicOwnerAfter = await nftContractAsUser1.methods.owner_of(TOKEN_ID).simulate(); expect(publicOwnerAfter).toEqual(user2Wallet.getAddress()); }); From 28312f6e263cd10fa3c54a0c69a3739fe62dc710 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 11 Oct 2024 14:17:21 +0000 Subject: [PATCH 31/36] making test slower so that Nico is happy ;) --- .../contracts/nft_contract/src/test/transfer_to_private.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 5fa3a63a7f7..fb7aad88875 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -77,8 +77,10 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); - // We set random value for the commitment as the NFT owner check is before we use the value - let hiding_point_slot = random(); + // (For this specific test we could set a random value for the commitment and not do the call to `prepare...` + // as the NFT owner check is before we use the value but that would made the test less robust against changes + // in the contract.) + let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(not_owner)); // Try transferring someone else's public NFT env.impersonate(not_owner); From 63f9e9bab9dc413e6b19470e083cb5d3f035ef04 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 21 Oct 2024 14:28:58 +0000 Subject: [PATCH 32/36] fmt --- noir-projects/noir-contracts/contracts/nft_contract/src/main.nr | 2 +- .../contracts/nft_contract/src/test/transfer_to_private.nr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 517f3f9f560..3dc561f4c6a 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -218,7 +218,7 @@ contract NFT { #[public] #[internal] - fn _finalize_transfer_to_private_unsafe(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { + fn _finalize_transfer_to_private_unsafe(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { _finalize_transfer_to_private(from, token_id, hiding_point_slot, &mut context, storage); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index fb7aad88875..8d06b5a5dd5 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -60,7 +60,7 @@ unconstrained fn transfer_to_private_external_orchestration() { utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id); } -#[test(should_fail_with="transfer not prepared")] +#[test(should_fail_with = "transfer not prepared")] unconstrained fn transfer_to_private_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); From 121a7533ad36ee98233bc036ba23d64dc1b32ea1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 23 Oct 2024 18:18:50 +0000 Subject: [PATCH 33/36] fix --- .../contracts/nft_contract/src/main.nr | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 3dc561f4c6a..26189542b23 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -11,8 +11,8 @@ contract NFT { use dep::aztec::{ oracle::random::random, prelude::{ - NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateSet, - AztecAddress, PublicContext + NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateContext, + PrivateSet, AztecAddress, PublicContext }, encrypted_logs::{encrypted_note_emission::{encode_and_encrypt_note, encrypt_and_emit_partial_log}}, keys::getters::get_public_keys, note::constants::MAX_NOTES_PER_PAGE, @@ -143,7 +143,7 @@ contract NFT { let nft = NFT::at(context.this_address()); // We prepare the transfer. - let hiding_point_slot = nft._prepare_transfer_to_private(to).call(&mut context); + let hiding_point_slot = _prepare_transfer_to_private(to, &mut context, storage); // At last we finalize the transfer. Usafe of the `unsafe` method here is safe because we set the `from` // function argument to a message sender, guaranteeing that he can transfer only his own NFTs. @@ -154,7 +154,7 @@ contract NFT { /// finalized by calling `finalize_transfer_to_private`. Returns a hiding point slot. #[private] fn prepare_transfer_to_private(to: AztecAddress) -> Field { - NFT::at(context.this_address())._prepare_transfer_to_private(to).call(&mut context) + _prepare_transfer_to_private(to, &mut context, storage) } /// This function exists separately from `prepare_transfer_to_private` solely as an optimization as it allows @@ -162,9 +162,12 @@ contract NFT { /// /// TODO(#9180): Consider adding macro support for functions callable both as an entrypoint and as an internal /// function. - #[private] - #[internal] - fn _prepare_transfer_to_private(to: AztecAddress) -> Field { + #[contract_library_method] + fn _prepare_transfer_to_private( + to: AztecAddress, + context: &mut PrivateContext, + storage: Storage<&mut PrivateContext> + ) -> Field { let to_keys = get_public_keys(to); let to_npk_m_hash = to_keys.npk_m.hash(); let to_note_slot = storage.private_nfts.at(to).storage_slot; @@ -178,7 +181,7 @@ contract NFT { let note_setup_payload = NFTNote::setup_payload().new(to_npk_m_hash, note_randomness, to_note_slot); // We encrypt and emit the partial note log - encrypt_and_emit_partial_log(&mut context, note_setup_payload.log_plaintext, to_keys, to); + encrypt_and_emit_partial_log(context, note_setup_payload.log_plaintext, to_keys, to); // Using the x-coordinate as a hiding point slot is safe against someone else interfering with it because // we have a guarantee that the public functions of the transaction are executed right after the private ones @@ -196,7 +199,7 @@ contract NFT { // We don't need to perform a check that the value overwritten by `_store_point_in_transient_storage_unsafe` // is zero because the slot is the x-coordinate of the hiding point and hence we could only overwrite // the value in the slot with the same value. This makes usage of the `unsafe` method safe. - NFT::at(context.this_address())._store_point_in_transient_storage_unsafe(hiding_point_slot, note_setup_payload.hiding_point).enqueue(&mut context); + NFT::at(context.this_address())._store_point_in_transient_storage_unsafe(hiding_point_slot, note_setup_payload.hiding_point).enqueue(context); hiding_point_slot } From 8e90fb27a214e95c33fe11878e1caaf578c00915 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 24 Oct 2024 14:38:47 +0000 Subject: [PATCH 34/36] fixes --- .../contracts/nft_contract/src/main.nr | 2 +- .../src/test/transfer_to_private.nr | 17 ++++++++++++----- .../contracts/nft_contract/src/test/utils.nr | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index e3057a53a97..29e71e1f206 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -12,7 +12,7 @@ contract NFT { oracle::random::random, prelude::{ NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateSet, - AztecAddress, + AztecAddress, PrivateContext, PublicContext }, encrypted_logs::encrypted_note_emission::{ encode_and_encrypt_note, encrypt_and_emit_partial_log, diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 9f20d408c28..e67a1eba5fe 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -37,10 +37,13 @@ unconstrained fn transfer_to_private_external_orchestration() { let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(recipient)); + let hiding_point_slot: Field = NFT::at(nft_contract_address).prepare_transfer_to_private(recipient).call(&mut env.private()); + // Finalize the transfer of the NFT (message sender owns the NFT in public) - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot).call( + &mut env.public(), + ); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. @@ -76,7 +79,9 @@ unconstrained fn transfer_to_private_transfer_not_prepared() { let hiding_point_slot = random(); // Try finalizing the transfer without preparing it - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot).call( + &mut env.public(), + ); } #[test(should_fail_with = "invalid NFT owner")] @@ -88,9 +93,11 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { // (For this specific test we could set a random value for the commitment and not do the call to `prepare...` // as the NFT owner check is before we use the value but that would made the test less robust against changes // in the contract.) - let hiding_point_slot: Field = env.call_private(NFT::at(nft_contract_address).prepare_transfer_to_private(not_owner)); + let hiding_point_slot: Field = NFT::at(nft_contract_address).prepare_transfer_to_private(not_owner).call(&mut env.private()); // Try transferring someone else's public NFT env.impersonate(not_owner); - env.call_public(NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot)); + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot).call( + &mut env.public(), + ); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr index f5e2014e014..e83067023ee 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr @@ -64,7 +64,7 @@ pub unconstrained fn setup_mint_and_transfer_to_private( let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We transfer the public NFT to private. - env.call_private_void(NFT::at(nft_contract_address).transfer_to_private(owner, minted_token_id)); + NFT::at(nft_contract_address).transfer_to_private(owner, minted_token_id).call(&mut env.private()); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. From e0a1ee94d15862cc41459df29d74a6d910932d3c Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 24 Oct 2024 16:16:00 +0000 Subject: [PATCH 35/36] fmt --- .../contracts/nft_contract/src/main.nr | 23 ++++++++++++++----- .../src/test/transfer_to_private.nr | 14 +++++++---- .../contracts/nft_contract/src/test/utils.nr | 4 +++- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 29e71e1f206..bdab2dac015 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -12,7 +12,7 @@ contract NFT { oracle::random::random, prelude::{ NoteGetterOptions, NoteViewerOptions, Map, PublicMutable, SharedImmutable, PrivateSet, - AztecAddress, PrivateContext, PublicContext + AztecAddress, PrivateContext, PublicContext, }, encrypted_logs::encrypted_note_emission::{ encode_and_encrypt_note, encrypt_and_emit_partial_log, @@ -154,7 +154,9 @@ contract NFT { // At last we finalize the transfer. Usafe of the `unsafe` method here is safe because we set the `from` // function argument to a message sender, guaranteeing that he can transfer only his own NFTs. - nft._finalize_transfer_to_private_unsafe(from, token_id, hiding_point_slot).enqueue(&mut context); + nft._finalize_transfer_to_private_unsafe(from, token_id, hiding_point_slot).enqueue( + &mut context, + ); } /// Prepares a transfer to a private balance of `to`. The transfer then needs to be @@ -173,7 +175,7 @@ contract NFT { fn _prepare_transfer_to_private( to: AztecAddress, context: &mut PrivateContext, - storage: Storage<&mut PrivateContext> + storage: Storage<&mut PrivateContext>, ) -> Field { let to_keys = get_public_keys(to); let to_npk_m_hash = to_keys.npk_m.hash(); @@ -205,7 +207,12 @@ contract NFT { // We don't need to perform a check that the value overwritten by `_store_point_in_transient_storage_unsafe` // is zero because the slot is the x-coordinate of the hiding point and hence we could only overwrite // the value in the slot with the same value. This makes usage of the `unsafe` method safe. - NFT::at(context.this_address())._store_point_in_transient_storage_unsafe(hiding_point_slot, note_setup_payload.hiding_point).enqueue(context); + NFT::at(context.this_address()) + ._store_point_in_transient_storage_unsafe( + hiding_point_slot, + note_setup_payload.hiding_point, + ) + .enqueue(context); hiding_point_slot } @@ -227,7 +234,11 @@ contract NFT { #[public] #[internal] - fn _finalize_transfer_to_private_unsafe(from: AztecAddress, token_id: Field, hiding_point_slot: Field) { + fn _finalize_transfer_to_private_unsafe( + from: AztecAddress, + token_id: Field, + hiding_point_slot: Field, + ) { _finalize_transfer_to_private(from, token_id, hiding_point_slot, &mut context, storage); } @@ -237,7 +248,7 @@ contract NFT { token_id: Field, hiding_point_slot: Field, context: &mut PublicContext, - storage: Storage<&mut PublicContext> + storage: Storage<&mut PublicContext>, ) { let public_owners_storage = storage.public_owners.at(token_id); assert(public_owners_storage.read().eq(from), "invalid NFT owner"); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index e67a1eba5fe..9d48bdb4977 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -1,7 +1,7 @@ use crate::{test::utils, types::nft_note::NFTNote, NFT}; use dep::aztec::{ keys::getters::get_public_keys, prelude::{AztecAddress, NoteHeader}, oracle::random::random, - protocol_types::storage::map::derive_storage_slot_in_map + protocol_types::storage::map::derive_storage_slot_in_map, }; use std::test::OracleMock; @@ -29,7 +29,8 @@ unconstrained fn transfer_to_private_internal_orchestration() { #[test] unconstrained fn transfer_to_private_external_orchestration() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, nft_contract_address, _, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); + let (env, nft_contract_address, _, recipient, token_id) = + utils::setup_and_mint( /* with_account_contracts */ false); let note_randomness = random(); @@ -37,8 +38,9 @@ unconstrained fn transfer_to_private_external_orchestration() { let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - let hiding_point_slot: Field = NFT::at(nft_contract_address).prepare_transfer_to_private(recipient).call(&mut env.private()); - + let hiding_point_slot: Field = NFT::at(nft_contract_address) + .prepare_transfer_to_private(recipient) + .call(&mut env.private()); // Finalize the transfer of the NFT (message sender owns the NFT in public) NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, hiding_point_slot).call( @@ -93,7 +95,9 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { // (For this specific test we could set a random value for the commitment and not do the call to `prepare...` // as the NFT owner check is before we use the value but that would made the test less robust against changes // in the contract.) - let hiding_point_slot: Field = NFT::at(nft_contract_address).prepare_transfer_to_private(not_owner).call(&mut env.private()); + let hiding_point_slot: Field = NFT::at(nft_contract_address) + .prepare_transfer_to_private(not_owner) + .call(&mut env.private()); // Try transferring someone else's public NFT env.impersonate(not_owner); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr index e83067023ee..bd4829003eb 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr @@ -64,7 +64,9 @@ pub unconstrained fn setup_mint_and_transfer_to_private( let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We transfer the public NFT to private. - NFT::at(nft_contract_address).transfer_to_private(owner, minted_token_id).call(&mut env.private()); + NFT::at(nft_contract_address).transfer_to_private(owner, minted_token_id).call( + &mut env.private(), + ); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. From 93ae40911a3613217e17ddfb9deb65b970c838c1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 24 Oct 2024 16:38:25 +0000 Subject: [PATCH 36/36] fmt 2 --- noir-projects/aztec-nr/aztec/src/prelude.nr | 9 ++- .../aztec-nr/aztec/src/utils/bytes.nr | 55 +++++++++++++------ 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/prelude.nr b/noir-projects/aztec-nr/aztec/src/prelude.nr index 4eaed773154..bfa103d66f8 100644 --- a/noir-projects/aztec-nr/aztec/src/prelude.nr +++ b/noir-projects/aztec-nr/aztec/src/prelude.nr @@ -5,11 +5,10 @@ pub use dep::protocol_types::{ }; pub use crate::{ state_vars::{ - map::Map, private_immutable::PrivateImmutable, private_mutable::PrivateMutable, - public_immutable::PublicImmutable, public_mutable::PublicMutable, private_set::PrivateSet, - shared_immutable::SharedImmutable, shared_mutable::SharedMutable, storage::Storable -}, - context::{PrivateContext, PackedReturns, FunctionReturns, PublicContext}, + map::Map, private_immutable::PrivateImmutable, private_mutable::PrivateMutable, + public_immutable::PublicImmutable, public_mutable::PublicMutable, private_set::PrivateSet, + shared_immutable::SharedImmutable, shared_mutable::SharedMutable, storage::Storable, + }, context::{PrivateContext, PackedReturns, FunctionReturns, PublicContext}, note::{ note_header::NoteHeader, note_interface::{NoteInterface, NullifiableNote}, note_getter_options::NoteGetterOptions, note_viewer_options::NoteViewerOptions, diff --git a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr index 58bbd233065..355ddad2b57 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/bytes.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/bytes.nr @@ -75,7 +75,8 @@ mod test { #[test] fn test_bytes_to_1_field() { let input = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, 30, 31, ]; let output = bytes_to_fields::<31, 1>(input); @@ -88,9 +89,11 @@ mod test { let output = fields_to_bytes::<31, 1>(input); assert_eq( - output, [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 - ] + output, + [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, + 24, 25, 26, 27, 28, 29, 30, 31, + ], ); } @@ -101,9 +104,13 @@ mod test { // Each field should occupy 31 bytes with the non-zero value being placed in the last one. assert_eq( - output, [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3 - ] + output, + [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 3, + ], ); } @@ -116,16 +123,21 @@ mod test { // field should occupy 1 byte. There is not information destruction here because the last field fits into // 1 byte. assert_eq( - output, [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 3 - ] + output, + [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 2, 3, + ], ); } #[test] fn test_bytes_to_2_fields() { let input = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, + 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, ]; let output = bytes_to_fields::<59, 2>(input); @@ -136,14 +148,18 @@ mod test { #[test] fn test_2_fields_to_bytes() { let input = [ - 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f, 0x202122232425262728292a2b2c2d2e2f303132333435363738393a3b + 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f, + 0x202122232425262728292a2b2c2d2e2f303132333435363738393a3b, ]; let output = fields_to_bytes::<62, 2>(input); assert_eq( - output, [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 0, 0, 0, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59 - ] + output, + [ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, + 24, 25, 26, 27, 28, 29, 30, 31, 0, 0, 0, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, + 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + ], ); } @@ -165,11 +181,16 @@ mod test { input3: [u64; 5], input4: [u32; 5], input5: [u16; 5], - input6: [u8; 5] + input6: [u8; 5], ) { let mut input = [0; 5]; for i in 0..5 { - input[i] = (input1[i] as Field * 2.pow_32(184)) + (input2[i] as Field * 2.pow_32(120)) + (input3[i] as Field * 2.pow_32(56)) + (input4[i] as Field * 2.pow_32(24)) + (input5[i] as Field * 2.pow_32(8)) + input6[i] as Field; + input[i] = (input1[i] as Field * 2.pow_32(184)) + + (input2[i] as Field * 2.pow_32(120)) + + (input3[i] as Field * 2.pow_32(56)) + + (input4[i] as Field * 2.pow_32(24)) + + (input5[i] as Field * 2.pow_32(8)) + + input6[i] as Field; } let output = fields_to_bytes::<155, 5>(input);