refactor(peer_loop): Validate SyncChallenge

Ensure that sync challenge does not reference block 1 (as that would
necessitate sending genesis block) and all referenced heights are
smaller than tip.

Co-authored-by: Alan Szepieniec <[email protected]>

src/models/blockchain/block/mod.rs

@@ -1065,11 +1065,11 @@ mod block_tests {
     use crate::models::state::wallet::transaction_output::TxOutput;
     use crate::models::state::wallet::utxo_notification::UtxoNotificationMedium;
     use crate::models::state::wallet::WalletSecret;
+    use crate::tests::shared::fake_valid_successor_for_tests;
     use crate::tests::shared::invalid_block_with_transaction;
     use crate::tests::shared::make_mock_block;
     use crate::tests::shared::make_mock_transaction;
     use crate::tests::shared::mock_genesis_global_state;
-    use crate::tests::shared::valid_successor_for_tests;
     use crate::util_types::mutator_set::archival_mmr::ArchivalMmr;
 
     #[test]
@@ -1157,7 +1157,7 @@ mod block_tests {
         let now = genesis_block.kernel.header.timestamp + Timestamp::hours(2);
         let mut rng: StdRng = SeedableRng::seed_from_u64(2225550001);
 
-        let mut block1 = valid_successor_for_tests(&genesis_block, now, rng.gen()).await;
+        let mut block1 = fake_valid_successor_for_tests(&genesis_block, now, rng.gen()).await;
 
         let timestamp = block1.kernel.header.timestamp;
         assert!(block1.is_valid(&genesis_block, timestamp).await);
@@ -1266,7 +1266,7 @@ mod block_tests {
         use super::*;
         use crate::mine_loop::mine_loop_tests::make_coinbase_transaction_from_state;
         use crate::models::state::wallet::address::KeyType;
-        use crate::tests::shared::valid_successor_for_tests;
+        use crate::tests::shared::fake_valid_successor_for_tests;
 
         #[traced_test]
         #[tokio::test]
@@ -1285,7 +1285,7 @@ mod block_tests {
             let plus_seven_months = genesis_block.kernel.header.timestamp + Timestamp::months(7);
             let mut rng: StdRng = SeedableRng::seed_from_u64(2225550001);
             let block1 =
-                valid_successor_for_tests(&genesis_block, plus_seven_months, rng.gen()).await;
+                fake_valid_successor_for_tests(&genesis_block, plus_seven_months, rng.gen()).await;
 
             let alice_wallet = WalletSecret::devnet_wallet();
             let mut alice = mock_genesis_global_state(
@@ -1444,7 +1444,7 @@ mod block_tests {
             let mut now = genesis_block.kernel.header.timestamp + Timestamp::hours(2);
             let mut rng: StdRng = SeedableRng::seed_from_u64(2225550001);
 
-            let mut block1 = valid_successor_for_tests(&genesis_block, now, rng.gen()).await;
+            let mut block1 = fake_valid_successor_for_tests(&genesis_block, now, rng.gen()).await;
 
             // Set block timestamp 1 hour in the future.  (is valid)
             let future_time1 = now + Timestamp::hours(1);

src/models/peer.rs

@@ -738,6 +738,8 @@ pub(crate) struct SyncChallengeResponse {
 }
 
 impl SyncChallengeResponse {
+    /// Determine whether the `SyncChallengeResponse` answers the given
+    /// `IssuedSyncChallenge`, and not some other one.
     pub(crate) fn matches(&self, issued_challenge: IssuedSyncChallenge) -> bool {
         let Ok(tip_predecessor) = Block::try_from(self.tip_parent.clone()) else {
             return false;
@@ -755,6 +757,8 @@ impl SyncChallengeResponse {
             && tip.has_proof_of_work(tip_predecessor.header())
     }
 
+    /// Determine whether the proofs in `SyncChallengeResponse` are valid. Also
+    /// checks proof-of-work.
     pub(crate) async fn is_valid(&self, now: Timestamp) -> bool {
         let Ok(tip_predecessor) = Block::try_from(self.tip_parent.clone()) else {
             return false;

src/models/peer/transfer_block.rs

@@ -79,8 +79,8 @@ mod test {
     use super::*;
     use crate::models::peer::Network;
     use crate::models::proof_abstractions::timestamp::Timestamp;
+    use crate::tests::shared::fake_valid_sequence_of_blocks_for_tests;
     use crate::tests::shared::invalid_empty_block;
-    use crate::tests::shared::valid_sequence_of_blocks_for_tests;
 
     #[test]
     fn cannot_transfer_blocks_that_are_not_single_proof_supported() {
@@ -105,7 +105,7 @@ mod test {
         // TransferBlock::into() will panic if it
         // encounters the genesis block.
         let genesis = Block::genesis_block(network);
-        let [block1] = valid_sequence_of_blocks_for_tests(
+        let [block1] = fake_valid_sequence_of_blocks_for_tests(
             &genesis,
             Timestamp::hours(1),
             StdRng::seed_from_u64(5550001).gen(),

src/models/proof_abstractions/verifier.rs

@@ -44,8 +44,8 @@ pub(crate) async fn cache_true_claim(claim: Claim) {
 #[cfg(test)]
 pub(crate) mod test {
     use itertools::Itertools;
-    use rand::{thread_rng, Rng};
-
+    use rand::thread_rng;
+    use rand::Rng;
     use tasm_lib::prelude::Tip5;
     use triton_vm::prelude::BFieldCodec;
 

src/models/state/mod.rs

@@ -1482,36 +1482,47 @@ impl GlobalState {
         // Ok(())
     }
 
-    /// Returns (parent, child) pair of blocks.
-    pub(crate) async fn fetch_block_pair(&self, child_digest: Digest) -> Option<(Block, Block)> {
-        let child = self
-            .chain
-            .archival_state()
-            .get_block(child_digest)
-            .await
-            .expect("fetching block from archival state should work.");
-        let Some(child) = child else {
-            warn!("Got sync challenge for unknown tip");
-
-            return None;
-        };
-        let parent_digest = child.header().prev_block_digest;
-        let parent = self
-            .chain
-            .archival_state()
-            .get_block(parent_digest)
-            .await
-            .expect("fetching block from archival state should work.")
-            .expect("parent of known block from archival state must exist.");
-
-        Some((parent, child))
-    }
-
     pub(crate) async fn response_to_sync_challenge(
         &self,
         sync_challenge: SyncChallenge,
     ) -> Result<SyncChallengeResponse> {
-        let Some((tip_parent, tip)) = self.fetch_block_pair(sync_challenge.tip_digest).await else {
+        async fn fetch_block_pair(
+            state: &GlobalState,
+            child_digest: Digest,
+        ) -> Option<(Block, Block)> {
+            let child = state
+                .chain
+                .archival_state()
+                .get_block(child_digest)
+                .await
+                .expect("fetching block from archival state should work.");
+            let Some(child) = child else {
+                warn!("Got sync challenge for unknown tip");
+
+                return None;
+            };
+            if child.header().height < 2.into() {
+                warn!("Got sync challenge for tip of too low height; cannot send genesis block");
+
+                return None;
+            }
+
+            let parent_digest = child.header().prev_block_digest;
+            let parent = state
+                .chain
+                .archival_state()
+                .get_block(parent_digest)
+                .await
+                .expect("fetching block from archival state should work.")
+                .expect(
+                    "parent of known block from archival state must exist, if height exceeds 1.",
+                );
+
+            Some((parent, child))
+        }
+
+        let Some((tip_parent, tip)) = fetch_block_pair(self, sync_challenge.tip_digest).await
+        else {
             bail!("could not fetch tip and tip predecessor");
         };
 
@@ -1525,6 +1536,9 @@ impl GlobalState {
             if h < 2u64.into() {
                 bail!("challenge asks for genesis block");
             }
+            if h >= tip.header().height {
+                bail!("challenge asks for height that's not ancestor to tip.");
+            }
 
             let Some(child_digest) = self
                 .chain
@@ -1535,7 +1549,7 @@ impl GlobalState {
             else {
                 bail!("could not get leaf from archival block mmr");
             };
-            let Some((p, c)) = self.fetch_block_pair(child_digest).await else {
+            let Some((p, c)) = fetch_block_pair(self, child_digest).await else {
                 bail!("could not fetch indicated block pair");
             };
 
@@ -1636,9 +1650,9 @@ mod global_state_tests {
     use crate::config_models::network::Network;
     use crate::mine_loop::mine_loop_tests::make_coinbase_transaction_from_state;
     use crate::models::blockchain::block::Block;
+    use crate::tests::shared::fake_valid_successor_for_tests;
     use crate::tests::shared::make_mock_block;
     use crate::tests::shared::mock_genesis_global_state;
-    use crate::tests::shared::valid_successor_for_tests;
     use crate::tests::shared::wallet_state_has_all_valid_mps;
 
     #[traced_test]
@@ -2666,7 +2680,7 @@ mod global_state_tests {
         let genesis_block = Block::genesis_block(network);
         let now = genesis_block.kernel.header.timestamp + Timestamp::hours(1);
 
-        let block1 = valid_successor_for_tests(&genesis_block, now, Default::default()).await;
+        let block1 = fake_valid_successor_for_tests(&genesis_block, now, Default::default()).await;
 
         global_state_lock.set_new_tip(block1).await.unwrap();