Skip to content

Commit

Permalink
fix: remove optional timestamp verification bypass (#5552)
Browse files Browse the repository at this point in the history
Description
---
In previous refactors we had to bypass timestamp checks during header
sync as it we where asking the database for the header timestamps. We
now cache the timestamps when syncing negating this limitation.
We can thus remove this optional check and always check the header
timestamps pass the median rule

How Has This Been Tested?
---
Pass all unit tests
Manual sync of multiple networks.
  • Loading branch information
SWvheerden authored Jun 30, 2023
1 parent 891990a commit b5a5bed
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 35 deletions.
2 changes: 1 addition & 1 deletion applications/tari_base_node/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async fn build_node_context(
let difficulty_calculator = DifficultyCalculator::new(rules.clone(), randomx_factory.clone());
let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone()),
BlockBodyInternalConsistencyValidator::new(
rules.clone(),
app_config.base_node.bypass_range_proof_verification,
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_base_node/src/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub async fn run_recovery(node_config: &BaseNodeConfig) -> Result<(), anyhow::Er
let difficulty_calculator = DifficultyCalculator::new(rules.clone(), randomx_factory);
let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone()),
BlockBodyInternalConsistencyValidator::new(
rules.clone(),
node_config.bypass_range_proof_verification,
Expand Down
10 changes: 5 additions & 5 deletions base_layer/core/src/base_node/sync/header_sync/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct State {
impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
pub fn new(db: AsyncBlockchainDb<B>, consensus_rules: ConsensusManager, randomx_factory: RandomXFactory) -> Self {
let difficulty_calculator = DifficultyCalculator::new(consensus_rules.clone(), randomx_factory);
let validator = HeaderFullValidator::new(consensus_rules.clone(), difficulty_calculator, true);
let validator = HeaderFullValidator::new(consensus_rules.clone(), difficulty_calculator);
Self {
db,
state: None,
Expand Down Expand Up @@ -309,16 +309,16 @@ mod test {

#[tokio::test]
async fn it_fails_if_height_is_not_serial() {
let (mut validator, _, tip) = setup_with_headers(2).await;
let (mut validator, _, tip) = setup_with_headers(12).await;
validator.initialize_state(tip.hash()).await.unwrap();
let mut next = BlockHeader::from_previous(tip.header());
next.height = 10;
next.height = 14;
let err = validator.validate(next).unwrap_err();
unpack_enum!(BlockHeaderSyncError::ValidationFailed(val_err) = err);
unpack_enum!(ValidationError::BlockHeaderError(header_err) = val_err);
unpack_enum!(BlockHeaderValidationError::InvalidHeight { actual, expected } = header_err);
assert_eq!(actual, 10);
assert_eq!(expected, 3);
assert_eq!(actual, 14);
assert_eq!(expected, 13);
}
}
}
1 change: 0 additions & 1 deletion base_layer/core/src/chain_storage/blockchain_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3274,7 +3274,6 @@ mod test {
let header_validator = Box::new(HeaderFullValidator::new(
consensus.clone(),
difficulty_calculator.clone(),
false,
));
let post_orphan_body_validator = Box::new(MockValidator::new(true));
let chain_strength_comparer = strongest_chain().by_sha3x_difficulty().build();
Expand Down
14 changes: 3 additions & 11 deletions base_layer/core/src/validation/header/header_full_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,13 @@ pub const LOG_TARGET: &str = "c::val::header_full_validator";
pub struct HeaderFullValidator {
rules: ConsensusManager,
difficulty_calculator: DifficultyCalculator,
bypass_prev_timestamp_verification: bool,
}

impl HeaderFullValidator {
pub fn new(
rules: ConsensusManager,
difficulty_calculator: DifficultyCalculator,
bypass_prev_timestamp_verification: bool,
) -> Self {
pub fn new(rules: ConsensusManager, difficulty_calculator: DifficultyCalculator) -> Self {
Self {
rules,
difficulty_calculator,
bypass_prev_timestamp_verification,
}
}
}
Expand All @@ -75,10 +69,8 @@ impl<B: BlockchainBackend> HeaderChainLinkedValidator<B> for HeaderFullValidator

check_blockchain_version(constants, header.version)?;

if !self.bypass_prev_timestamp_verification {
check_timestamp_count(header, prev_timestamps, constants)?;
check_header_timestamp_greater_than_median(header, prev_timestamps)?;
}
check_timestamp_count(header, prev_timestamps, constants)?;
check_header_timestamp_greater_than_median(header, prev_timestamps)?;

check_height(header, prev_header)?;
check_prev_hash(header, prev_header)?;
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/validation/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ mod header_validators {
let mut header = BlockHeader::from_previous(genesis.header());
header.version = u16::MAX;
let difficulty_calculator = DifficultyCalculator::new(consensus_manager.clone(), Default::default());
let validator = HeaderFullValidator::new(consensus_manager, difficulty_calculator, false);
let validator = HeaderFullValidator::new(consensus_manager, difficulty_calculator);

let err = validator
.validate(&*db.db_read_access().unwrap(), &header, genesis.header(), &[], None)
Expand Down
30 changes: 17 additions & 13 deletions base_layer/core/tests/tests/block_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ async fn test_monero_blocks() {
.build()
.unwrap();
let difficulty_calculator = DifficultyCalculator::new(cm.clone(), RandomXFactory::default());
let header_validator = HeaderFullValidator::new(cm.clone(), difficulty_calculator, false);
let header_validator = HeaderFullValidator::new(cm.clone(), difficulty_calculator);
let db = create_store_with_consensus_and_validators(
cm.clone(),
Validators::new(MockValidator::new(true), header_validator, MockValidator::new(true)),
Expand Down Expand Up @@ -283,7 +283,7 @@ async fn test_orphan_validator() {

let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone()),
orphan_validator.clone(),
);
let db = BlockchainDatabase::new(
Expand Down Expand Up @@ -422,10 +422,10 @@ async fn test_orphan_body_validation() {
let backend = create_test_db();
let difficulty_calculator = DifficultyCalculator::new(rules.clone(), Default::default());
let body_only_validator = BlockBodyFullValidator::new(rules.clone(), true);
let header_validator = HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), true);
let header_validator = HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone());
let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator, true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator),
BlockBodyInternalConsistencyValidator::new(rules.clone(), false, factories.clone()),
);
let db = BlockchainDatabase::new(
Expand Down Expand Up @@ -454,13 +454,14 @@ OutputFeatures::default()),
let mut new_block = db.prepare_new_block(template.clone()).unwrap();
new_block.header.nonce = OsRng.next_u64();

let timestamps = db.fetch_block_timestamps(new_block.header.prev_hash).unwrap();
find_header_with_achieved_difficulty(&mut new_block.header, Difficulty::from_u64(10).unwrap());
let achieved_target_diff = header_validator
.validate(
&*db.db_read_access().unwrap(),
&new_block.header,
genesis.header(),
&[],
&timestamps,
None,
)
.unwrap();
Expand Down Expand Up @@ -524,14 +525,14 @@ OutputFeatures::default()),
];
new_block.body = AggregateBody::new(inputs, template.body.outputs().clone(), template.body.kernels().clone());
new_block.header.nonce = OsRng.next_u64();

let timestamps = db.fetch_block_timestamps(new_block.header.prev_hash).unwrap();
find_header_with_achieved_difficulty(&mut new_block.header, Difficulty::from_u64(10).unwrap());
let achieved_target_diff = header_validator
.validate(
&*db.db_read_access().unwrap(),
&new_block.header,
&prev_header,
&[],
&timestamps,
None,
)
.unwrap();
Expand All @@ -557,12 +558,13 @@ OutputFeatures::default()),
new_block.header.nonce = OsRng.next_u64();

find_header_with_achieved_difficulty(&mut new_block.header, Difficulty::from_u64(10).unwrap());
let timestamps = db.fetch_block_timestamps(new_block.header.prev_hash).unwrap();
let achieved_target_diff = header_validator
.validate(
&*db.db_read_access().unwrap(),
&new_block.header,
genesis.header(),
&[],
&timestamps,
None,
)
.unwrap();
Expand All @@ -586,12 +588,13 @@ OutputFeatures::default()),
new_block.header.nonce = OsRng.next_u64();

find_header_with_achieved_difficulty(&mut new_block.header, Difficulty::from_u64(10).unwrap());
let timestamps = db.fetch_block_timestamps(new_block.header.prev_hash).unwrap();
let achieved_target_diff = header_validator
.validate(
&*db.db_read_access().unwrap(),
&new_block.header,
&prev_header,
&[],
&timestamps,
None,
)
.unwrap();
Expand Down Expand Up @@ -634,10 +637,10 @@ async fn test_header_validation() {
.unwrap();
let backend = create_test_db();
let difficulty_calculator = DifficultyCalculator::new(rules.clone(), Default::default());
let header_validator = HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), true);
let header_validator = HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone());
let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator.clone()),
BlockBodyInternalConsistencyValidator::new(rules.clone(), false, factories.clone()),
);
let db = BlockchainDatabase::new(
Expand Down Expand Up @@ -665,14 +668,15 @@ OutputFeatures::default()),
let (template, _) = chain_block_with_new_coinbase(&genesis, vec![tx01, tx02], &rules, None, &key_manager).await;
let mut new_block = db.prepare_new_block(template.clone()).unwrap();
new_block.header.nonce = OsRng.next_u64();
let timestamps = db.fetch_block_timestamps(new_block.header.prev_hash).unwrap();

