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

DKG and Randomness Beacon pallets #7820

Closed
mike1729 opened this issue Jan 4, 2021 · 5 comments
Closed

DKG and Randomness Beacon pallets #7820

mike1729 opened this issue Jan 4, 2021 · 5 comments
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known.

Comments

@mike1729
Copy link

mike1729 commented Jan 4, 2021

Together with my colleagues, we implemented a pallet for Distributed Key Generation (DKG) and a randomness beacon pallet using keys generated by the DKG. Our work was founded by a web3 grant.

Here is a description of the architecture of the dkg pallet, here of the randomness beacon pallet, here are some slides explaining a high level overview of both pallets, and here are some benchmarks results and further details on how to use the pallets.

This is our first-time working with substrate, so while we did our best to learn it from your excellent docs, by reading your high quality code, and asking questions on riot, we are sure that our architecture and code is not perfect, and would love to fix/adjust whatever you believe is missing or not optimal.

The main question is however, whether you are at all interested in merging such pallets. Possibly just the DKG pallet? (There is no point in merging just randomness beacon as it needs DKG for the setup.)
If you would be interested in merging, but require us to make even substantial changes, then we would be more than happy to introduce them.

I assume you are well aware of the benefits of such general purpose pallets as DKG and randomness beacon but let me recall some of the most popular ones: threshold cryptography, privacy preserving messaging services, e-voting protocols, gambling and lottery services.

@github-actions github-actions bot added the J2-unconfirmed Issue might be valid, but it’s not yet known. label Jan 4, 2021
@bkchr
Copy link
Member

bkchr commented Jan 4, 2021

I think from a high level we don't want to merge external pallets. The problem with that is that it increases our maintenance load and that is already relative high.

However, I think that we can find someone who can give you some tips on what can be improved.

CC @shawntabrizi @danforbes

@danforbes
Copy link
Contributor

I have conferred with @shawntabrizi on this topic and we are in agreement with @bkchr; in fact, I would amplify his statement by adding that we intend on minimizing the number of pallets in the Substrate frame directory. The mental model I would assume may be similar to C++, where the std library provides a relatively narrowly defined set of capabilities while making room for community-maintained "add-ons" like Boost (I don't know if there would ever be an exact analogue for Boost because it's pretty fully-featured, but think in terms of individual pallets providing those types of capabilities). Community-maintained FRAME pallets may be exposed via package manager-type services, such as Substrate Marketplace (still in staging). The Open Runtime Module Library (ORML) is a great example of a set of community-maintained pallets, and in fact may be the closest thing we have to Boost. You may even want to consider transferring this Issue to the ORML repo, to see if its maintainers would be interested in including your pallet(s). For now, I am going to close this Issue; please feel free to reopen it if you believe I have closed it in haste, or add comments if anyone disagrees w/ anything I've said. Thank you very much for your interest in contributing, @mike1729 🙏🏻 I would also be very happy to list your pallet(s) on Awesome Substrate 😎 if you'd care to make a PR.

@mike1729
Copy link
Author

mike1729 commented Jan 7, 2021

Thanks for the exhaustive answer. If you consider substrate as std lib in C++ then indeed it makes little sense to integrate external pallets that don't provide basic functionality. We weren't aware of the existence of neither the Marketplace nor the ORML, so thanks a lot for bringing them to our attention!

We'll try our luck in ORML then and will make a PR to the awesome list:) Thanks!

@burdges
Copy link

burdges commented Jan 7, 2021

I'd suggest basing production randomness beacons off some future production version of https://github.com/kobigurk/aggregatable-dkg which implements https://eprint.iacr.org/2021/005.pdf

I'd expect this could could provide a framework for their crypto of course, but some issues present themselves:

Although 2021/005 massively increases the viable DKG size, I'm still unsure really huge DKG sized become possible, so one might still require the fibering ideas from RandHound. I guess this is future work since maybe not required.

It appears this code produces only a VUF not a VRF because it fails to hash the output and inputs together. We made this mistake in BABE like twice, so yeah everyone does this their first time. It's partially because academics have until 2021/005 employed a crappy formulation of VRF in their papers.

It takes block hashes as VUF input, which gives the handy property of binding randomness to chain content, but enables bias if compromised, and permits chain fork manipulation if not. We could discuss this further but it's actually hard to exploit that randomness is tied to chain content so if I did not require that property then I'd restrict the input the previous output. In that way, randomness proceeds unchangably until you rerun the DKG, which presumably you do one epoch in advance.

As for the crypto in mike's code, I'll warn that https://github.com/Cardinal-Cryptography/substrate/blob/randomness-beacon-m3/primitives/dkg/src/threshold_signatures.rs#L256 should be replaced with a call to unimplemented!() or producing a point by any desired means, like even slow ones like guessing x-coordinates. As is, someone might use it, but it admits forgeries.

@burdges
Copy link

burdges commented Feb 5, 2021

I only looked superficially but https://github.com/Cardinal-Cryptography/substrate/blob/randomness-beacon-m3/primitives/dkg/src/threshold_signatures.rs does not look like any VSS scheme.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known.
Projects
None yet
Development

No branches or pull requests

4 participants