fix(sha256): Fix upper bound when building msg block and delay final block compression under certain cases

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -1435,7 +1435,6 @@ impl<F: AcirField> AcirContext<F> {
             name,
             BlackBoxFunc::MultiScalarMul
                 | BlackBoxFunc::Keccakf1600
-                | BlackBoxFunc::Sha256Compression
                 | BlackBoxFunc::Blake2s
                 | BlackBoxFunc::Blake3
                 | BlackBoxFunc::AND

noir_stdlib/src/hash/sha256.nr

@@ -39,14 +39,14 @@ unconstrained fn build_msg_block_iter<let N: u32>(
     msg_start: u32
 ) -> ([u8; 64], u64) {
     let mut msg_byte_ptr: u64 = 0; // Message byte pointer
-    for k in msg_start..N {
+    let mut msg_end = msg_start + BLOCK_SIZE;
+    if msg_end > N {
+        msg_end = N;
+    }
+    for k in msg_start..msg_end {
         if k as u64 < message_size {
             msg_block[msg_byte_ptr] = msg[k];
             msg_byte_ptr = msg_byte_ptr + 1;
-
-            if msg_byte_ptr == 64 {
-                msg_byte_ptr = 0;
-            }
         }
     }
     (msg_block, msg_byte_ptr)
@@ -60,17 +60,17 @@ fn verify_msg_block<let N: u32>(
     msg_start: u32
 ) -> u64 {
     let mut msg_byte_ptr: u64 = 0; // Message byte pointer
-    for k in msg_start..N {
+    let mut msg_end = msg_start + BLOCK_SIZE;
+    if msg_end > N {
+        msg_end = N;
+    }
+    for k in msg_start..msg_end {
         if k as u64 < message_size {
             assert_eq(msg_block[msg_byte_ptr], msg[k]);
             msg_byte_ptr = msg_byte_ptr + 1;
-            if msg_byte_ptr == 64 {
-                // Enough to hash block
-                msg_byte_ptr = 0;
-            }
         } else {
             // Need to assert over the msg block in the else case as well 
-            if N < 64 {
+            if N < BLOCK_SIZE {
                 assert_eq(msg_block[msg_byte_ptr], 0);
             } else {
                 assert_eq(msg_block[msg_byte_ptr], msg[k]);
@@ -89,31 +89,42 @@ pub fn sha256_var<let N: u32>(msg: [u8; N], message_size: u64) -> [u8; 32] {
     let mut h: [u32; 8] = [1779033703, 3144134277, 1013904242, 2773480762, 1359893119, 2600822924, 528734635, 1541459225]; // Intermediate hash, starting with the canonical initial value
     let mut msg_byte_ptr = 0; // Pointer into msg_block
 
-    if num_blocks == 0 {
+    for i in 0..num_blocks {
         unsafe {
-            let (new_msg_block, new_msg_byte_ptr)  = build_msg_block_iter(msg, message_size, msg_block, 0);
+            let (new_msg_block, new_msg_byte_ptr)  = build_msg_block_iter(msg, message_size, msg_block, BLOCK_SIZE * i);
             msg_block = new_msg_block;
             msg_byte_ptr = new_msg_byte_ptr;
         }
 
         if !crate::runtime::is_unconstrained() {
-            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, 0);
+            assert(msg_byte_ptr <= BLOCK_SIZE as u64);
+            // Verify the block we are compressing was appropriately constructed
+            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, BLOCK_SIZE * i);
         }
+
+        // Compress the block
+        h = sha256_compression(msg_u8_to_u32(msg_block), h);
     }
 
-    for i in 0..num_blocks {
+    let modulo = N % BLOCK_SIZE;
+    // Handle setup of the final msg block.
+    // This case is only hit if the msg is less than the block size, 
+    // or our message cannot be evenly split into blocks. 
+    if modulo != 0 {
         unsafe {
-            let (new_msg_block, new_msg_byte_ptr)  = build_msg_block_iter(msg, message_size, msg_block, BLOCK_SIZE * i);
+            let (new_msg_block, new_msg_byte_ptr)  = build_msg_block_iter(msg, message_size, msg_block, BLOCK_SIZE * num_blocks);
             msg_block = new_msg_block;
             msg_byte_ptr = new_msg_byte_ptr;
         }
+
         if !crate::runtime::is_unconstrained() {
-            // Verify the block we are compressing was appropriately constructed
-            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, BLOCK_SIZE * i);
+            assert(msg_byte_ptr <= BLOCK_SIZE as u64);
+            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, BLOCK_SIZE * num_blocks);
         }
+    }
 
-        // Hash the block
-        h = sha256_compression(msg_u8_to_u32(msg_block), h);
+    if msg_byte_ptr == BLOCK_SIZE as u64 {
+        msg_byte_ptr = 0;
     }
 
     let last_block = msg_block;

test_programs/execution_success/sha256_regression_5836/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "sha256_regression_5836"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.33.0"
+
+[dependencies]

test_programs/execution_success/sha256_regression_5836/Prover.toml

@@ -0,0 +1,2 @@
+# Result matched against ethers library
+result = [112, 144, 73, 182, 208, 98, 9, 238, 54, 229, 61, 145, 222, 17, 72, 62, 148, 222, 186, 55, 192, 82, 220, 35, 66, 47, 193, 200, 22, 38, 26, 186]

test_programs/execution_success/sha256_regression_5836/src/main.nr

@@ -0,0 +1,7 @@
+fn main(result: [u8; 32]) {
+    let headers = [
+        102, 114, 111, 109, 58, 114, 117, 110, 110, 105, 101, 114, 46, 108, 101, 97, 103, 117, 101, 115, 46, 48, 106, 64, 105, 99, 108, 111, 117, 100, 46, 99, 111, 109, 13, 10, 99, 111, 110, 116, 101, 110, 116, 45, 116, 121, 112, 101, 58, 116, 101, 120, 116, 47, 112, 108, 97, 105, 110, 59, 32, 99, 104, 97, 114, 115, 101, 116, 61, 117, 115, 45, 97, 115, 99, 105, 105, 13, 10, 109, 105, 109, 101, 45, 118, 101, 114, 115, 105, 111, 110, 58, 49, 46, 48, 32, 40, 77, 97, 99, 32, 79, 83, 32, 88, 32, 77, 97, 105, 108, 32, 49, 54, 46, 48, 32, 92, 40, 51, 55, 51, 49, 46, 53, 48, 48, 46, 50, 51, 49, 92, 41, 41, 13, 10, 115, 117, 98, 106, 101, 99, 116, 58, 72, 101, 108, 108, 111, 13, 10, 109, 101, 115, 115, 97, 103, 101, 45, 105, 100, 58, 60, 56, 70, 56, 49, 57, 68, 51, 50, 45, 66, 54, 65, 67, 45, 52, 56, 57, 68, 45, 57, 55, 55, 70, 45, 52, 51, 56, 66, 66, 67, 52, 67, 65, 66, 50, 55, 64, 109, 101, 46, 99, 111, 109, 62, 13, 10, 100, 97, 116, 101, 58, 83, 97, 116, 44, 32, 50, 54, 32, 65, 117, 103, 32, 50, 48, 50, 51, 32, 49, 50, 58, 50, 53, 58, 50, 50, 32, 43, 48, 52, 48, 48, 13, 10, 116, 111, 58, 122, 107, 101, 119, 116, 101, 115, 116, 64, 103, 109, 97, 105, 108, 46, 99, 111, 109, 13, 10, 100, 107, 105, 109, 45, 115, 105, 103, 110, 97, 116, 117, 114, 101, 58, 118, 61, 49, 59, 32, 97, 61, 114, 115, 97, 45, 115, 104, 97, 50, 53, 54, 59, 32, 99, 61, 114, 101, 108, 97, 120, 101, 100, 47, 114, 101, 108, 97, 120, 101, 100, 59, 32, 100, 61, 105, 99, 108, 111, 117, 100, 46, 99, 111, 109, 59, 32, 115, 61, 49, 97, 49, 104, 97, 105, 59, 32, 116, 61, 49, 54, 57, 51, 48, 51, 56, 51, 51, 55, 59, 32, 98, 104, 61, 55, 120, 81, 77, 68, 117, 111, 86, 86, 85, 52, 109, 48, 87, 48, 87, 82, 86, 83, 114, 86, 88, 77, 101, 71, 83, 73, 65, 83, 115, 110, 117, 99, 75, 57, 100, 74, 115, 114, 99, 43, 118, 85, 61, 59, 32, 104, 61, 102, 114, 111, 109, 58, 67, 111, 110, 116, 101, 110, 116, 45, 84, 121, 112, 101, 58, 77, 105, 109, 101, 45, 86, 101, 114, 115, 105, 111, 110, 58, 83, 117, 98, 106, 101, 99, 116, 58, 77, 101, 115, 115, 97, 103, 101, 45, 73, 100, 58, 68, 97, 116, 101, 58, 116, 111, 59, 32, 98, 61
+    ];
+    let hash = std::hash::sha256_var(headers, headers.len() as u64);
+    assert_eq(hash, result);
+}