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.

Sign up for GitHub

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

Jump to bottom

Create a cache for BLS #453

Merged

matt-o-how merged 58 commits into main from bls_cache_rebased

May 14, 2024

Contributor

matt-o-how commented Apr 2, 2024 •

edited

Loading

Creates a BLSCache struct and Py bindings for it.
Also adds a requirements.txt

matt-o-how requested a review from arvidn

April 2, 2024 15:55

socket-security bot commented Apr 2, 2024 •

edited

Loading

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

Contributor

arvidn commented Apr 3, 2024

@matt-o-how I imagine the security warnings comes from just adding the requirements.txt. I don't see the dependency graph for those wheels though. Do we need pluggy on there? I imagine most of them come via pytest though

matt-o-how force-pushed the bls_cache_rebased branch from a6c238b to 6b58212 Compare

April 5, 2024 15:00

matt-o-how requested a review from Rigidity

April 5, 2024 16:05

coveralls-official bot commented Apr 5, 2024 •

edited

Loading

Pull Request Test Coverage Report for Build 9081446412

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

165 of 165 (100.0%) changed or added relevant lines in 2 files are covered.
128 unchanged lines in 12 files lost coverage.
Overall coverage increased (+0.09%) to 84.163%

Files with Coverage Reduction	New Missed Lines	%
crates/chia-protocol/src/full_node_protocol.rs	2	0.0%
crates/chia-puzzles/src/puzzles/standard.rs	2	93.75%
crates/chia-puzzles/src/puzzles/did.rs	3	76.92%
crates/chia-puzzles/src/puzzles/singleton.rs	5	26.67%
crates/chia-puzzles/src/puzzles/offer.rs	5	44.44%
crates/chia-consensus/src/fast_forward.rs	8	97.99%
crates/chia-protocol/src/chia_protocol.rs	9	0.0%
crates/clvm-utils/src/tree_hash.rs	16	93.47%
crates/chia-protocol/src/wallet_protocol.rs	17	0.0%
wheel/src/api.rs	18	77.92%

Totals
Change from base Build 9011508250:	0.09%
Covered Lines:	12010
Relevant Lines:	14270

💛 - Coveralls

Rigidity reviewed

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

wheel/README.md Outdated Show resolved Hide resolved

matt-o-how force-pushed the bls_cache_rebased branch from 2c6a195 to 11c3527 Compare

April 25, 2024 14:55

arvidn requested changes

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/gtelement.rs Outdated Show resolved Hide resolved

Contributor

arvidn commented Apr 29, 2024

this is a sensitive part of the consensus. we need really thorough tests and fuzzers. I think we should have some benchmarks as well

arvidn reviewed

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

arvidn reviewed

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

arvidn reviewed

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

matt-o-how added 17 commits

May 1, 2024 15:44


          rebase bls_cache off main

02f4438


          fix tests and update readme

0f543f0

fmt

28b309c


          pyi stubs

e42f49c


          mypy fixes

02181c2


          no bytes conversion

ffd3862


          bytes length

d333221


          add byteorder

d273c8e


          black test

7c096c7


          remove unnecessary bytes32/48 type


          fix typo

13939b9


          shorten return

f156286


          implement Rigidity's review comments

9bb3b9f


          fmt new changes

290fba2


          delete requirements.txt

5891fd7


          remove readme from wheel

0b4aff6


          converts msg to [u8] slice

3dff7c0

matt-o-how and others added 17 commits

May 9, 2024 16:28


          add benchmarking test

ca5e347


          add cargo bench funcs for cache

d827fc3


          test more representative number pairs and clone cache

a823c4c


          update benchmark

1c3c14b


          Merge pull request #510 from Chia-Network/bls-cache-benchmark

532e86c

update benchmark


          Merge branch 'main' into bls_cache_rebased

ee330c5


          benchmarks

dcfef4f


          test both old and new blscache


          Merge branch 'bls_cache_rebased' of https://github.com/Chia-Network/c…

f56e629

…hia_rs into bls_cache_rebased


          black tests

94c8f7b


          swap to nonzerousize and remove generator

cf1fe69


          fmt + clippy


          change pystubs

35e8658


          type stubs typo fix

81ed976


          add self to init

1252af5


          attempt to fix mypy error in CI

a6b9923


          black pytest

405728f

arvidn reviewed

View reviewed changes

Contributor

arvidn left a comment

do you feel confident the unit tests cover all interesting cases?
A few cases are:
0% cache hit rate
100% cache hit rate
validating a signature of 0 messages and keys
validating a signature of 1 message and key

Can you think if any aspect that might benefit from fuzzing?

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

tests/test_blscache.py Show resolved Hide resolved

matt-o-how added 4 commits

May 14, 2024 14:04


          return and test overflow error in python

ee22bd1


          clippy + black

3ee0028


          change unwrap to expect with message

b2257a2


          add tests for empty validation

c03c79c

arvidn reviewed

View reviewed changes

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved

crates/chia-bls/src/cached_bls.rs Outdated Show resolved Hide resolved


          make tests not pub

ed86a1b

arvidn reviewed

View reviewed changes

tests/test_blscache.py Outdated Show resolved Hide resolved

matt-o-how added 2 commits

May 14, 2024 15:47


          assert pytest.raises

44da724


          black formatting

c617acd

arvidn approved these changes

View reviewed changes

matt-o-how merged commit 090ff48 into main

64 checks passed

matt-o-how deleted the bls_cache_rebased branch

May 14, 2024 15:25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet