Skip to content
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

Modify LTChip to take Value<F> instead of F #38

Closed
2 tasks done
enricobottazzi opened this issue Jun 5, 2023 · 0 comments · Fixed by #40
Closed
2 tasks done

Modify LTChip to take Value<F> instead of F #38

enricobottazzi opened this issue Jun 5, 2023 · 0 comments · Fixed by #40
Assignees

Comments

@enricobottazzi
Copy link
Member

enricobottazzi commented Jun 5, 2023

The bug has been discussed here

The root cause of the problem is the design of the assign function from the LTChip gadget from the zkevm.

As you can see the witness assignment function takes as input lhs and rhs of typeF. Because of that this trick had to be performed in order to extract F from an assigned cell as pass it to the chip.

total_assets_cell
.value()
.zip(computed_sum_cell.value())
.map(|(total_assets, computed_sum)| {
if let Err(e) = chip.assign(
&mut region,
0,
computed_sum.to_owned(),
total_assets.to_owned(),
) {
println!("Error: {:?}", e);
};
});

The problem of such implementation is that a circuit assignment function is conditional on a witness value being Some. This is dangerous because "the keygen synthesises the circuit without having access to the witness; whereas the prover runs synthesis with the witness. Since this closure is conditioned on witness values being Some, it will only execute in the prover, meaning we would end up with an inconsistent circuit structure cf. keygen."

  • Create PR to Zkevm
  • Modify our circuit accordingly
@enricobottazzi enricobottazzi self-assigned this Jun 5, 2023
@enricobottazzi enricobottazzi moved this to Backlog in ∑ Summa Jun 5, 2023
@enricobottazzi enricobottazzi added this to the Chip readiness milestone Jun 5, 2023
@enricobottazzi enricobottazzi moved this from Backlog to In Progress in ∑ Summa Jun 5, 2023
@enricobottazzi enricobottazzi linked a pull request Jun 7, 2023 that will close this issue
@enricobottazzi enricobottazzi moved this from In Progress to Under Review in ∑ Summa Jun 7, 2023
@github-project-automation github-project-automation bot moved this from Under Review to Merged in ∑ Summa Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant