Skip to content

Commit

Permalink
payment: refactor validate allocation to return an enum instead of bool
Browse files Browse the repository at this point in the history
  • Loading branch information
kamirr committed Jul 9, 2024
1 parent fc85b43 commit a844351
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 32 deletions.
9 changes: 8 additions & 1 deletion core/model/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,16 @@ impl ValidateAllocation {
}
}

#[derive(Clone, Debug, Serialize, Deserialize)]
pub enum ValidateAllocationResult {
InsufficientFunds,
TimeoutExceedsDeposit,
Valid,
}

impl RpcMessage for ValidateAllocation {
const ID: &'static str = "ValidateAllocation";
type Item = bool;
type Item = ValidateAllocationResult;
type Error = GenericError;
}

Expand Down
4 changes: 2 additions & 2 deletions core/model/src/payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum RpcMessageError {

pub mod local {
use super::{public::Ack, *};
use crate::driver::{AccountMode, GasDetails, PaymentConfirmation};
use crate::driver::{AccountMode, GasDetails, PaymentConfirmation, ValidateAllocationResult};
use bigdecimal::{BigDecimal, Zero};
use chrono::{DateTime, Utc};
use erc20_payment_lib::rpc_pool::{Web3ExternalSources, Web3FullNodeData};
Expand Down Expand Up @@ -431,7 +431,7 @@ pub mod local {

impl RpcMessage for ValidateAllocation {
const ID: &'static str = "ValidateAllocation";
type Item = bool;
type Item = ValidateAllocationResult;
type Error = ValidateAllocationError;
}

Expand Down
2 changes: 1 addition & 1 deletion core/payment-driver/base/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub trait PaymentDriver {

caller: String,
msg: ValidateAllocation,
) -> Result<bool, GenericError>;
) -> Result<ValidateAllocationResult, GenericError>;

async fn release_deposit(
&self,
Expand Down
4 changes: 2 additions & 2 deletions core/payment-driver/dummy/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ async fn validate_allocation(
_db: (),
_caller: String,
_msg: ValidateAllocation,
) -> Result<bool, GenericError> {
Ok(true)
) -> Result<ValidateAllocationResult, GenericError> {
Ok(ValidateAllocationResult::Valid)
}

async fn fund(_db: (), _caller: String, _msg: Fund) -> Result<String, GenericError> {
Expand Down
37 changes: 20 additions & 17 deletions core/payment-driver/erc20/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ impl Erc20Driver {
&self,
caller: String,
msg: ValidateAllocation,
) -> Result<bool, GenericError> {
) -> Result<ValidateAllocationResult, GenericError> {
if msg.deposit.is_some() {
Err(GenericError::new(
"validate_allocation_internal called with not empty deposit",
Expand Down Expand Up @@ -391,14 +391,18 @@ impl Erc20Driver {
total_allocated_amount,
);

Ok(msg.amount <= account_balance - total_allocated_amount)
Ok(if msg.amount > account_balance {
ValidateAllocationResult::InsufficientFunds
} else {
ValidateAllocationResult::Valid
})
}

async fn validate_allocation_deposit(
&self,
msg: ValidateAllocation,
deposit: Deposit,
) -> Result<bool, GenericError> {
) -> Result<ValidateAllocationResult, GenericError> {
let network = msg
.platform
.split('-')
Expand Down Expand Up @@ -435,37 +439,36 @@ impl Erc20Driver {
deposit_timeout,
);

let valid_amount = msg.amount <= deposit_balance;

if !valid_amount {
log::info!(
if msg.amount > deposit_balance {
log::debug!(
"Deposit validation failed: requested amount [{}] > deposit balance [{}]",
msg.amount,
deposit_balance
);
}

let valid_timeout = if let Some(timeout) = msg.timeout {
let valid_timeout = timeout <= deposit_details.valid_to;
return Ok(ValidateAllocationResult::InsufficientFunds);
}

if !valid_timeout {
log::info!(
if let Some(timeout) = msg.timeout {
if timeout > deposit_details.valid_to {
log::debug!(
"Deposit validation failed: requested timeout [{}] > deposit timeout [{}]",
timeout,
deposit_timeout
);

return Ok(ValidateAllocationResult::TimeoutExceedsDeposit);
}
valid_timeout
} else {
log::info!(
log::debug!(
"Deposit validation failed: allocations with deposits must have a timeout. Deposit timeout: {}",
deposit_timeout
);

false
return Ok(ValidateAllocationResult::TimeoutExceedsDeposit);
};

Ok(valid_amount && valid_timeout)
Ok(ValidateAllocationResult::Valid)
}
}

Expand Down Expand Up @@ -1003,7 +1006,7 @@ impl PaymentDriver for Erc20Driver {
&self,
caller: String,
mut msg: ValidateAllocation,
) -> Result<bool, GenericError> {
) -> Result<ValidateAllocationResult, GenericError> {
log::debug!("Validate_allocation: {:?}", msg);

if let Some(deposit) = msg.deposit.take() {
Expand Down
35 changes: 29 additions & 6 deletions core/payment/src/api/allocations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use ya_client_model::NodeId;
use ya_agreement_utils::{ClauseOperator, ConstraintKey, Constraints};
use ya_client_model::payment::allocation::{PaymentPlatform, PaymentPlatformEnum};
use ya_client_model::payment::*;
use ya_core_model::driver::ValidateAllocationResult;
use ya_core_model::payment::local::{
get_token_from_network_name, DriverName, NetworkName, ReleaseDeposit, ValidateAllocation,
ValidateAllocationError, BUS_ID as LOCAL_SERVICE,
Expand Down Expand Up @@ -344,11 +345,17 @@ async fn create_allocation(
};

match async move { Ok(bus::service(LOCAL_SERVICE).send(validate_msg).await??) }.await {
Ok(true) => {}
Ok(false) => {
Ok(ValidateAllocationResult::Valid) => {}
Ok(ValidateAllocationResult::InsufficientFunds) => {
return bad_req_and_log(format!("Insufficient funds to make allocation for payment platform {payment_triple}. \
Top up your account or release all existing allocations to unlock the funds via `yagna payment release-allocations`"));
}
Ok(ValidateAllocationResult::TimeoutExceedsDeposit) => {
return bad_req_and_log(
"Requested allocation timeout either not set or exceeds deposit timeout"
.to_string(),
);
}
Err(Error::Rpc(RpcMessageError::ValidateAllocation(
ValidateAllocationError::AccountNotRegistered,
))) => {
Expand Down Expand Up @@ -455,6 +462,11 @@ async fn amend_allocation(
body: Json<AllocationUpdate>,
id: Identity,
) -> HttpResponse {
let bad_req_and_log = |err_msg: String| -> HttpResponse {
log::error!("{}", err_msg);
response::bad_request(&err_msg)
};

let allocation_id = path.allocation_id.clone();
let node_id = id.identity;
let new_allocation: AllocationUpdate = body.into_inner();
Expand All @@ -477,6 +489,8 @@ async fn amend_allocation(
Err(e) => return response::bad_request(&e),
};

let payment_triple = amended_allocation.payment_platform.clone();

// validation will take into account all existing allocation, including the one
// being currently modified. This means we only need to validate the increase.
let amount_to_validate =
Expand All @@ -494,11 +508,20 @@ async fn amend_allocation(
deposit: amended_allocation.deposit.clone(),
};
match async move { Ok(bus::service(LOCAL_SERVICE).send(validate_msg).await??) }.await {
Ok(true) => {}
Ok(false) => return response::bad_request(&"Insufficient funds to make allocation. Top up your account or release all existing allocations to unlock the funds via `yagna payment release-allocations`"),
Ok(ValidateAllocationResult::Valid) => {}
Ok(ValidateAllocationResult::InsufficientFunds) => {
return bad_req_and_log(format!("Insufficient funds to make allocation for payment platform {payment_triple}. \
Top up your account or release all existing allocations to unlock the funds via `yagna payment release-allocations`"));
}
Ok(ValidateAllocationResult::TimeoutExceedsDeposit) => {
return bad_req_and_log(
"Requested allocation timeout either not set or exceeds deposit timeout"
.to_string(),
);
}
Err(Error::Rpc(RpcMessageError::ValidateAllocation(
ValidateAllocationError::AccountNotRegistered,
))) => return response::bad_request(&"Account not registered"),
ValidateAllocationError::AccountNotRegistered,
))) => return response::bad_request(&"Account not registered"),
Err(e) => return response::server_error(&e),
}

Expand Down
4 changes: 2 additions & 2 deletions core/payment/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use ya_client_model::payment::{
};
use ya_core_model::driver::{
self, driver_bus_id, AccountMode, DriverReleaseDeposit, GasDetails, GetRpcEndpointsResult,
PaymentConfirmation, PaymentDetails, ShutDown, ValidateAllocation,
PaymentConfirmation, PaymentDetails, ShutDown, ValidateAllocation, ValidateAllocationResult,
};
use ya_core_model::payment::local::{
GenericError, GetAccountsError, GetDriversError, NotifyPayment, RegisterAccount,
Expand Down Expand Up @@ -804,7 +804,7 @@ impl PaymentProcessor {
amount: BigDecimal,
timeout: Option<DateTime<Utc>>,
deposit: Option<Deposit>,
) -> Result<bool, ValidateAllocationError> {
) -> Result<ValidateAllocationResult, ValidateAllocationError> {
if self.in_shutdown.load(Ordering::SeqCst) {
return Err(ValidateAllocationError::Shutdown);
}
Expand Down
3 changes: 2 additions & 1 deletion core/payment/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ mod local {
},
NodeId,
};
use ya_core_model::driver::ValidateAllocationResult;
use ya_core_model::payment::public::Ack;
use ya_core_model::{
driver::{driver_bus_id, DriverStatus, DriverStatusError},
Expand Down Expand Up @@ -413,7 +414,7 @@ mod local {
processor: Arc<PaymentProcessor>,
sender: String,
msg: ValidateAllocation,
) -> Result<bool, ValidateAllocationError> {
) -> Result<ValidateAllocationResult, ValidateAllocationError> {
Ok(processor
.validate_allocation(
msg.platform,
Expand Down

0 comments on commit a844351

Please sign in to comment.