Merge pull request #109 from semiotic-ai/105-tap_manager-always-requesting-rav-using-all-receipts-even-when-some-receipts-included-in-previous-rav

105 tap manager always requesting rav using all receipts even when some receipts included in previous rav

Author: ColePBryan

CONTRIBUTING.md

@@ -66,33 +66,33 @@ Depending on how large the project is, you may want to outsource the questioning
 > ```
 > Developer Certificate of Origin
 > Version 1.1
-> 
+>
 > Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
-> 
+>
 > Everyone is permitted to copy and distribute verbatim copies of this
 > license document, but changing it is not allowed.
-> 
-> 
+>
+>
 > Developer's Certificate of Origin 1.1
-> 
+>
 > By making a contribution to this project, I certify that:
-> 
+>
 > (a) The contribution was created in whole or in part by me and I
 >     have the right to submit it under the open source license
 >     indicated in the file; or
-> 
+>
 > (b) The contribution is based upon previous work that, to the best
 >     of my knowledge, is covered under an appropriate open source
 >     license and I have the right under that license to submit that
 >     work with modifications, whether created in whole or in part
 >     by me, under the same open source license (unless I am
 >     permitted to submit under a different license), as indicated
 >     in the file; or
-> 
+>
 > (c) The contribution was provided directly to me by some other
 >     person who certified (a), (b) or (c) and I have not modified
 >     it.
-> 
+>
 > (d) I understand and agree that this project and the contribution
 >     are public and that a record of the contribution (including all
 >     personal information I submit with it, including my sign-off) is
@@ -146,7 +146,7 @@ Once it's filed:
 
 ### Suggesting Enhancements
 
-This section guides you through submitting an enhancement suggestion for H2S2, **including completely new features and minor improvements to existing functionality**. Following these guidelines will help maintainers and the community to understand your suggestion and find related suggestions.
+This section guides you through submitting an enhancement suggestion for TAP, **including completely new features and minor improvements to existing functionality**. Following these guidelines will help maintainers and the community to understand your suggestion and find related suggestions.
 
 <!-- omit in toc -->
 #### Before Submitting an Enhancement
@@ -165,7 +165,7 @@ Enhancement suggestions are tracked as [GitHub issues](https://github.com/semiot
 - Provide a **step-by-step description of the suggested enhancement** in as many details as possible.
 - **Describe the current behavior** and **explain which behavior you expected to see instead** and why. At this point you can also tell which alternatives do not work for you.
 - You may want to **include screenshots and animated GIFs** which help you demonstrate the steps or point out the part which the suggestion is related to. You can use [this tool](https://www.cockos.com/licecap/) to record GIFs on macOS and Windows, and [this tool](https://github.com/colinkeenan/silentcast) or [this tool](https://github.com/GNOME/byzanz) on Linux. <!-- this should only be included if the project has a GUI -->
-- **Explain why this enhancement would be useful** to most H2S2 users. You may also want to point out the other projects that solved it better and which could serve as inspiration.
+- **Explain why this enhancement would be useful** to most TAP users. You may also want to point out the other projects that solved it better and which could serve as inspiration.
 
 <!-- You might want to create an issue template for enhancement suggestions that can be used as a guide and that defines the structure of the information to be included. If you do so, reference it here in the description. -->
 

tap_core/src/adapters/receipt_storage_adapter.rs

@@ -1,6 +1,8 @@
 // Copyright 2023-, Semiotic AI, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use std::ops::Range;
+
 use crate::tap_receipt::ReceivedReceipt;
 
 pub trait ReceiptStorageAdapter {
@@ -20,11 +22,19 @@ pub trait ReceiptStorageAdapter {
         &self,
         timestamp_ns: u64,
     ) -> Result<Vec<(u64, ReceivedReceipt)>, Self::AdapterError>;
+    fn retrieve_receipts_in_timestamp_range(
+        &self,
+        timestamp_range_ns: Range<u64>,
+    ) -> Result<Vec<(u64, ReceivedReceipt)>, Self::AdapterError>;
     fn update_receipt_by_id(
         &mut self,
         receipt_id: u64,
         receipt: ReceivedReceipt,
     ) -> Result<(), Self::AdapterError>;
     fn remove_receipt_by_id(&mut self, receipt_id: u64) -> Result<(), Self::AdapterError>;
     fn remove_receipts_by_ids(&mut self, receipt_ids: &[u64]) -> Result<(), Self::AdapterError>;
+    fn remove_receipts_in_timestamp_range(
+        &mut self,
+        timestamp_ns: Range<u64>,
+    ) -> Result<(), Self::AdapterError>;
 }

tap_core/src/adapters/test/receipt_storage_adapter_mock.rs

@@ -3,6 +3,7 @@
 
 use std::{
     collections::HashMap,
+    ops::Range,
     sync::{Arc, RwLock},
 };
 
@@ -35,7 +36,12 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
     type AdapterError = AdpaterErrorMock;
     fn store_receipt(&mut self, receipt: ReceivedReceipt) -> Result<u64, Self::AdapterError> {
         let id = self.unique_id;
-        let mut receipt_storage = self.receipt_storage.write().unwrap();
+        let mut receipt_storage =
+            self.receipt_storage
+                .write()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
         receipt_storage.insert(id, receipt);
         self.unique_id += 1;
         Ok(id)
@@ -44,7 +50,12 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
         &self,
         receipt_id: u64,
     ) -> Result<ReceivedReceipt, Self::AdapterError> {
-        let receipt_storage = self.receipt_storage.read().unwrap();
+        let receipt_storage =
+            self.receipt_storage
+                .read()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
 
         receipt_storage
             .get(&receipt_id)
@@ -57,7 +68,12 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
         &self,
         timestamp_ns: u64,
     ) -> Result<Vec<(u64, ReceivedReceipt)>, Self::AdapterError> {
-        let receipt_storage = self.receipt_storage.read().unwrap();
+        let receipt_storage =
+            self.receipt_storage
+                .read()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
         Ok(receipt_storage
             .iter()
             .filter(|(_, rx_receipt)| {
@@ -70,7 +86,12 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
         &self,
         timestamp_ns: u64,
     ) -> Result<Vec<(u64, ReceivedReceipt)>, Self::AdapterError> {
-        let receipt_storage = self.receipt_storage.read().unwrap();
+        let receipt_storage =
+            self.receipt_storage
+                .read()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
         Ok(receipt_storage
             .iter()
             .filter(|(_, rx_receipt)| {
@@ -79,12 +100,35 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
             .map(|(&id, rx_receipt)| (id, rx_receipt.clone()))
             .collect())
     }
+    fn retrieve_receipts_in_timestamp_range(
+        &self,
+        timestamp_range_ns: Range<u64>,
+    ) -> Result<Vec<(u64, ReceivedReceipt)>, Self::AdapterError> {
+        let receipt_storage =
+            self.receipt_storage
+                .read()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
+        Ok(receipt_storage
+            .iter()
+            .filter(|(_, rx_receipt)| {
+                timestamp_range_ns.contains(&rx_receipt.signed_receipt.message.timestamp_ns)
+            })
+            .map(|(&id, rx_receipt)| (id, rx_receipt.clone()))
+            .collect())
+    }
     fn update_receipt_by_id(
         &mut self,
         receipt_id: u64,
         receipt: ReceivedReceipt,
     ) -> Result<(), Self::AdapterError> {
-        let mut receipt_storage = self.receipt_storage.write().unwrap();
+        let mut receipt_storage =
+            self.receipt_storage
+                .write()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
 
         if !receipt_storage.contains_key(&receipt_id) {
             return Err(AdpaterErrorMock::AdapterError {
@@ -97,7 +141,12 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
         Ok(())
     }
     fn remove_receipt_by_id(&mut self, receipt_id: u64) -> Result<(), Self::AdapterError> {
-        let mut receipt_storage = self.receipt_storage.write().unwrap();
+        let mut receipt_storage =
+            self.receipt_storage
+                .write()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
         receipt_storage
             .remove(&receipt_id)
             .map(|_| ())
@@ -111,4 +160,19 @@ impl ReceiptStorageAdapter for ReceiptStorageAdapterMock {
         }
         Ok(())
     }
+    fn remove_receipts_in_timestamp_range(
+        &mut self,
+        timestamp_ns: Range<u64>,
+    ) -> Result<(), Self::AdapterError> {
+        let mut receipt_storage =
+            self.receipt_storage
+                .write()
+                .map_err(|e| Self::AdapterError::AdapterError {
+                    error: e.to_string(),
+                })?;
+        receipt_storage.retain(|_, rx_receipt| {
+            !timestamp_ns.contains(&rx_receipt.signed_receipt.message.timestamp_ns)
+        });
+        Ok(())
+    }
 }

tap_core/src/error.rs

@@ -39,6 +39,11 @@ pub enum Error {
     AdapterError { source_error_message: String },
     #[error("Failed to produce rav request, no valid receipts")]
     NoValidReceiptsForRAVRequest,
+    #[error("Timestamp range error: min_timestamp_ns: {min_timestamp_ns}, max_timestamp_ns: {max_timestamp_ns}. Adjust timestamp buffer.")]
+    TimestampRangeError {
+        min_timestamp_ns: u64,
+        max_timestamp_ns: u64,
+    },
 }
 
 pub type Result<T> = StdResult<T, Error>;

tap_core/src/tap_manager/manager.rs

@@ -129,12 +129,15 @@ impl<
             });
         }
 
-        self.rav_storage_adapter
+        let rav_id = self
+            .rav_storage_adapter
             .store_rav(signed_rav)
             .map_err(|err| Error::AdapterError {
                 source_error_message: err.to_string(),
             })?;
 
+        self.current_rav_id = Some(rav_id);
+
         Ok(())
     }
 
@@ -146,18 +149,26 @@ impl<
     ///
     /// Returns [`Error::AdapterError`] if unable to fetch previous RAV or if unable to fetch previous receipts
     ///
+    /// Returns [`Error::TimestampRangeError`] if the max timestamp of the previous RAV is greater than the min timestamp. Caused by timestamp buffer being too large, or requests coming too soon.
+    ///
     pub fn create_rav_request(&mut self, timestamp_buffer_ns: u64) -> Result<RAVRequest, Error> {
         let previous_rav = self.get_previous_rav()?;
+        let min_timestamp_ns = previous_rav
+            .as_ref()
+            .map(|rav| rav.message.timestamp_ns + 1)
+            .unwrap_or(0);
 
-        let (valid_receipts, invalid_receipts) = self.collect_receipts(timestamp_buffer_ns)?;
+        let (valid_receipts, invalid_receipts) =
+            self.collect_receipts(timestamp_buffer_ns, min_timestamp_ns)?;
 
-        let expected_rav = Self::generate_expected_rav(&valid_receipts, previous_rav)?;
+        let expected_rav = Self::generate_expected_rav(&valid_receipts, previous_rav.clone())?;
 
         self.receipt_auditor
             .update_min_timestamp_ns(expected_rav.timestamp_ns + 1);
 
         Ok(RAVRequest {
             valid_receipts,
+            previous_rav,
             invalid_receipts,
             expected_rav,
         })
@@ -181,11 +192,19 @@ impl<
     fn collect_receipts(
         &mut self,
         timestamp_buffer_ns: u64,
+        min_timestamp_ns: u64,
     ) -> Result<(Vec<SignedReceipt>, Vec<SignedReceipt>), Error> {
-        let cutoff_timestamp = crate::get_current_timestamp_u64_ns()? - timestamp_buffer_ns;
+        let max_timestamp_ns = crate::get_current_timestamp_u64_ns()? - timestamp_buffer_ns;
+
+        if min_timestamp_ns > max_timestamp_ns {
+            return Err(Error::TimestampRangeError {
+                min_timestamp_ns,
+                max_timestamp_ns,
+            });
+        }
         let received_receipts = self
             .receipt_storage_adapter
-            .retrieve_receipts_upto_timestamp(cutoff_timestamp)
+            .retrieve_receipts_in_timestamp_range(min_timestamp_ns..max_timestamp_ns)
             .map_err(|err| Error::AdapterError {
                 source_error_message: err.to_string(),
             })?;

tap_core/src/tap_manager/rav_request.rs

@@ -3,13 +3,14 @@
 
 use serde::{Deserialize, Serialize};
 
-use super::SignedReceipt;
+use super::{SignedRAV, SignedReceipt};
 use crate::receipt_aggregate_voucher::ReceiptAggregateVoucher;
 
 #[derive(Debug, Serialize, Deserialize, Clone)]
 
 pub struct RAVRequest {
     pub valid_receipts: Vec<SignedReceipt>,
+    pub previous_rav: Option<SignedRAV>,
     pub invalid_receipts: Vec<SignedReceipt>,
     pub expected_rav: ReceiptAggregateVoucher,
 }