feat: Enhance boolean operations in bellpepper crate#55
feat: Enhance boolean operations in bellpepper crate#55huitseeker merged 3 commits intolurk-lang:mainfrom
Conversation
- Introduce a new module for variadic boolean utility functions,
- Implement new variadic versions of boolean OR and AND operations, i.e., `or_v`, `or_v_unchecked_for_optimization`, `and_v`.
- Integrate boolean `or!` and `and!` macros and add allocation function to verify zero values & arity.
- Conduct tests for the added and/or macros and various utility functions.
- Implemented a new `negative_num` module in the `bellpepper` crate's `gadgets` directory, which includes the function `allocate_is_negative` for determining number negativity. - Overhauled the module declarations order in `gadgets/mod.rs` for improved structuring.
1a1c759 to
7bfaded
Compare
|
|
||
| /// Allocate a Boolean which is true if and only if `num` is zero. | ||
| pub fn alloc_num_is_zero<CS: ConstraintSystem<F>, F: PrimeField>( | ||
| mut cs: CS, |
There was a problem hiding this comment.
If we are establishing a new set of gadgets, it may be worth taking a moment to consider unification of similar alternate approaches for two reasons:
- This may lead to the most coherent/efficient foundation.
- This may simplify achieving the goal of migrating downstream projects to use a common base.
To that end, let's take a moment to make an active decision. The current function (alloc_num_is_zero) can be implemented in terms of the (also important) alloc_equal_const (https://github.com/lurk-lab/lurk-rs/blob/dae0d6c478a83c80e3b3753c29f89e4ecd2a6313/src/circuit/gadgets/constraints.rs#L495) [UPDATE: actually alloc_num_is_zero operates on a Num — so my points here are not quite germane to this work. The general point stands as something we should sort out as part of a 'base gadgets' project, though.].
Arecibo, for path-dependent reasons associated with its own internal consistency has the slightly-differently-named alloc_num_equals_const: https://github.com/lurk-lab/arecibo/blob/6a9e8707e868068abf8c6b8da999443c3f3d0895/src/gadgets/utils.rs#L200-L237.
In addition to the slight naming difference, that function has a more concise (arguably simpler — if also less didactic) definition, itself derived from its alloc_num_equals (https://github.com/lurk-lab/arecibo/blob/6a9e8707e868068abf8c6b8da999443c3f3d0895/src/gadgets/utils.rs#L156-L197), which has a similar relationship to Lurk's equivalent alloc_num_equal (https://github.com/lurk-lab/lurk-rs/blob/dae0d6c478a83c80e3b3753c29f89e4ecd2a6313/src/circuit/gadgets/constraints.rs#L440-L492C2).
The point of this comment is to suggest we take a moment to actively decide:
- About names, do we want to use:
- the
lurk-rsnames; - the
nova/arecibonames; - some (potentially) more lexically-consistent names considered from the ground up to solve the larger design problem now faced.
- the
- About implementations, do we want to use:
- the
lurk-rsimplementations; - the
nova/areciboimplementations; - fresh implementations [note: probably not a good idea in this work]
- the
The reason this is worth considering is that any refactoring of both arecibo (which implies the hope for an eventual change in nova) and lurk-rs will have audit/correctness implications for at least one of those projects. We should seek to minimize global cost and maximize eventual global confidence in correctness with whatever we do here.
I don't think we need to overthink this, just make an active decision. I suggest there are only four reasonable choices, and all are more-or-less acceptable with the right argument:
lurk-rsnames and implementations (status quo of this PR)arecibonames and implementations (status quo if we had started from the premise of refactoring arecibo)lurk-rsnames andareciboimplementations (possible lessening of the lower-level crypto's auditing burden as a matter of policy, or in deference to the lowest-level's cryptographic weight).arecibonames andlurk-rsimplementations (in deference tolurk-rsand many of the gadgets in question more broadly than just this PR having undergone one round of audit).
I would say that the last option probably makes little sense. If the logic for lurk-rs implementations is strong, we should probably also use lurk-rs names. This is because lurk-rs names are probably the better choice anyway, and changes of naming for purpose of gadget standardization should be non-threatening to arecibo/nova.
In the end, I think my vote is for lurk-rs names and either implementation. If we know we will prefer one implementation over the other, it's probably best to initialize this work with that choice so we can follow a policy of not gratuitously changing implementations of gadgets — since this has a substantive effect on any circuit depending on them; making it a deeply breaking change in ways that are not instantly obvious.
porcuquine
left a comment
There was a problem hiding this comment.
I'm blocking this to leave space for us to consider the design question posed in my comment before merging anything.
porcuquine
left a comment
There was a problem hiding this comment.
Having started the discussion in #55 (comment) — and with the added context that this PR is primarily about the need to re-home these gadgets so they can be moved out of lurk-rs, I'm now approving this.
8b14765 to
1954dc0
Compare
1954dc0 to
d4fd9fc
Compare
or_v,or_v_unchecked_for_optimization,and_v. - Integrate booleanor!andand!macros and add allocation function to verify zero values & arity.Imported from lurk-lab/lurk-beta#874