diff --git a/tap_aggregator/src/aggregator/v1.rs b/tap_aggregator/src/aggregator/v1.rs index 6234a54f..c7556a57 100644 --- a/tap_aggregator/src/aggregator/v1.rs +++ b/tap_aggregator/src/aggregator/v1.rs @@ -19,7 +19,7 @@ pub fn check_and_aggregate_receipts( wallet: &PrivateKeySigner, accepted_addresses: &HashSet
, ) -> Result> { - check_signatures_unique(domain_separator, receipts)?; + check_signatures_unique(receipts)?; // Check that the receipts are signed by an accepted signer address receipts.par_iter().try_for_each(|receipt| { @@ -93,17 +93,14 @@ fn check_allocation_id( Ok(()) } -fn check_signatures_unique( - domain_separator: &Eip712Domain, - receipts: &[Eip712SignedMessage], -) -> Result<()> { +fn check_signatures_unique(receipts: &[Eip712SignedMessage]) -> Result<()> { let mut receipt_signatures = HashSet::new(); for receipt in receipts.iter() { - let signature = receipt.unique_id(domain_separator)?; + let signature = receipt.unique_id(); if !receipt_signatures.insert(signature) { return Err(tap_core::Error::DuplicateReceiptSignature(format!( "{:?}", - receipt.unique_id(domain_separator)? + receipt.unique_id() )) .into()); } @@ -221,7 +218,7 @@ mod tests { // This should return an error because the signatures are different // but the messages are the same, which if allowed would present a security vulnerability - let result = check_signatures_unique(&domain_separator, &receipts); + let result = check_signatures_unique(&receipts); // The result should be an error because the malleated signature is not treated as unique // and is detected as a duplicate @@ -246,7 +243,7 @@ mod tests { receipts.push(receipt.clone()); receipts.push(receipt); - let res = check_signatures_unique(&domain_separator, &receipts); + let res = check_signatures_unique(&receipts); assert!(res.is_err()); } @@ -273,7 +270,7 @@ mod tests { .unwrap(), ]; - let res = check_signatures_unique(&domain_separator, &receipts); + let res = check_signatures_unique(&receipts); assert!(res.is_ok()); } diff --git a/tap_aggregator/src/aggregator/v2.rs b/tap_aggregator/src/aggregator/v2.rs index 0abb5f31..484ad4e8 100644 --- a/tap_aggregator/src/aggregator/v2.rs +++ b/tap_aggregator/src/aggregator/v2.rs @@ -19,7 +19,7 @@ pub fn check_and_aggregate_receipts( wallet: &PrivateKeySigner, accepted_addresses: &HashSet
, ) -> Result> { - check_signatures_unique(domain_separator, receipts)?; + check_signatures_unique(receipts)?; // Check that the receipts are signed by an accepted signer address receipts.par_iter().try_for_each(|receipt| { @@ -148,17 +148,14 @@ fn check_allocation_id( Ok(()) } -fn check_signatures_unique( - domain_separator: &Eip712Domain, - receipts: &[Eip712SignedMessage], -) -> Result<()> { +fn check_signatures_unique(receipts: &[Eip712SignedMessage]) -> Result<()> { let mut receipt_signatures = HashSet::new(); for receipt in receipts.iter() { - let signature = receipt.unique_id(domain_separator)?; + let signature = receipt.unique_id(); if !receipt_signatures.insert(signature) { return Err(tap_core::Error::DuplicateReceiptSignature(format!( "{:?}", - receipt.unique_id(domain_separator)? + receipt.unique_id() )) .into()); } @@ -254,7 +251,7 @@ mod tests { receipts.push(receipt.clone()); receipts.push(receipt); - let res = super::check_signatures_unique(&domain_separator, &receipts); + let res = super::check_signatures_unique(&receipts); assert!(res.is_err()); } @@ -284,7 +281,7 @@ mod tests { .unwrap(), ]; - let res = super::check_signatures_unique(&domain_separator, &receipts); + let res = super::check_signatures_unique(&receipts); assert!(res.is_ok()); } diff --git a/tap_core/src/manager/tap_manager.rs b/tap_core/src/manager/tap_manager.rs index 6320de2a..47f5670f 100644 --- a/tap_core/src/manager/tap_manager.rs +++ b/tap_core/src/manager/tap_manager.rs @@ -138,13 +138,12 @@ where let mut failed_receipts = vec![]; // check for timestamp - let (checking_receipts, already_failed) = TimestampCheck(min_timestamp_ns) - .check_batch(&self.domain_separator, checking_receipts)?; + let (checking_receipts, already_failed) = + TimestampCheck(min_timestamp_ns).check_batch(checking_receipts)?; failed_receipts.extend(already_failed); // check for uniqueness - let (checking_receipts, already_failed) = - UniqueCheck.check_batch(&self.domain_separator, checking_receipts)?; + let (checking_receipts, already_failed) = UniqueCheck.check_batch(checking_receipts)?; failed_receipts.extend(already_failed); for receipt in checking_receipts.into_iter() { diff --git a/tap_core/tests/manager_test.rs b/tap_core/tests/manager_test.rs index cae341cc..e44badbd 100644 --- a/tap_core/tests/manager_test.rs +++ b/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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); stored_signed_receipts.push(signed_receipt.clone()); query_appraisals.write().unwrap().insert(query_id, value); assert!(manager diff --git a/tap_core/tests/received_receipt_test.rs b/tap_core/tests/received_receipt_test.rs index 8403a9c1..d7972fbc 100644 --- a/tap_core/tests/received_receipt_test.rs +++ b/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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); // 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(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); // add escrow for sender escrow_storage @@ -203,7 +203,7 @@ async fn standard_lifetime_valid_receipt( ) .unwrap(); - let query_id = signed_receipt.unique_hash(&domain_separator).unwrap(); + let query_id = signed_receipt.unique_hash(); // add escrow for sender escrow_storage diff --git a/tap_eip712_message/src/lib.rs b/tap_eip712_message/src/lib.rs index a347c382..e0899081 100644 --- a/tap_eip712_message/src/lib.rs +++ b/tap_eip712_message/src/lib.rs @@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize}; use thegraph_core::alloy::{ dyn_abi::Eip712Domain, - primitives::{keccak256, Address, Signature}, + primitives::{Address, Signature}, signers::{local::PrivateKeySigner, SignerSync}, sol_types::SolStruct, }; @@ -105,21 +105,8 @@ impl Eip712SignedMessage { Ok(recovered_address) } - /// Generate a uniqueness identifier using both the message content and the recovered signer - pub fn unique_hash(&self, domain_separator: &Eip712Domain) -> Result { - // 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)) + /// Use this as a simple key for testing + pub fn unique_hash(&self) -> MessageId { + MessageId(self.message.eip712_hash_struct().into()) } } diff --git a/tap_receipt/src/checks.rs b/tap_receipt/src/checks.rs index 44acbd9e..c48aa425 100644 --- a/tap_receipt/src/checks.rs +++ b/tap_receipt/src/checks.rs @@ -37,7 +37,6 @@ use std::{ }; use tap_eip712_message::Eip712Error; -use thegraph_core::alloy::dyn_abi::Eip712Domain; use super::{ state::{Checking, Failed}, @@ -97,7 +96,6 @@ type CheckBatchResponse = ( pub trait CheckBatch { fn check_batch( &self, - domain_separator: &Eip712Domain, receipts: Vec>, ) -> Result, Eip712Error>; } @@ -157,7 +155,6 @@ where { fn check_batch( &self, - _domain_separator: &Eip712Domain, receipts: Vec>, ) -> Result, Eip712Error> { let (mut checking, mut failed) = (vec![], vec![]); @@ -189,14 +186,13 @@ where { fn check_batch( &self, - domain_separator: &Eip712Domain, receipts: Vec>, ) -> Result, 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(domain_separator)?; + let unique_id = received_receipt.receipt.unique_id(); if signatures.insert(unique_id) { checking.push(received_receipt); } else { @@ -277,10 +273,7 @@ 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 domain_separator = create_test_domain_separator(); - let (valid_receipts, invalid_receipts) = UniqueCheck - .check_batch(&domain_separator, receipts_batch) - .unwrap(); + let (valid_receipts, invalid_receipts) = UniqueCheck.check_batch(receipts_batch).unwrap(); assert_eq!(valid_receipts.len(), 2); assert_eq!(invalid_receipts.len(), 1); } @@ -290,12 +283,10 @@ mod tests { 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(&domain_separator, receipts_batch) + .check_batch(receipts_batch) .unwrap(); assert_eq!(valid_receipts.len(), 1); assert_eq!(invalid_receipts.len(), 1); diff --git a/tap_receipt/src/lib.rs b/tap_receipt/src/lib.rs index 1188a2ef..f3f3e05c 100644 --- a/tap_receipt/src/lib.rs +++ b/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::{Eip712Error, Eip712SignedMessage, MessageId}; -use thegraph_core::alloy::{dyn_abi::Eip712Domain, sol_types::SolStruct}; +use tap_eip712_message::{Eip712SignedMessage, SignatureBytes, SignatureBytesExt}; +use thegraph_core::alloy::sol_types::SolStruct; /// Result type for receipt pub type ReceiptResult = Result; @@ -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, domain_separator: &Eip712Domain) -> Result; + fn unique_id(&self) -> Self::Output; } impl WithValueAndTimestamp for Eip712SignedMessage @@ -65,9 +65,9 @@ impl WithUniqueId for Eip712SignedMessage where T: SolStruct, { - type Output = MessageId; + type Output = SignatureBytes; - fn unique_id(&self, domain_separator: &Eip712Domain) -> Result { - self.unique_hash(domain_separator) + fn unique_id(&self) -> Self::Output { + self.signature.normalized_s().get_signature_bytes() } }