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

@@ -1,3 +1,5 @@
+use crate::runtime::is_unconstrained;
+
 // Implementation of SHA-256 mapping a byte array of variable length to
 // 32 bytes.
 
@@ -32,21 +34,17 @@ fn msg_u8_to_u32(msg: [u8; 64]) -> [u32; 16] {
     msg32
 }
 
-unconstrained fn build_msg_block_iter<let N: u32>(
-    msg: [u8; N],
-    message_size: u64,
-    mut msg_block: [u8; 64],
-    msg_start: u32
-) -> ([u8; 64], u64) {
+unconstrained fn build_msg_block_iter<let N: u32>(msg: [u8; N], message_size: u64, msg_start: u32) -> ([u8; 64], u64) {
+    let mut msg_block: [u8; BLOCK_SIZE] = [0; BLOCK_SIZE];
     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,23 +58,27 @@ 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;
+    let mut extra_bytes = 0;
+    if msg_end > N {
+        msg_end = N;
+        extra_bytes = 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;
-            }
+        }
+    }
+
+    for i in 0..BLOCK_SIZE {
+        if i as u64 >= msg_byte_ptr {
+            assert_eq(msg_block[i], 0);
         } else {
-            // Need to assert over the msg block in the else case as well 
-            if N < 64 {
-                assert_eq(msg_block[msg_byte_ptr], 0);
-            } else {
-                assert_eq(msg_block[msg_byte_ptr], msg[k]);
-            }
+            assert_eq(msg_block[i], msg[msg_start + i - extra_bytes]);
         }
     }
+
     msg_byte_ptr
 }
 
@@ -89,31 +91,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 {
-        unsafe {
-            let (new_msg_block, new_msg_byte_ptr)  = build_msg_block_iter(msg, message_size, msg_block, 0);
-            msg_block = new_msg_block;
+    for i in 0..num_blocks {
+        let (new_msg_block, new_msg_byte_ptr) = unsafe {
+            build_msg_block_iter(msg, message_size, BLOCK_SIZE * i)
+        };
+        msg_block = new_msg_block;
+
+        if !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);
+        } else {
             msg_byte_ptr = new_msg_byte_ptr;
         }
 
-        if !crate::runtime::is_unconstrained() {
-            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, 0);
-        }
+        // Compress the block
+        h = sha256_compression(msg_u8_to_u32(msg_block), h);
     }
 
-    for i in 0..num_blocks {
-        unsafe {
-            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;
+    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 {
+        let (new_msg_block, new_msg_byte_ptr) = unsafe {
+            build_msg_block_iter(msg, message_size, BLOCK_SIZE * num_blocks)
+        };
+        msg_block = new_msg_block;
+
+        if !is_unconstrained() {
+            msg_byte_ptr = verify_msg_block(msg, message_size, msg_block, BLOCK_SIZE * num_blocks);
+        } else {
             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);
-        }
+    }
 
-        // 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;
@@ -154,15 +167,15 @@ pub fn sha256_var<let N: u32>(msg: [u8; N], message_size: u64) -> [u8; 32] {
         msg_byte_ptr = 0;
     }
 
-    unsafe {
-        msg_block = attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size);
-    }
+    msg_block = unsafe {
+        attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size)
+    };
 
     if !crate::runtime::is_unconstrained() {
         if msg_byte_ptr != 0 {
             for i in 0..64 {
                 if i as u64 < msg_byte_ptr - 1 {
-                    assert_eq(msg_block[i], last_block[i]);
+                    assert_eq(msg_block[i], msg[BLOCK_SIZE * num_blocks + i]);
                 }
             }
             assert_eq(msg_block[msg_byte_ptr - 1], 1 << 7);
@@ -205,11 +218,7 @@ unconstrained fn pad_msg_block(mut msg_block: [u8; 64], mut msg_byte_ptr: u64) -
     (msg_block, msg_byte_ptr)
 }
 
-unconstrained fn attach_len_to_msg_block(
-    mut msg_block: [u8; 64],
-    mut msg_byte_ptr: u64,
-    message_size: u64
-) -> [u8; 64] {
+unconstrained fn attach_len_to_msg_block(mut msg_block: [u8; 64], mut msg_byte_ptr: u64, message_size: u64) -> [u8; 64] {
     let len = 8 * message_size;
     let len_bytes = (len as Field).to_le_bytes(8);
     for _i in 0..64 {

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,5 @@
+# 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]
+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]
+#  Result matched against ethers library
+# result = [91, 122, 146, 93, 52, 109, 133, 148, 171, 61, 156, 70, 189, 238, 153, 7, 222, 184, 94, 24, 65, 114, 192, 244, 207, 199, 87, 232, 192, 224, 171, 207]
+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,4 @@
+fn main(headers: [u8; 472], result: pub [u8; 32]) {
+    let hash = std::hash::sha256_var(headers, headers.len() as u64);
+    assert_eq(hash, result);
+}