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

Remove cache from validation #646

Open
wants to merge 6 commits into
base: validate_clvm_and_sigs
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/chia-bls/src/bls_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ impl BlsCache {
let hash: [u8; 32] = hasher.finalize();

// If the pairing is in the cache, we don't need to recalculate it.
if let Some(pairing) = self.cache.get(&hash).cloned() {
return pairing;
if let Some(pairing) = self.cache.get(&hash) {
return *pairing;
}

// Otherwise, we need to calculate the pairing and add it to the cache.
Expand All @@ -79,7 +79,7 @@ impl BlsCache {
let hash: [u8; 32] = hasher.finalize();

let pairing = aug_hash.pair(pk.borrow());
self.cache.put(hash, pairing.clone());
self.cache.put(hash, pairing);
added.push((hash, pairing.to_bytes().to_vec()));
pairing
});
Expand Down
4 changes: 2 additions & 2 deletions crates/chia-bls/src/gtelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::ops::{Mul, MulAssign};
pyo3::pyclass,
derive(chia_py_streamable_macro::PyStreamable)
)]
#[derive(Clone)]
#[derive(Clone, Copy)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the GTElement is more than 500 bytes large. I think it's pretty good to require explicit cloning, to see where we pay that cost.

pub struct GTElement(pub(crate) blst_fp12);

impl GTElement {
Expand Down Expand Up @@ -114,7 +114,7 @@ impl GTElement {

#[must_use]
pub fn __mul__(&self, rhs: &Self) -> Self {
let mut ret = self.clone();
let mut ret = *self;
ret *= rhs;
ret
}
Expand Down
2 changes: 1 addition & 1 deletion crates/chia-bls/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ where
return *sig == Signature::default();
};

let mut agg = agg.borrow().clone();
let mut agg = *agg.borrow();
for gt in data {
agg *= gt.borrow();
}
Expand Down
92 changes: 49 additions & 43 deletions crates/chia-consensus/src/spendbundle_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,35 @@ use crate::gen::opcodes::{
use crate::gen::owned_conditions::OwnedSpendBundleConditions;
use crate::gen::validation_error::ErrorCode;
use crate::spendbundle_conditions::get_conditions_from_spendbundle;
use chia_bls::BlsCache;
use chia_bls::PairingInfo;
use chia_bls::GTElement;
use chia_bls::{aggregate_verify_gt, hash_to_g2};
use chia_protocol::SpendBundle;
use clvmr::sha2::Sha256;
use clvmr::{ENABLE_BLS_OPS_OUTSIDE_GUARD, ENABLE_FIXED_DIV, LIMIT_HEAP};
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant};

// type definition makes clippy happy
pub type ValidationPair = ([u8; 32], GTElement);

// currently in mempool_manager.py
// called in threads from pre_validate_spend_bundle()
// returns (error, cached_results, new_cache_entries, duration)
// pybinding returns (error, cached_results, new_cache_entries, duration)
pub fn validate_clvm_and_signature(
spend_bundle: &SpendBundle,
max_cost: u64,
constants: &ConsensusConstants,
height: u32,
cache: &Arc<Mutex<BlsCache>>,
) -> Result<(OwnedSpendBundleConditions, Vec<PairingInfo>, Duration), ErrorCode> {
) -> Result<(OwnedSpendBundleConditions, Vec<ValidationPair>, Duration), ErrorCode> {
let start_time = Instant::now();
let mut a = make_allocator(LIMIT_HEAP);
let sbc = get_conditions_from_spendbundle(&mut a, spend_bundle, max_cost, height, constants)
.map_err(|e| e.1)?;
let npcresult = OwnedSpendBundleConditions::from(&a, sbc);
let iter = npcresult.spends.iter().flat_map(|spend| {

// Collect all pairs in a single vector to avoid multiple iterations
let mut pairs = Vec::new();

for spend in &npcresult.spends {
let condition_items_pairs = [
(AGG_SIG_PARENT, &spend.agg_sig_parent),
(AGG_SIG_PUZZLE, &spend.agg_sig_puzzle),
Expand All @@ -41,37 +47,46 @@ pub fn validate_clvm_and_signature(
(AGG_SIG_PARENT_PUZZLE, &spend.agg_sig_parent_puzzle),
(AGG_SIG_ME, &spend.agg_sig_me),
];
condition_items_pairs
.into_iter()
.flat_map(move |(condition, items)| {
let spend_clone = spend.clone();
items.iter().map(move |(pk, msg)| {
(
pk,
make_aggsig_final_message(
condition,
msg.as_slice(),
&spend_clone,
constants,
),
)
})
})
});
let unsafe_items = npcresult
.agg_sig_unsafe
.iter()
.map(|(pk, msg)| (pk, msg.as_slice().to_vec()));
let iter = iter.chain(unsafe_items);

for (condition, items) in condition_items_pairs {
for (pk, msg) in items {
let mut aug_msg = pk.to_bytes().to_vec();
let msg = make_aggsig_final_message(condition, msg.as_slice(), spend, constants);
aug_msg.extend_from_slice(msg.as_ref());
let aug_hash = hash_to_g2(&aug_msg);
let pairing = aug_hash.pair(pk);
pairs.push((hash_pk_and_msg(&pk.to_bytes(), &msg), pairing));
}
}
}

// Adding unsafe items
for (pk, msg) in &npcresult.agg_sig_unsafe {
let mut aug_msg = pk.to_bytes().to_vec();
aug_msg.extend_from_slice(msg.as_ref());
let aug_hash = hash_to_g2(&aug_msg);
let pairing = aug_hash.pair(pk);
pairs.push((hash_pk_and_msg(&pk.to_bytes(), msg), pairing));
}

// Verify aggregated signature
let (result, added) = cache
.lock()
.unwrap()
.aggregate_verify(iter, &spend_bundle.aggregated_signature);
let result = aggregate_verify_gt(
&spend_bundle.aggregated_signature,
pairs.iter().map(|tuple| tuple.1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pairs.iter().map(|tuple| tuple.1),
pairs.iter().map(|tuple| &tuple.1),

does this work?

);
if !result {
return Err(ErrorCode::BadAggregateSignature);
}
Ok((npcresult, added, start_time.elapsed()))

// Collect results
Ok((npcresult, pairs, start_time.elapsed()))
}

fn hash_pk_and_msg(pk: &[u8], msg: &[u8]) -> [u8; 32] {
let mut hasher = Sha256::new();
hasher.update(pk);
hasher.update(msg);
hasher.finalize()
}

pub fn get_flags_for_height_and_constants(height: u32, constants: &ConsensusConstants) -> u32 {
Expand Down Expand Up @@ -117,7 +132,6 @@ mod tests {
use hex::FromHex;
use hex_literal::hex;
use rstest::rstest;
use std::sync::Arc;

#[rstest]
#[case(0, 0)]
Expand Down Expand Up @@ -161,7 +175,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
236,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down Expand Up @@ -193,7 +206,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
236,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down Expand Up @@ -222,15 +234,13 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm / 2, // same as mempool_manager default
&TEST_CONSTANTS,
236,
&Arc::new(Mutex::new(BlsCache::default())),
);
assert!(matches!(result, Ok(..)));
let result = validate_clvm_and_signature(
&spend_bundle,
TEST_CONSTANTS.max_block_cost_clvm / 3, // lower than mempool_manager default
&TEST_CONSTANTS,
236,
&Arc::new(Mutex::new(BlsCache::default())),
);
assert!(matches!(result, Err(ErrorCode::CostExceeded)));
}
Expand Down Expand Up @@ -271,7 +281,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
1,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down Expand Up @@ -321,7 +330,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
TEST_CONSTANTS.hard_fork_height + 1,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down Expand Up @@ -365,7 +373,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
TEST_CONSTANTS.hard_fork_height + 1,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down Expand Up @@ -409,7 +416,6 @@ ff01\
TEST_CONSTANTS.max_block_cost_clvm,
&TEST_CONSTANTS,
TEST_CONSTANTS.hard_fork_height + 1,
&Arc::new(Mutex::new(BlsCache::default())),
)
.expect("SpendBundle should be valid for this test");
}
Expand Down
1 change: 0 additions & 1 deletion tests/test_blscache.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ def test_validate_clvm_and_sig():
DEFAULT_CONSTANTS.MAX_BLOCK_COST_CLVM,
DEFAULT_CONSTANTS,
DEFAULT_CONSTANTS.HARD_FORK_HEIGHT + 1,
cache,
)

