fix(eip712): enhance receipt uniqueness verification

Signed-off-by: Joseph Livesey <[email protected]>

Author: suchapalaver

tap_core/src/manager/tap_manager.rs

@@ -138,12 +138,13 @@ where
         let mut failed_receipts = vec![];
 
         // check for timestamp
-        let (checking_receipts, already_failed) =
-            TimestampCheck(min_timestamp_ns).check_batch(checking_receipts);
+        let (checking_receipts, already_failed) = TimestampCheck(min_timestamp_ns)
+            .check_batch(&self.domain_separator, checking_receipts)?;
         failed_receipts.extend(already_failed);
 
         // check for uniqueness
-        let (checking_receipts, already_failed) = UniqueCheck.check_batch(checking_receipts);
+        let (checking_receipts, already_failed) =
+            UniqueCheck.check_batch(&self.domain_separator, checking_receipts)?;
         failed_receipts.extend(already_failed);
 
         for receipt in checking_receipts.into_iter() {

tap_core/tests/manager_test.rs

@@ -139,7 +139,7 @@ async fn manager_verify_and_store_varying_initial_checks(
         &signer,
     )
     .unwrap();
-    let query_id = signed_receipt.unique_hash();
+    let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
     query_appraisals.write().unwrap().insert(query_id, value);
     escrow_storage
         .write()
@@ -182,7 +182,7 @@ async fn manager_create_rav_request_all_valid_receipts(
             &signer,
         )
         .unwrap();
-        let query_id = signed_receipt.unique_hash();
+        let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
         stored_signed_receipts.push(signed_receipt.clone());
         query_appraisals.write().unwrap().insert(query_id, value);
         assert!(manager
@@ -277,7 +277,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts(
             &signer,
         )
         .unwrap();
-        let query_id = signed_receipt.unique_hash();
+        let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
         stored_signed_receipts.push(signed_receipt.clone());
         query_appraisals.write().unwrap().insert(query_id, value);
         assert!(manager
@@ -321,7 +321,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts(
         )
         .unwrap();
 
-        let query_id = signed_receipt.unique_hash();
+        let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
         stored_signed_receipts.push(signed_receipt.clone());
         query_appraisals.write().unwrap().insert(query_id, value);
         assert!(manager
@@ -389,7 +389,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts_consecutive_tim
         receipt.timestamp_ns = starting_min_timestamp + query_id + 1;
         let signed_receipt = Eip712SignedMessage::new(&domain_separator, receipt, &signer).unwrap();
 
-        let query_id = signed_receipt.unique_hash();
+        let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
         stored_signed_receipts.push(signed_receipt.clone());
         query_appraisals.write().unwrap().insert(query_id, value);
         assert!(manager
@@ -436,7 +436,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts_consecutive_tim
         let mut receipt = Receipt::new(allocation_ids[0], value).unwrap();
         receipt.timestamp_ns = starting_min_timestamp + query_id + 1;
         let signed_receipt = Eip712SignedMessage::new(&domain_separator, receipt, &signer).unwrap();
-        let query_id = signed_receipt.unique_hash();
+        let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
         stored_signed_receipts.push(signed_receipt.clone());
         query_appraisals.write().unwrap().insert(query_id, value);
         assert!(manager

tap_core/tests/received_receipt_test.rs

@@ -113,7 +113,7 @@ async fn partial_then_full_check_valid_receipt(
     )
     .unwrap();
 
-    let query_id = signed_receipt.unique_hash();
+    let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
 
     // add escrow for sender
     escrow_storage
@@ -156,7 +156,7 @@ async fn partial_then_finalize_valid_receipt(
         &signer,
     )
     .unwrap();
-    let query_id = signed_receipt.unique_hash();
+    let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
 
     // add escrow for sender
     escrow_storage
@@ -203,7 +203,7 @@ async fn standard_lifetime_valid_receipt(
     )
     .unwrap();
 
-    let query_id = signed_receipt.unique_hash();
+    let query_id = signed_receipt.unique_hash(&domain_separator).unwrap();
 
     // add escrow for sender
     escrow_storage

tap_eip712_message/src/lib.rs

@@ -25,7 +25,7 @@
 use serde::{Deserialize, Serialize};
 use thegraph_core::alloy::{
     dyn_abi::Eip712Domain,
-    primitives::{Address, Signature},
+    primitives::{keccak256, Address, Signature},
     signers::{local::PrivateKeySigner, SignerSync},
     sol_types::SolStruct,
 };
@@ -105,8 +105,21 @@ impl<M: SolStruct> Eip712SignedMessage<M> {
         Ok(recovered_address)
     }
 
-    /// Use this as a simple key for testing
-    pub fn unique_hash(&self) -> MessageId {
-        MessageId(self.message.eip712_hash_struct().into())
+    /// Generate a uniqueness identifier using both the message content and the recovered signer
+    pub fn unique_hash(&self, domain_separator: &Eip712Domain) -> Result<MessageId, Eip712Error> {
+        // Get the hash of just the message content
+        let message_hash = self.message.eip712_hash_struct();
+
+        // Recover the signer address
+        let signer = self.recover_signer(domain_separator)?;
+
+        // Create a new hash combining both the message hash and the signer address
+        let mut input = Vec::with_capacity(32 + 20); // 32 bytes for hash, 20 bytes for address
+        input.extend_from_slice(&message_hash.0);
+        input.extend_from_slice(signer.as_ref());
+
+        let combined_hash = keccak256(&input);
+
+        Ok(MessageId(*combined_hash))
     }
 }

tap_receipt/src/checks.rs

@@ -36,6 +36,9 @@ use std::{
     sync::{Arc, RwLock},
 };
 
+use tap_eip712_message::Eip712Error;
+use thegraph_core::alloy::dyn_abi::Eip712Domain;
+
 use super::{
     state::{Checking, Failed},
     Context, ReceiptError, ReceiptWithState, WithUniqueId, WithValueAndTimestamp,
@@ -94,8 +97,9 @@ type CheckBatchResponse<Rcpt> = (
 pub trait CheckBatch<Rcpt> {
     fn check_batch(
         &self,
+        domain_separator: &Eip712Domain,
         receipts: Vec<ReceiptWithState<Checking, Rcpt>>,
-    ) -> CheckBatchResponse<Rcpt>;
+    ) -> Result<CheckBatchResponse<Rcpt>, Eip712Error>;
 }
 
 /// Provides a built-in check to verify that the timestamp of a receipt
@@ -153,11 +157,9 @@ where
 {
     fn check_batch(
         &self,
+        _domain_separator: &Eip712Domain,
         receipts: Vec<ReceiptWithState<Checking, Rcpt>>,
-    ) -> (
-        Vec<ReceiptWithState<Checking, Rcpt>>,
-        Vec<ReceiptWithState<Failed, Rcpt>>,
-    ) {
+    ) -> Result<CheckBatchResponse<Rcpt>, Eip712Error> {
         let (mut checking, mut failed) = (vec![], vec![]);
         for receipt in receipts.into_iter() {
             let receipt_timestamp_ns = receipt.signed_receipt().timestamp_ns();
@@ -171,7 +173,7 @@ where
                 }));
             }
         }
-        (checking, failed)
+        Ok((checking, failed))
     }
 }
 
@@ -187,23 +189,21 @@ where
 {
     fn check_batch(
         &self,
+        domain_separator: &Eip712Domain,
         receipts: Vec<ReceiptWithState<Checking, Rcpt>>,
-    ) -> (
-        Vec<ReceiptWithState<Checking, Rcpt>>,
-        Vec<ReceiptWithState<Failed, Rcpt>>,
-    ) {
+    ) -> Result<CheckBatchResponse<Rcpt>, Eip712Error> {
         let mut signatures = HashSet::new();
         let (mut checking, mut failed) = (vec![], vec![]);
 
         for received_receipt in receipts.into_iter() {
-            let unique_id = received_receipt.receipt.unique_id();
+            let unique_id = received_receipt.receipt.unique_id(domain_separator)?;
             if signatures.insert(unique_id) {
                 checking.push(received_receipt);
             } else {
                 failed.push(received_receipt.perform_state_error(ReceiptError::NonUniqueReceipt));
             }
         }
-        (checking, failed)
+        Ok((checking, failed))
     }
 }
 
@@ -236,16 +236,20 @@ mod tests {
         }
     }
 
-    fn create_signed_receipt_with_custom_value(
-        value: u128,
-    ) -> ReceiptWithState<Checking, Eip712SignedMessage<MyReceipt>> {
-        let wallet: PrivateKeySigner = PrivateKeySigner::random();
-        let eip712_domain_separator: Eip712Domain = eip712_domain! {
+    fn create_test_domain_separator() -> Eip712Domain {
+        eip712_domain! {
             name: "TAP",
             version: "1",
             chain_id: 1,
             verifying_contract: Address:: from([0x11u8; 20]),
-        };
+        }
+    }
+
+    fn create_signed_receipt_with_custom_value(
+        value: u128,
+    ) -> ReceiptWithState<Checking, Eip712SignedMessage<MyReceipt>> {
+        let wallet: PrivateKeySigner = PrivateKeySigner::random();
+        let eip712_domain_separator = create_test_domain_separator();
 
         let timestamp = SystemTime::now()
             .duration_since(SystemTime::UNIX_EPOCH)
@@ -273,7 +277,10 @@ mod tests {
         let signed_receipt_2 = create_signed_receipt_with_custom_value(15);
         let signed_receipt_copy = signed_receipt.clone();
         let receipts_batch = vec![signed_receipt, signed_receipt_2, signed_receipt_copy];
-        let (valid_receipts, invalid_receipts) = UniqueCheck.check_batch(receipts_batch);
+        let domain_separator = create_test_domain_separator();
+        let (valid_receipts, invalid_receipts) = UniqueCheck
+            .check_batch(&domain_separator, receipts_batch)
+            .unwrap();
         assert_eq!(valid_receipts.len(), 2);
         assert_eq!(invalid_receipts.len(), 1);
     }
@@ -282,10 +289,14 @@ mod tests {
     async fn test_receipt_timestamp_check() {
         let signed_receipt = create_signed_receipt_with_custom_value(10);
         let signed_receipt_2 = create_signed_receipt_with_custom_value(15);
+
+        let domain_separator = create_test_domain_separator();
+
         let receipts_batch = vec![signed_receipt.clone(), signed_receipt_2];
         let min_time_stamp = signed_receipt.receipt.message.timestamp_ns + 1;
-        let (valid_receipts, invalid_receipts) =
-            TimestampCheck(min_time_stamp).check_batch(receipts_batch);
+        let (valid_receipts, invalid_receipts) = TimestampCheck(min_time_stamp)
+            .check_batch(&domain_separator, receipts_batch)
+            .unwrap();
         assert_eq!(valid_receipts.len(), 1);
         assert_eq!(invalid_receipts.len(), 1);
     }

tap_receipt/src/lib.rs

@@ -27,8 +27,8 @@ pub mod state;
 
 pub use error::ReceiptError;
 pub use received_receipt::ReceiptWithState;
-use tap_eip712_message::{Eip712SignedMessage, SignatureBytes, SignatureBytesExt};
-use thegraph_core::alloy::sol_types::SolStruct;
+use tap_eip712_message::{Eip712Error, Eip712SignedMessage, MessageId};
+use thegraph_core::alloy::{dyn_abi::Eip712Domain, sol_types::SolStruct};
 
 /// Result type for receipt
 pub type ReceiptResult<T> = Result<T, ReceiptError>;
@@ -45,7 +45,7 @@ pub trait WithValueAndTimestamp {
 /// Extension that allows UniqueCheck for any SolStruct receipt
 pub trait WithUniqueId {
     type Output: Eq + std::hash::Hash;
-    fn unique_id(&self) -> Self::Output;
+    fn unique_id(&self, domain_separator: &Eip712Domain) -> Result<Self::Output, Eip712Error>;
 }
 
 impl<T> WithValueAndTimestamp for Eip712SignedMessage<T>
@@ -65,9 +65,9 @@ impl<T> WithUniqueId for Eip712SignedMessage<T>
 where
     T: SolStruct,
 {
-    type Output = SignatureBytes;
+    type Output = MessageId;
 
-    fn unique_id(&self) -> Self::Output {
-        self.signature.get_signature_bytes()
+    fn unique_id(&self, domain_separator: &Eip712Domain) -> Result<Self::Output, Eip712Error> {
+        self.unique_hash(domain_separator)
     }
 }