-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OrchardZSA backward compatibility using "if"s #25
Conversation
…ization to generate both vanilla and zsa circuit
@@ -59,6 +59,8 @@ where | |||
pub(super) generator_table: GeneratorTableConfig, | |||
/// An advice column configured to perform lookup range checks. | |||
lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>, | |||
/// FIXME: add a proper comment | |||
is_zsa_variant: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename option: "is_zsa_variant" --> "is_Q_private". Previously, Q was public in the old version, i.e., y_q = fixed_y_q. Now, Q is private, and y_q is assigned to x_p.prev.
The comment can be: is_Q_private is a boolean flag used to determine whether Q is a private point. When this flag is set to true, it indicates that Q is a private point, and y_Q is assigned to x_P. When this flag is set to false, it indicates that Q is a public point, and y_Q is assigned to fixed_y_q.
@@ -515,8 +515,12 @@ where | |||
Error, | |||
> { | |||
assert_eq!(self.M.sinsemilla_chip, message.chip); | |||
|
|||
// FIXME: it's not a breaking change because `blinding_factor` simply wraps `R.mul` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the problem?
@@ -181,6 +183,8 @@ where | |||
table_range_check_tag: lookup.3, | |||
}, | |||
lookup_config: range_check, | |||
// FIXME: consider passing is_zsa_enabled to `configure` function explicitly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the problem?
@@ -41,7 +41,11 @@ where | |||
), | |||
Error, | |||
> { | |||
let (offset, x_a, y_a) = self.public_initialization(region, Q)?; | |||
let (offset, x_a, y_a) = if self.config.is_zsa_variant { | |||
self.public_initialization_zsa(region, Q)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public_initialization_zsa --> private_initialization. I think the "private_initialization" would be better since Q is now a private point.
|
||
Ok((offset, x_a, y_a)) | ||
} | ||
|
||
#[allow(non_snake_case)] | ||
/// Assign the coordinates of the initial public point `Q` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public point Q
--> private point Q
. Not all comments are aligned with the update. Q is a private point in the updated version, whereas Q is a public point in the old version.
// FIXME: add a proper doc | ||
/// ZsaExtension | ||
#[derive(Eq, PartialEq, Debug, Clone, Copy)] | ||
pub struct ZsaExtension { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ZsaExtension" --> "lookup_extension" or "lookup_decomposition_extension". The updated version extend the lookup range check to include the optimized short range check on 4 and 5 bits.
running_sum: Column<Advice>, | ||
table_idx: TableColumn, | ||
table_range_check_tag: TableColumn, | ||
zsa: Option<ZsaExtension>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"zsa"--> "extended_lookup_inputs"
closed in favor of #31 |
This PR introduces modifications to the circuit configuration and generation routines to ensure backward compatibility in the Orchard crate for generating both vanilla and ZSA circuits.