Skip to content

Commit

Permalink
feat: add ability to bypass rangeproof (#3265)
Browse files Browse the repository at this point in the history
Description
---
Adds the ability to bypass rangeproof verification. 

Motivation and Context
---
Warning: This should not be done by default as it can cause a fork. By default this should always be set to verify rangeproofs, but in some scenarios, you want to disable it to quickly download a chain or run on a slim device. 

The rangeproof verification also takes the majority of time when profiling, so by disabling it, we can monitor other performance bottlenecks

How Has This Been Tested?
---
Manually

> Note that I disabled checking of rangeproofs during wallet sending because it adds little value to validate a rangeproof that you created
  • Loading branch information
stringhandler committed Aug 31, 2021
1 parent 8a36fb5 commit 055271f
Show file tree
Hide file tree
Showing 19 changed files with 101 additions and 34 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ members = [
"applications/tari_stratum_transcoder",
"applications/tari_mining_node",
]
#
#[profile.release]
#debug = true
11 changes: 9 additions & 2 deletions applications/tari_base_node/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,11 @@ async fn build_node_context(
let validators = Validators::new(
BodyOnlyValidator::default(),
HeaderValidator::new(rules.clone()),
OrphanBlockValidator::new(rules.clone(), factories.clone()),
OrphanBlockValidator::new(
rules.clone(),
config.base_node_bypass_range_proof_verification,
factories.clone(),
),
);
let db_config = BlockchainDatabaseConfig {
orphan_storage_capacity: config.orphan_storage_capacity,
Expand All @@ -236,7 +240,10 @@ async fn build_node_context(
cleanup_orphans_at_startup,
)?;
let mempool_validator = MempoolValidator::new(vec![
Box::new(TxInternalConsistencyValidator::new(factories.clone())),
Box::new(TxInternalConsistencyValidator::new(
factories.clone(),
config.base_node_bypass_range_proof_verification,
)),
Box::new(TxInputAndMaturityValidator::new(blockchain_db.clone())),
Box::new(TxConsensusValidator::new(blockchain_db.clone())),
]);
Expand Down
6 changes: 5 additions & 1 deletion applications/tari_base_node/src/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ pub async fn run_recovery(node_config: &GlobalConfig) -> Result<(), anyhow::Erro
let validators = Validators::new(
BodyOnlyValidator::default(),
HeaderValidator::new(rules.clone()),
OrphanBlockValidator::new(rules.clone(), factories.clone()),
OrphanBlockValidator::new(
rules.clone(),
node_config.base_node_bypass_range_proof_verification,
factories.clone(),
),
);
let db_config = BlockchainDatabaseConfig {
orphan_storage_capacity: node_config.orphan_storage_capacity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ where B: BlockchainBackend + 'static
let connectivity = handles.expect_handle::<ConnectivityRequester>();
let peer_manager = handles.expect_handle::<Arc<PeerManager>>();

let sync_validators = SyncValidators::full_consensus(rules.clone(), factories);
let sync_validators =
SyncValidators::full_consensus(rules.clone(), factories, config.bypass_range_proof_verification);
let max_randomx_vms = config.max_randomx_vms;

let node = BaseNodeStateMachine::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub struct BaseNodeStateMachineConfig {
pub pruning_horizon: u64,
pub max_randomx_vms: usize,
pub blocks_behind_before_considered_lagging: u64,
pub bypass_range_proof_verification: bool,
}

/// A Tari full node, aka Base Node.
Expand Down
8 changes: 6 additions & 2 deletions base_layer/core/src/base_node/sync/validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,13 @@ impl<B: BlockchainBackend + 'static> SyncValidators<B> {
}
}

pub fn full_consensus(rules: ConsensusManager, factories: CryptoFactories) -> Self {
pub fn full_consensus(
rules: ConsensusManager,
factories: CryptoFactories,
bypass_range_proof_verification: bool,
) -> Self {
Self::new(
BlockValidator::new(rules.clone(), factories.clone()),
BlockValidator::new(rules.clone(), bypass_range_proof_verification, factories.clone()),
ChainBalanceValidator::<B>::new(rules, factories),
)
}
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/test_helpers/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub fn create_store_with_consensus(rules: ConsensusManager) -> BlockchainDatabas
let validators = Validators::new(
BodyOnlyValidator::default(),
MockValidator::new(true),
OrphanBlockValidator::new(rules.clone(), factories),
OrphanBlockValidator::new(rules.clone(), false, factories),
);
create_store_with_consensus_and_validators(rules, validators)
}
Expand Down
5 changes: 4 additions & 1 deletion base_layer/core/src/transactions/aggregated_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ impl AggregateBody {
&self,
tx_offset: &BlindingFactor,
script_offset: &BlindingFactor,
bypass_range_proof_verification: bool,
total_reward: MicroTari,
factories: &CryptoFactories,
) -> Result<(), TransactionError> {
Expand All @@ -316,7 +317,9 @@ impl AggregateBody {
self.verify_kernel_signatures()?;
self.validate_kernel_sum(total_offset, &factories.commitment)?;

self.validate_range_proofs(&factories.range_proof)?;
if !bypass_range_proof_verification {
self.validate_range_proofs(&factories.range_proof)?;
}
self.verify_metadata_signatures()?;
self.validate_script_offset(script_offset_g, &factories.commitment)
}
Expand Down
1 change: 1 addition & 0 deletions base_layer/core/src/transactions/coinbase_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ mod test {
tx.body.validate_internal_consistency(
&BlindingFactor::default(),
&PrivateKey::default(),
false,
block_reward,
&factories
),
Expand Down
24 changes: 16 additions & 8 deletions base_layer/core/src/transactions/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,12 +1109,18 @@ impl Transaction {
#[allow(clippy::erasing_op)] // This is for 0 * uT
pub fn validate_internal_consistency(
&self,
bypass_range_proof_verification: bool,
factories: &CryptoFactories,
reward: Option<MicroTari>,
) -> Result<(), TransactionError> {
let reward = reward.unwrap_or_else(|| 0 * uT);
self.body
.validate_internal_consistency(&self.offset, &self.script_offset, reward, factories)
self.body.validate_internal_consistency(
&self.offset,
&self.script_offset,
bypass_range_proof_verification,
reward,
factories,
)
}

pub fn get_body(&self) -> &AggregateBody {
Expand Down Expand Up @@ -1264,7 +1270,7 @@ impl TransactionBuilder {
if let (Some(script_offset), Some(offset)) = (self.script_offset, self.offset) {
let (i, o, k) = self.body.dissolve();
let tx = Transaction::new(i, o, k, offset, script_offset);
tx.validate_internal_consistency(factories, self.reward)?;
tx.validate_internal_consistency(true, factories, self.reward)?;
Ok(tx)
} else {
Err(TransactionError::ValidationError(
Expand Down Expand Up @@ -1514,7 +1520,7 @@ mod test {
let (tx, _, _) = helpers::create_tx(5000.into(), 15.into(), 1, 2, 1, 4);

let factories = CryptoFactories::default();
assert!(tx.validate_internal_consistency(&factories, None).is_ok());
assert!(tx.validate_internal_consistency(false, &factories, None).is_ok());
}

#[test]
Expand All @@ -1527,7 +1533,7 @@ mod test {
assert_eq!(tx.body.kernels().len(), 1);

let factories = CryptoFactories::default();
assert!(tx.validate_internal_consistency(&factories, None).is_ok());
assert!(tx.validate_internal_consistency(false, &factories, None).is_ok());

let schema = txn_schema!(from: vec![outputs[1].clone()], to: vec![1 * T, 2 * T]);
let (tx2, _outputs, _) = helpers::spend_utxos(schema);
Expand Down Expand Up @@ -1558,10 +1564,12 @@ mod test {
}

// Validate basis transaction where cut-through has not been applied.
assert!(tx3.validate_internal_consistency(&factories, None).is_ok());
assert!(tx3.validate_internal_consistency(false, &factories, None).is_ok());

// tx3_cut_through has manual cut-through, it should not be possible so this should fail
assert!(tx3_cut_through.validate_internal_consistency(&factories, None).is_err());
assert!(tx3_cut_through
.validate_internal_consistency(false, &factories, None)
.is_err());
}

#[test]
Expand Down Expand Up @@ -1598,7 +1606,7 @@ mod test {
tx.body.inputs_mut()[0].input_data = stack;

let factories = CryptoFactories::default();
let err = tx.validate_internal_consistency(&factories, None).unwrap_err();
let err = tx.validate_internal_consistency(false, &factories, None).unwrap_err();
assert!(matches!(err, TransactionError::InvalidSignatureError(_)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ impl SenderTransactionProtocol {
}
let transaction = result.unwrap();
let result = transaction
.validate_internal_consistency(factories, None)
.validate_internal_consistency(true, factories, None)
.map_err(TPE::TransactionBuildError);
if let Err(e) = result {
self.state = SenderState::Failed(e.clone());
Expand Down Expand Up @@ -965,7 +965,10 @@ mod test {
assert_eq!(tx.body.inputs().len(), 1);
assert_eq!(tx.body.inputs()[0], utxo);
assert_eq!(tx.body.outputs().len(), 2);
assert!(tx.clone().validate_internal_consistency(&factories, None).is_ok());
assert!(tx
.clone()
.validate_internal_consistency(false, &factories, None)
.is_ok());
}

#[test]
Expand Down
27 changes: 22 additions & 5 deletions base_layer/core/src/validation/block_validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,17 @@ pub const LOG_TARGET: &str = "c::val::block_validators";
#[derive(Clone)]
pub struct OrphanBlockValidator {
rules: ConsensusManager,
bypass_range_proof_verification: bool,
factories: CryptoFactories,
}

impl OrphanBlockValidator {
pub fn new(rules: ConsensusManager, factories: CryptoFactories) -> Self {
Self { rules, factories }
pub fn new(rules: ConsensusManager, bypass_range_proof_verification: bool, factories: CryptoFactories) -> Self {
Self {
rules,
bypass_range_proof_verification,
factories,
}
}
}

Expand Down Expand Up @@ -101,7 +106,12 @@ impl OrphanValidation for OrphanBlockValidator {
trace!(target: LOG_TARGET, "SV - Output constraints are ok for {} ", &block_id);
check_coinbase_output(block, &self.rules, &self.factories)?;
trace!(target: LOG_TARGET, "SV - Coinbase output is ok for {} ", &block_id);
check_accounting_balance(block, &self.rules, &self.factories)?;
check_accounting_balance(
block,
&self.rules,
self.bypass_range_proof_verification,
&self.factories,
)?;
trace!(target: LOG_TARGET, "SV - accounting balance correct for {}", &block_id);
debug!(
target: LOG_TARGET,
Expand Down Expand Up @@ -311,15 +321,17 @@ fn check_mmr_roots<B: BlockchainBackend>(block: &Block, db: &B) -> Result<(), Va
/// the block body using the header. It is assumed that the `BlockHeader` has already been validated.
pub struct BlockValidator<B: BlockchainBackend> {
rules: ConsensusManager,
bypass_range_proof_verification: bool,
factories: CryptoFactories,
phantom_data: PhantomData<B>,
}

impl<B: BlockchainBackend> BlockValidator<B> {
pub fn new(rules: ConsensusManager, factories: CryptoFactories) -> Self {
pub fn new(rules: ConsensusManager, bypass_range_proof_verification: bool, factories: CryptoFactories) -> Self {
Self {
rules,
factories,
bypass_range_proof_verification,
phantom_data: Default::default(),
}
}
Expand Down Expand Up @@ -428,7 +440,12 @@ impl<B: BlockchainBackend> CandidateBlockBodyValidation<B> for BlockValidator<B>
self.check_inputs(block)?;
self.check_outputs(block)?;

check_accounting_balance(block, &self.rules, &self.factories)?;
check_accounting_balance(
block,
&self.rules,
self.bypass_range_proof_verification,
&self.factories,
)?;
trace!(target: LOG_TARGET, "SV - accounting balance correct for {}", &block_id);
debug!(
target: LOG_TARGET,
Expand Down
9 changes: 8 additions & 1 deletion base_layer/core/src/validation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ pub fn check_block_weight(block: &Block, consensus_constants: &ConsensusConstant
pub fn check_accounting_balance(
block: &Block,
rules: &ConsensusManager,
bypass_range_proof_verification: bool,
factories: &CryptoFactories,
) -> Result<(), ValidationError> {
if block.header.height == 0 {
Expand All @@ -210,7 +211,13 @@ pub fn check_accounting_balance(
let total_coinbase = rules.calculate_coinbase_and_fees(block);
block
.body
.validate_internal_consistency(&offset, &script_offset, total_coinbase, factories)
.validate_internal_consistency(
&offset,
&script_offset,
bypass_range_proof_verification,
total_coinbase,
factories,
)
.map_err(|err| {
warn!(
target: LOG_TARGET,
Expand Down
10 changes: 7 additions & 3 deletions base_layer/core/src/validation/transaction_validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,21 @@ pub const LOG_TARGET: &str = "c::val::transaction_validators";
/// This function does NOT check that inputs come from the UTXO set
pub struct TxInternalConsistencyValidator {
factories: CryptoFactories,
bypass_range_proof_verification: bool,
}

impl TxInternalConsistencyValidator {
pub fn new(factories: CryptoFactories) -> Self {
Self { factories }
pub fn new(factories: CryptoFactories, bypass_range_proof_verification: bool) -> Self {
Self {
factories,
bypass_range_proof_verification,
}
}
}

impl MempoolTransactionValidation for TxInternalConsistencyValidator {
fn validate(&self, tx: &Transaction) -> Result<(), ValidationError> {
tx.validate_internal_consistency(&self.factories, None)
tx.validate_internal_consistency(self.bypass_range_proof_verification, &self.factories, None)
.map_err(ValidationError::TransactionError)?;
Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/tests/block_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ fn test_genesis_block() {
let validators = Validators::new(
BodyOnlyValidator::default(),
HeaderValidator::new(rules.clone()),
OrphanBlockValidator::new(rules.clone(), factories),
OrphanBlockValidator::new(rules.clone(), false, factories),
);
let db = BlockchainDatabase::new(
backend,
Expand Down Expand Up @@ -216,7 +216,7 @@ fn inputs_are_not_malleable() {
input_mut.input_data = malicious_input.input_data;
input_mut.script_signature = malicious_input.script_signature;

let validator = BlockValidator::new(blockchain.consensus_manager().clone(), CryptoFactories::default());
let validator = BlockValidator::new(blockchain.consensus_manager().clone(), true, CryptoFactories::default());
let err = validator
.validate_body(&block, &*blockchain.store().db_read_access().unwrap())
.unwrap_err();
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/tests/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ async fn consensus_validation_large_tx() {

// make sure the tx was correctly made and is valid
let factories = CryptoFactories::default();
assert!(tx.validate_internal_consistency(&factories, None).is_ok());
assert!(tx.validate_internal_consistency(true, &factories, None).is_ok());
let weight = tx.calculate_weight();

let height = blocks.len() as u64;
Expand Down
6 changes: 3 additions & 3 deletions base_layer/core/tests/node_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ async fn propagate_and_forward_invalid_block() {
.with_consensus_constants(consensus_constants)
.with_block(block0.clone())
.build();
let stateless_block_validator = OrphanBlockValidator::new(rules.clone(), factories);
let stateless_block_validator = OrphanBlockValidator::new(rules.clone(), true, factories);

let mock_validator = MockValidator::new(false);
let (mut dan_node, rules) = BaseNodeBuilder::new(network.into())
Expand Down Expand Up @@ -643,7 +643,7 @@ async fn local_get_new_block_with_zero_conf() {
.with_validators(
BodyOnlyValidator::default(),
HeaderValidator::new(rules.clone()),
OrphanBlockValidator::new(rules, factories.clone()),
OrphanBlockValidator::new(rules, true, factories.clone()),
)
.start(temp_dir.path().to_str().unwrap())
.await;
Expand Down Expand Up @@ -720,7 +720,7 @@ async fn local_get_new_block_with_combined_transaction() {
.with_validators(
BodyOnlyValidator::default(),
HeaderValidator::new(rules.clone()),
OrphanBlockValidator::new(rules, factories.clone()),
OrphanBlockValidator::new(rules, true, factories.clone()),
)
.start(temp_dir.path().to_str().unwrap())
.await;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ where TBackend: TransactionBackend + 'static
);

finalized_transaction
.validate_internal_consistency(&self.resources.factories, None)
.validate_internal_consistency(true, &self.resources.factories, None)
.map_err(|e| TransactionServiceProtocolError::new(self.id, TransactionServiceError::from(e)))?;

// Find your own output in the transaction
Expand Down
Loading

0 comments on commit 055271f

Please sign in to comment.