Frost to merge

[oxarbitrage]

The audit actions are of 2 types: Security and Others. This ticket is to keep track of the "Others". On each point we need a PR or an explication on why we consider no de change is needed.

Audit can be found in https://github.com/ZcashFoundation/redjubjub/blob/main/zcash-frost-audit-report-20210323.pdf

3.1 Potential API improvements

• Restrict the maximum number of signers and threshold parameters. At the moment these parameters are defined using u32 type. Consider using u8 or to put some hard-coded constant for the maximal intended numbers.

  Comments: We are actually using u8 now but this might change.

• Document what the library does in relation to how to import secret keys. This can be required in certain use cases. The trusted dealer will have a way to instead of randomly picking the secret key, to take existing one. - Document intended use for FROST #104

• Derive Debug , Eq , PartialEq for participants local public keys. This will allow implementers more freedom to work with public outputs of the key generation.

• Serialization and deserialization may be implemented for messages. It is hard to imagine how the code can be run over distributed network without such functionality.

  Comments: Work in progress, the design is Messages RFC #85 and an initial implementation PR is at Introduce messages and validation to FROST #95

Unused values

• The KeyPackage structure is never used, which leads to the warnings.

  Potential fix at Fix Unused values #99

3.2 Outdated dependencies

• The byteorder and rand_core crates are outdated versions (presumably, rand_core is on purpose pre-0.6). We suggest to update to the latest version after testing it, and potentially reviewing the changes.

  core_rand updated at Upgrade rand, rand_core, and rand_chacha together #55 and byteorder in Upgrade byteorder to 1.4 #100

3.3 Test coverage

• The amount of unit testing should be extended to better cover different success and failure scenarios, and generally better code coverage. For example, we recommend tests for various number of participants and thresholds, valid and invalid ones, and for different preprocessing scenarios.

3.4 unwrap() risks

• We recommend to be careful with unwrap() usage, as it could lead to panics upon None/Err . Instead, use pattern matching to gracefully fail.

  There is a PR with some work in this subject at Analysis on audit "3.4 unwrap() risks" #101

3.5 Null ID support

• When performing Shamir secret sharing, a polynomial f(x) is used to generate each party’s share of the secret. The actual secret is f(0) and the party with ID i will be given a share with value f(i) . Since a DKG may be implemented in the future, we recommend that the ID 0 be declared invalid.

3.6 Typo in a comment

• "langrange" instead of "lagrange" in

  redjubjub/src/frost.rs

  Line 492 in 2ebc08f

  /// Generates the langrange coefficient for the i'th participant.

  PR: Fix audit 3.6: Typo in a comment #97

[chelseakomlo]

As far as importing secret keys, we decided as a team that implementations can handle this functionality on their own. We do allow for serialization/deserialization of key material, which allows them to/restore save key material to/from disk. We might want to document this for users in the library documentation, though.