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

Have a plan for increasing commitment tree depth #821

Closed
daira opened this issue Apr 3, 2016 · 5 comments
Closed

Have a plan for increasing commitment tree depth #821

daira opened this issue Apr 3, 2016 · 5 comments
Labels
A-circuit Area: zk-SNARK circuits A-consensus Area: Consensus rules A-crypto Area: Cryptography C-future-proofing Category: Changes that minimise the effects of shocks and stresses of future events. I-SECURITY Problems and improvements related to security. M-requires-zip This change would need to be specified in a ZIP. NU1-sapling Network upgrade: Sapling-specific tasks S-blocking-scalability Status: Blocks on scalability R&D special to Daira

Comments

@daira
Copy link
Contributor

daira commented Apr 3, 2016

Desired properties:

  • maintain the anonymity set including all prior notes. This seems as though it requires being able to graft the old tree into the new one. If a different number of hashes is needed for each input note, the circuit must be given this as a private input.
  • minimise the design, security analysis, and implementation complexity of the system after the transition. Minimising analysis complexity requires for example being able to easily rule out hash-for-data and similar substitution attacks.

Having a concrete plan for how to do this could help to dispel potential FUD about the scalability of the number of Zcash notes. We could possibly aim to do this as an early hard fork.

@daira daira added I-SECURITY Problems and improvements related to security. A-crypto Area: Cryptography A-consensus Area: Consensus rules C-future-proofing Category: Changes that minimise the effects of shocks and stresses of future events. A-circuit Area: zk-SNARK circuits special to Daira engineering meeting agenda maybe in 2.0 and removed engineering meeting agenda labels Apr 3, 2016
@defuse
Copy link
Contributor

defuse commented Jun 8, 2016

For a not-completely-insane amount of money, an attacker can max out our network's transaction rate which will exhaust the tree earlier than we expect. We may think there are years of "normal use" left and de-prioritize the depth increase, when there may actually only be months of "attacker is flooding the network use" left. Let's not let that catch us off guard. See my analysis here: #994 (comment)

@ebfull
Copy link
Contributor

ebfull commented Jun 8, 2016

In an emergency, we can extend the depth of the tree by adding an additional accumulator. An attack begins and we add a new accumulator. Before the attack completes we hardfork to allow anchors to the new accumulator as well. I'm not concerned about a spam attack scenario as long as we have enough notice to implement that change.

If we aren't attacked in such a way, we can increase the depth when we change our tree's CRH.

@daira daira added the M-requires-zip This change would need to be specified in a ZIP. label Sep 29, 2016
@daira
Copy link
Contributor Author

daira commented Mar 12, 2017

In #2173 I describe how to expand the number of supported note commitments to ~246 without increasing the number of SHA256Compress hashes.

@daira daira added the NU1-sapling Network upgrade: Sapling-specific tasks label Apr 5, 2017
@daira
Copy link
Contributor Author

daira commented Aug 12, 2018

For Sapling we increased the depth from 29 to 32 (by creating a new commitment tree). This is essentially what @ebfull suggested above, although not prompted by an attack.

@str4d str4d added the S-blocking-scalability Status: Blocks on scalability R&D label Mar 11, 2019
str4d added a commit to str4d/zcash that referenced this issue Oct 21, 2020
c6b6b8f1b Merge zcash#830: Rip out non-endomorphism code + dependencies
c582abade Consistency improvements to the comments
63c6b7161 Reorder comments/function around scalar_split_lambda
2edc514c9 WNAF of lambda_split output has max size 129
4232e5b7d Rip out non-endomorphism code
ebad8414b Check correctness of lambda split without -DVERIFY
fe7fc1fda Make lambda constant accessible
9d2f2b44d Add tests to exercise lambda split near bounds
9aca2f7f0 Add secp256k1_split_lambda_verify
acab934d2 Detailed comments for secp256k1_scalar_split_lambda
76ed922a5 Increase precision of g1 and g2
6173839c9 Switch to our own memcmp function
63150ab4d Merge zcash#827: Rename testrand functions to have test in name
c5257aed0 Merge zcash#821: travis: Explicitly set --with-valgrind
bb1f54280 Merge zcash#818: Add static assertion that uint32_t is unsigned int or wider
a45c1fa63 Rename testrand functions to have test in name
5006895bd Merge zcash#808: Exhaustive test improvements + exhaustive schnorrsig tests
4eecb4d6e travis: VALGRIND->RUN_VALGRIND to avoid confusion with WITH_VALGRIND
66a765c77 travis: Explicitly set --with-valgrind
d7838ba6a Merge zcash#813: Enable configuring Valgrind support
7ceb0b761 Merge zcash#819: Enable -Wundef warning
8b7dcdd95 Add exhaustive test for extrakeys and schnorrsig
08d7d8929 Make pubkey parsing test whether points are in the correct subgroup
87af00b51 Abstract out challenge computation in schnorrsig
63e1b2aa7 Disable output buffering in tests_exhaustive.c
39f67dd07 Support splitting exhaustive tests across cores
e99b26fcd Give exhaustive_tests count and seed cmdline inputs
49e6630bc refactor: move RNG seeding to testrand
b110c106f Change exhaustive test groups so they have a point with X=1
cec7b18a3 Select exhaustive lambda in function of order
78f6cdfaa Make the curve B constant a secp256k1_fe
d7f39ae4b Delete gej_is_valid_var: unused outside tests
8bcd78cd7 Make secp256k1_scalar_b32 detect overflow in scalar_low
c498366e5 Move exhaustive tests for recovery to module
be3179154 Make group order purely compile-time in exhaustive tests
e73ff3092 Enable -Wundef warning
c0041b5cf Add static assertion that uint32_t is unsigned int or wider
4ad408faf Merge zcash#782: Check if variable=yes instead of if var is set in travis.sh
412bf874d configure: Allow specifying --with[out]-valgrind explicitly
34debf7a6 Modify .travis.yml to explictly pass no in env vars instead of setting to nothing
a0e99fc12 Merge zcash#814: tests: Initialize random group elements fully
5738e8622 tests: Initialize random group elements fully
c9939ba55 Merge zcash#812: travis: run bench_schnorrsig
a51f2af62 travis: run bench_schnorrsig
ef37761fe Change travis.sh to check if variables are equal to yes instead of not-empty. Before this, setting `VALGRIND=wat` was considered as true, and to make it evaluate as false you had to unset the variable `VALGRIND=` but not it checks if `VALGRIND=yes` and if it's not `yes` then it's evaluated to false

git-subtree-dir: src/secp256k1
git-subtree-split: c6b6b8f1bb044d7d1aa065ebb674adde98a36a8e
@nuttycom
Copy link
Contributor

The plan is to decide on a new commitment tree depth for each new shielded protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-circuit Area: zk-SNARK circuits A-consensus Area: Consensus rules A-crypto Area: Cryptography C-future-proofing Category: Changes that minimise the effects of shocks and stresses of future events. I-SECURITY Problems and improvements related to security. M-requires-zip This change would need to be specified in a ZIP. NU1-sapling Network upgrade: Sapling-specific tasks S-blocking-scalability Status: Blocks on scalability R&D special to Daira
Projects
None yet
Development

No branches or pull requests

6 participants