Skip to content

Commit

Permalink
fix: block sync validation (#3236)
Browse files Browse the repository at this point in the history
Description
---
This fixes the block sync block body validation to validate the correct items
This consolidates all the syncing helper functions
This adds in unit tests for the validators

Motivation and Context
---
Block sync needs to validate all the required steps. This also consolidates all the possible validation functions to remove duplicated pieces of code between the validators so we use a single piece to test a specific consensus item. 

How Has This Been Tested?
---
Synced a node up to the tip height 22014. 
Added new unit tests. 
Ran all integration and unit tests.
  • Loading branch information
SWvheerden committed Sep 8, 2021
1 parent a9d079a commit fd081c8
Show file tree
Hide file tree
Showing 13 changed files with 903 additions and 421 deletions.
6 changes: 3 additions & 3 deletions base_layer/core/src/base_node/sync/block_sync/synchronizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::{
proto::base_node::SyncBlocksRequest,
tari_utilities::{hex::Hex, Hashable},
transactions::aggregated_body::AggregateBody,
validation::CandidateBlockBodyValidation,
validation::BlockSyncBodyValidation,
};
use futures::StreamExt;
use log::*;
Expand All @@ -55,7 +55,7 @@ pub struct BlockSynchronizer<B> {
db: AsyncBlockchainDb<B>,
connectivity: ConnectivityRequester,
sync_peer: Option<PeerConnection>,
block_validator: Arc<dyn CandidateBlockBodyValidation<B>>,
block_validator: Arc<dyn BlockSyncBodyValidation<B>>,
hooks: Hooks,
}

Expand All @@ -65,7 +65,7 @@ impl<B: BlockchainBackend + 'static> BlockSynchronizer<B> {
db: AsyncBlockchainDb<B>,
connectivity: ConnectivityRequester,
sync_peer: Option<PeerConnection>,
block_validator: Arc<dyn CandidateBlockBodyValidation<B>>,
block_validator: Arc<dyn BlockSyncBodyValidation<B>>,
) -> Self {
Self {
config,
Expand Down
6 changes: 3 additions & 3 deletions base_layer/core/src/base_node/sync/validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,22 @@ use crate::{
transactions::CryptoFactories,
validation::{
block_validators::BlockValidator,
CandidateBlockBodyValidation,
BlockSyncBodyValidation,
ChainBalanceValidator,
FinalHorizonStateValidation,
},
};

#[derive(Clone)]
pub struct SyncValidators<B: BlockchainBackend> {
pub block_body: Arc<dyn CandidateBlockBodyValidation<B>>,
pub block_body: Arc<dyn BlockSyncBodyValidation<B>>,
pub final_horizon_state: Arc<dyn FinalHorizonStateValidation<B>>,
}

impl<B: BlockchainBackend + 'static> SyncValidators<B> {
pub fn new<TBody, TFinal>(block_body: TBody, final_state: TFinal) -> Self
where
TBody: CandidateBlockBodyValidation<B> + 'static,
TBody: BlockSyncBodyValidation<B> + 'static,
TFinal: FinalHorizonStateValidation<B> + 'static,
{
Self {
Expand Down
9 changes: 7 additions & 2 deletions base_layer/core/src/blocks/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ pub enum BlockValidationError {
TransactionError(#[from] TransactionError),
#[error("Invalid input in block")]
InvalidInput,
#[error("Contains kernels or inputs that are not yet spendable")]
MaturityError,
#[error("Mismatched MMR roots")]
MismatchedMmrRoots,
#[error("MMR size for {mmr_tree} does not match. Expected: {expected}, received: {actual}")]
Expand Down Expand Up @@ -109,9 +111,12 @@ impl Block {
Ok(())
}

/// Checks that all STXO rules (maturity etc) are followed
pub fn check_stxo_rules(&self) -> Result<(), BlockValidationError> {
/// Checks that all STXO rules (maturity etc) and kernel heights are followed
pub fn check_spend_rules(&self) -> Result<(), BlockValidationError> {
self.body.check_stxo_rules(self.header.height)?;
if self.body.max_kernel_timelock() > self.header.height {
return Err(BlockValidationError::MaturityError);
}
Ok(())
}

Expand Down
29 changes: 18 additions & 11 deletions base_layer/core/src/transactions/aggregated_body.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
use std::fmt::{Display, Error, Formatter};

use log::*;
use serde::{Deserialize, Serialize};
use tari_crypto::{
commitment::HomomorphicCommitmentFactory,
keys::PublicKey as PublicKeyTrait,
ristretto::pedersen::PedersenCommitment,
tari_utilities::hex::Hex,
};

// Copyright 2019, The Tari Project
//
// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the
Expand All @@ -31,6 +20,12 @@ use tari_crypto::{
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
use crate::transactions::{crypto_factories::CryptoFactories, fee::Fee, tari_amount::*, transaction::*};
use log::*;
use serde::{Deserialize, Serialize};
use std::{
cmp::max,
fmt::{Display, Error, Formatter},
};
use tari_common_types::types::{
BlindingFactor,
Commitment,
Expand All @@ -39,6 +34,12 @@ use tari_common_types::types::{
PublicKey,
RangeProofService,
};
use tari_crypto::{
commitment::HomomorphicCommitmentFactory,
keys::PublicKey as PublicKeyTrait,
ristretto::pedersen::PedersenCommitment,
tari_utilities::hex::Hex,
};

pub const LOG_TARGET: &str = "c::tx::aggregated_body";

Expand Down Expand Up @@ -450,6 +451,12 @@ impl AggregateBody {
self.kernels.len()
)
}

pub fn max_kernel_timelock(&self) -> u64 {
self.kernels()
.iter()
.fold(0, |max_timelock, kernel| max(max_timelock, kernel.lock_height))
}
}

/// This will strip away the offset of the transaction returning a pure aggregate body
Expand Down
5 changes: 1 addition & 4 deletions base_layer/core/src/transactions/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,10 +1156,7 @@ impl Transaction {

/// Returns the maximum time lock of the kernels inside of the transaction
pub fn max_kernel_timelock(&self) -> u64 {
self.body
.kernels()
.iter()
.fold(0, |max_timelock, kernel| max(max_timelock, kernel.lock_height))
self.body.max_kernel_timelock()
}

/// Returns the height of the minimum height where the transaction is spendable. This is calculated from the
Expand Down
Loading

0 comments on commit fd081c8

Please sign in to comment.