[DRAFT] Add BLSCache struct / pyobject #408
Conversation
Pull Request Test Coverage Report for Build 8006278210Details
💛 - Coveralls |
| @@ -0,0 +1,275 @@ | |||
| extern crate lru; | |||
There was a problem hiding this comment.
I don't think this line is necessary. I don't recall ever having to use extern crate. ChatGPT suggests that since the rust 2018 edition you can import with use instead.
chia-bls/src/cached_bls.rs
Outdated
| impl BLSCache { | ||
|
|
||
| pub fn generator(cache_size: Option<usize>) -> Self { | ||
| let cache: LruCache<Bytes32, GTElement> = LruCache::new(NonZeroUsize::new(cache_size.unwrap_or(50000)).unwrap()); |
There was a problem hiding this comment.
I wasn't familiar with NonZeroUsize. Seems a bit strange they'd use that in their API. Maybe too clever by half.
Note that using .unwrap is a bad smell because it panics on failure. There's probably a way to do this without .unwrap. Maybe just do cache_size = cache_size | 50000.
pub fn generator(cache_size: usize) -> Self {
let cache: LruCache<Bytes32, GTElement> = LruCache::new(NonZeroUsize::new(cache_size | 50000).unwrap());
Self{cache}
}
I haven't test this.
| let mut aug_msg = pk.to_vec(); | ||
| aug_msg.extend_from_slice(msg.borrow()); // pk + msg | ||
| let mut hasher = Sha256::new(); | ||
| hasher.update(aug_msg); |
There was a problem hiding this comment.
You can call hasher.update multiple times, which means you don't actually have to create aug_msg.
- let mut aug_msg = pk.to_vec();
- aug_msg.extend_from_slice(msg.borrow()); // pk + msg
let mut hasher = Sha256::new();
+ hasher.update(pk);
+ hasher.update(msg);
- hasher.update(aug_msg);There was a problem hiding this comment.
(also not tested, so some types may be off by a & or something)
|
|
||
| } | ||
|
|
||
| // G1Element.from_bytes can be expensive due to subgroup check, so we avoid recomputing it with this cache |
There was a problem hiding this comment.
Not sure why there are two loops instead of one. Seems like a natural way to do this would be do dig for the answer in the cache, and if it ain't there, calculate it (and possible cache the result). Maybe it has something to do with the heuristic above. I realize this is just a port so you may not know either.
| }); | ||
|
|
||
| let pairing: GTElement = aug_hash.pair(pk_parsed); | ||
| let mut hasher = Sha256::new(); |
There was a problem hiding this comment.
We do this same hash again, not sure why.
| let res: bool = agg_ver(sig, data); | ||
| return res | ||
| } | ||
| let pairings_prod = pairings.pop(); // start with the first pairing |
There was a problem hiding this comment.
Note that the GT group has an "identity" element which acts like 1 for multiplication of "regular" (ie. real) numbers. If you set the partial product prod to this identity element, you don't have to do this weird .pop which treats the first argument as special. In fact, you can probably use .fold https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.fold
There are still two cases: an empty list and a non-empty list. It's a bit weird to me that you don't check the signature for the empty list case, but it might be a consensus change to do so depending on how empty blocks are treated (ie. blocks that don't have spends with any AGG_* conditions).
| @@ -19,7 +19,8 @@ use pyo3::exceptions::PyValueError; | |||
| #[cfg(feature = "py-bindings")] | |||
| use pyo3::{pyclass, pymethods, IntoPy, PyAny, PyObject, PyResult, Python}; | |||
|
|
|||
| #[cfg_attr(feature = "py-bindings", pyclass, derive(PyStreamable, Clone))] | |||
| #[cfg_attr(feature = "py-bindings", pyclass, derive(PyStreamable))] | |||
| #[derive(Clone)] | |||
There was a problem hiding this comment.
I'm puzzled as to why this might be necessary.
There was a problem hiding this comment.
We want to Clone the GTElement in Rust so we move it out of the py-binding exclusive derives
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
matt-o-how
force-pushed
the
bls_cache
branch
3 times, most recently
from
6bdb0d2
to
cb09152
Compare
No description provided.