fix: some details, better error handling

Author: pcarranzav

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,41 +277,48 @@ pub async fn validate_and_create_agreement(
     allowed_payers: impl AsRef<[Address]>,
     voucher: Vec<u8>,
 ) -> Result<Uuid, DipsError> {
-    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(
     store: Arc<dyn AgreementStore>,
     domain: &Eip712Domain,
     cancellation_request: Vec<u8>,
 ) -> Result<Uuid, DipsError> {
-    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(())

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<Option<(SignedIndexingAgreementVoucher, bool)>>;
+    ) -> Result<Option<(SignedIndexingAgreementVoucher, bool)>, 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<Option<(SignedIndexingAgreementVoucher, bool)>> {
-        Ok(self.data.try_read()?.get(&id).cloned())
+    ) -> Result<Option<(SignedIndexingAgreementVoucher, bool)>, 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(())
     }

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,54 +18,59 @@ 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, DipsError> {
+    BigDecimal::from_str(&value.to_string())
+        .map_err(|e| DipsError::InvalidVoucher(format!("{}: {}", field, e)))
 }
 
 #[async_trait]
 impl AgreementStore for PsqlAgreementStore {
     async fn get_by_id(
         &self,
         id: Uuid,
-    ) -> anyhow::Result<Option<(SignedIndexingAgreementVoucher, bool)>> {
+    ) -> Result<Option<(SignedIndexingAgreementVoucher, bool)>, DipsError> {
         let item = sqlx::query!("SELECT * FROM indexing_agreements WHERE id=$1", id,)
             .fetch_one(&self.pool)
             .await;
 
         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)))
     }
     async fn create_agreement(
         &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,18 +226,17 @@ 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 =
             <indexer_dips::SubgraphIndexingVoucherMetadata as SolType>::abi_decode(
                 retrieved_voucher.metadata.as_ref(),
                 true,
             )
             .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