From 84024a683de12fa82d63ac3a3d374ee74e58e62b Mon Sep 17 00:00:00 2001 From: Pablo Carranza Velez Date: Thu, 6 Feb 2025 09:00:30 -0300 Subject: [PATCH] fix: some details, better error handling --- crates/dips/src/lib.rs | 48 +++++++++++++---------- crates/dips/src/store.rs | 26 ++++++++----- crates/service/src/database/dips.rs | 60 +++++++++++++++-------------- 3 files changed, 76 insertions(+), 58 deletions(-) diff --git a/crates/dips/src/lib.rs b/crates/dips/src/lib.rs index 2c59b42fc..ea00ef060 100644 --- a/crates/dips/src/lib.rs +++ b/crates/dips/src/lib.rs @@ -147,6 +147,8 @@ pub enum DipsError { AbiDecoding(String), #[error("agreement is cancelled")] AgreementCancelled, + #[error("invalid voucher: {0}")] + InvalidVoucher(String), } // TODO: send back messages @@ -275,17 +277,23 @@ pub async fn validate_and_create_agreement( allowed_payers: impl AsRef<[Address]>, voucher: Vec, ) -> Result { - let voucher = SignedIndexingAgreementVoucher::abi_decode(voucher.as_ref(), true) + let decoded_voucher = SignedIndexingAgreementVoucher::abi_decode(voucher.as_ref(), true) .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; - let metadata = - SubgraphIndexingVoucherMetadata::abi_decode(voucher.voucher.metadata.as_ref(), true) - .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; + let metadata = SubgraphIndexingVoucherMetadata::abi_decode( + decoded_voucher.voucher.metadata.as_ref(), + true, + ) + .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; - voucher.validate(domain, expected_payee, allowed_payers)?; + decoded_voucher.validate(domain, expected_payee, allowed_payers)?; - store.create_agreement(voucher.clone(), metadata).await?; + store + .create_agreement(decoded_voucher.clone(), metadata) + .await?; - Ok(Uuid::from_bytes(voucher.voucher.agreement_id.into())) + Ok(Uuid::from_bytes( + decoded_voucher.voucher.agreement_id.into(), + )) } pub async fn validate_and_cancel_agreement( @@ -293,23 +301,24 @@ pub async fn validate_and_cancel_agreement( domain: &Eip712Domain, cancellation_request: Vec, ) -> Result { - let request = SignedCancellationRequest::abi_decode(cancellation_request.as_ref(), true) - .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; + let decoded_request = + SignedCancellationRequest::abi_decode(cancellation_request.as_ref(), true) + .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; let result = store - .get_by_id(Uuid::from_bytes(request.request.agreement_id.into())) + .get_by_id(Uuid::from_bytes( + decoded_request.request.agreement_id.into(), + )) .await?; - let agreement_and_cancelled = result.ok_or(DipsError::AgreementNotFound)?; - let agreement = agreement_and_cancelled.0; - let cancelled = agreement_and_cancelled.1; + let (agreement, cancelled) = result.ok_or(DipsError::AgreementNotFound)?; if cancelled { return Err(DipsError::AgreementCancelled); } let expected_signer = agreement.voucher.payer; - let id = Uuid::from_bytes(request.request.agreement_id.into()); - request.validate(domain, &expected_signer)?; + let id = Uuid::from_bytes(decoded_request.request.agreement_id.into()); + decoded_request.validate(domain, &expected_signer)?; - store.cancel_agreement(request).await?; + store.cancel_agreement(decoded_request).await?; Ok(id) } @@ -384,9 +393,7 @@ mod test { let actual = store.get_by_id(actual_id).await.unwrap(); - let actual_voucher_and_cancelled = actual.unwrap(); - let actual_voucher = actual_voucher_and_cancelled.0; - let actual_cancelled = actual_voucher_and_cancelled.1; + let (actual_voucher, actual_cancelled) = actual.unwrap(); assert_eq!(voucher, actual_voucher); assert!(!actual_cancelled); Ok(()) @@ -590,8 +597,7 @@ mod test { // Verify agreement is cancelled let result = store.get_by_id(agreement_id).await?; - let agreement_and_cancelled = result.ok_or(DipsError::AgreementNotFound)?; - let cancelled = agreement_and_cancelled.1; + let (_, cancelled) = result.ok_or(DipsError::AgreementNotFound)?; assert!(cancelled); Ok(()) diff --git a/crates/dips/src/store.rs b/crates/dips/src/store.rs index f1fae55c2..e437f7cc9 100644 --- a/crates/dips/src/store.rs +++ b/crates/dips/src/store.rs @@ -16,12 +16,12 @@ pub trait AgreementStore: Sync + Send + std::fmt::Debug { async fn get_by_id( &self, id: Uuid, - ) -> anyhow::Result>; + ) -> Result, DipsError>; async fn create_agreement( &self, agreement: SignedIndexingAgreementVoucher, metadata: SubgraphIndexingVoucherMetadata, - ) -> anyhow::Result<()>; + ) -> Result<(), DipsError>; async fn cancel_agreement( &self, signed_cancellation: SignedCancellationRequest, @@ -38,18 +38,26 @@ impl AgreementStore for InMemoryAgreementStore { async fn get_by_id( &self, id: Uuid, - ) -> anyhow::Result> { - Ok(self.data.try_read()?.get(&id).cloned()) + ) -> Result, DipsError> { + Ok(self + .data + .try_read() + .map_err(|e| DipsError::UnknownError(e.into()))? + .get(&id) + .cloned()) } async fn create_agreement( &self, agreement: SignedIndexingAgreementVoucher, _medatadata: SubgraphIndexingVoucherMetadata, - ) -> anyhow::Result<()> { - self.data.try_write()?.insert( - Uuid::from_bytes(agreement.voucher.agreement_id.into()), - (agreement.clone(), false), - ); + ) -> Result<(), DipsError> { + self.data + .try_write() + .map_err(|e| DipsError::UnknownError(e.into()))? + .insert( + Uuid::from_bytes(agreement.voucher.agreement_id.into()), + (agreement.clone(), false), + ); Ok(()) } diff --git a/crates/service/src/database/dips.rs b/crates/service/src/database/dips.rs index f1ca073f7..2784a374f 100644 --- a/crates/service/src/database/dips.rs +++ b/crates/service/src/database/dips.rs @@ -3,7 +3,6 @@ use std::str::FromStr; -use anyhow::bail; use axum::async_trait; use build_info::chrono::{DateTime, Utc}; use indexer_dips::{ @@ -19,13 +18,9 @@ pub struct PsqlAgreementStore { pub pool: PgPool, } -// Utility to convert alloy::alloy_primitives::Uint<256, 4> into sqlx::BigDecimal -fn uint256_to_bigdecimal(uint256: &uint256) -> BigDecimal { - BigDecimal::from_str(&uint256.to_string()).unwrap() -} - -fn uint32_to_i64(uint32: u32) -> i64 { - uint32.into() +fn uint256_to_bigdecimal(value: &uint256, field: &str) -> Result { + BigDecimal::from_str(&value.to_string()) + .map_err(|e| DipsError::InvalidVoucher(format!("{}: {}", field, e))) } #[async_trait] @@ -33,7 +28,7 @@ impl AgreementStore for PsqlAgreementStore { async fn get_by_id( &self, id: Uuid, - ) -> anyhow::Result> { + ) -> Result, DipsError> { let item = sqlx::query!("SELECT * FROM indexing_agreements WHERE id=$1", id,) .fetch_one(&self.pool) .await; @@ -41,11 +36,11 @@ impl AgreementStore for PsqlAgreementStore { let item = match item { Ok(item) => item, Err(sqlx::Error::RowNotFound) => return Ok(None), - Err(err) => bail!(err), + Err(err) => return Err(DipsError::UnknownError(err.into())), }; - let signed = - SignedIndexingAgreementVoucher::abi_decode(item.signed_payload.as_ref(), true)?; + let signed = SignedIndexingAgreementVoucher::abi_decode(item.signed_payload.as_ref(), true) + .map_err(|e| DipsError::AbiDecoding(e.to_string()))?; let cancelled = item.cancelled_at.is_some(); Ok(Some((signed, cancelled))) } @@ -53,20 +48,29 @@ impl AgreementStore for PsqlAgreementStore { &self, agreement: SignedIndexingAgreementVoucher, metadata: SubgraphIndexingVoucherMetadata, - ) -> anyhow::Result<()> { + ) -> Result<(), DipsError> { let id = Uuid::from_bytes(agreement.voucher.agreement_id.into()); let bs = agreement.encode_vec(); let now = Utc::now(); - let deadline = - DateTime::from_timestamp(agreement.voucher.deadline.try_into().unwrap(), 0).unwrap(); - let base_price_per_epoch = uint256_to_bigdecimal(&metadata.basePricePerEpoch); - let price_per_entity = uint256_to_bigdecimal(&metadata.pricePerEntity); - let duration_epochs = uint32_to_i64(agreement.voucher.durationEpochs); - let max_initial_amount = uint256_to_bigdecimal(&agreement.voucher.maxInitialAmount); - let max_ongoing_amount_per_epoch = - uint256_to_bigdecimal(&agreement.voucher.maxOngoingAmountPerEpoch); - let min_epochs_per_collection = uint32_to_i64(agreement.voucher.minEpochsPerCollection); - let max_epochs_per_collection = uint32_to_i64(agreement.voucher.maxEpochsPerCollection); + let deadline_i64: i64 = agreement + .voucher + .deadline + .try_into() + .map_err(|_| DipsError::InvalidVoucher("deadline".to_string()))?; + let deadline = DateTime::from_timestamp(deadline_i64, 0) + .ok_or(DipsError::InvalidVoucher("deadline".to_string()))?; + let base_price_per_epoch = + uint256_to_bigdecimal(&metadata.basePricePerEpoch, "basePricePerEpoch")?; + let price_per_entity = uint256_to_bigdecimal(&metadata.pricePerEntity, "pricePerEntity")?; + let duration_epochs: i64 = agreement.voucher.durationEpochs.into(); + let max_initial_amount = + uint256_to_bigdecimal(&agreement.voucher.maxInitialAmount, "maxInitialAmount")?; + let max_ongoing_amount_per_epoch = uint256_to_bigdecimal( + &agreement.voucher.maxOngoingAmountPerEpoch, + "maxOngoingAmountPerEpoch", + )?; + let min_epochs_per_collection: i64 = agreement.voucher.minEpochsPerCollection.into(); + let max_epochs_per_collection: i64 = agreement.voucher.maxEpochsPerCollection.into(); sqlx::query!( "INSERT INTO indexing_agreements VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,null,null,null)", id, @@ -90,7 +94,8 @@ impl AgreementStore for PsqlAgreementStore { now ) .execute(&self.pool) - .await?; + .await + .map_err(|e| DipsError::UnknownError(e.into()))?; Ok(()) } @@ -221,10 +226,9 @@ pub(crate) mod test { .unwrap(); // Retrieve agreement - let retrieved = store.get_by_id(id).await.unwrap().unwrap(); + let (retrieved_signed_voucher, cancelled) = store.get_by_id(id).await.unwrap().unwrap(); - let retrieved_voucher = &retrieved.0.voucher; - let cancelled = retrieved.1; + let retrieved_voucher = &retrieved_signed_voucher.voucher; let retrieved_metadata = ::abi_decode( retrieved_voucher.metadata.as_ref(), @@ -232,7 +236,7 @@ pub(crate) mod test { ) .unwrap(); // Verify retrieved agreement matches original - assert_eq!(retrieved.0.signature, agreement.signature); + assert_eq!(retrieved_signed_voucher.signature, agreement.signature); assert_eq!( retrieved_voucher.durationEpochs, agreement.voucher.durationEpochs