fix(tap-manager): receipts being used after being added to RAV

receipts were being added to RAVs but the next RAV request would still contain the receipts causing RAV requests to fail.

BREAKING CHANGE: RAV Request definition is updated to include optional previous RAV

Signed-off-by: Bryan Cole <[email protected]>

Author: ColePBryan

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(())
     }
 
@@ -148,16 +151,22 @@ impl<
     ///
     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 +190,12 @@ 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;
         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,
 }

tap_core/src/tap_manager/test/manager_test.rs

@@ -176,6 +176,7 @@ mod manager_unit_test {
             get_full_list_of_checks(),
             starting_min_timestamp,
         );
+        collateral_storage.write().unwrap().insert(keys.1, 999999);
 
         let mut stored_signed_receipts = Vec::new();
         for query_id in 0..10 {
@@ -189,7 +190,6 @@ mod manager_unit_test {
                 .write()
                 .unwrap()
                 .insert(query_id, value);
-            collateral_storage.write().unwrap().insert(keys.1, 999999);
             assert!(manager
                 .verify_and_store_receipt(signed_receipt, query_id, initial_checks.clone())
                 .is_ok());
@@ -213,4 +213,125 @@ mod manager_unit_test {
             .verify_and_store_rav(rav_request.expected_rav, signed_rav)
             .is_ok());
     }
+
+    #[rstest]
+    #[case::full_checks(get_full_list_of_checks())]
+    #[case::partial_checks(vec![ReceiptCheck::CheckSignature])]
+    #[case::no_checks(Vec::<ReceiptCheck>::new())]
+    async fn manager_create_multiple_rav_requests_all_valid_receipts(
+        rav_storage_adapter: RAVStorageAdapterMock,
+        collateral_adapters: (CollateralAdapterMock, Arc<RwLock<HashMap<Address, u128>>>),
+        receipt_adapters: (
+            ReceiptStorageAdapterMock,
+            ReceiptChecksAdapterMock,
+            Arc<RwLock<HashMap<u64, u128>>>,
+        ),
+        keys: (LocalWallet, Address),
+        allocation_ids: Vec<Address>,
+        #[case] initial_checks: Vec<ReceiptCheck>,
+    ) {
+        let (collateral_adapter, collateral_storage) = collateral_adapters;
+        let (receipt_storage_adapter, receipt_checks_adapter, query_appraisal_storage) =
+            receipt_adapters;
+        // give receipt 5 second variance for min start time
+        let starting_min_timestamp = get_current_timestamp_u64_ns().unwrap() - 500000000;
+
+        let mut manager = Manager::new(
+            collateral_adapter,
+            receipt_checks_adapter,
+            rav_storage_adapter,
+            receipt_storage_adapter,
+            get_full_list_of_checks(),
+            starting_min_timestamp,
+        );
+
+        collateral_storage.write().unwrap().insert(keys.1, 999999);
+
+        let mut stored_signed_receipts = Vec::new();
+        let mut expected_accumulated_value = 0;
+        for query_id in 0..10 {
+            let value = 20u128;
+            let signed_receipt =
+                EIP712SignedMessage::new(Receipt::new(allocation_ids[0], value).unwrap(), &keys.0)
+                    .await
+                    .unwrap();
+            stored_signed_receipts.push(signed_receipt.clone());
+            query_appraisal_storage
+                .write()
+                .unwrap()
+                .insert(query_id, value);
+            assert!(manager
+                .verify_and_store_receipt(signed_receipt, query_id, initial_checks.clone())
+                .is_ok());
+            expected_accumulated_value += value;
+        }
+        let rav_request_result = manager.create_rav_request(0);
+        assert!(rav_request_result.is_ok());
+
+        let rav_request = rav_request_result.unwrap();
+        // all receipts passing
+        assert_eq!(
+            rav_request.valid_receipts.len(),
+            stored_signed_receipts.len()
+        );
+        // no receipts failing
+        assert_eq!(rav_request.invalid_receipts.len(), 0);
+        // accumulated value is correct
+        assert_eq!(
+            rav_request.expected_rav.value_aggregate,
+            expected_accumulated_value
+        );
+        // no previous rav
+        assert!(rav_request.previous_rav.is_none());
+
+        let signed_rav = EIP712SignedMessage::new(rav_request.expected_rav.clone(), &keys.0)
+            .await
+            .unwrap();
+        assert!(manager
+            .verify_and_store_rav(rav_request.expected_rav, signed_rav)
+            .is_ok());
+
+        stored_signed_receipts.clear();
+        for query_id in 10..20 {
+            let value = 20u128;
+            let signed_receipt =
+                EIP712SignedMessage::new(Receipt::new(allocation_ids[0], value).unwrap(), &keys.0)
+                    .await
+                    .unwrap();
+            stored_signed_receipts.push(signed_receipt.clone());
+            query_appraisal_storage
+                .write()
+                .unwrap()
+                .insert(query_id, value);
+            assert!(manager
+                .verify_and_store_receipt(signed_receipt, query_id, initial_checks.clone())
+                .is_ok());
+            expected_accumulated_value += value;
+        }
+        let rav_request_result = manager.create_rav_request(0);
+        assert!(rav_request_result.is_ok());
+
+        let rav_request = rav_request_result.unwrap();
+        // all receipts passing
+        assert_eq!(
+            rav_request.valid_receipts.len(),
+            stored_signed_receipts.len()
+        );
+        // no receipts failing
+        assert_eq!(rav_request.invalid_receipts.len(), 0);
+        // accumulated value is correct
+        assert_eq!(
+            rav_request.expected_rav.value_aggregate,
+            expected_accumulated_value
+        );
+        // no previous rav
+        assert!(rav_request.previous_rav.is_some());
+
+        let signed_rav = EIP712SignedMessage::new(rav_request.expected_rav.clone(), &keys.0)
+            .await
+            .unwrap();
+        assert!(manager
+            .verify_and_store_rav(rav_request.expected_rav, signed_rav)
+            .is_ok());
+    }
 }