From 637c24ca6264da2fd616289d1f8039421abbbe05 Mon Sep 17 00:00:00 2001 From: Gustavo Inacio Date: Fri, 8 Mar 2024 10:25:45 -0300 Subject: [PATCH] refactor: use verify_signer from escrow_adapter Signed-off-by: Gustavo Inacio --- tap_core/Cargo.toml | 1 - tap_core/src/adapters/escrow_adapter.rs | 5 +++ tap_core/src/adapters/mock/executor_mock.rs | 16 +++++++- tap_core/src/tap_manager/manager.rs | 16 ++------ tap_core/src/tap_manager/test/manager_test.rs | 34 ++++----------- tap_core/src/tap_receipt/receipt_auditor.rs | 41 ++++++++----------- .../received_receipt_unit_test.rs | 4 +- .../tests/indexer_mock/mod.rs | 14 +------ tap_integration_tests/tests/showcase.rs | 3 +- 9 files changed, 52 insertions(+), 82 deletions(-) diff --git a/tap_core/Cargo.toml b/tap_core/Cargo.toml index c09e3590..624c5bfc 100644 --- a/tap_core/Cargo.toml +++ b/tap_core/Cargo.toml @@ -24,7 +24,6 @@ strum = "0.24.1" strum_macros = "0.24.3" async-trait = "0.1.72" tokio = { version = "1.29.1", features = ["macros", "rt-multi-thread"] } -futures = "0.3.17" [dev-dependencies] criterion = { version = "0.5", features = ["async_std"] } diff --git a/tap_core/src/adapters/escrow_adapter.rs b/tap_core/src/adapters/escrow_adapter.rs index 764beddd..7cb03bfe 100644 --- a/tap_core/src/adapters/escrow_adapter.rs +++ b/tap_core/src/adapters/escrow_adapter.rs @@ -53,4 +53,9 @@ pub trait EscrowAdapter { sender_id: Address, value: u128, ) -> Result<(), Self::AdapterError>; + + async fn verify_signer( + &self, + signer_address: Address + ) -> Result; } diff --git a/tap_core/src/adapters/mock/executor_mock.rs b/tap_core/src/adapters/mock/executor_mock.rs index ee824ccf..7a807c33 100644 --- a/tap_core/src/adapters/mock/executor_mock.rs +++ b/tap_core/src/adapters/mock/executor_mock.rs @@ -37,10 +37,9 @@ pub struct ExecutorMock { rav_storage: RAVStorage, receipt_storage: ReceiptStorage, unique_id: Arc>, - sender_escrow_storage: EscrowStorage, - timestamp_check: Arc, + sender_address: Option
, } impl ExecutorMock { @@ -56,9 +55,15 @@ impl ExecutorMock { unique_id: Arc::new(RwLock::new(0)), sender_escrow_storage, timestamp_check, + sender_address: None, } } + pub fn with_sender_address(mut self, sender_address: Address) -> Self { + self.sender_address = Some(sender_address); + self + } + pub async fn retrieve_receipt_by_id( &self, receipt_id: u64, @@ -241,4 +246,11 @@ impl EscrowAdapter for ExecutorMock { ) -> Result<(), Self::AdapterError> { self.reduce_escrow(sender_id, value) } + + async fn verify_signer(&self, signer_address: Address) -> Result { + Ok(self + .sender_address + .map(|sender| signer_address == sender) + .unwrap_or(false)) + } } diff --git a/tap_core/src/tap_manager/manager.rs b/tap_core/src/tap_manager/manager.rs index 5bc85e71..58ad686e 100644 --- a/tap_core/src/tap_manager/manager.rs +++ b/tap_core/src/tap_manager/manager.rs @@ -1,9 +1,7 @@ // Copyright 2023-, Semiotic AI, Inc. // SPDX-License-Identifier: Apache-2.0 -use alloy_primitives::Address; use alloy_sol_types::Eip712Domain; -use futures::Future; use super::{RAVRequest, SignedRAV, SignedReceipt}; use crate::{ @@ -59,7 +57,7 @@ where impl Manager where - E: RAVStore, + E: RAVStore + EscrowAdapter, { /// Verify `signed_rav` matches all values on `expected_rav`, and that `signed_rav` has a valid signer. /// @@ -67,19 +65,13 @@ where /// /// Returns [`Error::AdapterError`] if there are any errors while storing RAV /// - pub async fn verify_and_store_rav( + pub async fn verify_and_store_rav( &self, expected_rav: ReceiptAggregateVoucher, signed_rav: SignedRAV, - verify_signer: F, - ) -> std::result::Result<(), Error> - where - F: FnOnce(Address) -> Fut, - Fut: Future>, - Err: std::fmt::Display, - { + ) -> std::result::Result<(), Error> { self.receipt_auditor - .check_rav_signature(&signed_rav, verify_signer) + .check_rav_signature(&signed_rav) .await?; if signed_rav.message != expected_rav { diff --git a/tap_core/src/tap_manager/test/manager_test.rs b/tap_core/src/tap_manager/test/manager_test.rs index 300a7fb8..4c8f6bfe 100644 --- a/tap_core/src/tap_manager/test/manager_test.rs +++ b/tap_core/src/tap_manager/test/manager_test.rs @@ -73,6 +73,7 @@ fn executor_mock( domain_separator: Eip712Domain, allocation_ids: Vec
, sender_ids: Vec
, + keys: (LocalWallet, Address), ) -> ExecutorFixture { let escrow_storage = Arc::new(RwLock::new(HashMap::new())); let rav_storage = Arc::new(RwLock::new(None)); @@ -84,7 +85,8 @@ fn executor_mock( receipt_storage.clone(), escrow_storage.clone(), timestamp_check.clone(), - ); + ) + .with_sender_address(keys.1); let mut checks = get_full_list_of_checks( domain_separator, @@ -189,11 +191,7 @@ async fn manager_create_rav_request_all_valid_receipts( EIP712SignedMessage::new(&domain_separator, rav_request.expected_rav.clone(), &keys.0) .unwrap(); assert!(manager - .verify_and_store_rav( - rav_request.expected_rav, - signed_rav, - |address: Address| async move { Ok::(keys.1 == address) } - ) + .verify_and_store_rav(rav_request.expected_rav, signed_rav) .await .is_ok()); } @@ -260,11 +258,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts( EIP712SignedMessage::new(&domain_separator, rav_request.expected_rav.clone(), &keys.0) .unwrap(); assert!(manager - .verify_and_store_rav( - rav_request.expected_rav, - signed_rav, - |address: Address| async move { Ok::(keys.1 == address) } - ) + .verify_and_store_rav(rav_request.expected_rav, signed_rav) .await .is_ok()); @@ -310,11 +304,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts( EIP712SignedMessage::new(&domain_separator, rav_request.expected_rav.clone(), &keys.0) .unwrap(); assert!(manager - .verify_and_store_rav( - rav_request.expected_rav, - signed_rav, - |address: Address| async move { Ok::(keys.1 == address) } - ) + .verify_and_store_rav(rav_request.expected_rav, signed_rav) .await .is_ok()); } @@ -391,11 +381,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts_consecutive_tim ) .unwrap(); assert!(manager - .verify_and_store_rav( - rav_request_1.expected_rav, - signed_rav_1, - |address: Address| async move { Ok::(keys.1 == address) } - ) + .verify_and_store_rav(rav_request_1.expected_rav, signed_rav_1) .await .is_ok()); @@ -456,11 +442,7 @@ async fn manager_create_multiple_rav_requests_all_valid_receipts_consecutive_tim ) .unwrap(); assert!(manager - .verify_and_store_rav( - rav_request_2.expected_rav, - signed_rav_2, - |address: Address| async move { Ok::(keys.1 == address) } - ) + .verify_and_store_rav(rav_request_2.expected_rav, signed_rav_2) .await .is_ok()); } diff --git a/tap_core/src/tap_receipt/receipt_auditor.rs b/tap_core/src/tap_receipt/receipt_auditor.rs index 982c097c..f3c0683a 100644 --- a/tap_core/src/tap_receipt/receipt_auditor.rs +++ b/tap_core/src/tap_receipt/receipt_auditor.rs @@ -1,9 +1,7 @@ // Copyright 2023-, Semiotic AI, Inc. // SPDX-License-Identifier: Apache-2.0 -use alloy_primitives::Address; use alloy_sol_types::Eip712Domain; -use futures::Future; use crate::{ adapters::escrow_adapter::EscrowAdapter, @@ -26,29 +24,6 @@ impl ReceiptAuditor { executor, } } - - pub async fn check_rav_signature( - &self, - signed_rav: &SignedRAV, - verify_signer: F, - ) -> Result<(), Error> - where - F: FnOnce(Address) -> Fut, - Fut: Future>, - Err: std::fmt::Display, - { - let recovered_address = signed_rav.recover_signer(&self.domain_separator)?; - if verify_signer(recovered_address) - .await - .map_err(|e| Error::FailedToVerifySigner(e.to_string()))? - { - Ok(()) - } else { - Err(Error::InvalidRecoveredSigner { - address: recovered_address, - }) - } - } } impl ReceiptAuditor @@ -77,4 +52,20 @@ where Ok(()) } + + pub async fn check_rav_signature(&self, signed_rav: &SignedRAV) -> Result<(), Error> { + let recovered_address = signed_rav.recover_signer(&self.domain_separator)?; + if self + .executor + .verify_signer(recovered_address) + .await + .map_err(|e| Error::FailedToVerifySigner(e.to_string()))? + { + Ok(()) + } else { + Err(Error::InvalidRecoveredSigner { + address: recovered_address, + }) + } + } } diff --git a/tap_core/src/tap_receipt/received_receipt/received_receipt_unit_test.rs b/tap_core/src/tap_receipt/received_receipt/received_receipt_unit_test.rs index 5452fc3e..8c74ef60 100644 --- a/tap_core/src/tap_receipt/received_receipt/received_receipt_unit_test.rs +++ b/tap_core/src/tap_receipt/received_receipt/received_receipt_unit_test.rs @@ -70,6 +70,7 @@ fn executor_mock( domain_separator: Eip712Domain, allocation_ids: Vec
, sender_ids: Vec
, + keys: (LocalWallet, Address), ) -> ExecutorFixture { let escrow_storage = Arc::new(RwLock::new(HashMap::new())); let rav_storage = Arc::new(RwLock::new(None)); @@ -82,7 +83,8 @@ fn executor_mock( receipt_storage.clone(), escrow_storage.clone(), timestamp_check.clone(), - ); + ) + .with_sender_address(keys.1); let mut checks = get_full_list_of_checks( domain_separator, sender_ids.iter().cloned().collect(), diff --git a/tap_integration_tests/tests/indexer_mock/mod.rs b/tap_integration_tests/tests/indexer_mock/mod.rs index 24ac809d..81cc28f6 100644 --- a/tap_integration_tests/tests/indexer_mock/mod.rs +++ b/tap_integration_tests/tests/indexer_mock/mod.rs @@ -5,7 +5,6 @@ use std::sync::{ Arc, }; -use alloy_primitives::Address; use alloy_sol_types::Eip712Domain; use anyhow::{Error, Result}; use jsonrpsee::{ @@ -51,7 +50,6 @@ pub struct RpcManager { receipt_count: Arc, // Thread-safe atomic counter for receipts threshold: u64, // The count at which a RAV request will be triggered aggregator_client: (HttpClient, String), // HTTP client for sending requests to the aggregator server - sender_id: Address, // The sender address } /// Implementation for `RpcManager`, includes the constructor and the `request` method. @@ -66,7 +64,6 @@ where executor: E, required_checks: Checks, threshold: u64, - sender_id: Address, aggregate_server_address: String, aggregate_server_api_version: String, ) -> Result { @@ -78,7 +75,6 @@ where )), receipt_count: Arc::new(AtomicU64::new(0)), threshold, - sender_id, aggregator_client: ( HttpClientBuilder::default().build(aggregate_server_address)?, aggregate_server_api_version, @@ -118,7 +114,6 @@ where time_stamp_buffer, &self.aggregator_client, self.threshold as usize, - self.sender_id, ) .await { @@ -146,7 +141,6 @@ pub async fn run_server( threshold: u64, // The count at which a RAV request will be triggered aggregate_server_address: String, // Address of the aggregator server aggregate_server_api_version: String, // API version of the aggregator server - sender_id: Address, // The sender address ) -> Result<(ServerHandle, std::net::SocketAddr)> where E: ReceiptStore @@ -172,7 +166,6 @@ where executor, required_checks, threshold, - sender_id, aggregate_server_address, aggregate_server_api_version, )?; @@ -187,7 +180,6 @@ async fn request_rav( time_stamp_buffer: u64, // Buffer for timestamping, see tap_core for details aggregator_client: &(HttpClient, String), // HttpClient for making requests to the tap_aggregator server threshold: usize, - expected_sender_id: Address, ) -> Result<()> where E: ReceiptRead + RAVRead + RAVStore + EscrowAdapter, @@ -208,11 +200,7 @@ where .request("aggregate_receipts", params) .await?; manager - .verify_and_store_rav( - rav_request.expected_rav, - remote_rav_result.data, - |address| async move { Ok::(address == expected_sender_id) }, - ) + .verify_and_store_rav(rav_request.expected_rav, remote_rav_result.data) .await?; // For these tests, we expect every receipt to be valid, i.e. there should be no invalid receipts, nor any missing receipts (less than the expected threshold). diff --git a/tap_integration_tests/tests/showcase.rs b/tap_integration_tests/tests/showcase.rs index a5da35c5..41b4b4f7 100644 --- a/tap_integration_tests/tests/showcase.rs +++ b/tap_integration_tests/tests/showcase.rs @@ -871,12 +871,11 @@ async fn start_indexer_server( let (server_handle, socket_addr) = indexer_mock::run_server( http_port, domain_separator, - executor, + executor.with_sender_address(sender_id), required_checks, receipt_threshold, aggregate_server_address, aggregate_server_api_version(), - sender_id, ) .await?;