rlp + tests #1
rlp + tests #1
Conversation
| builder.split_le(x, n) | ||
| } | ||
|
|
||
| pub fn bits_to_num<F: RichField + Extendable<D>, const D: usize>( |
There was a problem hiding this comment.
This appears to do the same thing as the le_sum gadget in plonky2/src/gadgets/split_base.rs
| res | ||
| } | ||
|
|
||
| pub fn less_than<F: RichField + Extendable<D>, const D: usize>( |
There was a problem hiding this comment.
These comparison operators could probably be rewritten using the range_check gadget
| x: Target, | ||
| shift: usize, | ||
| ) -> Target { | ||
| // nikko: Is this legit ? an IF depending on the witness seems |
There was a problem hiding this comment.
This seems fishy to me as well. I'm not familiar enough with Plonky2 to know what happens behind the scenes in the circuit for the true/false cases. Perhaps right_shift adds a single row with a trivial permutation in both cases, which could be OK (depending on the circuit config?). But even if it's OK I think it should be rewritten to remove the if statement. Such things don't belong in circuits!
There was a problem hiding this comment.
I think there are several opportunities to rewrite self-rolled functions using built-in plonky2 gadgets which are (hopefully) well-optimized, well-tested, and well-documented. We can compare the constraint counts of circuits using the self-rolled versions to using the built-ins to see if there is an improvement or not. Those optimizations can wait though...this code looks correct to me and will make a good base on which to iterate on. Good work!
|
Sounds good, I've created an issue for it https://github.com/Lagrange-Labs/mapreduce-plonky2/issues/4 |
This code and associated test comes mostly from the previous codebase from recursive-batch-proof repo. It's a light version of the RLP stuff.
There is still a bunch of warnings because some functions do get used in other modules so currently not really used.