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

CSUB-336 weighted VRFs #831

Merged
merged 9 commits into from
Jan 24, 2023
Merged

CSUB-336 weighted VRFs #831

merged 9 commits into from
Jan 24, 2023

Conversation

CAGS295
Copy link
Contributor

@CAGS295 CAGS295 commented Dec 23, 2022

Description of proposed changes:
This is an early implementation of the basics. I will be rebasing without notice until the interface is stabilized.
With this, you can generate a Transcript that you can use along a sr25519 public to generate a provable seed. The seed is extended with personal data to generate a random number. The provable seed can be verified publically and the result of such proof is the extended provable seed that can be used directly to generate a random number. The PR also contains a pseudo-probabilistic model to generate a threshold based on the stake and a sample ratio hyperparameter.

I also included rudimentary positive tests.

Non.exhaustive TODOs:

  • Stabilize interface.
  • Error handling.
  • specialize functions.
  • better parameters at the runtime_interface boundaries.

Practical tips for PR review & merge:

  • All GitHub Actions report PASS
  • Newly added code/functions have unit tests
    • Coverage tools report all newly added lines as covered
    • The positive scenario is exercised
    • Negative scenarios are exercised, e.g. assert on all possible errors
    • Assert on events triggered if applicable
    • Assert on changes made to storage if applicable
  • Modified behavior/functions - try to make sure above test items are covered
  • Integration tests are added if applicable/needed

@CAGS295 CAGS295 force-pushed the feature/CSUB-336-3 branch 2 times, most recently from 498f904 to edc74b6 Compare January 3, 2023 23:48
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

For full LLVM coverage report click here!

@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2023

Codecov Report

Merging #831 (cde7be9) into dev (af5da66) will increase coverage by 0.19%.
The diff coverage is 87.37%.

@@            Coverage Diff             @@
##              dev     #831      +/-   ##
==========================================
+ Coverage   78.26%   78.45%   +0.19%     
==========================================
  Files          68       68              
  Lines       10724    10856     +132     
==========================================
+ Hits         8393     8517     +124     
- Misses       2331     2339       +8     
Impacted Files Coverage Δ
pallets/difficulty/src/lib.rs 93.05% <ø> (ø)
primitives/src/lib.rs 93.33% <ø> (ø)
runtime/src/lib.rs 24.13% <0.00%> (+0.35%) ⬆️
primitives/src/vrf.rs 94.09% <89.00%> (-3.11%) ⬇️
sha3pow/src/lib.rs 94.38% <100.00%> (ø)
node/src/main.rs 16.66% <0.00%> (ø)
test/runtime-utils/src/lib.rs 100.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@atodorov atodorov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes requested.

I think I need some more context here before I can review the tests further.

primitives/src/vrf.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@atodorov atodorov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes to names of variables to make the tests more intuitive to understand.

primitives/src/vrf.rs Outdated Show resolved Hide resolved
primitives/src/vrf.rs Outdated Show resolved Hide resolved
primitives/src/vrf.rs Outdated Show resolved Hide resolved
primitives/src/vrf.rs Outdated Show resolved Hide resolved
primitives/src/vrf.rs Outdated Show resolved Hide resolved
primitives/src/vrf.rs Outdated Show resolved Hide resolved
atodorov
atodorov previously approved these changes Jan 11, 2023
@atodorov atodorov enabled auto-merge (rebase) January 16, 2023 15:16
Copy link
Contributor

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass at review, just have a few questions

primitives/src/vrf.rs Show resolved Hide resolved
primitives/src/vrf.rs Show resolved Hide resolved
primitives/src/vrf.rs Show resolved Hide resolved
Copy link
Contributor

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@atodorov atodorov merged commit dc257f1 into dev Jan 24, 2023
@atodorov atodorov deleted the feature/CSUB-336-3 branch January 24, 2023 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants