Skip to content

Commit 731b626

Browse files
authored
fix(avm): sha256 tracegen batched tag checks (#19244)
When I implemented the batched tag checks, I made the invalid assumption the values would be within 64 bits ( 1-width bigger than the 32-bits that Sha256 uses). This can easily be shown to be wrong `(Tag::FF - Tag::U32 => p - 4)`, good thing the fuzzer caught it
1 parent 8d8ecf7 commit 731b626

File tree

1 file changed

+9
-5
lines changed

1 file changed

+9
-5
lines changed

barretenberg/cpp/src/barretenberg/vm2/tracegen/sha256_trace.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -404,17 +404,21 @@ void Sha256TraceBuilder::process(
404404

405405
if (invalid_state_tag_err) {
406406
// This is the more efficient batched tag check we perform in the circuit
407-
uint64_t batched_check = 0;
407+
FF batched_tag_check = 0;
408408
// Batch the state tag checks
409+
FF target_tag = FF(static_cast<uint8_t>(MemoryTag::U32));
409410
for (uint32_t i = 0; i < event.state.size(); i++) {
410-
batched_check |=
411-
(static_cast<uint64_t>(event.state[i].get_tag()) - static_cast<uint64_t>(MemoryTag::U32))
412-
<< (i * 3);
411+
// Compute the batched tag check step by step to match the circuit implementation
412+
FF mem_tag = FF(static_cast<uint8_t>(event.state[i].get_tag()));
413+
FF state_tag_diff = mem_tag - target_tag;
414+
FF exponent = FF(1 << (i * 3)); // exponent is 1, 8, 64, 512, ...
415+
batched_tag_check += state_tag_diff * exponent;
413416
}
414417
trace.set(row,
415418
{ {
416419
{ C::sha256_sel_invalid_state_tag_err, 1 },
417-
{ C::sha256_batch_tag_inv, FF(batched_check).invert() },
420+
// Guaranteed non-zero (so inversion is safe) since we have an invalid tag
421+
{ C::sha256_batch_tag_inv, FF(batched_tag_check).invert() },
418422
{ C::sha256_latch, 1 },
419423
{ C::sha256_err, 1 }, // Set the error flag
420424
} });

0 commit comments

Comments
 (0)