microsoft / Nova

Nova: High-speed recursive arguments from folding schemes
MIT License
673 stars 176 forks source link

chore: Remove redundant absorption to the transcript #287

Closed storojs72 closed 6 months ago

storojs72 commented 6 months ago

Fixes https://github.com/microsoft/Nova/issues/284

storojs72 commented 6 months ago

@microsoft-github-policy-service agree company="LurkLab"

srinathsetty commented 6 months ago

Hi! Thanks for the PR! I'm a little confused about the diff. The simplest fix for the issue is to just remove the following two lines. Isn't that cleaner than what is proposed here? I'm not entirely sure why there should be any other edits (e.g., regarding the absorption of comm_L_row and comm_L_col).

Prover: https://github.com/microsoft/Nova/blob/fe5b93234ec9b972dde2d0406fbe2d1e9984acfc/src/spartan/ppsnark.rs#L1066 Verifier: https://github.com/microsoft/Nova/blob/fe5b93234ec9b972dde2d0406fbe2d1e9984acfc/src/spartan/ppsnark.rs#L1367

storojs72 commented 6 months ago

Right. But, let's consider verification flow. Why do we use this manually crafted slice, if later we create eval_vec with exactly same elements? So I thought that moving absorptions without violation of their order (&eval_vec.as_slice() first, and &vec![comm_L_row, comm_L_col].as_slice second) after introducing eval_vec would be more readable.