Merge branch 'stefan/1499-remove_adapter_hacks' into 'master'

chore(NET-1499): remove request cache

 

See merge request dfinity-lab/public/ic!14823

rs/bitcoin/adapter/src/get_successors_handler.rs

@@ -62,7 +62,6 @@ pub struct GetSuccessorsResponse {
 pub struct GetSuccessorsHandler {
     state: Arc<Mutex<BlockchainState>>,
     blockchain_manager_tx: Sender<BlockchainManagerRequest>,
-    last_request: std::sync::Mutex<Option<(GetSuccessorsRequest, GetSuccessorsResponse)>>,
     network: Network,
     metrics: GetSuccessorMetrics,
 }
@@ -79,7 +78,6 @@ impl GetSuccessorsHandler {
         Self {
             state,
             blockchain_manager_tx,
-            last_request: std::sync::Mutex::new(None),
             network: config.network,
             metrics: GetSuccessorMetrics::new(metrics_registry),
         }
@@ -99,18 +97,6 @@ impl GetSuccessorsHandler {
             .processed_block_hashes
             .observe(request.processed_block_hashes.len() as f64);
 
-        if self.network == Network::Testnet || self.network == Network::Regtest {
-            // A cache entry has to be discarded after after a new request.
-            // Indexing the cache based on the request is not safe since the response
-            // depends on the internal state of the adapter.
-            if let Some((cached_request, cached_response)) =
-                self.last_request.lock().unwrap().take()
-            {
-                if cached_request == request {
-                    return Ok(cached_response);
-                }
-            }
-        }
         let response = {
             let state = self.state.lock().await;
             let anchor_height = state
@@ -144,13 +130,6 @@ impl GetSuccessorsHandler {
         self.metrics
             .response_blocks
             .observe(response.blocks.len() as f64);
-        // Set cache value
-        if self.network == Network::Testnet || self.network == Network::Regtest {
-            self.last_request
-                .lock()
-                .unwrap()
-                .replace((request.clone(), response.clone()));
-        }
 
         if !response.next.is_empty() {
             // TODO: better handling of full channel as the receivers are never closed.
@@ -490,69 +469,6 @@ mod test {
         assert_eq!(response.next.len(), 3);
     }
 
-    #[tokio::test]
-    async fn test_get_successors_cache() {
-        let config = ConfigBuilder::new().with_network(Network::Regtest).build();
-        let blockchain_state = BlockchainState::new(&config, &MetricsRegistry::default());
-        let genesis = *blockchain_state.genesis();
-        let genesis_hash = genesis.block_hash();
-        let (blockchain_manager_tx, _blockchain_manager_rx) =
-            channel::<BlockchainManagerRequest>(10);
-        let handler = GetSuccessorsHandler::new(
-            &config,
-            Arc::new(Mutex::new(blockchain_state)),
-            blockchain_manager_tx,
-            &MetricsRegistry::default(),
-        );
-
-        // Set up the following chain:
-        // 0 -> 1 -> 2 -> 3 -> 4 -> 5
-        let main_chain = generate_headers(genesis_hash, genesis.time, 5, &[]);
-        let main_block_1 = Block {
-            header: main_chain[0],
-            txdata: vec![],
-        };
-        let main_block_2 = Block {
-            header: main_chain[1],
-            txdata: vec![],
-        };
-        {
-            let mut blockchain = handler.state.lock().await;
-            blockchain.add_headers(&main_chain);
-            blockchain
-                .add_block(main_block_1.clone())
-                .expect("invalid block");
-            blockchain
-                .add_block(main_block_2.clone())
-                .expect("invalid block");
-        }
-        let request = GetSuccessorsRequest {
-            anchor: genesis_hash,
-            processed_block_hashes: vec![],
-        };
-
-        let cached_response = handler.get_successors(request.clone()).await.unwrap();
-        assert!(cached_response.blocks.len() == 2);
-        // Check that cached value is returned even though the state has changed
-        {
-            let mut blockchain = handler.state.lock().await;
-            let main_block_3 = Block {
-                header: main_chain[2],
-                txdata: vec![],
-            };
-            blockchain
-                .add_block(main_block_3.clone())
-                .expect("invalid block");
-        }
-        let response_from_cache = handler.get_successors(request.clone()).await.unwrap();
-        assert_eq!(cached_response, response_from_cache);
-        assert!(response_from_cache.blocks.len() == 2);
-
-        let non_cached_response = handler.get_successors(request).await.unwrap();
-        assert!(cached_response != non_cached_response);
-        assert!(non_cached_response.blocks.len() == 3);
-    }
-
     /// This tests ensures that `BlockchainManager::handle_client_request(...)` returns multiple
     /// blocks from the main chain and a fork. Order should be preserved.
     #[tokio::test]