Artificial checkpoint only for 1st batch

Author: ii-cruz

blockchain/src/blockchain/history_sync.rs

@@ -199,54 +199,46 @@ impl Blockchain {
         let mut block_state = vec![];
         let mut block_transactions = vec![];
         let mut block_inherents = vec![];
-
-        let mut prev_batch = 0;
         let mut prev_block = 0;
+        let mut prev_batch = this
+            .chain_store
+            .get_block(&this.state.head_hash, false, Some(&txn))
+            .map_or(0, |head| Policy::batch_at(head.block_number()));
 
         for hist_tx in history.iter().skip(first_new_hist_tx) {
             if hist_tx.block_number > prev_block {
-                // If a macro block does not have any history items, we need to add it here so that
-                // we always commit FinalizeBatch/FinalizeEpoch inherents.
-                // FIXME We're missing the block timestamp to do this correctly.
-                // Also, this works only if a single macro block is missing between history items.
-                let batch_number = Policy::batch_at(hist_tx.block_number);
-                if batch_number > prev_batch
-                    && block_state.last().is_some_and(|block_state: &BlockState| {
-                        !Policy::is_macro_block_at(block_state.number)
-                    })
-                {
+                let new_batch = Policy::batch_at(hist_tx.block_number);
+                // If we are crossing a batch boundary we must artificially put the macro block, so that we have the
+                // finalize batch inherent generated as well. However, since checkpoint blocks have reward txs, the only macro block
+                // we may have a gap for is the first checkpoint block due to the batch delay on reward payouts.
+                if prev_batch < new_batch {
+                    if prev_batch + 1 < new_batch && (prev_batch != 0 || new_batch != 2) {
+                        error!(
+                            new_batch,
+                            "We cannot skip over macro blocks after batch 1 due to reward payout txs."
+                        );
+                        #[cfg(feature = "metrics")]
+                        this.metrics.note_invalid_block();
+                        return Err(PushError::InvalidBlock(BlockError::InvalidHistoryRoot));
+                    }
+                    // If we are missing the very first macro block we add it artificially here and this also
+                    // means we lost its timestamp.
                     debug!(
                         history_item_block_number = hist_tx.block_number,
                         prev_block,
-                        history_item_batch = batch_number,
+                        history_item_batch = new_batch,
                         prev_batch,
                         last_block = ?block_state.last(),
-                        "Inserting macro block"
+                        "Adding the first checkpoint block manually since there weren't any txs on it."
                     );
-                    if batch_number != prev_batch + 1 {
-                        warn!(
-                            %block,
-                            reason = "missing batch in history",
-                            history_item_block_number = hist_tx.block_number,
-                            history_item_batch = batch_number,
-                            prev_batch,
-                            "Rejecting block",
-                        );
-                        txn.abort();
-                        #[cfg(feature = "metrics")]
-                        this.metrics.note_invalid_block();
-                        return Err(PushError::InvalidBlock(BlockError::InvalidHistoryRoot));
-                    }
-
                     block_state.push(BlockState {
-                        number: Policy::macro_block_of(prev_batch).unwrap(),
-                        time: 0,                                        // FIXME
+                        number: Policy::macro_block_before(hist_tx.block_number),
+                        time: 0,
                         protocol_version: this.state.current_version(), // Cannot change, protocol version upgrades only on election blocks.
                     });
                     block_transactions.push(vec![]);
                     block_inherents.push(vec![]);
                 }
-
                 block_state.push(BlockState {
                     number: hist_tx.block_number,
                     time: hist_tx.block_time,
@@ -255,8 +247,8 @@ impl Blockchain {
                 block_transactions.push(vec![]);
                 block_inherents.push(vec![]);
 
-                prev_batch = batch_number;
                 prev_block = hist_tx.block_number;
+                prev_batch = new_batch;
             }
 
             match &hist_tx.data {

blockchain/tests/history_sync.rs

@@ -1,11 +1,12 @@
-use std::sync::Arc;
+use std::{collections::BTreeSet, sync::Arc};
 
+use nimiq_account::Account;
 use nimiq_block::BlockError;
 use nimiq_blockchain::{interface::HistoryInterface, BlockProducer, Blockchain, BlockchainConfig};
 use nimiq_blockchain_interface::{AbstractBlockchain, BlockchainEvent, PushError, PushResult};
-use nimiq_database::mdbx::MdbxDatabase;
+use nimiq_database::{mdbx::MdbxDatabase, traits::WriteTransaction};
 use nimiq_genesis::NetworkId;
-use nimiq_primitives::policy::Policy;
+use nimiq_primitives::{key_nibbles::KeyNibbles, policy::Policy};
 use nimiq_serde::{Deserialize, Serialize};
 use nimiq_test_log::test;
 use nimiq_test_utils::{
@@ -288,12 +289,10 @@ fn history_sync_emits_protocol_upgrade_event_when_version_changes() {
     ));
 
     let producer = BlockProducer::new(signing_key(), voting_key());
-    produce_macro_blocks_with_txns(
+    produce_macro_blocks(
         &producer,
         &blockchain1,
         (Policy::batches_per_epoch() - 1) as usize,
-        1,
-        0,
     );
     let upgrade_version = signal_next_protocol_version_via_tx(&producer, &blockchain1);
     fill_micro_blocks_with_txns(&producer, &blockchain1, 1, 0);
@@ -308,7 +307,7 @@ fn history_sync_emits_protocol_upgrade_event_when_version_changes() {
     let upgrade_history = blockchain1
         .read()
         .history_store
-        .get_epoch_transactions(1, None);
+        .get_epoch_transactions(upgrade_block.epoch_number(), None);
 
     let time2 = Arc::new(OffsetTime::new());
     let env2 = MdbxDatabase::new_volatile(Default::default()).unwrap();
@@ -341,6 +340,253 @@ fn history_sync_emits_protocol_upgrade_event_when_version_changes() {
     );
 }
 
+#[test]
+fn history_sync_replays_empty_first_batch_when_it_changes_state() {
+    fn seed_current_batch_punished_slot(blockchain: &Arc<RwLock<Blockchain>>) {
+        let blockchain = blockchain.write();
+        let mut staking_contract = blockchain.get_staking_contract();
+        let validator_address = staking_contract
+            .active_validators
+            .keys()
+            .next()
+            .expect("missing active validator")
+            .clone();
+
+        staking_contract
+            .punished_slots
+            .current_batch_punished_slots
+            .clear();
+        staking_contract
+            .punished_slots
+            .previous_batch_punished_slots
+            .clear();
+
+        let mut punished_slots = BTreeSet::new();
+        punished_slots.insert(0);
+        staking_contract
+            .punished_slots
+            .current_batch_punished_slots
+            .insert(validator_address, punished_slots);
+
+        let mut txn = blockchain.write_transaction();
+        blockchain
+            .state
+            .accounts
+            .tree
+            .put(
+                &mut (&mut txn).into(),
+                &KeyNibbles::from(&Policy::STAKING_CONTRACT_ADDRESS),
+                Account::Staking(staking_contract),
+            )
+            .unwrap();
+        txn.commit();
+    }
+
+    let genesis_block_number = Policy::genesis_block_number();
+    let num_macro_blocks = 2;
+
+    // Create a blockchain to produce the macro blocks.
+    let time = Arc::new(OffsetTime::new());
+    let env = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain = Arc::new(RwLock::new(
+        Blockchain::new(
+            env,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+    seed_current_batch_punished_slot(&blockchain);
+
+    // Create a second blockchain to push blocks to.
+    let time = Arc::new(OffsetTime::new());
+    let env2 = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain2 = Arc::new(RwLock::new(
+        Blockchain::new(
+            env2,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+    seed_current_batch_punished_slot(&blockchain2);
+
+    // Produce the blocks on blockchain1.
+    let producer = BlockProducer::new(signing_key(), voting_key());
+    produce_macro_blocks(&producer, &blockchain, num_macro_blocks);
+
+    // Get the first checkpoint and corresponding history tree transactions.
+    let blockchain_rg = blockchain.read();
+    let election_txs_1 = blockchain_rg.history_store.get_epoch_transactions(1, None);
+
+    let checkpoint_block_1_2 = blockchain_rg
+        .chain_store
+        .get_block_at(
+            Policy::blocks_per_batch() * 2 + genesis_block_number,
+            true,
+            None,
+        )
+        .unwrap();
+    let mut checkpoint_txs_1_2 = vec![];
+
+    for hist_tx in &election_txs_1 {
+        if hist_tx.block_number > Policy::blocks_per_batch() * 2 + genesis_block_number {
+            break;
+        }
+        checkpoint_txs_1_2.push(hist_tx.clone());
+    }
+
+    // The first checkpoint of the first epoch has no history items of its own. Replaying it still
+    // matters here because its FinalizeBatch clears the seeded punished slots before the next
+    // checkpoint reward is applied.
+    assert_eq!(
+        Blockchain::push_history_sync(
+            blockchain2.upgradable_read(),
+            checkpoint_block_1_2,
+            &checkpoint_txs_1_2
+        ),
+        Ok(PushResult::Extended)
+    );
+}
+
+#[test]
+fn history_sync_replays_missing_first_checkpoint_before_second_batch_history() {
+    let genesis_block_number = Policy::genesis_block_number();
+
+    let time = Arc::new(OffsetTime::new());
+    let env = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain1 = Arc::new(RwLock::new(
+        Blockchain::new(
+            env,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+
+    let producer = BlockProducer::new(signing_key(), voting_key());
+
+    // Leave the first batch empty so the first checkpoint is missing from history, then add
+    // transactions in the second batch before producing the target checkpoint.
+    produce_macro_blocks(&producer, &blockchain1, 1);
+    fill_micro_blocks_with_txns(&producer, &blockchain1, 1, 0);
+    produce_macro_blocks(&producer, &blockchain1, 1);
+
+    let blockchain_rg = blockchain1.read();
+    let checkpoint_block_1_2 = blockchain_rg
+        .chain_store
+        .get_block_at(
+            Policy::blocks_per_batch() * 2 + genesis_block_number,
+            true,
+            None,
+        )
+        .unwrap();
+    let checkpoint_txs_1_2 = blockchain_rg.history_store.get_epoch_transactions(1, None);
+
+    assert!(
+        checkpoint_txs_1_2
+            .iter()
+            .all(|hist_tx| hist_tx.block_number > Policy::blocks_per_batch() + genesis_block_number),
+        "test setup must keep the first batch empty",
+    );
+    assert!(
+        checkpoint_txs_1_2
+            .iter()
+            .any(|hist_tx| Policy::batch_at(hist_tx.block_number) == 2),
+        "test setup must retain second-batch history",
+    );
+
+    let time = Arc::new(OffsetTime::new());
+    let env2 = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain2 = Arc::new(RwLock::new(
+        Blockchain::new(
+            env2,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+
+    assert_eq!(
+        Blockchain::push_history_sync(
+            blockchain2.upgradable_read(),
+            checkpoint_block_1_2,
+            &checkpoint_txs_1_2
+        ),
+        Ok(PushResult::Extended)
+    );
+    assert_eq!(blockchain_rg.head(), blockchain2.read().head());
+}
+
+#[test]
+fn history_sync_rejects_missing_non_first_batch_history() {
+    let genesis_block_number = Policy::genesis_block_number();
+    let num_macro_blocks = 3;
+
+    let time = Arc::new(OffsetTime::new());
+    let env = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain = Arc::new(RwLock::new(
+        Blockchain::new(
+            env,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+
+    let producer = BlockProducer::new(signing_key(), voting_key());
+    produce_macro_blocks_with_txns(&producer, &blockchain, num_macro_blocks, 1, 0);
+
+    let blockchain_rg = blockchain.read();
+    let election_txs_1 = blockchain_rg.history_store.get_epoch_transactions(1, None);
+    let checkpoint_block_1_3 = blockchain_rg
+        .chain_store
+        .get_block_at(
+            Policy::blocks_per_batch() * 3 + genesis_block_number,
+            true,
+            None,
+        )
+        .unwrap();
+
+    let checkpoint_txs_1_3: Vec<_> = election_txs_1
+        .into_iter()
+        .filter(|hist_tx| {
+            hist_tx.block_number > Policy::blocks_per_batch() * 2 + genesis_block_number
+        })
+        .collect();
+
+    assert!(
+        !checkpoint_txs_1_3.is_empty(),
+        "test setup must retain history after the skipped batch",
+    );
+
+    let time = Arc::new(OffsetTime::new());
+    let env2 = MdbxDatabase::new_volatile(Default::default()).unwrap();
+    let blockchain2 = Arc::new(RwLock::new(
+        Blockchain::new(
+            env2,
+            BlockchainConfig::default(),
+            NetworkId::UnitAlbatross,
+            time,
+        )
+        .unwrap(),
+    ));
+
+    assert_eq!(
+        Blockchain::push_history_sync(
+            blockchain2.upgradable_read(),
+            checkpoint_block_1_3,
+            &checkpoint_txs_1_3
+        ),
+        Err(PushError::InvalidBlock(BlockError::InvalidHistoryRoot))
+    );
+}
+
 // Tests if the history sync works when micro blocks have already been pushed in the blockchain.
 // This basically tests if we can go from the history sync to the normal follow mode and back.
 #[test]