assert sbc is not None
Expand Down
1 change: 0 additions & 1 deletion wheel/generate_type_stubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ def validate_clvm_and_signature(
max_cost: int,
constants: ConsensusConstants,
peak_height: int,
cache: Optional[BLSCache],
) -> Tuple[SpendBundleConditions, List[Tuple[SpendBundleConditions, List[Tuple[bytes32, bytes]]]], float]: ...

def get_conditions_from_spendbundle(
Expand Down
1 change: 0 additions & 1 deletion wheel/python/chia_rs/chia_rs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ def validate_clvm_and_signature(
max_cost: int,
constants: ConsensusConstants,
peak_height: int,
cache: Optional[BLSCache],
) -> Tuple[SpendBundleConditions, List[Tuple[SpendBundleConditions, List[Tuple[bytes32, bytes]]]], float]: ...

def get_conditions_from_spendbundle(
Expand Down
23 changes: 9 additions & 14 deletions wheel/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ use pyo3::types::PyList;
use pyo3::types::PyTuple;
use pyo3::wrap_pyfunction;
use std::iter::zip;
use std::sync::{Arc, Mutex};

use crate::run_program::{run_chia_program, serialized_length};

Expand Down Expand Up @@ -368,20 +367,16 @@ pub fn py_validate_clvm_and_signature(
max_cost: u64,
constants: &ConsensusConstants,
peak_height: u32,
cache: Option<BlsCache>,
) -> PyResult<(OwnedSpendBundleConditions, Vec<PairingInfo>, f32)> {
let real_cache = cache.unwrap_or_default();
let (owned_conditions, additions, duration) = validate_clvm_and_signature(
new_spend,
max_cost,
constants,
peak_height,
&Arc::new(Mutex::new(real_cache)), // TODO: use cache properly
)
.map_err(|e| {
let error_code: u32 = e.into();
PyErr::new::<PyTypeError, _>(error_code)
})?; // cast validation error to int
let (owned_conditions, additions, duration) =
validate_clvm_and_signature(new_spend, max_cost, constants, peak_height).map_err(|e| {
let error_code: u32 = e.into();
PyErr::new::<PyTypeError, _>(error_code)
})?; // cast validation error to int
let additions = additions
.into_iter()
.map(|tuple| (tuple.0, tuple.1.to_bytes().to_vec()))
.collect();
Ok((owned_conditions, additions, duration.as_secs_f32()))
}

Expand Down
Loading