From 4d3c5d7252561929cddb30161844fd626f48af61 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 20 May 2022 11:13:52 +0800 Subject: [PATCH 1/5] fix tag byte overlap issue --- .../src/state_circuit/lexicographic_ordering.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 3a1f89fb55..80efab255f 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -319,7 +319,15 @@ fn rw_to_be_limbs(row: &Rw) -> Vec { // check that the first byte of id is not used, and overwrites it with packed // tags. assert_eq!(be_bytes[0], 0); - be_bytes[0] = row.field_tag().unwrap_or_default() as u8 + ((row.tag() as u8) << 4); + assert_eq!(be_bytes[1], 0); + let tag = row.tag() as u64; + let tag_packed_value = row.field_tag().unwrap_or_default() + tag << 5; + let tag_packed_bytes = tag_packed_value.to_le_bytes(); + be_bytes[0] = tag_packed_bytes[0]; + be_bytes[1] = tag_packed_bytes[1]; + if tag_packed_value > 255 { + be_bytes[0] = 255; + } be_bytes .iter() From 729bc58b0a30c1231b8e0734d6dfb3922a9cc1c3 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 20 May 2022 11:40:44 +0800 Subject: [PATCH 2/5] fix clippy --- zkevm-circuits/src/state_circuit/lexicographic_ordering.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 80efab255f..d20e96a2a0 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -59,7 +59,7 @@ use std::ops::Mul; // non-zero value, then we assign lower_limb_difference to be the value of C29. // Packing the field into 480 bits: -// 4 bits for tag, +// 5 bits for tag, // + 4 bits for field_tag // TODO: this actually needs 5 bits. Either reduce id // + 24 bits for id // to 23 bits, or add diff_3 etc. // + 160 bits for address, @@ -321,7 +321,7 @@ fn rw_to_be_limbs(row: &Rw) -> Vec { assert_eq!(be_bytes[0], 0); assert_eq!(be_bytes[1], 0); let tag = row.tag() as u64; - let tag_packed_value = row.field_tag().unwrap_or_default() + tag << 5; + let tag_packed_value = row.field_tag().unwrap_or_default() + (tag << 5); let tag_packed_bytes = tag_packed_value.to_le_bytes(); be_bytes[0] = tag_packed_bytes[0]; be_bytes[1] = tag_packed_bytes[1]; From b4dcfa262d1fe02852fcafe2665a5c122d40ec8a Mon Sep 17 00:00:00 2001 From: z2trillion Date: Fri, 20 May 2022 15:26:20 -0400 Subject: [PATCH 3/5] Update constraints and bit packing for 5 bit field tags --- .../state_circuit/lexicographic_ordering.rs | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index d20e96a2a0..8b9f109c80 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -59,9 +59,9 @@ use std::ops::Mul; // non-zero value, then we assign lower_limb_difference to be the value of C29. // Packing the field into 480 bits: -// 5 bits for tag, -// + 4 bits for field_tag // TODO: this actually needs 5 bits. Either reduce id -// + 24 bits for id // to 23 bits, or add diff_3 etc. +// 4 bits for tag, +// + 5 bits for field_tag +// + 24 bits for id // + 160 bits for address, // + 256 bits for storage key // + 32 bits for rw_counter @@ -280,7 +280,7 @@ impl Queries { } fn packed_tags(&self) -> Expression { - (1u64 << 4).expr() * self.tag.clone() + self.field_tag.clone() + (1u64 << 5).expr() * self.tag.clone() + self.field_tag.clone() } fn storage_key_be_limbs(&self) -> Vec> { @@ -302,33 +302,27 @@ impl Queries { .chain(self.rw_counter_limbs.iter().rev()) .cloned() .collect(); - // most significant byte of id should be 0, so safe to overwrite it with packed - // tags. - limbs[0] = limbs[0].clone() + self.packed_tags() * (1u64 << 8).expr(); + // most significant 9 bits of id are assumed be 0, so we can overwrite them with + // packed tags. + limbs[0] = limbs[0].clone() + self.packed_tags() * (1u64 << 7).expr(); limbs } } fn rw_to_be_limbs(row: &Rw) -> Vec { + let mut id = row.id().unwrap_or_default() as u32; + assert_eq!(id.to_be_bytes().len(), 4); + // check that the most significant 9 bits of id are 0, and pack tag and then + // field_tag into them. + assert!(id < (1 << 23)); + id += (((row.tag() as u32) << 5) + (row.field_tag().unwrap_or_default() as u32)) << 23; + let mut be_bytes = vec![]; - be_bytes.extend_from_slice(&(row.id().unwrap_or_default() as u32).to_be_bytes()); + be_bytes.extend_from_slice(&id.to_be_bytes()); be_bytes.extend_from_slice(&(row.address().unwrap_or_default().0)); be_bytes.extend_from_slice(&(row.storage_key().unwrap_or_default().to_be_bytes())); be_bytes.extend_from_slice(&((row.rw_counter() as u32).to_be_bytes())); - // check that the first byte of id is not used, and overwrites it with packed - // tags. - assert_eq!(be_bytes[0], 0); - assert_eq!(be_bytes[1], 0); - let tag = row.tag() as u64; - let tag_packed_value = row.field_tag().unwrap_or_default() + (tag << 5); - let tag_packed_bytes = tag_packed_value.to_le_bytes(); - be_bytes[0] = tag_packed_bytes[0]; - be_bytes[1] = tag_packed_bytes[1]; - if tag_packed_value > 255 { - be_bytes[0] = 255; - } - be_bytes .iter() .tuples() From f8314b0ebe9246744344c56f02c89ab327ac83d3 Mon Sep 17 00:00:00 2001 From: z2trillion Date: Mon, 23 May 2022 21:20:00 -0400 Subject: [PATCH 4/5] Update comments --- .../src/state_circuit/lexicographic_ordering.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 8b9f109c80..70818f1f62 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -61,7 +61,7 @@ use std::ops::Mul; // Packing the field into 480 bits: // 4 bits for tag, // + 5 bits for field_tag -// + 24 bits for id +// + 23 bits for id // + 160 bits for address, // + 256 bits for storage key // + 32 bits for rw_counter @@ -302,8 +302,8 @@ impl Queries { .chain(self.rw_counter_limbs.iter().rev()) .cloned() .collect(); - // most significant 9 bits of id are assumed be 0, so we can overwrite them with - // packed tags. + // The packed tags are shifted left by 7 bits so that they occupy the most + // significant 9 bits of the first 16-bit limb. limbs[0] = limbs[0].clone() + self.packed_tags() * (1u64 << 7).expr(); limbs } @@ -312,8 +312,8 @@ impl Queries { fn rw_to_be_limbs(row: &Rw) -> Vec { let mut id = row.id().unwrap_or_default() as u32; assert_eq!(id.to_be_bytes().len(), 4); - // check that the most significant 9 bits of id are 0, and pack tag and then - // field_tag into them. + // id is u32, but it is at most 2^23 - 1, so the 9 most significant bits will be + // 0. We overwrite these 9 bits with the tag and field tag. assert!(id < (1 << 23)); id += (((row.tag() as u32) << 5) + (row.field_tag().unwrap_or_default() as u32)) << 23; From 22a8bbb3e89ab2511957d72e7d585b6a3b521819 Mon Sep 17 00:00:00 2001 From: DreamWuGit Date: Fri, 27 May 2022 08:46:55 +0800 Subject: [PATCH 5/5] minor update for comment minor update for comment Co-authored-by: Haichen Shen --- zkevm-circuits/src/state_circuit/lexicographic_ordering.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 992a684e64..cba31c48a0 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -326,8 +326,8 @@ impl Queries { fn rw_to_be_limbs(row: &Rw) -> Vec { let mut id = row.id().unwrap_or_default() as u32; assert_eq!(id.to_be_bytes().len(), 4); - // id is u32, but it is at most 2^23 - 1, so the 9 most significant bits will be - // 0. We overwrite these 9 bits with the tag and field tag. + // The max value of `id` is 2^23 - 1, so the 9 most significant bits should be + // 0. We use these 9 bits to hold value of `tag` and `field_tag`. assert!(id < (1 << 23)); id += (((row.tag() as u32) << 5) + (row.field_tag().unwrap_or_default() as u32)) << 23;