From 9e7f875566ed2eb84424c7b01bba955f58b0221a Mon Sep 17 00:00:00 2001 From: Gustavo Inacio Date: Mon, 11 Nov 2024 12:52:42 -0500 Subject: [PATCH 1/2] fix: remove escrow adapter check Signed-off-by: Gustavo Inacio --- crates/tap-agent/src/agent/sender_account.rs | 6 - .../tap-agent/src/agent/sender_allocation.rs | 18 +- crates/tap-agent/src/tap/context.rs | 5 - crates/tap-agent/src/tap/context/escrow.rs | 21 ++- crates/tap-agent/src/tap/context/rav.rs | 6 +- crates/tap-agent/src/tap/context/receipt.rs | 29 +-- crates/tap-agent/src/tap/escrow_adapter.rs | 168 ------------------ crates/tap-agent/src/tap/mod.rs | 1 - 8 files changed, 28 insertions(+), 226 deletions(-) delete mode 100644 crates/tap-agent/src/tap/escrow_adapter.rs diff --git a/crates/tap-agent/src/agent/sender_account.rs b/crates/tap-agent/src/agent/sender_account.rs index 3140eddce..e28766c08 100644 --- a/crates/tap-agent/src/agent/sender_account.rs +++ b/crates/tap-agent/src/agent/sender_account.rs @@ -33,7 +33,6 @@ use crate::adaptative_concurrency::AdaptiveLimiter; use crate::agent::sender_allocation::{AllocationConfig, SenderAllocationMessage}; use crate::agent::unaggregated_receipts::UnaggregatedReceipts; use crate::backoff::BackoffInfo; -use crate::tap::escrow_adapter::EscrowAdapter; use crate::tracker::{SenderFeeTracker, SimpleFeeTracker}; use lazy_static::lazy_static; @@ -166,7 +165,6 @@ pub struct State { escrow_subgraph: &'static SubgraphClient, network_subgraph: &'static SubgraphClient, - escrow_adapter: EscrowAdapter, domain_separator: Eip712Domain, pgpool: PgPool, sender_aggregator: jsonrpsee::http_client::HttpClient, @@ -221,7 +219,6 @@ impl State { sender: self.sender, escrow_accounts: self.escrow_accounts.clone(), escrow_subgraph: self.escrow_subgraph, - escrow_adapter: self.escrow_adapter.clone(), domain_separator: self.domain_separator.clone(), sender_account_ref: sender_account_ref.clone(), sender_aggregator: self.sender_aggregator.clone(), @@ -584,8 +581,6 @@ impl Actor for SenderAccount { } }); - let escrow_adapter = EscrowAdapter::new(escrow_accounts.clone(), sender_id); - // Get deny status from the scalar_tap_denylist table let denied = sqlx::query!( r#" @@ -640,7 +635,6 @@ impl Actor for SenderAccount { escrow_accounts, escrow_subgraph, network_subgraph, - escrow_adapter, domain_separator, pgpool, sender_aggregator, diff --git a/crates/tap-agent/src/agent/sender_allocation.rs b/crates/tap-agent/src/agent/sender_allocation.rs index 403db3306..b01859584 100644 --- a/crates/tap-agent/src/agent/sender_allocation.rs +++ b/crates/tap-agent/src/agent/sender_allocation.rs @@ -35,9 +35,9 @@ use crate::agent::sender_account::SenderAccountMessage; use crate::agent::sender_accounts_manager::NewReceiptNotification; use crate::agent::unaggregated_receipts::UnaggregatedReceipts; use crate::{ + tap::context::checks::AllocationId, tap::context::{checks::Signature, TapAgentContext}, tap::signers_trimmed, - tap::{context::checks::AllocationId, escrow_adapter::EscrowAdapter}, }; use thiserror::Error; @@ -137,7 +137,6 @@ pub struct SenderAllocationArgs { pub sender: Address, pub escrow_accounts: Receiver, pub escrow_subgraph: &'static SubgraphClient, - pub escrow_adapter: EscrowAdapter, pub domain_separator: Eip712Domain, pub sender_account_ref: ActorRef, pub sender_aggregator: jsonrpsee::http_client::HttpClient, @@ -340,7 +339,6 @@ impl SenderAllocationState { sender, escrow_accounts, escrow_subgraph, - escrow_adapter, domain_separator, sender_account_ref, sender_aggregator, @@ -368,7 +366,6 @@ impl SenderAllocationState { allocation_id, sender, escrow_accounts.clone(), - escrow_adapter, ); let latest_rav = context.last_rav().await.unwrap_or_default(); let tap_manager = TapManager::new( @@ -869,13 +866,9 @@ pub mod tests { sender_accounts_manager::NewReceiptNotification, unaggregated_receipts::UnaggregatedReceipts, }, - tap::{ - escrow_adapter::EscrowAdapter, - test_utils::{ - create_rav, create_received_receipt, store_invalid_receipt, store_rav, - store_receipt, ALLOCATION_ID_0, INDEXER, SENDER, SIGNER, - TAP_EIP712_DOMAIN_SEPARATOR, - }, + tap::test_utils::{ + create_rav, create_received_receipt, store_invalid_receipt, store_rav, store_receipt, + ALLOCATION_ID_0, INDEXER, SENDER, SIGNER, TAP_EIP712_DOMAIN_SEPARATOR, }, }; use futures::future::join_all; @@ -995,8 +988,6 @@ pub mod tests { )) .1; - let escrow_adapter = EscrowAdapter::new(escrow_accounts_rx.clone(), SENDER.1); - let sender_account_ref = match sender_account { Some(sender) => sender, None => create_mock_sender_account().await.1, @@ -1011,7 +1002,6 @@ pub mod tests { sender: SENDER.1, escrow_accounts: escrow_accounts_rx, escrow_subgraph, - escrow_adapter, domain_separator: TAP_EIP712_DOMAIN_SEPARATOR.clone(), sender_account_ref, sender_aggregator, diff --git a/crates/tap-agent/src/tap/context.rs b/crates/tap-agent/src/tap/context.rs index 3a936b78f..b49a34d88 100644 --- a/crates/tap-agent/src/tap/context.rs +++ b/crates/tap-agent/src/tap/context.rs @@ -6,8 +6,6 @@ use indexer_common::escrow_accounts::EscrowAccounts; use sqlx::PgPool; use tokio::sync::watch::Receiver; -use super::escrow_adapter::EscrowAdapter; - pub mod checks; mod error; mod escrow; @@ -22,7 +20,6 @@ pub struct TapAgentContext { allocation_id: Address, sender: Address, escrow_accounts: Receiver, - escrow_adapter: EscrowAdapter, } impl TapAgentContext { @@ -31,14 +28,12 @@ impl TapAgentContext { allocation_id: Address, sender: Address, escrow_accounts: Receiver, - escrow_adapter: EscrowAdapter, ) -> Self { Self { pgpool, allocation_id, sender, escrow_accounts, - escrow_adapter, } } } diff --git a/crates/tap-agent/src/tap/context/escrow.rs b/crates/tap-agent/src/tap/context/escrow.rs index a362f6921..bded312e5 100644 --- a/crates/tap-agent/src/tap/context/escrow.rs +++ b/crates/tap-agent/src/tap/context/escrow.rs @@ -16,19 +16,30 @@ impl From for AdapterError { } } +// we don't need these checks anymore because there are being done before triggering +// a rav request. +// +// In any case, we don't want to fail a receipt because of this. +// The receipt is fine, just the escrow account that is not. #[async_trait] impl EscrowAdapterTrait for TapAgentContext { type AdapterError = AdapterError; - async fn get_available_escrow(&self, signer: Address) -> Result { - self.escrow_adapter.get_available_escrow(signer).await + async fn get_available_escrow(&self, _signer: Address) -> Result { + Ok(0) } - async fn subtract_escrow(&self, signer: Address, value: u128) -> Result<(), AdapterError> { - self.escrow_adapter.subtract_escrow(signer, value).await + async fn subtract_escrow(&self, _signer: Address, _value: u128) -> Result<(), AdapterError> { + Ok(()) } async fn verify_signer(&self, signer: Address) -> Result { - self.escrow_adapter.verify_signer(signer).await + let escrow_accounts = self.escrow_accounts.borrow(); + let sender = escrow_accounts + .get_sender_for_signer(&signer) + .map_err(|_| AdapterError::ValidationError { + error: format!("Could not find the sender for the signer {}", signer), + })?; + Ok(sender == self.sender) } } diff --git a/crates/tap-agent/src/tap/context/rav.rs b/crates/tap-agent/src/tap/context/rav.rs index 0fd1531ff..99ff2d666 100644 --- a/crates/tap-agent/src/tap/context/rav.rs +++ b/crates/tap-agent/src/tap/context/rav.rs @@ -133,10 +133,7 @@ mod test { use tokio::sync::watch; use super::*; - use crate::tap::{ - escrow_adapter::EscrowAdapter, - test_utils::{create_rav, ALLOCATION_ID_0, SENDER, SIGNER}, - }; + use crate::tap::test_utils::{create_rav, ALLOCATION_ID_0, SENDER, SIGNER}; #[derive(Debug)] struct TestableRav(SignedRAV); @@ -159,7 +156,6 @@ mod test { *ALLOCATION_ID_0, SENDER.1, watch::channel(EscrowAccounts::default()).1, - EscrowAdapter::mock(), ); // Insert a rav diff --git a/crates/tap-agent/src/tap/context/receipt.rs b/crates/tap-agent/src/tap/context/receipt.rs index 07f521db7..bc890cbc3 100644 --- a/crates/tap-agent/src/tap/context/receipt.rs +++ b/crates/tap-agent/src/tap/context/receipt.rs @@ -193,12 +193,9 @@ impl ReceiptDelete for TapAgentContext { #[cfg(test)] mod test { use super::*; - use crate::tap::{ - escrow_adapter::EscrowAdapter, - test_utils::{ - create_received_receipt, store_receipt, wallet, ALLOCATION_ID_0, SENDER, SIGNER, - TAP_EIP712_DOMAIN_SEPARATOR, - }, + use crate::tap::test_utils::{ + create_received_receipt, store_receipt, wallet, ALLOCATION_ID_0, SENDER, SIGNER, + TAP_EIP712_DOMAIN_SEPARATOR, }; use alloy::{primitives::U256, signers::local::PrivateKeySigner}; use anyhow::Result; @@ -224,13 +221,8 @@ mod test { )) .1; - let storage_adapter = TapAgentContext::new( - pgpool, - *ALLOCATION_ID_0, - SENDER.1, - escrow_accounts.clone(), - EscrowAdapter::mock(), - ); + let storage_adapter = + TapAgentContext::new(pgpool, *ALLOCATION_ID_0, SENDER.1, escrow_accounts.clone()); let received_receipt = create_received_receipt(&ALLOCATION_ID_0, &SIGNER.0, u64::MAX, u64::MAX, u128::MAX); @@ -445,7 +437,6 @@ mod test { *ALLOCATION_ID_0, SENDER.1, escrow_accounts.clone(), - EscrowAdapter::mock(), ); // Creating 100 receipts with timestamps 42 to 141 @@ -514,7 +505,6 @@ mod test { *ALLOCATION_ID_0, SENDER.1, escrow_accounts.clone(), - EscrowAdapter::mock(), ); // Creating 10 receipts with timestamps 42 to 51 @@ -638,13 +628,8 @@ mod test { )) .1; - let storage_adapter = TapAgentContext::new( - pgpool, - *ALLOCATION_ID_0, - SENDER.1, - escrow_accounts.clone(), - EscrowAdapter::mock(), - ); + let storage_adapter = + TapAgentContext::new(pgpool, *ALLOCATION_ID_0, SENDER.1, escrow_accounts.clone()); // Creating 10 receipts with timestamps 42 to 51 let mut received_receipt_vec = Vec::new(); diff --git a/crates/tap-agent/src/tap/escrow_adapter.rs b/crates/tap-agent/src/tap/escrow_adapter.rs deleted file mode 100644 index 10db98c57..000000000 --- a/crates/tap-agent/src/tap/escrow_adapter.rs +++ /dev/null @@ -1,168 +0,0 @@ -// Copyright 2023-, Edge & Node, GraphOps, and Semiotic Labs. -// SPDX-License-Identifier: Apache-2.0 - -use std::sync::{Arc, RwLock}; - -use alloy::primitives::Address; -use async_trait::async_trait; -use indexer_common::escrow_accounts::EscrowAccounts; -use tap_core::manager::adapters::EscrowHandler as EscrowAdapterTrait; -use tokio::sync::watch::Receiver; - -use super::context::AdapterError; - -/// The EscrowAdapter is used to track the available escrow for all senders. It is updated when -/// receipt checks are finalized (right before a RAV request). -/// -/// It is to be shared between all Account instances. Note that it is Arc internally, so it can be -/// shared through clones. -/// -/// It is not used to track unaggregated fees (yet?), because we are currently batch finalizing -/// receipt checks only when we need to send a RAV request. -#[derive(Clone)] -pub struct EscrowAdapter { - escrow_accounts: Receiver, - sender_id: Address, - sender_pending_fees: Arc>, -} - -impl EscrowAdapter { - pub fn new(escrow_accounts: Receiver, sender_id: Address) -> Self { - Self { - escrow_accounts, - sender_pending_fees: Arc::new(RwLock::new(0)), - sender_id, - } - } -} - -#[async_trait] -impl EscrowAdapterTrait for EscrowAdapter { - type AdapterError = AdapterError; - - async fn get_available_escrow(&self, signer: Address) -> Result { - let escrow_accounts = self.escrow_accounts.borrow(); - - let sender = escrow_accounts.get_sender_for_signer(&signer)?; - - let balance = escrow_accounts.get_balance_for_sender(&sender)?.to_owned(); - let balance: u128 = balance - .try_into() - .map_err(|_| AdapterError::BalanceTooLarge { - sender: sender.to_owned(), - })?; - - let fees = *self.sender_pending_fees.read().unwrap(); - Ok(balance - fees) - } - - async fn subtract_escrow(&self, signer: Address, value: u128) -> Result<(), AdapterError> { - let current_available_escrow = self.get_available_escrow(signer).await?; - - let sender = self - .escrow_accounts - .borrow() - .get_sender_for_signer(&signer)?; - - let mut fees = self.sender_pending_fees.write().unwrap(); - if current_available_escrow < value { - return Err(AdapterError::NotEnoughEscrow { - sender: sender.to_owned(), - fees: value, - balance: current_available_escrow, - }); - } - *fees += value; - Ok(()) - } - - async fn verify_signer(&self, signer: Address) -> Result { - let escrow_accounts = self.escrow_accounts.borrow(); - let sender = escrow_accounts - .get_sender_for_signer(&signer) - .map_err(|_| AdapterError::ValidationError { - error: format!("Could not find the sender for the signer {}", signer), - })?; - Ok(sender == self.sender_id) - } -} - -#[cfg(test)] -mod test { - use std::{collections::HashMap, vec}; - - use alloy::primitives::U256; - use tokio::sync::watch; - - use crate::tap::test_utils::{SENDER, SIGNER}; - - use super::*; - - impl super::EscrowAdapter { - pub fn mock() -> Self { - let escrow_accounts = watch::channel(EscrowAccounts::new( - HashMap::from([(SENDER.1, U256::from(1000))]), - HashMap::from([(SENDER.1, vec![SIGNER.1])]), - )) - .1; - Self { - escrow_accounts, - sender_pending_fees: Arc::new(RwLock::new(0)), - sender_id: Address::ZERO, - } - } - } - - #[tokio::test] - async fn test_subtract_escrow() { - let escrow_accounts = watch::channel(EscrowAccounts::new( - HashMap::from([(SENDER.1, U256::from(1000))]), - HashMap::from([(SENDER.1, vec![SIGNER.1])]), - )) - .1; - - let sender_pending_fees = Arc::new(RwLock::new(500)); - - let adapter = EscrowAdapter { - escrow_accounts, - sender_pending_fees, - sender_id: Address::ZERO, - }; - adapter - .subtract_escrow(SIGNER.1, 500) - .await - .expect("Subtract escrow."); - let available_escrow = adapter - .get_available_escrow(SIGNER.1) - .await - .expect("Get available escrow."); - assert_eq!(available_escrow, 0); - } - - #[tokio::test] - async fn test_subtract_escrow_overflow() { - let escrow_accounts = watch::channel(EscrowAccounts::new( - HashMap::from([(SENDER.1, U256::from(1000))]), - HashMap::from([(SENDER.1, vec![SIGNER.1])]), - )) - .1; - - let sender_pending_fees = Arc::new(RwLock::new(500)); - - let adapter = EscrowAdapter { - escrow_accounts, - sender_pending_fees, - sender_id: Address::ZERO, - }; - adapter - .subtract_escrow(SIGNER.1, 250) - .await - .expect("Subtract escrow."); - assert!(adapter.subtract_escrow(SIGNER.1, 251).await.is_err()); - let available_escrow = adapter - .get_available_escrow(SIGNER.1) - .await - .expect("Get available escrow."); - assert_eq!(available_escrow, 250); - } -} diff --git a/crates/tap-agent/src/tap/mod.rs b/crates/tap-agent/src/tap/mod.rs index 56be0537e..535538ca6 100644 --- a/crates/tap-agent/src/tap/mod.rs +++ b/crates/tap-agent/src/tap/mod.rs @@ -7,7 +7,6 @@ use indexer_common::escrow_accounts::EscrowAccounts; use tokio::sync::watch::Receiver; pub mod context; -pub mod escrow_adapter; #[cfg(test)] pub mod test_utils; From 3878e9c005a298859c5b16427d0ba94fd2fa51ad Mon Sep 17 00:00:00 2001 From: Gustavo Inacio Date: Mon, 11 Nov 2024 13:43:15 -0500 Subject: [PATCH 2/2] test: turn receipts into invalid Signed-off-by: Gustavo Inacio --- crates/tap-agent/src/agent/sender_allocation.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/tap-agent/src/agent/sender_allocation.rs b/crates/tap-agent/src/agent/sender_allocation.rs index b01859584..5e2d7a6bd 100644 --- a/crates/tap-agent/src/agent/sender_allocation.rs +++ b/crates/tap-agent/src/agent/sender_allocation.rs @@ -1702,18 +1702,20 @@ pub mod tests { .register( Mock::given(method("POST")) .and(body_string_contains("transactions")) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(json!({ "data": { "transactions": []}})), - ), + .respond_with(ResponseTemplate::new(200).set_body_json( + json!({ "data": { "transactions": [ + { + "id": "redeemed" + } + ]}}), + )), ) .await; - // Add invalid receipts to the database. ( receipts are older than rav ) + // Add invalid receipts to the database. ( already redeemed ) let timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("Time went backwards") - .as_nanos() as u64 - - 10000; + .as_nanos() as u64; const RECEIPT_VALUE: u128 = 1622018441284756158; const TOTAL_RECEIPTS: u64 = 10; const TOTAL_SUM: u128 = RECEIPT_VALUE * TOTAL_RECEIPTS as u128;