Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Gadgets that works with naive main gate #71

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from
Open

Conversation

saitima
Copy link
Member

@saitima saitima commented Aug 8, 2024

What ❔

This PR adds some gadgets that can works with a main gate that has only 3 variables: qAA + qBB + qABAB + qC*C + qConst

Why ❔

It allowed to keep number of the polynomials in the fflonk implementation smaller so that cheaper evm verification cost.

Checklist

  • [ x] PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • [ x] Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@saitima saitima requested a review from shamatar August 8, 2024 20:03
minus_one.negate();

let a_minus_b = a.sub(cs, &b)?;
let a_minus_b_cond = Self::mask(cs, &a_minus_b, &condition)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't address the fact that such multiplication can be merged with naive gate's multiplication

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be addressed by latest commit. @shamatar


assert_eq!(used_constant, true);

if used_constant == false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'll be checking the layout above, this line looks very invalid (compare to false after assert to true).

Also, you should never allocate constants (this feature is only needed for lookups), and instead add into gate.

Also, starting a sum with allocating zero variable is also not the most rational thing, you can just take first element of intermediate sums

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants