Closed GhostOfGauss closed 2 years ago
I'd say its worth making a test where BRIDGE_SIZE
and INS
are different. As they are likely to be different in practice since we're using 2-2 and 16-2 join/split txes in Solidity.
We can have a bridge where the root set is of length 3 and the inputs are size 2. This shouldn't be tough to implement right?
:exclamation: No coverage uploaded for pull request base (
master@b3a8f9f
). Click here to learn what that means. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #169 +/- ##
=========================================
Coverage ? 66.49%
=========================================
Files ? 66
Lines ? 4539
Branches ? 0
=========================================
Hits ? 3018
Misses ? 1521
Partials ? 0
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b3a8f9f...2e0ca85. Read the comment docs.
I've changed the Vanchor tests to have a single 16-2 success test and I assume that all remaining tests are 2-2 transactions. This solves the issues that I was concerned with above. Please take a look at the 2 success tests (here and here). If there are no objections to the way they are written then I will change the failure tests to reflect the changes I made to those success tests.
A question on this TODO : we can get the field size in bits as F::size_in_bits()
. What should the bound on transaction size be? If there are only 2 outputs then having each amount \< (field size)/2 is sufficient, but if you wanted to make sure that no overflow could occur when someone combines 16 inputs then you should enforce outputs to be \< (field size)/16. Maybe it's a good idea to use F::size_in_bits() - 10
just to be conservative? That's still allowing for a huge transaction size, since it's something like a 244 bit number.
I believe 248 bits is what is used in other projects.
On Fri, Feb 25, 2022 at 12:46 PM Todd Norton @.***> wrote:
A question on this TODO https://github.com/webb-tools/arkworks-gadgets/blob/7f1e465feb3f03afa1c90ce6579a7580635e6777/arkworks-plonk-circuits/src/vanchor/mod.rs#L282 : we can get the field size in bits as F::size_in_bits(). What should the bound on transaction size be? If there are only 2 outputs then having each amount < (field size)/2 is sufficient, but if you wanted to make sure that no overflow could occur when someone combines 16 inputs then you should enforce outputs to be < (field size)/16. Maybe it's a good idea to use F::size_in_bits() - 10 just to be conservative? That's still allowing for a huge transaction size, since it's something like a 244 bit number.
— Reply to this email directly, view it on GitHub https://github.com/webb-tools/arkworks-gadgets/pull/169#issuecomment-1051063740, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADELLF6I3AFUZGHY5NIUBFLU466AFANCNFSM5PFG2OEA . You are receiving this because your review was requested.Message ID: @.***>
This adds the anchor and vanchor plonk circuits to the protocol, so it closes #86 and closes #87.
Aside from the anchor and vanchor there are a few new things:
gadget_tester
(see here). The reason is thatgadget_tester
has issues with failure tests. One thing I didn't like aboutgadget_tester
was that it lacks functionality for failure tests where the prover/verifier disagree on what the public input values are. The newprove_then_verify
function improves on this by adding an optionalverifier_public_inputs
argument. This is useful for testing things likeshould_fail_to_verify_wrong_chain_id
, for example.check_membership_is_enabled
function to use for dummy inputs to the vanchor circuit (see here).To-Do: the vanchor tests are still ugly and I would like to clean them up. My main question regarding these tests is
input_paths
in the obvious way would be to define somedefault_path
and initialize the array as[default_path; INS]
. ButPathGadget
s don't implement Copy and deriving Copy doesn't seem to work. So what I'm doing now is ugly, see here.INS = BRIDGE_SIZE
so that the root set is the same size as the number of inputs. I don't really know what I'd do to make the tests allow for those two parameters to be unequal. (Maybe it's not worth doing?)