find_header_with_achieved_difficulty(&mut new_block.header, Difficulty::from_u64(20).unwrap());
assert!(header_validator
.validate(
&*db.db_read_access().unwrap(),
&new_block.header,
genesis.header(),
&[],
&timestamps,
None
)
.is_ok());
Expand Down Expand Up @@ -745,7 +749,7 @@ async fn test_block_sync_body_validator() {
let difficulty_calculator = DifficultyCalculator::new(rules.clone(), Default::default());
let validators = Validators::new(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator, false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator),
BlockBodyInternalConsistencyValidator::new(rules.clone(), false, factories.clone()),
);

Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/tests/tests/node_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ async fn local_get_new_block_with_zero_conf() {
.with_consensus_manager(rules.clone())
.with_validators(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator, false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator),
BlockBodyInternalConsistencyValidator::new(rules, true, factories.clone()),
)
.start(temp_dir.path().to_str().unwrap())
Expand Down Expand Up @@ -639,7 +639,7 @@ async fn local_get_new_block_with_combined_transaction() {
.with_consensus_manager(rules.clone())
.with_validators(
BlockBodyFullValidator::new(rules.clone(), true),
HeaderFullValidator::new(rules.clone(), difficulty_calculator, false),
HeaderFullValidator::new(rules.clone(), difficulty_calculator),
BlockBodyInternalConsistencyValidator::new(rules, true, factories.clone()),
)
.start(temp_dir.path().to_str().unwrap())
Expand Down

0 comments on commit b5a5bed

Please sign in to comment.