From 439c58cb21a8d877e1ffd74f9f80c948ecf10d7a Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 7 Jan 2025 15:53:42 +0000 Subject: [PATCH 01/15] Try to reproduce the bytecode size blowup. --- .../bytecode_size_regression/Nargo.toml | 7 ++ .../bytecode_size_regression/src/main.nr | 116 ++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml create mode 100644 test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr diff --git a/test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml b/test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml new file mode 100644 index 00000000000..f219724ea21 --- /dev/null +++ b/test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "bytecode_size_regression" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr b/test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr new file mode 100644 index 00000000000..9cdf50ddf45 --- /dev/null +++ b/test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr @@ -0,0 +1,116 @@ +// The code below is inspired by [compute_encrypted_log](https://github.com/AztecProtocol/aztec-packages/blob/b42756bc10175fea9eb60544759e9dbe41ae5e76/noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr#L111) +// which resulted in a bytecode size blowup when compiled to ACIR, see https://github.com/noir-lang/noir/issues/6929 +// The issue was around `encrypted_bytes[offset + i]` generating large amounts of gates, as per the `flamegraph.sh` tool in aztec-packages. +// The details around encryption and addresses have been stripped away, focusing on just copying bytes of equivalent size arrays. + +use std::aes128::aes128_encrypt; + +global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; +global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; +global EPH_PK_SIZE: u32 = 32; +global HEADER_SIZE: u32 = 48; +global OVERHEAD_PADDING: u32 = 15; +global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; +global PLAINTEXT_LENGTH_SIZE: u32 = 2; +global MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES: u32 = + ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - OVERHEAD_SIZE - PLAINTEXT_LENGTH_SIZE - 1 /* aes padding */; + +fn main( + eph_pk_bytes: [u8; EPH_PK_SIZE], + aes_secret: [u8; 32], + incoming_header_ciphertext: [u8; HEADER_SIZE], + extended_plaintext: [u8; MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES + PLAINTEXT_LENGTH_SIZE], + shift: u32, +) -> pub [u8; ENCRYPTED_PAYLOAD_SIZE_IN_BYTES] { + // unsafe { + // compute_encrypted_log_unconstrained( + // eph_pk_bytes, + // aes_secret, + // incoming_header_ciphertext, + // extended_plaintext, + // shift, + // ) + // } + compute_encrypted_log( + eph_pk_bytes, + aes_secret, + incoming_header_ciphertext, + extended_plaintext, + shift, + ) +} + +unconstrained fn compute_encrypted_log_unconstrained( + eph_pk_bytes: [u8; EPH_PK_SIZE], + aes_secret: [u8; 32], + incoming_header_ciphertext: [u8; HEADER_SIZE], + plaintext: [u8; MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES + PLAINTEXT_LENGTH_SIZE], + shift: u32, +) -> [u8; M] { + compute_encrypted_log( + eph_pk_bytes, + aes_secret, + incoming_header_ciphertext, + plaintext, + shift, + ) +} + +fn compute_encrypted_log( + eph_pk_bytes: [u8; EPH_PK_SIZE], + aes_secret: [u8; 32], + incoming_header_ciphertext: [u8; HEADER_SIZE], + plaintext: [u8; P], + shift: u32, +) -> [u8; M] { + let mut encrypted_bytes = [0; M]; + let mut offset = 0; + + // eph_pk + for i in 0..EPH_PK_SIZE { + encrypted_bytes[offset + i] = eph_pk_bytes[i]; + } + offset += EPH_PK_SIZE; + + // incoming_header + for i in 0..HEADER_SIZE { + encrypted_bytes[offset + i] = incoming_header_ciphertext[i]; + } + offset += HEADER_SIZE; + + // Padding. + offset += OVERHEAD_PADDING; + + // incoming_body + let incoming_body_ciphertext = compute_incoming_body_ciphertext(plaintext, aes_secret); + // Then we fill in the rest as the incoming body ciphertext + let size = M - offset; + + // NOTE: This made the bytecode size blowup disappear in aztec packages, + // but in this reproduction the size seems to be statically known regardless. + //let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING; + + assert_eq(size, incoming_body_ciphertext.len(), "ciphertext length mismatch"); + for i in 0..size { + // NOTE: Adding `shift` makes the index dynamic enough to force ACIR to use memory operations. + // Without `shift` it assigns directly to witnesses, which is good, but not what we wanted to reproduce. + encrypted_bytes[offset + i + shift] = incoming_body_ciphertext[i]; + } + + encrypted_bytes +} + +pub fn compute_incoming_body_ciphertext( + plaintext: [u8; P], + aes_secret: [u8; 32], +) -> [u8] { + let full_key = aes_secret; + let mut sym_key = [0; 16]; + let mut iv = [0; 16]; + + for i in 0..16 { + sym_key[i] = full_key[i]; + iv[i] = full_key[i + 16]; + } + aes128_encrypt(plaintext, iv, sym_key) +} From b84e84f6654cc47b4e82209aa3fa4ac20be7a17a Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 7 Jan 2025 15:54:41 +0000 Subject: [PATCH 02/15] Rename example --- .../Nargo.toml | 2 +- .../src/main.nr | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test_programs/compile_success_no_bug/{bytecode_size_regression => encrypted_log_regression}/Nargo.toml (71%) rename test_programs/compile_success_no_bug/{bytecode_size_regression => encrypted_log_regression}/src/main.nr (100%) diff --git a/test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml b/test_programs/compile_success_no_bug/encrypted_log_regression/Nargo.toml similarity index 71% rename from test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml rename to test_programs/compile_success_no_bug/encrypted_log_regression/Nargo.toml index f219724ea21..5c2623b3a1c 100644 --- a/test_programs/compile_success_no_bug/bytecode_size_regression/Nargo.toml +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "bytecode_size_regression" +name = "encrypted_log_regression" type = "bin" authors = [""] compiler_version = ">=0.31.0" diff --git a/test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr similarity index 100% rename from test_programs/compile_success_no_bug/bytecode_size_regression/src/main.nr rename to test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr From 9a3290372ac16db8538534c688e7ec4b0ce4b05c Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 12:41:47 +0000 Subject: [PATCH 03/15] Use a conditional variable to reproduce the blowup --- .../encrypted_log_regression/src/main.nr | 84 +++++++------------ 1 file changed, 32 insertions(+), 52 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index 9cdf50ddf45..e8d9bd27f34 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -20,39 +20,14 @@ fn main( aes_secret: [u8; 32], incoming_header_ciphertext: [u8; HEADER_SIZE], extended_plaintext: [u8; MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES + PLAINTEXT_LENGTH_SIZE], - shift: u32, + flag: bool, ) -> pub [u8; ENCRYPTED_PAYLOAD_SIZE_IN_BYTES] { - // unsafe { - // compute_encrypted_log_unconstrained( - // eph_pk_bytes, - // aes_secret, - // incoming_header_ciphertext, - // extended_plaintext, - // shift, - // ) - // } compute_encrypted_log( eph_pk_bytes, aes_secret, incoming_header_ciphertext, extended_plaintext, - shift, - ) -} - -unconstrained fn compute_encrypted_log_unconstrained( - eph_pk_bytes: [u8; EPH_PK_SIZE], - aes_secret: [u8; 32], - incoming_header_ciphertext: [u8; HEADER_SIZE], - plaintext: [u8; MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES + PLAINTEXT_LENGTH_SIZE], - shift: u32, -) -> [u8; M] { - compute_encrypted_log( - eph_pk_bytes, - aes_secret, - incoming_header_ciphertext, - plaintext, - shift, + flag, ) } @@ -61,40 +36,45 @@ fn compute_encrypted_log( aes_secret: [u8; 32], incoming_header_ciphertext: [u8; HEADER_SIZE], plaintext: [u8; P], - shift: u32, + flag: bool, ) -> [u8; M] { let mut encrypted_bytes = [0; M]; let mut offset = 0; - // eph_pk - for i in 0..EPH_PK_SIZE { - encrypted_bytes[offset + i] = eph_pk_bytes[i]; - } - offset += EPH_PK_SIZE; + // NOTE: Adding a conditional variable here results in the array being fully copied, item by item, + // in each iteration in the second loop that copies incoming_body_ciphertext into encrypted_bytes. + // If we move `flag` to be after the copying of the header, or just before the loop, then we don't + // get the item-by-item copying, but just a single variable gets read and a new array constructed + // in each iteration. + if flag { + // eph_pk + for i in 0..EPH_PK_SIZE { + encrypted_bytes[offset + i] = eph_pk_bytes[i]; + } + offset += EPH_PK_SIZE; - // incoming_header - for i in 0..HEADER_SIZE { - encrypted_bytes[offset + i] = incoming_header_ciphertext[i]; - } - offset += HEADER_SIZE; + // incoming_header + for i in 0..HEADER_SIZE { + encrypted_bytes[offset + i] = incoming_header_ciphertext[i]; + } + offset += HEADER_SIZE; - // Padding. - offset += OVERHEAD_PADDING; + // Padding. + offset += OVERHEAD_PADDING; - // incoming_body - let incoming_body_ciphertext = compute_incoming_body_ciphertext(plaintext, aes_secret); - // Then we fill in the rest as the incoming body ciphertext - let size = M - offset; + // incoming_body + let incoming_body_ciphertext = compute_incoming_body_ciphertext(plaintext, aes_secret); + // Then we fill in the rest as the incoming body ciphertext + let size = M - offset; - // NOTE: This made the bytecode size blowup disappear in aztec packages, - // but in this reproduction the size seems to be statically known regardless. - //let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING; + // NOTE: This made the bytecode size blowup disappear in aztec packages, + // but in this reproduction the size seems to be statically known regardless. + //let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING; - assert_eq(size, incoming_body_ciphertext.len(), "ciphertext length mismatch"); - for i in 0..size { - // NOTE: Adding `shift` makes the index dynamic enough to force ACIR to use memory operations. - // Without `shift` it assigns directly to witnesses, which is good, but not what we wanted to reproduce. - encrypted_bytes[offset + i + shift] = incoming_body_ciphertext[i]; + assert_eq(size, incoming_body_ciphertext.len(), "ciphertext length mismatch"); + for i in 0..size { + encrypted_bytes[offset + i] = incoming_body_ciphertext[i]; + } } encrypted_bytes From 8a80fb829ecc97b621af79b135e9175409f7089f Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 14:56:34 +0000 Subject: [PATCH 04/15] Return array from compute_incoming_body_ciphertext --- .../encrypted_log_regression/src/main.nr | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index 4911e07ab0b..f078aa69dda 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -46,17 +46,16 @@ fn compute_encrypted_log( // Depending on where we place the `flag` we either get the item-by-item copying (blowup), // or just a single array item gets read and a new array constructed in each iteration (no blowup). - // If the `flag` is here then it blows up. - //if flag { - - // eph_pk - for i in 0..EPH_PK_SIZE { - encrypted_bytes[offset + i] = eph_pk_bytes[i]; - } - offset += EPH_PK_SIZE; - // If the `flag` is here then it blows up. if flag { + // eph_pk + for i in 0..EPH_PK_SIZE { + encrypted_bytes[offset + i] = eph_pk_bytes[i]; + } + offset += EPH_PK_SIZE; + + // If the `flag` is here then it blows up. + // if flag { // incoming_header for i in 0..HEADER_SIZE { encrypted_bytes[offset + i] = incoming_header_ciphertext[i]; @@ -76,7 +75,7 @@ fn compute_encrypted_log( // NOTE: This made the bytecode size blowup disappear in aztec packages, // but in this reproduction the size seems to be statically known regardless. - //let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING; + // let size = M - 32 - HEADER_SIZE - OVERHEAD_PADDING; assert_eq(size, incoming_body_ciphertext.len(), "ciphertext length mismatch"); for i in 0..size { @@ -90,7 +89,7 @@ fn compute_encrypted_log( pub fn compute_incoming_body_ciphertext( plaintext: [u8; P], aes_secret: [u8; 32], -) -> [u8] { +) -> [u8; P + 16 - P % 16] { let full_key = aes_secret; let mut sym_key = [0; 16]; let mut iv = [0; 16]; From f8983e88371d37e5b3c300785f5608f343e6ce18 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 15:02:41 +0000 Subject: [PATCH 05/15] Remove the encryption parts, they don't make a difference --- .../encrypted_log_regression/src/main.nr | 43 ++++++------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index f078aa69dda..d44a2526759 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -3,39 +3,36 @@ // The issue was around `encrypted_bytes[offset + i]` generating large amounts of gates, as per the `flamegraph.sh` tool in aztec-packages. // The details around encryption and addresses have been stripped away, focusing on just copying bytes of equivalent size arrays. -use std::aes128::aes128_encrypt; - global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; global EPH_PK_SIZE: u32 = 32; global HEADER_SIZE: u32 = 48; global OVERHEAD_PADDING: u32 = 15; -global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; -global PLAINTEXT_LENGTH_SIZE: u32 = 2; -global MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES: u32 = - ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - OVERHEAD_SIZE - PLAINTEXT_LENGTH_SIZE - 1 /* aes padding */; +// global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; +// global PLAINTEXT_LENGTH_SIZE: u32 = 2; +// global MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES: u32 = +// ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - OVERHEAD_SIZE - PLAINTEXT_LENGTH_SIZE - 1 /* aes padding */; +global BODY_SIZE: u32 = + ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - EPH_PK_SIZE - HEADER_SIZE - OVERHEAD_PADDING; fn main( eph_pk_bytes: [u8; EPH_PK_SIZE], - aes_secret: [u8; 32], incoming_header_ciphertext: [u8; HEADER_SIZE], - extended_plaintext: [u8; MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES + PLAINTEXT_LENGTH_SIZE], + incoming_body_ciphertext: [u8; BODY_SIZE], flag: bool, ) -> pub [u8; ENCRYPTED_PAYLOAD_SIZE_IN_BYTES] { compute_encrypted_log( eph_pk_bytes, - aes_secret, incoming_header_ciphertext, - extended_plaintext, + incoming_body_ciphertext, flag, ) } -fn compute_encrypted_log( +fn compute_encrypted_log( eph_pk_bytes: [u8; EPH_PK_SIZE], - aes_secret: [u8; 32], incoming_header_ciphertext: [u8; HEADER_SIZE], - plaintext: [u8; P], + incoming_body_ciphertext: [u8; BODY_SIZE], flag: bool, ) -> [u8; M] { let mut encrypted_bytes = [0; M]; @@ -56,6 +53,7 @@ fn compute_encrypted_log( // If the `flag` is here then it blows up. // if flag { + // incoming_header for i in 0..HEADER_SIZE { encrypted_bytes[offset + i] = incoming_header_ciphertext[i]; @@ -66,10 +64,8 @@ fn compute_encrypted_log( offset += OVERHEAD_PADDING; // If the `flag` is here then it does not blow up. - // if flag { - + //if flag { // incoming_body - let incoming_body_ciphertext = compute_incoming_body_ciphertext(plaintext, aes_secret); // Then we fill in the rest as the incoming body ciphertext let size = M - offset; @@ -85,18 +81,3 @@ fn compute_encrypted_log( encrypted_bytes } - -pub fn compute_incoming_body_ciphertext( - plaintext: [u8; P], - aes_secret: [u8; 32], -) -> [u8; P + 16 - P % 16] { - let full_key = aes_secret; - let mut sym_key = [0; 16]; - let mut iv = [0; 16]; - - for i in 0..16 { - sym_key[i] = full_key[i]; - iv[i] = full_key[i + 16]; - } - aes128_encrypt(plaintext, iv, sym_key) -} From 239f6949ca512cf3e4b16ca9e8819e21dc9abcfb Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 15:15:10 +0000 Subject: [PATCH 06/15] Smaller numbers for smaller SSA --- .../encrypted_log_regression/src/main.nr | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index d44a2526759..72c6efc2c74 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -3,11 +3,12 @@ // The issue was around `encrypted_bytes[offset + i]` generating large amounts of gates, as per the `flamegraph.sh` tool in aztec-packages. // The details around encryption and addresses have been stripped away, focusing on just copying bytes of equivalent size arrays. -global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; -global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; -global EPH_PK_SIZE: u32 = 32; -global HEADER_SIZE: u32 = 48; -global OVERHEAD_PADDING: u32 = 15; +// Using the same formulas with smaller numbers; the effect is the same, but the SSA is more manageable. +global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 4; // Originally 18 +global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 5; // Originally 31 +global EPH_PK_SIZE: u32 = 3; // Originally 32 +global HEADER_SIZE: u32 = 2; // Originally 48 +global OVERHEAD_PADDING: u32 = 1; // Originally 15 // global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; // global PLAINTEXT_LENGTH_SIZE: u32 = 2; // global MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES: u32 = From b1743974fb40f7b77bce1451bbc8ac5654ef0f0d Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 15:57:56 +0000 Subject: [PATCH 07/15] Add another mem2reg before flattening to remove load and store --- compiler/noirc_evaluator/src/ssa.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 2500b8685a1..3018f510c8b 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -170,10 +170,11 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result Date: Wed, 8 Jan 2025 16:00:55 +0000 Subject: [PATCH 08/15] Using the original numbers to be sure --- .../encrypted_log_regression/src/main.nr | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index 72c6efc2c74..f8863f45ec8 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -3,16 +3,26 @@ // The issue was around `encrypted_bytes[offset + i]` generating large amounts of gates, as per the `flamegraph.sh` tool in aztec-packages. // The details around encryption and addresses have been stripped away, focusing on just copying bytes of equivalent size arrays. +// Original values which resulted in huge bytecode even on this example (500K long SSA) +global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; +global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; +global EPH_PK_SIZE: u32 = 32; +global HEADER_SIZE: u32 = 48; +global OVERHEAD_PADDING: u32 = 15; + // Using the same formulas with smaller numbers; the effect is the same, but the SSA is more manageable. -global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 4; // Originally 18 -global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 5; // Originally 31 -global EPH_PK_SIZE: u32 = 3; // Originally 32 -global HEADER_SIZE: u32 = 2; // Originally 48 -global OVERHEAD_PADDING: u32 = 1; // Originally 15 +// global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 4; +// global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 5; +// global EPH_PK_SIZE: u32 = 3; +// global HEADER_SIZE: u32 = 2; +// global OVERHEAD_PADDING: u32 = 1; + +// Unused because encryption didn't play a role: // global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; // global PLAINTEXT_LENGTH_SIZE: u32 = 2; // global MAX_PRIVATE_LOG_PLAINTEXT_SIZE_IN_BYTES: u32 = // ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - OVERHEAD_SIZE - PLAINTEXT_LENGTH_SIZE - 1 /* aes padding */; + global BODY_SIZE: u32 = ENCRYPTED_PAYLOAD_SIZE_IN_BYTES - EPH_PK_SIZE - HEADER_SIZE - OVERHEAD_PADDING; From 8a9652ed0d41c3d1cf310aed665d83fcb76a11f1 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 16:01:33 +0000 Subject: [PATCH 09/15] Check in with the small numbers; if we see a regression it will be in percentages --- .../encrypted_log_regression/src/main.nr | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr index f8863f45ec8..c65f580b0c8 100644 --- a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr +++ b/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr @@ -4,18 +4,18 @@ // The details around encryption and addresses have been stripped away, focusing on just copying bytes of equivalent size arrays. // Original values which resulted in huge bytecode even on this example (500K long SSA) -global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; -global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; -global EPH_PK_SIZE: u32 = 32; -global HEADER_SIZE: u32 = 48; -global OVERHEAD_PADDING: u32 = 15; +// global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 18; +// global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 31; +// global EPH_PK_SIZE: u32 = 32; +// global HEADER_SIZE: u32 = 48; +// global OVERHEAD_PADDING: u32 = 15; // Using the same formulas with smaller numbers; the effect is the same, but the SSA is more manageable. -// global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 4; -// global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 5; -// global EPH_PK_SIZE: u32 = 3; -// global HEADER_SIZE: u32 = 2; -// global OVERHEAD_PADDING: u32 = 1; +global PRIVATE_LOG_SIZE_IN_FIELDS: u32 = 4; +global ENCRYPTED_PAYLOAD_SIZE_IN_BYTES: u32 = (PRIVATE_LOG_SIZE_IN_FIELDS - 1) * 5; +global EPH_PK_SIZE: u32 = 3; +global HEADER_SIZE: u32 = 2; +global OVERHEAD_PADDING: u32 = 1; // Unused because encryption didn't play a role: // global OVERHEAD_SIZE: u32 = EPH_PK_SIZE + HEADER_SIZE + OVERHEAD_PADDING; From b086d5033814badbac6221420b38f3e8dfff2899 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 21:33:05 +0000 Subject: [PATCH 10/15] Only use the original bit size for upcasts --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 9 +++++++-- .../noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs | 5 ++--- test_programs/execution_success/sha2_byte/src/main.nr | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 8f1b360a120..90512951c61 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -416,10 +416,15 @@ impl DataFlowGraph { pub(crate) fn get_value_max_num_bits(&self, value: ValueId) -> u32 { match self[value] { Value::Instruction { instruction, .. } => { + let value_bit_size = self.type_of_value(value).bit_size(); if let Instruction::Cast(original_value, _) = self[instruction] { - self.type_of_value(original_value).bit_size() + let original_bit_size = self.type_of_value(original_value).bit_size(); + // We might have cast e.g. `u1` to `u8` to be able to do arithmetic, + // in which case we want to recover the original smaller bit size; + // OTOH if we cast down, then we don't need the higher original size. + value_bit_size.min(original_bit_size) } else { - self.type_of_value(value).bit_size() + value_bit_size } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 61d53cbd960..4fc3751de01 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -7,7 +7,7 @@ use crate::ssa::{ basic_block::BasicBlockId, call_stack::CallStackId, dfg::InsertInstructionResult, - function::{Function, RuntimeType}, + function::Function, instruction::{Binary, BinaryOp, Endian, Instruction, InstructionId, Intrinsic}, types::{NumericType, Type}, value::ValueId, @@ -32,7 +32,7 @@ impl Function { /// The structure of this pass is simple: /// Go through each block and re-insert all instructions. pub(crate) fn remove_bit_shifts(&mut self) { - if matches!(self.runtime(), RuntimeType::Brillig(_)) { + if self.runtime().is_brillig() { return; } @@ -120,7 +120,6 @@ impl Context<'_> { let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2), typ); let max_lhs_bits = self.function.dfg.get_value_max_num_bits(lhs); - (max_lhs_bits + bit_shift_size, pow) } else { // we use a predicate to nullify the result in case of overflow diff --git a/test_programs/execution_success/sha2_byte/src/main.nr b/test_programs/execution_success/sha2_byte/src/main.nr index 1b4bed1a643..a1663642c69 100644 --- a/test_programs/execution_success/sha2_byte/src/main.nr +++ b/test_programs/execution_success/sha2_byte/src/main.nr @@ -1,8 +1,8 @@ // Test Noir implementations of SHA256 and SHA512 on a one-byte message. fn main(x: Field, result256: [u8; 32], result512: [u8; 64]) { - let digest256 = std::sha256::digest([x as u8]); + let digest256 = std::hash::sha256([x as u8]); assert(digest256 == result256); - let digest512 = std::sha512::digest([x as u8]); + let digest512 = std::hash::sha512::digest([x as u8]); assert(digest512 == result512); } From 3e3b8856b51328852621e73b749cd080875eb760 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 21:42:40 +0000 Subject: [PATCH 11/15] Restrict truncation to 254 bits --- compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 4fc3751de01..30c66b2ccec 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -253,6 +253,9 @@ impl Context<'_> { bit_size: u32, max_bit_size: u32, ) -> ValueId { + // There is no point trying to truncate to more than the Field size. + // A higher `max_bit_size` input can come from trying to left-shift a Field. + let max_bit_size = max_bit_size.min(NumericType::NativeField.bit_size()); self.insert_instruction(Instruction::Truncate { value, bit_size, max_bit_size }, None) .first() } From 9ba164c7d835e7499ee1f721412b48ac4b53b8b9 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 8 Jan 2025 21:49:08 +0000 Subject: [PATCH 12/15] Restrict truncation to 254 bits in the formula --- .../noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 30c66b2ccec..0af8fbb0b5e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -120,7 +120,11 @@ impl Context<'_> { let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2), typ); let max_lhs_bits = self.function.dfg.get_value_max_num_bits(lhs); - (max_lhs_bits + bit_shift_size, pow) + let max_bit_size = max_lhs_bits + bit_shift_size; + // There is no point trying to truncate to more than the Field size. + // A higher `max_lhs_bits` input can come from trying to left-shift a Field. + let max_bit_size = max_bit_size.min(NumericType::NativeField.bit_size()); + (max_bit_size, pow) } else { // we use a predicate to nullify the result in case of overflow let u8_type = NumericType::unsigned(8); @@ -253,9 +257,6 @@ impl Context<'_> { bit_size: u32, max_bit_size: u32, ) -> ValueId { - // There is no point trying to truncate to more than the Field size. - // A higher `max_bit_size` input can come from trying to left-shift a Field. - let max_bit_size = max_bit_size.min(NumericType::NativeField.bit_size()); self.insert_instruction(Instruction::Truncate { value, bit_size, max_bit_size }, None) .first() } From 8dac3ba82ecbdf6f55d01eb6fe8ade38bf90c251 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 9 Jan 2025 11:33:34 +0000 Subject: [PATCH 13/15] Fix typo in directory name --- .../Nargo.toml | 0 .../src/main.nr | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test_programs/compile_success_no_bug/{check_uncostrained_regression => check_unconstrained_regression}/Nargo.toml (100%) rename test_programs/compile_success_no_bug/{check_uncostrained_regression => check_unconstrained_regression}/src/main.nr (100%) diff --git a/test_programs/compile_success_no_bug/check_uncostrained_regression/Nargo.toml b/test_programs/compile_success_no_bug/check_unconstrained_regression/Nargo.toml similarity index 100% rename from test_programs/compile_success_no_bug/check_uncostrained_regression/Nargo.toml rename to test_programs/compile_success_no_bug/check_unconstrained_regression/Nargo.toml diff --git a/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr b/test_programs/compile_success_no_bug/check_unconstrained_regression/src/main.nr similarity index 100% rename from test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr rename to test_programs/compile_success_no_bug/check_unconstrained_regression/src/main.nr From 13b58717e5a95174fb00d99d51b8b58a26c92740 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 9 Jan 2025 11:45:47 +0000 Subject: [PATCH 14/15] Move to be under execution_success --- .../encrypted_log_regression/Nargo.toml | 0 .../encrypted_log_regression/Prover.toml | 9 +++++++++ .../encrypted_log_regression/src/main.nr | 0 3 files changed, 9 insertions(+) rename test_programs/{compile_success_no_bug => execution_success}/encrypted_log_regression/Nargo.toml (100%) create mode 100644 test_programs/execution_success/encrypted_log_regression/Prover.toml rename test_programs/{compile_success_no_bug => execution_success}/encrypted_log_regression/src/main.nr (100%) diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/Nargo.toml b/test_programs/execution_success/encrypted_log_regression/Nargo.toml similarity index 100% rename from test_programs/compile_success_no_bug/encrypted_log_regression/Nargo.toml rename to test_programs/execution_success/encrypted_log_regression/Nargo.toml diff --git a/test_programs/execution_success/encrypted_log_regression/Prover.toml b/test_programs/execution_success/encrypted_log_regression/Prover.toml new file mode 100644 index 00000000000..7ca8c21a692 --- /dev/null +++ b/test_programs/execution_success/encrypted_log_regression/Prover.toml @@ -0,0 +1,9 @@ +# Using the smaller sizes defined in `main.nr`. +# The reason this program is in the `execution_success` directory is because +# `rebuild.sh` only goes over these programs, but all we really care about is +# any potential future bytecode size regression. +eph_pk_bytes = [1, 2, 3] +incoming_header_ciphertext = [1, 2] +incoming_body_ciphertext = [9, 8, 7, 6, 5, 4, 3, 2, 1] +flag = true +return = [1, 2, 3, 1, 2, 0, 9, 8, 7, 6, 5, 4, 3, 2, 1] diff --git a/test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr b/test_programs/execution_success/encrypted_log_regression/src/main.nr similarity index 100% rename from test_programs/compile_success_no_bug/encrypted_log_regression/src/main.nr rename to test_programs/execution_success/encrypted_log_regression/src/main.nr From 434315877d2fea00a08702bbe89fa4810e26a296 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 9 Jan 2025 12:03:53 +0000 Subject: [PATCH 15/15] . --- .../{.failures.jsonl.does_not_compile => .failures.jsonl} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/critical_libraries_status/noir-lang/noir_json_parser/{.failures.jsonl.does_not_compile => .failures.jsonl} (100%) diff --git a/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl.does_not_compile b/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl similarity index 100% rename from .github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl.does_not_compile rename to .github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl