From fe7b5ec9b3b77d9f9f4199d085fb50c96bd16c85 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 1 Mar 2022 19:35:30 +0200 Subject: [PATCH 01/13] POST /v5/channel/:id/pay route handler + cargo fmt/clippy --- primitives/src/sentry.rs | 5 + sentry/src/db/accounting.rs | 5 +- sentry/src/lib.rs | 36 ++++--- sentry/src/routes.rs | 114 +++++++++++------------ sentry/src/routes/channel.rs | 93 +++++++++++++++++- validator_worker/src/heartbeat.rs | 4 +- validator_worker/src/sentry_interface.rs | 28 +++--- 7 files changed, 197 insertions(+), 88 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index f799c6dca..6d37d3adb 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -591,6 +591,11 @@ pub struct AllSpendersQuery { pub page: u64, } +#[derive(Debug, Serialize, Deserialize)] +pub struct ChannelPayBody { + pub payouts: HashMap, +} + #[derive(Serialize, Deserialize, Debug)] pub struct ValidatorMessage { pub from: ValidatorId, diff --git a/sentry/src/db/accounting.rs b/sentry/src/db/accounting.rs index 595e7a8d1..400e7a2d0 100644 --- a/sentry/src/db/accounting.rs +++ b/sentry/src/db/accounting.rs @@ -9,6 +9,7 @@ use tokio_postgres::{ }; use super::{DbPool, PoolError}; +use serde::Serialize; use thiserror::Error; static UPDATE_ACCOUNTING_STATEMENT: &str = "INSERT INTO accounting(channel_id, side, address, amount, updated, created) VALUES($1, $2, $3, $4, $5, $6) ON CONFLICT ON CONSTRAINT accounting_pkey DO UPDATE SET amount = accounting.amount + $4, updated = $6 WHERE accounting.channel_id = $1 AND accounting.side = $2 AND accounting.address = $3 RETURNING channel_id, side, address, amount, updated, created"; @@ -27,7 +28,7 @@ impl From for Error { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct Accounting { pub channel_id: ChannelId, pub side: Side, @@ -50,7 +51,7 @@ impl From<&Row> for Accounting { } } -#[derive(Debug, Clone, Copy, ToSql, FromSql, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, ToSql, FromSql, PartialEq, Eq, Serialize)] #[postgres(name = "accountingside")] pub enum Side { Earner, diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index ff245a7ca..b2ab24a2f 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -27,25 +27,24 @@ use { routes::{ campaign, campaign::{campaign_list, create_campaign, update_campaign}, - get_cfg, - get_analytics, channel::{ - add_spender_leaf, channel_list, get_accounting_for_channel, get_all_spender_limits, - get_spender_limits, last_approved, + add_spender_leaf, channel_list, channel_payout, get_accounting_for_channel, + get_all_spender_limits, get_spender_limits, last_approved, validator_message::{ create_validator_messages, extract_params, list_validator_messages, }, }, + get_analytics, get_cfg, }, }; -pub mod analytics; -pub mod middleware; -pub mod routes; pub mod access; +pub mod analytics; pub mod application; pub mod db; +pub mod middleware; pub mod payout; +pub mod routes; pub mod spender; static LAST_APPROVED_BY_CHANNEL_ID: Lazy = Lazy::new(|| { @@ -80,6 +79,9 @@ static CHANNEL_ACCOUNTING: Lazy = Lazy::new(|| { Regex::new(r"^/v5/channel/0x([a-zA-Z0-9]{64})/accounting/?$") .expect("The regex should be valid") }); +static CHANNEL_PAY: Lazy = Lazy::new(|| { + Regex::new(r"^/v5/channel/0x([a-zA-Z0-9]{64})/pay/?$").expect("The regex should be valid") +}); /// Regex extracted parameters. /// This struct is created manually on each of the matched routes. @@ -217,7 +219,6 @@ async fn analytics_router( mut req: Request, app: &Application, ) -> Result, ResponseError> { - let (route, method) = (req.uri().path(), req.method()); match (route, method) { @@ -267,10 +268,6 @@ async fn channels_router( let (path, method) = (req.uri().path().to_owned(), req.method()); // TODO AIP#61: Add routes for: - // - POST /channel/:id/pay - // #[serde(rename_all = "camelCase")] - // Pay { payout: BalancesMap }, - // // - GET /channel/:id/spender/:addr // - GET /channel/:id/spender/all // - POST /channel/:id/spender/:addr @@ -381,6 +378,21 @@ async fn channels_router( .await?; get_accounting_for_channel(req, app).await + } + // POST /v5/channel/:id/pay + else if let (Some(caps), &Method::POST) = (CHANNEL_PAY.captures(&path), method) { + let param = RouteParams(vec![caps + .get(1) + .map_or("".to_string(), |m| m.as_str().to_string())]); + req.extensions_mut().insert(param); + + req = Chain::new() + .chain(AuthRequired) + .chain(ChannelLoad) + .apply(req, app) + .await?; + + channel_payout(req, app).await } else { Err(ResponseError::NotFound) } diff --git a/sentry/src/routes.rs b/sentry/src/routes.rs index 086f7d2be..58feed512 100644 --- a/sentry/src/routes.rs +++ b/sentry/src/routes.rs @@ -1,124 +1,124 @@ //! Sentry REST API documentation -//! +//! //! ## Channel -//! +//! //! All routes are implemented under module [channel]. -//! +//! //! - [`GET /v5/channel/list`](crate::routes::channel::channel_list) -//! +//! //! todo -//! +//! //! - [`GET /v5/channel/:id/accounting`](channel::get_accounting_for_channel) -//! +//! //! todo -//! +//! //! - [`GET /v5/channel/:id/spender/:addr`](channel::get_spender_limits) (auth required) -//! +//! //! todo -//! +//! //! - [`POST /v5/channel/:id/spender/:addr`](channel::add_spender_leaf) (auth required) -//! +//! //! todo -//! +//! //! - [`GET /v5/channel/:id/spender/all`](channel::get_all_spender_limits) (auth required) -//! +//! //! todo -//! +//! //! - [`GET /v5/channel/:id/validator-messages`](channel::validator_message::list_validator_messages) -//! +//! //! - `GET /v5/channel/:id/validator-messages/:ValidatorId` - filter by ValidatorId //! - `GET /v5/channel/:id/validator-messages/:ValidatorId/NewState+ApproveState` - filters by a given [`primitives::ValidatorId`] and a //! [`Validator message types`](primitives::validator::MessageTypes). -//! +//! //! Request query parameters: [channel::validator_message::ValidatorMessagesListQuery] //! Response: [primitives::sentry::ValidatorMessageResponse] -//! +//! //! - [`POST /v5/channel/:id/validator-messages`](channel::validator_message::create_validator_messages) (auth required) -//! +//! //! todo -//! +//! //! - [`POST /v5/channel/:id/last-approved`](channel::last_approved) -//! +//! //! todo -//! +//! //! - `POST /v5/channel/:id/pay` (auth required) -//! +//! //! TODO: implement and document as part of issue #382 -//! +//! //! Channel Payout with authentication of the spender -//! +//! //! Withdrawals of advertiser funds - re-introduces the PAY event with a separate route. -//! +//! //! - `GET /v5/channel/:id/get-leaf` -//! +//! //! TODO: implement and document as part of issue #382 -//! +//! //! This route gets the latest approved state (`NewState`/`ApproveState` pair), //! and finds the given `spender`/`earner` in the balances tree, and produce a merkle proof for it. //! This is useful for the Platform to verify if a spender leaf really exists. -//! +//! //! Query parameters: -//! +//! //! - `spender=[0x...]` or `earner=[0x...]` (required) -//! +//! //! Example Spender: -//! +//! //! `/get-leaf?spender=0x...` -//! +//! //! Example Earner: -//! +//! //! `/get-leaf?earner=0x....` //! This module includes all routes for `Sentry` and the documentation of each Request/Response. -//! +//! //! ## Campaign -//! +//! //! All routes are implemented under module [campaign]. -//! +//! //! - `GET /v5/campaign/list` -//! +//! //! Lists all campaigns with pagination and orders them in descending order (`DESC`) by `Campaign.created`. This ensures that the order in the pages will not change if a new `Campaign` is created while still retrieving a page. -//! +//! //! Query parameters: //! - `page=[integer]` (optional) default: `0` //! - `creator=[0x....]` (optional) - address of the creator to be filtered by //! - `activeTo=[integer]` (optional) in seconds - filters campaigns by `Campaign.active.to > query.activeTo` -//! - `validator=[0x...]` or `leader=[0x...]` (optional) - address of the validator to be filtered by. You can either +//! - `validator=[0x...]` or `leader=[0x...]` (optional) - address of the validator to be filtered by. You can either //! - `validator=[0x...]` - it will return all `Campaign`s where this address is **either** `Channel.leader` or `Channel.follower` //! - `leader=[0x...]` - it will return all `Campaign`s where this address is `Channel.leader` -//! -//! +//! +//! //! - `POST /v5/campaign` (auth required) -//! +//! //! Create a new Campaign. -//! +//! //! It will make sure the `Channel` is created if new and it will update the spendable amount using the `Adapter::get_deposit()`. -//! +//! //! Authentication: **required** to validate `Campaign.creator == Auth.uid` -//! +//! //! Request Body: [`primitives::sentry::campaign_create::CreateCampaign`] (json) -//! +//! //! `POST /v5/campaign/:id/close` (auth required) -//! +//! //! todo -//! +//! //! ## Analytics -//! +//! //! - `GET /v5/analytics` -//! +//! //! todo -//! +//! //! - `GET /v5/analytics/for-publisher` (auth required) -//! +//! //! todo -//! +//! //! - `GET /v5/analytics/for-advertiser` (auth required) -//! +//! //! todo -//! +//! //! - `GET /v5/analytics/for-admin` (auth required) -//! +//! //! todo -//! +//! pub use analytics::analytics as get_analytics; pub use cfg::config as get_cfg; @@ -128,4 +128,4 @@ mod analytics; pub mod campaign; // `cfg` module has single request, so we only export this request mod cfg; -pub mod channel; \ No newline at end of file +pub mod channel; diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index c97d93143..8fd40a036 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -1,14 +1,19 @@ //! Channel - `/v5/channel` routes -//! +//! use crate::db::{ - accounting::{get_all_accountings_for_channel, update_accounting, Side}, + accounting::{ + get_accounting, get_all_accountings_for_channel, spend_amount, update_accounting, Side, + }, event_aggregate::{latest_approve_state_v5, latest_heartbeats, latest_new_state_v5}, insert_channel, list_channels, spendable::{fetch_spendable, get_all_spendables_for_channel, update_spendable}, DbPool, }; -use crate::{success_response, Application, ResponseError, RouteParams}; +use crate::{ + campaign::fetch_campaign_ids_for_channel, success_response, Application, ResponseError, + RouteParams, +}; use adapter::{client::Locked, Adapter}; use futures::future::try_join_all; use hyper::{Body, Request, Response}; @@ -17,7 +22,8 @@ use primitives::{ config::TokenInfo, sentry::{ channel_list::ChannelListQuery, AccountingResponse, AllSpendersQuery, AllSpendersResponse, - LastApproved, LastApprovedQuery, LastApprovedResponse, SpenderResponse, SuccessResponse, + ChannelPayBody, LastApproved, LastApprovedQuery, LastApprovedResponse, SpenderResponse, + SuccessResponse, }, spender::{Spendable, Spender}, validator::NewState, @@ -379,6 +385,83 @@ pub async fn get_accounting_for_channel( Ok(success_response(serde_json::to_string(&res)?)) } +pub async fn channel_payout( + req: Request, + app: &Application, +) -> Result, ResponseError> { + let channel = req + .extensions() + .get::() + .expect("Request should have Channel") + .to_owned(); + + let body = hyper::body::to_bytes(req.into_body()).await?; + + let body = serde_json::from_slice::(&body) + .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; + + let channel_campaigns = + fetch_campaign_ids_for_channel(&app.pool, &channel.id(), app.config.campaigns_find_limit) + .await?; + + let campaigns_remaining_sum = app + .campaign_remaining + .get_multiple(channel_campaigns.as_slice()) + .await? + .iter() + .sum::>() + .ok_or_else(|| ResponseError::BadRequest( + "Couldn't sum remaining amount for all campaigns".to_string(), + ))?; + + // A campaign is closed when its remaining == 0 + // therefore for all campaigns for a channel to be closed their total remaining sum should be 0 + if campaigns_remaining_sum.ge(&UnifiedNum::from_u64(0)) { + return Err(ResponseError::BadRequest( + "Not all campaigns are closed!".to_string(), + )); + } + + let mut unchecked_balances: Balances = Balances::default(); + for (address, amount) in body.payouts { + let accounting = + get_accounting(app.pool.clone(), channel.id(), address, Side::Spender).await?; + match accounting { + Some(accounting) => { + // We need to check if the amount this spender has is available for withdrawal + if accounting.amount.lt(&amount) { + return Err(ResponseError::BadRequest(format!( + "Spender amount for {} is less than the requested withdrawal amount", + address + ))); + } + // If so, we add it to the earner balances for the same address + unchecked_balances + .earners + .insert(accounting.address, accounting.amount); + } + None => { + return Err(ResponseError::BadRequest( + "No accounting earner entry for channel/address pair".to_string(), + )); + } + } + } + + let balances = match unchecked_balances.check() { + Ok(balances) => balances, + Err(error) => { + error!(&app.logger, "{}", &error; "module" => "channel_payout"); + return Err(ResponseError::FailedValidation( + "Earners sum is not equal to spenders sum for channel".to_string(), + )); + } + }; + + let res = spend_amount(app.pool.clone(), channel.id(), balances).await?; + + Ok(success_response(serde_json::to_string(&res)?)) +} /// [`Channel`] [validator messages](primitives::validator::MessageTypes) routes /// starting with `/v5/channel/0xXXX.../validator-messages` /// @@ -522,7 +605,7 @@ pub mod validator_message { #[cfg(test)] mod test { use super::*; - use crate::db::{accounting::spend_amount, insert_channel}; + use crate::db::insert_channel; use crate::test_util::setup_dummy_app; use adapter::primitives::Deposit; use hyper::StatusCode; diff --git a/validator_worker/src/heartbeat.rs b/validator_worker/src/heartbeat.rs index 19dee87e7..0f8021009 100644 --- a/validator_worker/src/heartbeat.rs +++ b/validator_worker/src/heartbeat.rs @@ -27,7 +27,9 @@ pub async fn heartbeat( iface: &SentryApi, channel: Channel, ) -> Result { - let validator_message_response = iface.get_our_latest_msg(channel.id(), &["Heartbeat"]).await?; + let validator_message_response = iface + .get_our_latest_msg(channel.id(), &["Heartbeat"]) + .await?; let heartbeat_msg = match validator_message_response { Some(MessageTypes::Heartbeat(heartbeat)) => Some(heartbeat), _ => None, diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index ca3b981d9..a78c4eda5 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -323,17 +323,23 @@ impl SentryApi { channel: Channel, messages: &[&MessageTypes], ) -> Vec { - join_all(self.propagate_to.iter().filter(|(validator_id, _)| { - channel.leader == **validator_id || channel.follower == **validator_id - }).map(|(validator_id, validator)| { - propagate_to::( - &self.client, - self.config.propagation_timeout, - channel.id(), - (*validator_id, validator), - messages, - ) - })).await + join_all( + self.propagate_to + .iter() + .filter(|(validator_id, _)| { + channel.leader == **validator_id || channel.follower == **validator_id + }) + .map(|(validator_id, validator)| { + propagate_to::( + &self.client, + self.config.propagation_timeout, + channel.id(), + (*validator_id, validator), + messages, + ) + }), + ) + .await } } From d1d8dea71a9bc6af5b265e6183742bf3fe3c7e26 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 4 Mar 2022 12:44:42 +0200 Subject: [PATCH 02/13] requested changes --- primitives/src/sentry.rs | 6 +- sentry/src/routes/channel.rs | 116 +++++++++++++++++++---------------- 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index 352fc481e..213bc30c0 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -3,7 +3,7 @@ use crate::{ balances::BalancesState, spender::Spender, validator::{ApproveState, Heartbeat, MessageTypes, NewState, Type as MessageType}, - Address, Balances, CampaignId, UnifiedNum, ValidatorId, IPFS, + Address, Balances, CampaignId, UnifiedMap, UnifiedNum, ValidatorId, IPFS, }; use chrono::{ serde::ts_milliseconds, Date, DateTime, Duration, NaiveDate, TimeZone, Timelike, Utc, @@ -577,8 +577,8 @@ pub struct AllSpendersQuery { } #[derive(Debug, Serialize, Deserialize)] -pub struct ChannelPayBody { - pub payouts: HashMap, +pub struct ChannelPayRequest { + pub payouts: UnifiedMap, } #[derive(Serialize, Deserialize, Debug)] diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 1d79ae700..9825168d8 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -11,8 +11,8 @@ use crate::db::{ DbPool, }; use crate::{ - campaign::fetch_campaign_ids_for_channel, success_response, Application, ResponseError, - RouteParams, + campaign::{fetch_campaign_ids_for_channel, update_latest_spendable}, + success_response, Application, ResponseError, RouteParams, }; use adapter::{client::Locked, Adapter}; use futures::future::try_join_all; @@ -21,7 +21,7 @@ use primitives::{ balances::{Balances, CheckedState, UncheckedState}, sentry::{ channel_list::ChannelListQuery, AccountingResponse, AllSpendersQuery, AllSpendersResponse, - ChannelPayBody, LastApproved, LastApprovedQuery, LastApprovedResponse, SpenderResponse, + ChannelPayRequest, LastApproved, LastApprovedQuery, LastApprovedResponse, SpenderResponse, SuccessResponse, }, spender::{Spendable, Spender}, @@ -213,7 +213,7 @@ pub async fn get_spender_limits( } }; - let new_state = match get_corresponding_new_state(&app.pool, &app.logger, &channel).await? { + let new_state = match get_corresponding_new_state(&app.pool, &app.logger, channel).await? { Some(new_state) => new_state, None => return spender_response_without_leaf(latest_spendable.deposit.total), }; @@ -402,20 +402,22 @@ pub async fn channel_payout( req: Request, app: &Application, ) -> Result, ResponseError> { - let channel = req + let channel_context = req .extensions() - .get::() - .expect("Request should have Channel") + .get::>() + .expect("Request should have Channel & Chain/TokenInfo") .to_owned(); let body = hyper::body::to_bytes(req.into_body()).await?; - - let body = serde_json::from_slice::(&body) + let to_pay = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; - let channel_campaigns = - fetch_campaign_ids_for_channel(&app.pool, &channel.id(), app.config.campaigns_find_limit) - .await?; + let channel_campaigns = fetch_campaign_ids_for_channel( + &app.pool, + channel_context.context.id(), + app.config.campaigns_find_limit, + ) + .await?; let campaigns_remaining_sum = app .campaign_remaining @@ -423,57 +425,67 @@ pub async fn channel_payout( .await? .iter() .sum::>() - .ok_or_else(|| ResponseError::BadRequest( - "Couldn't sum remaining amount for all campaigns".to_string(), - ))?; + .ok_or_else(|| { + ResponseError::BadRequest("Couldn't sum remaining amount for all campaigns".to_string()) + })?; // A campaign is closed when its remaining == 0 // therefore for all campaigns for a channel to be closed their total remaining sum should be 0 if campaigns_remaining_sum.ge(&UnifiedNum::from_u64(0)) { - return Err(ResponseError::BadRequest( - "Not all campaigns are closed!".to_string(), + return Err(ResponseError::FailedValidation( + "All campaigns should be closed or have no budget left".to_string(), )); } - let mut unchecked_balances: Balances = Balances::default(); - for (address, amount) in body.payouts { - let accounting = - get_accounting(app.pool.clone(), channel.id(), address, Side::Spender).await?; - match accounting { - Some(accounting) => { - // We need to check if the amount this spender has is available for withdrawal - if accounting.amount.lt(&amount) { - return Err(ResponseError::BadRequest(format!( - "Spender amount for {} is less than the requested withdrawal amount", - address - ))); - } - // If so, we add it to the earner balances for the same address - unchecked_balances - .earners - .insert(accounting.address, accounting.amount); - } - None => { - return Err(ResponseError::BadRequest( - "No accounting earner entry for channel/address pair".to_string(), - )); - } + let mut balances: Balances = Balances::new(); + for (address, amount_to_return) in to_pay.payouts { + let accounting_spent = get_accounting( + app.pool.clone(), + channel_context.context.id(), + address, + Side::Spender, + ) + .await? + .map(|accounting_spent| accounting_spent.amount) + .unwrap_or_default(); + let accounting_earned = get_accounting( + app.pool.clone(), + channel_context.context.id(), + address, + Side::Spender, + ) + .await? + .map(|accounting_spent| accounting_spent.amount) + .unwrap_or_default(); + let latest_spendable = + update_latest_spendable(&app.adapter, &app.pool, &channel_context, address) + .await + .map_err(|err| ResponseError::BadRequest(err.to_string()))?; + let total_deposited = latest_spendable.deposit.total; + + let available_for_return = total_deposited + .checked_sub(&accounting_spent) + .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? + .checked_add(&accounting_earned) + .ok_or_else(|| { + ResponseError::FailedValidation("No more budget remaining".to_string()) + })?; + // We need to check if the amount this spender has is available for withdrawal + if available_for_return.lt(&amount_to_return) { + return Err(ResponseError::BadRequest(format!( + "Spender amount for {} is less than the requested withdrawal amount", + address + ))); } + // If so, we add it to the earner balances for the same address + balances.spend(address, address, available_for_return)?; } - let balances = match unchecked_balances.check() { - Ok(balances) => balances, - Err(error) => { - error!(&app.logger, "{}", &error; "module" => "channel_payout"); - return Err(ResponseError::FailedValidation( - "Earners sum is not equal to spenders sum for channel".to_string(), - )); - } - }; - - let res = spend_amount(app.pool.clone(), channel.id(), balances).await?; + spend_amount(app.pool.clone(), channel_context.context.id(), balances).await?; - Ok(success_response(serde_json::to_string(&res)?)) + Ok(success_response(serde_json::to_string(&SuccessResponse { + success: true, + })?)) } /// [`Channel`] [validator messages](primitives::validator::MessageTypes) routes /// starting with `/v5/channel/0xXXX.../validator-messages` From b7b14c1728b266f6b1f68f0964992467f533328e Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 12:20:56 +0200 Subject: [PATCH 03/13] tests, fixes, documentation, fmt --- sentry/src/lib.rs | 4 - sentry/src/routes.rs | 11 +- sentry/src/routes/channel.rs | 294 +++++++++++++++++++++++++++++------ 3 files changed, 256 insertions(+), 53 deletions(-) diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 18de8fdbb..2b9cf5668 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -262,10 +262,6 @@ async fn analytics_router( } // TODO AIP#61: Add routes for: -// - POST /channel/:id/pay -// #[serde(rename_all = "camelCase")] -// Pay { payout: BalancesMap }, -// // - GET /channel/:id/get-leaf async fn channels_router( mut req: Request, diff --git a/sentry/src/routes.rs b/sentry/src/routes.rs index b218a253c..5c44350d4 100644 --- a/sentry/src/routes.rs +++ b/sentry/src/routes.rs @@ -76,9 +76,16 @@ //! //! Response: [`LastApprovedResponse`][primitives::sentry::LastApprovedResponse] //! -//! - `POST /v5/channel/:id/pay` (auth required) +//! - [`POST /v5/channel/:id/pay`](channel::channel_payout) //! -//! TODO: implement and document as part of issue #382 +//! This route handles withdrawals of advertiser funds for a spender. Has authentication of the spender. +//! It needs to ensure all campaigns are closed. It accepts a JSON body in the request which contains +//! all of the earners and updates their balances accordingly. Used when an advertiser/spender wants +//! to get their remaining funds back. +//! +//! Request JSON body: [`ChannelPayRequest`](primitives::sentry::ChannelPayRequest) +//! +//! Response: [`SuccessResponse`](primitives::sentry::SuccessResponse) //! //! Channel Payout with authentication of the spender //! diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 9825168d8..3d5c9e515 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -11,8 +11,8 @@ use crate::db::{ DbPool, }; use crate::{ - campaign::{fetch_campaign_ids_for_channel, update_latest_spendable}, - success_response, Application, ResponseError, RouteParams, + campaign::fetch_campaign_ids_for_channel, success_response, Application, Auth, ResponseError, + RouteParams, }; use adapter::{client::Locked, Adapter}; use futures::future::try_join_all; @@ -398,6 +398,11 @@ pub async fn get_accounting_for_channel( Ok(success_response(serde_json::to_string(&res)?)) } +/// `GET /v5/channel/0xXXX.../pay` request +/// +/// Body: [`ChannelPayRequest`] +/// +/// Response: [`SuccessResponse`] pub async fn channel_payout( req: Request, app: &Application, @@ -408,6 +413,14 @@ pub async fn channel_payout( .expect("Request should have Channel & Chain/TokenInfo") .to_owned(); + let auth = req + .extensions() + .get::() + .ok_or(ResponseError::Unauthorized)? + .to_owned(); + + let spender = auth.uid.to_address(); + let body = hyper::body::to_bytes(req.into_body()).await?; let to_pay = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; @@ -431,54 +444,58 @@ pub async fn channel_payout( // A campaign is closed when its remaining == 0 // therefore for all campaigns for a channel to be closed their total remaining sum should be 0 - if campaigns_remaining_sum.ge(&UnifiedNum::from_u64(0)) { + if campaigns_remaining_sum.gt(&UnifiedNum::from_u64(0)) { return Err(ResponseError::FailedValidation( "All campaigns should be closed or have no budget left".to_string(), )); } + let accounting_spent = get_accounting( + app.pool.clone(), + channel_context.context.id(), + spender, + Side::Spender, + ) + .await? + .map(|accounting_spent| accounting_spent.amount) + .unwrap_or_default(); + let accounting_earned = get_accounting( + app.pool.clone(), + channel_context.context.id(), + spender, + Side::Earner, + ) + .await? + .map(|accounting_spent| accounting_spent.amount) + .unwrap_or_default(); + let latest_spendable = + fetch_spendable(app.pool.clone(), &spender, &channel_context.context.id()) + .await + .map_err(|err| ResponseError::BadRequest(err.to_string()))? + .ok_or(ResponseError::BadRequest("Spendable not found".to_string()))?; + let total_deposited = latest_spendable.deposit.total; + + let available_for_return = total_deposited + .checked_sub(&accounting_spent) + .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? + .checked_add(&accounting_earned) + .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))?; + + let total_to_pay = to_pay + .payouts + .values() + .sum::>() + .ok_or_else(|| ResponseError::FailedValidation("Payouts amount overflow".to_string()))?; + + if total_to_pay.gt(&available_for_return) { + return Err(ResponseError::FailedValidation( + "Total payout amount exceeds the available for return".to_string(), + )); + } + let mut balances: Balances = Balances::new(); - for (address, amount_to_return) in to_pay.payouts { - let accounting_spent = get_accounting( - app.pool.clone(), - channel_context.context.id(), - address, - Side::Spender, - ) - .await? - .map(|accounting_spent| accounting_spent.amount) - .unwrap_or_default(); - let accounting_earned = get_accounting( - app.pool.clone(), - channel_context.context.id(), - address, - Side::Spender, - ) - .await? - .map(|accounting_spent| accounting_spent.amount) - .unwrap_or_default(); - let latest_spendable = - update_latest_spendable(&app.adapter, &app.pool, &channel_context, address) - .await - .map_err(|err| ResponseError::BadRequest(err.to_string()))?; - let total_deposited = latest_spendable.deposit.total; - - let available_for_return = total_deposited - .checked_sub(&accounting_spent) - .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? - .checked_add(&accounting_earned) - .ok_or_else(|| { - ResponseError::FailedValidation("No more budget remaining".to_string()) - })?; - // We need to check if the amount this spender has is available for withdrawal - if available_for_return.lt(&amount_to_return) { - return Err(ResponseError::BadRequest(format!( - "Spender amount for {} is less than the requested withdrawal amount", - address - ))); - } - // If so, we add it to the earner balances for the same address - balances.spend(address, address, available_for_return)?; + for (earner, amount) in to_pay.payouts.into_iter() { + balances.spend(spender, earner, amount)?; } spend_amount(app.pool.clone(), channel_context.context.id(), balances).await?; @@ -631,14 +648,14 @@ pub mod validator_message { #[cfg(test)] mod test { use super::*; - use crate::db::insert_channel; + use crate::db::{insert_campaign, insert_channel}; use crate::test_util::setup_dummy_app; - use adapter::primitives::Deposit; + use crate::CampaignRemaining; use hyper::StatusCode; use primitives::{ test_util::{ADVERTISER, CREATOR, GUARDIAN, PUBLISHER, PUBLISHER_2}, test_util::{DUMMY_CAMPAIGN, IDS}, - BigNum, + BigNum, Deposit, UnifiedMap, ValidatorId, }; #[tokio::test] @@ -941,4 +958,187 @@ mod test { balances.spenders.get(&CREATOR), ); } + + #[tokio::test] + async fn payouts_for_earners_test() { + let app = setup_dummy_app().await; + let channel_context = app + .config + .find_chain_of(DUMMY_CAMPAIGN.channel.token) + .expect("Dummy channel Token should be present in config!") + .with(DUMMY_CAMPAIGN.channel); + + insert_channel(&app.pool, channel_context.context) + .await + .expect("should insert channel"); + insert_campaign(&app.pool, &DUMMY_CAMPAIGN) + .await + .expect("should insert the campaign"); + + // Setting the initial remaining to 0 + let campaign_remaining = CampaignRemaining::new(app.redis.clone()); + campaign_remaining + .set_initial(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(0)) + .await + .expect("Should set value in redis"); + + let auth = Auth { + era: 0, + uid: ValidatorId::from(DUMMY_CAMPAIGN.creator), + chain: channel_context.chain.clone(), + }; + + let build_request = |channel_context: &ChainOf, payouts: UnifiedMap| { + let payouts = ChannelPayRequest { payouts }; + + let body = Body::from(serde_json::to_string(&payouts).expect("Should serialize")); + + Request::builder() + .extension(channel_context.clone()) + .extension(auth.clone()) + .body(body) + .expect("Should build Request") + }; + + // Adding a deposit to the DUMMY adapter + let deposit = Deposit { + total: BigNum::from_str("1000").expect("should convert"), // 1000 DAI + still_on_create2: BigNum::from_str("0").expect("should convert"), // 0 DAI + }; + app.adapter.client.add_deposit_call( + channel_context.context.id(), + auth.uid.to_address(), + deposit.clone(), + ); + + // Add accounting for spender -> 500 + update_accounting( + app.pool.clone(), + channel_context.context.id(), + auth.uid.to_address(), + Side::Spender, + UnifiedNum::from_u64(500), + ) + .await + .expect("should update"); + + // Add spendable for the channel_context where total deposit -> 1000 + let spendable = Spendable { + spender: auth.uid.to_address(), + channel: channel_context.context, + deposit: Deposit { + total: UnifiedNum::from_u64(1000), + still_on_create2: UnifiedNum::from_u64(0), + }, + }; + + // Add accounting for earner -> 100 + update_accounting( + app.pool.clone(), + channel_context.context.id(), + auth.uid.to_address(), + Side::Earner, + UnifiedNum::from_u64(100), + ) + .await + .expect("should update"); + // available for return is now 600 + + // Updating spendable so that we have a value for total_deposited + update_spendable(app.pool.clone(), &spendable) + .await + .expect("Should update spendable"); + + // Test with no body + // TODO: Discuss expected response for that case. + let res = + channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; + assert!(res.is_ok(), "Request with no JSON body and edge cases"); + + // add another deposit to the DUMMY adapter before calling channel_payout + app.adapter.client.add_deposit_call( + channel_context.context.id(), + auth.uid.to_address(), + deposit.clone(), + ); + // make a normal request and get accounting to see if its as expected + + let mut payouts = UnifiedMap::default(); + payouts.insert(*PUBLISHER, UnifiedNum::from_u64(500)); + + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + res.is_ok(), + "Request with JSON body with one address and no errors triggered on purpose" + ); + let accounting = get_accounting( + app.pool.clone(), + channel_context.context.id(), + *PUBLISHER, + Side::Earner, + ) + .await + .expect("should get accounting") + .expect("Should be some"); + assert_eq!( + accounting.amount, + UnifiedNum::from_u64(500), + "Accounting is updated to reflect the publisher's earnings" + ); + + // make a request where total - spent + earned will be a negative balance resulting in an error + update_accounting( + app.pool.clone(), + channel_context.context.id(), + auth.uid.to_address(), + Side::Spender, + UnifiedNum::from_u64(1000), + ) + .await + .expect("should update"); // total spent: 500 + 1000 + // add another deposit to the DUMMY adapter before calling channel_payout + app.adapter.client.add_deposit_call( + channel_context.context.id(), + auth.uid.to_address(), + deposit.clone(), + ); + + let res = + channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; + assert!( + matches!(res, Err(ResponseError::FailedValidation(..))), + "Failed validation because available_for_return is negative" + ); + + // make a request where "total_to_pay" will exceed available + payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); + app.adapter.client.add_deposit_call( + channel_context.context.id(), + auth.uid.to_address(), + deposit.clone(), + ); + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + matches!(res, Err(ResponseError::FailedValidation(..))), + "Failed validation because total_to_pay > available_for_return" + ); + + // make a request where campaigns will have available remaining + campaign_remaining + .increase_by(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(1000)) + .await + .expect("Should set value in redis"); + app.adapter.client.add_deposit_call( + channel_context.context.id(), + auth.uid.to_address(), + deposit.clone(), + ); + + let res = + channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; + assert!( + matches!(res, Err(ResponseError::FailedValidation(..))), + "Failed validation because the campaign has remaining funds" + ); + } } From 2b8b46dc59f805ce3c7450183381b61fc8364f8a Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 12:25:40 +0200 Subject: [PATCH 04/13] requested changes --- sentry/src/db/accounting.rs | 4 ++-- sentry/src/routes/channel.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/sentry/src/db/accounting.rs b/sentry/src/db/accounting.rs index 6ddab1e21..bbbd8c2c4 100644 --- a/sentry/src/db/accounting.rs +++ b/sentry/src/db/accounting.rs @@ -28,7 +28,7 @@ impl From for Error { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Accounting { pub channel_id: ChannelId, pub side: Side, @@ -51,7 +51,7 @@ impl From<&Row> for Accounting { } } -#[derive(Debug, Clone, Copy, ToSql, FromSql, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Copy, ToSql, FromSql, PartialEq, Eq)] #[postgres(name = "accountingside")] pub enum Side { Earner, diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 3d5c9e515..5452dbcfc 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -434,7 +434,7 @@ pub async fn channel_payout( let campaigns_remaining_sum = app .campaign_remaining - .get_multiple(channel_campaigns.as_slice()) + .get_multiple(&channel_campaigns) .await? .iter() .sum::>() @@ -444,7 +444,7 @@ pub async fn channel_payout( // A campaign is closed when its remaining == 0 // therefore for all campaigns for a channel to be closed their total remaining sum should be 0 - if campaigns_remaining_sum.gt(&UnifiedNum::from_u64(0)) { + if campaigns_remaining_sum > UnifiedNum::from_u64(0) { return Err(ResponseError::FailedValidation( "All campaigns should be closed or have no budget left".to_string(), )); @@ -476,18 +476,19 @@ pub async fn channel_payout( let total_deposited = latest_spendable.deposit.total; let available_for_return = total_deposited - .checked_sub(&accounting_spent) - .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? .checked_add(&accounting_earned) + .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? + .checked_sub(&accounting_spent) .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))?; + let total_to_pay = to_pay .payouts .values() .sum::>() .ok_or_else(|| ResponseError::FailedValidation("Payouts amount overflow".to_string()))?; - if total_to_pay.gt(&available_for_return) { + if total_to_pay > available_for_return { return Err(ResponseError::FailedValidation( "Total payout amount exceeds the available for return".to_string(), )); From 369b0728113a477bfa28b3b3373f98b0f9182b68 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 12:30:20 +0200 Subject: [PATCH 05/13] removed unused use --- sentry/src/db/accounting.rs | 1 - sentry/src/routes/channel.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/sentry/src/db/accounting.rs b/sentry/src/db/accounting.rs index bbbd8c2c4..3fdd15bd3 100644 --- a/sentry/src/db/accounting.rs +++ b/sentry/src/db/accounting.rs @@ -9,7 +9,6 @@ use tokio_postgres::{ }; use super::{DbPool, PoolError}; -use serde::Serialize; use thiserror::Error; static UPDATE_ACCOUNTING_STATEMENT: &str = "INSERT INTO accounting(channel_id, side, address, amount, updated, created) VALUES($1, $2, $3, $4, $5, $6) ON CONFLICT ON CONSTRAINT accounting_pkey DO UPDATE SET amount = accounting.amount + $4, updated = $6 WHERE accounting.channel_id = $1 AND accounting.side = $2 AND accounting.address = $3 RETURNING channel_id, side, address, amount, updated, created"; diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 5452dbcfc..a648e266f 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -481,7 +481,6 @@ pub async fn channel_payout( .checked_sub(&accounting_spent) .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))?; - let total_to_pay = to_pay .payouts .values() From 3329f74e58e273841305f63317793189150dd2e2 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 16:50:26 +0200 Subject: [PATCH 06/13] Requested changes + fmt --- adview-manager/serve/src/main.rs | 9 ++++---- sentry/src/db/channel.rs | 2 +- sentry/src/routes.rs | 7 +++--- sentry/src/routes/campaign.rs | 37 +++++++++++++++++--------------- sentry/src/routes/channel.rs | 12 +++++++++-- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/adview-manager/serve/src/main.rs b/adview-manager/serve/src/main.rs index 714503d14..900765e71 100644 --- a/adview-manager/serve/src/main.rs +++ b/adview-manager/serve/src/main.rs @@ -13,8 +13,7 @@ use wiremock::{ Mock, MockServer, ResponseTemplate, }; - -use tera::{Tera, Context}; +use tera::{Context, Tera}; #[tokio::main] async fn main() -> Result<(), Box> { @@ -22,9 +21,11 @@ async fn main() -> Result<(), Box> { let serve_dir = match std::env::current_dir().unwrap() { serve_path if serve_path.ends_with("serve") => serve_path, - adview_manager_path if adview_manager_path.ends_with("adview-manager") => adview_manager_path.join("serve"), + adview_manager_path if adview_manager_path.ends_with("adview-manager") => { + adview_manager_path.join("serve") + } // running from the Validator stack workspace - workspace_path => workspace_path.join("adview-manager/serve") + workspace_path => workspace_path.join("adview-manager/serve"), }; let templates_glob = format!("{}/templates/**/*.html", serve_dir.display()); diff --git a/sentry/src/db/channel.rs b/sentry/src/db/channel.rs index 9a9ec4330..46383a475 100644 --- a/sentry/src/db/channel.rs +++ b/sentry/src/db/channel.rs @@ -1,4 +1,4 @@ -use primitives::{Channel, ChannelId, ValidatorId}; +use primitives::{Channel, ChannelId}; pub use list_channels::list_channels; diff --git a/sentry/src/routes.rs b/sentry/src/routes.rs index 91142c89e..e6b8085dc 100644 --- a/sentry/src/routes.rs +++ b/sentry/src/routes.rs @@ -76,9 +76,9 @@ //! //! Response: [`LastApprovedResponse`][primitives::sentry::LastApprovedResponse] //! -//! - [`POST /v5/channel/:id/pay`](channel::channel_payout) -//! -//! This route handles withdrawals of advertiser funds for a spender. Has authentication of the spender. +//! - [`POST /v5/channel/:id/pay`](channel::channel_payout)(auth required) +//! Channel Payout with authentication of the spender. +//! This route handles withdrawals of advertiser funds for the authenticated spender. //! It needs to ensure all campaigns are closed. It accepts a JSON body in the request which contains //! all of the earners and updates their balances accordingly. Used when an advertiser/spender wants //! to get their remaining funds back. @@ -87,7 +87,6 @@ //! //! Response: [`SuccessResponse`](primitives::sentry::SuccessResponse) //! -//! Channel Payout with authentication of the spender //! //! Withdrawals of advertiser funds - re-introduces the PAY event with a separate route. //! diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 1c70f0189..af3216ba0 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -14,7 +14,6 @@ use crate::{ }; use adapter::{prelude::*, Adapter, Error as AdaptorError}; use deadpool_postgres::PoolError; -use futures::future::try_join_all; use hyper::{Body, Request, Response}; use primitives::{ campaign_validator::Validator, @@ -115,20 +114,18 @@ pub async fn fetch_campaign_ids_for_channel( if total_pages < 2 { Ok(campaign_ids) } else { - let other_pages: Vec> = try_join_all((1..total_pages).map(|i| { - get_campaign_ids_by_channel( - pool, - &channel_id, - limit.into(), - i.checked_mul(limit.into()).expect("TODO"), - ) - })) - .await?; - - let all_campaigns: Vec = std::iter::once(campaign_ids) - .chain(other_pages.into_iter()) - .flat_map(|campaign_ids| campaign_ids.into_iter()) - .collect(); + let mut other_pages: Vec> = Vec::new(); + for i in 1..total_pages { + let skip = i + .checked_mul(limit.into()) + .ok_or(ResponseError::FailedValidation( + "Calculating skip while fetching campaign ids results in an overflow" + .to_string(), + ))?; + let res = get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).await?; + other_pages.push(res); + } + let all_campaigns = other_pages.into_iter().flatten().collect(); Ok(all_campaigns) } @@ -1279,13 +1276,19 @@ mod test { let new_budget = UnifiedNum::from_u64(300 * multiplier); let delta_budget = get_delta_budget(&campaign_remaining, &campaign, new_budget).await; - assert!(matches!(&delta_budget, Err(Error::NewBudget(_))), "Got result: {delta_budget:?}"); + assert!( + matches!(&delta_budget, Err(Error::NewBudget(_))), + "Got result: {delta_budget:?}" + ); // campaign_spent == new_budget let new_budget = UnifiedNum::from_u64(400 * multiplier); let delta_budget = get_delta_budget(&campaign_remaining, &campaign, new_budget).await; - assert!(matches!(&delta_budget, Err(Error::NewBudget(_))), "Got result: {delta_budget:?}"); + assert!( + matches!(&delta_budget, Err(Error::NewBudget(_))), + "Got result: {delta_budget:?}" + ); } // Increasing budget { diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index a648e266f..88d92746d 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -477,9 +477,17 @@ pub async fn channel_payout( let available_for_return = total_deposited .checked_add(&accounting_earned) - .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))? + .ok_or_else(|| { + ResponseError::FailedValidation( + "Overflow while calculating available for return".to_string(), + ) + })? .checked_sub(&accounting_spent) - .ok_or_else(|| ResponseError::FailedValidation("No more budget remaining".to_string()))?; + .ok_or_else(|| { + ResponseError::FailedValidation( + "Underflow while calculating available for return".to_string(), + ) + })?; let total_to_pay = to_pay .payouts From 9b938776d24fdc44e18f4f0992e12662cdd84fa4 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 17:38:25 +0200 Subject: [PATCH 07/13] handle case where request body is empty --- sentry/src/routes/channel.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 88d92746d..c360cb257 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -425,6 +425,13 @@ pub async fn channel_payout( let to_pay = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; + // Handling the case where a request with an empty body comes through + if to_pay.payouts.len() == 0 { + return Err(ResponseError::FailedValidation( + "Request has an empty body".to_string(), + )); + } + let channel_campaigns = fetch_campaign_ids_for_channel( &app.pool, channel_context.context.id(), @@ -1058,11 +1065,12 @@ mod test { .expect("Should update spendable"); // Test with no body - // TODO: Discuss expected response for that case. let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - assert!(res.is_ok(), "Request with no JSON body and edge cases"); - + assert!( + matches!(res, Err(ResponseError::FailedValidation(..))), + "Failed validation because request body is empty" + ); // add another deposit to the DUMMY adapter before calling channel_payout app.adapter.client.add_deposit_call( channel_context.context.id(), From d9714c5172f496ba29435804be5d566a04f8ecc1 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 15 Mar 2022 17:47:45 +0200 Subject: [PATCH 08/13] fixed docs formatting --- sentry/src/routes.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry/src/routes.rs b/sentry/src/routes.rs index e6b8085dc..4c3f99461 100644 --- a/sentry/src/routes.rs +++ b/sentry/src/routes.rs @@ -77,7 +77,9 @@ //! Response: [`LastApprovedResponse`][primitives::sentry::LastApprovedResponse] //! //! - [`POST /v5/channel/:id/pay`](channel::channel_payout)(auth required) +//! //! Channel Payout with authentication of the spender. +//! //! This route handles withdrawals of advertiser funds for the authenticated spender. //! It needs to ensure all campaigns are closed. It accepts a JSON body in the request which contains //! all of the earners and updates their balances accordingly. Used when an advertiser/spender wants From 4f63ba984a04da6b390cc1423bbeaa2b1323acac Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Wed, 16 Mar 2022 19:15:16 +0200 Subject: [PATCH 09/13] removes unnecessary deposits to the dummy adapter --- sentry/src/routes/channel.rs | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index c360cb257..9e9dd8dd1 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -1015,17 +1015,6 @@ mod test { .expect("Should build Request") }; - // Adding a deposit to the DUMMY adapter - let deposit = Deposit { - total: BigNum::from_str("1000").expect("should convert"), // 1000 DAI - still_on_create2: BigNum::from_str("0").expect("should convert"), // 0 DAI - }; - app.adapter.client.add_deposit_call( - channel_context.context.id(), - auth.uid.to_address(), - deposit.clone(), - ); - // Add accounting for spender -> 500 update_accounting( app.pool.clone(), @@ -1071,12 +1060,6 @@ mod test { matches!(res, Err(ResponseError::FailedValidation(..))), "Failed validation because request body is empty" ); - // add another deposit to the DUMMY adapter before calling channel_payout - app.adapter.client.add_deposit_call( - channel_context.context.id(), - auth.uid.to_address(), - deposit.clone(), - ); // make a normal request and get accounting to see if its as expected let mut payouts = UnifiedMap::default(); @@ -1112,12 +1095,6 @@ mod test { ) .await .expect("should update"); // total spent: 500 + 1000 - // add another deposit to the DUMMY adapter before calling channel_payout - app.adapter.client.add_deposit_call( - channel_context.context.id(), - auth.uid.to_address(), - deposit.clone(), - ); let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; @@ -1128,11 +1105,7 @@ mod test { // make a request where "total_to_pay" will exceed available payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); - app.adapter.client.add_deposit_call( - channel_context.context.id(), - auth.uid.to_address(), - deposit.clone(), - ); + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; assert!( matches!(res, Err(ResponseError::FailedValidation(..))), @@ -1144,11 +1117,6 @@ mod test { .increase_by(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(1000)) .await .expect("Should set value in redis"); - app.adapter.client.add_deposit_call( - channel_context.context.id(), - auth.uid.to_address(), - deposit.clone(), - ); let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; From 7ce4e80226e93fd0c6f0d36c9505421e059edc89 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 18 Mar 2022 18:07:02 +0200 Subject: [PATCH 10/13] requested changes with broken code in fetch_campaign_ids_for_channel --- sentry/src/routes.rs | 4 +- sentry/src/routes/campaign.rs | 40 +++++++++---- sentry/src/routes/channel.rs | 108 ++++++++++++++++++++++++---------- 3 files changed, 104 insertions(+), 48 deletions(-) diff --git a/sentry/src/routes.rs b/sentry/src/routes.rs index 4c3f99461..0f0bc1999 100644 --- a/sentry/src/routes.rs +++ b/sentry/src/routes.rs @@ -76,7 +76,7 @@ //! //! Response: [`LastApprovedResponse`][primitives::sentry::LastApprovedResponse] //! -//! - [`POST /v5/channel/:id/pay`](channel::channel_payout)(auth required) +//! - [`POST /v5/channel/:id/pay`](channel::channel_payout) (auth required) //! //! Channel Payout with authentication of the spender. //! @@ -90,8 +90,6 @@ //! Response: [`SuccessResponse`](primitives::sentry::SuccessResponse) //! //! -//! Withdrawals of advertiser funds - re-introduces the PAY event with a separate route. -//! //! - `GET /v5/channel/:id/get-leaf` //! //! TODO: implement and document as part of issue #382 diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index af3216ba0..b051c7f85 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -28,6 +28,7 @@ use slog::error; use std::cmp::{max, Ordering}; use thiserror::Error; use tokio_postgres::error::SqlState; +use futures::{future::try_join_all}; #[derive(Debug, Error)] pub enum Error { @@ -114,19 +115,32 @@ pub async fn fetch_campaign_ids_for_channel( if total_pages < 2 { Ok(campaign_ids) } else { - let mut other_pages: Vec> = Vec::new(); - for i in 1..total_pages { - let skip = i - .checked_mul(limit.into()) - .ok_or(ResponseError::FailedValidation( - "Calculating skip while fetching campaign ids results in an overflow" - .to_string(), - ))?; - let res = get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).await?; - other_pages.push(res); - } - let all_campaigns = other_pages.into_iter().flatten().collect(); - + // let mut other_pages: Vec> = Vec::new(); + // for i in 1..total_pages { + // let skip = i + // .checked_mul(limit.into()) + // .ok_or(ResponseError::FailedValidation( + // "Calculating skip while fetching campaign ids results in an overflow" + // .to_string(), + // ))?; + // let res = get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).await?; + // other_pages.push(res); + // } + // let all_campaigns = other_pages.into_iter().flatten().collect(); + + let other_pages: Vec> = try_join_all((1..total_pages).map(|i| { + let skip = match i.checked_mul(limit.into()) { + Some(skip) => skip, + None => return Err(ResponseError::FailedValidation("Calculating skip while fetching campaign ids results in an overflow".to_string())) + }; + get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip) + })) + .await?; + + let all_campaigns: Vec = std::iter::once(campaign_ids) + .chain(other_pages.into_iter()) + .flat_map(|campaign_ids| campaign_ids.into_iter()) + .collect(); Ok(all_campaigns) } } diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 9e9dd8dd1..476442780 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -398,7 +398,7 @@ pub async fn get_accounting_for_channel( Ok(success_response(serde_json::to_string(&res)?)) } -/// `GET /v5/channel/0xXXX.../pay` request +/// `POST /v5/channel/0xXXX.../pay` request /// /// Body: [`ChannelPayRequest`] /// @@ -428,7 +428,7 @@ pub async fn channel_payout( // Handling the case where a request with an empty body comes through if to_pay.payouts.len() == 0 { return Err(ResponseError::FailedValidation( - "Request has an empty body".to_string(), + "Request has empty payouts".to_string(), )); } @@ -479,20 +479,20 @@ pub async fn channel_payout( fetch_spendable(app.pool.clone(), &spender, &channel_context.context.id()) .await .map_err(|err| ResponseError::BadRequest(err.to_string()))? - .ok_or(ResponseError::BadRequest("Spendable not found".to_string()))?; + .ok_or(ResponseError::BadRequest("There is no spendable amount for the spender in this Channel.".to_string()))?; let total_deposited = latest_spendable.deposit.total; - let available_for_return = total_deposited + let available_for_payout = total_deposited .checked_add(&accounting_earned) .ok_or_else(|| { ResponseError::FailedValidation( - "Overflow while calculating available for return".to_string(), + "Overflow while calculating available for payout".to_string(), ) })? .checked_sub(&accounting_spent) .ok_or_else(|| { ResponseError::FailedValidation( - "Underflow while calculating available for return".to_string(), + "Underflow while calculating available for payout".to_string(), ) })?; @@ -502,9 +502,9 @@ pub async fn channel_payout( .sum::>() .ok_or_else(|| ResponseError::FailedValidation("Payouts amount overflow".to_string()))?; - if total_to_pay > available_for_return { + if total_to_pay > available_for_payout { return Err(ResponseError::FailedValidation( - "Total payout amount exceeds the available for return".to_string(), + "The total requested payout amount exceeds the available payout".to_string(), )); } @@ -1015,6 +1015,17 @@ mod test { .expect("Should build Request") }; + // TODO: Add Body for this test + // Testing before Accounting/Spendable are inserted + // let res = + // channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; + // assert!( + // res.is_err(), + // "Should return an error when there is no Accounting/Spendable" + // ); + // let err = res.unwrap_err(); + // assert_eq!(err, ResponseError::FailedValidation("Request has empty payouts".to_string()), "Failed validation because payouts are empty"); + // Add accounting for spender -> 500 update_accounting( app.pool.clone(), @@ -1057,34 +1068,62 @@ mod test { let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; assert!( - matches!(res, Err(ResponseError::FailedValidation(..))), - "Failed validation because request body is empty" + res.is_err(), + "Should return an error when payouts are empty" ); + let err = res.unwrap_err(); + assert_eq!(err, ResponseError::FailedValidation("Request has empty payouts".to_string()), "Failed validation because payouts are empty"); // make a normal request and get accounting to see if its as expected let mut payouts = UnifiedMap::default(); payouts.insert(*PUBLISHER, UnifiedNum::from_u64(500)); let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!(res.is_ok(), "This request shouldn't result in an error"); + + let json = hyper::body::to_bytes(res.unwrap().into_body()) + .await + .expect("Should get json"); + + let res: SuccessResponse = serde_json::from_slice(&json).expect("Should get SuccessResponse"); assert!( - res.is_ok(), + matches!(res, + SuccessResponse { success: true }), "Request with JSON body with one address and no errors triggered on purpose" ); - let accounting = get_accounting( - app.pool.clone(), - channel_context.context.id(), - *PUBLISHER, - Side::Earner, - ) - .await - .expect("should get accounting") - .expect("Should be some"); - assert_eq!( - accounting.amount, - UnifiedNum::from_u64(500), - "Accounting is updated to reflect the publisher's earnings" - ); + // Checking if Earner/Spender balances have been updated by looking up the Accounting in the database + { + let accounting = get_accounting( + app.pool.clone(), + channel_context.context.id(), + *PUBLISHER, + Side::Earner, + ) + .await + .expect("should get accounting") + .expect("Should be some"); + assert_eq!( + accounting.amount, + UnifiedNum::from_u64(500), + "Accounting is updated to reflect the publisher's earnings" + ); + + let accounting = get_accounting( + app.pool.clone(), + channel_context.context.id(), + auth.uid.to_address(), + Side::Spender, + ) + .await + .expect("should get accounting") + .expect("Should be some"); + assert_eq!( + accounting.amount, + UnifiedNum::from_u64(500), + "Accounting is updated to reflect the amount deducted from the spender" + ); + } // make a request where total - spent + earned will be a negative balance resulting in an error update_accounting( app.pool.clone(), @@ -1099,19 +1138,22 @@ mod test { let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; assert!( - matches!(res, Err(ResponseError::FailedValidation(..))), - "Failed validation because available_for_return is negative" + res.is_err(), + "Should return err when available_for_payout is negative" ); + let err = res.unwrap_err(); + assert_eq!(err, ResponseError::FailedValidation("a".to_string()), "Failed validation because available_for_payout is negative"); // make a request where "total_to_pay" will exceed available payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; assert!( - matches!(res, Err(ResponseError::FailedValidation(..))), - "Failed validation because total_to_pay > available_for_return" + res.is_err(), + "Should return an error when total_to_pay > available_for_payout" ); - + let err = res.unwrap_err(); + assert_eq!(err, ResponseError::FailedValidation("b".to_string()), "Failed validation because total_to_pay > available_for_payout"); // make a request where campaigns will have available remaining campaign_remaining .increase_by(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(1000)) @@ -1121,8 +1163,10 @@ mod test { let res = channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; assert!( - matches!(res, Err(ResponseError::FailedValidation(..))), - "Failed validation because the campaign has remaining funds" + res.is_err(), + "Should return an error when a campaign has remaining funds" ); + let err = res.unwrap_err(); + assert_eq!(err, ResponseError::FailedValidation("c".to_string()), "Failed validation because the campaign has remaining funds"); } } From 2452cb4fab8ac57ff96f03e7b5d93e65f09f0918 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Fri, 18 Mar 2022 18:25:47 +0200 Subject: [PATCH 11/13] sentry - routes - fix future-closure issue --- sentry/src/routes/campaign.rs | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index b051c7f85..3f683559d 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -28,7 +28,7 @@ use slog::error; use std::cmp::{max, Ordering}; use thiserror::Error; use tokio_postgres::error::SqlState; -use futures::{future::try_join_all}; +use futures::{future::try_join_all, TryFutureExt}; #[derive(Debug, Error)] pub enum Error { @@ -115,32 +115,23 @@ pub async fn fetch_campaign_ids_for_channel( if total_pages < 2 { Ok(campaign_ids) } else { - // let mut other_pages: Vec> = Vec::new(); - // for i in 1..total_pages { - // let skip = i - // .checked_mul(limit.into()) - // .ok_or(ResponseError::FailedValidation( - // "Calculating skip while fetching campaign ids results in an overflow" - // .to_string(), - // ))?; - // let res = get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).await?; - // other_pages.push(res); - // } - // let all_campaigns = other_pages.into_iter().flatten().collect(); - - let other_pages: Vec> = try_join_all((1..total_pages).map(|i| { - let skip = match i.checked_mul(limit.into()) { - Some(skip) => skip, - None => return Err(ResponseError::FailedValidation("Calculating skip while fetching campaign ids results in an overflow".to_string())) - }; - get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip) + let pages_skip: Vec = (1..total_pages).map(|i| { + i.checked_mul(limit.into()).ok_or_else(|| { + ResponseError::FailedValidation("Calculating skip while fetching campaign ids results in an overflow".to_string()) + }) + }).collect::>()?; + + + let other_pages = try_join_all(pages_skip.into_iter().map(|skip| { + get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).map_err(|e| ResponseError::BadRequest(e.to_string())) })) .await?; - let all_campaigns: Vec = std::iter::once(campaign_ids) + let all_campaigns = std::iter::once(campaign_ids) .chain(other_pages.into_iter()) .flat_map(|campaign_ids| campaign_ids.into_iter()) .collect(); + Ok(all_campaigns) } } From 8025da6c092275a973410e75167b37a0dfb09438 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Mon, 21 Mar 2022 14:52:52 +0200 Subject: [PATCH 12/13] fixes on tests --- sentry/src/routes/campaign.rs | 19 ++-- sentry/src/routes/channel.rs | 185 ++++++++++++++++++++-------------- 2 files changed, 124 insertions(+), 80 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 3f683559d..925a3b2c3 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -14,6 +14,7 @@ use crate::{ }; use adapter::{prelude::*, Adapter, Error as AdaptorError}; use deadpool_postgres::PoolError; +use futures::{future::try_join_all, TryFutureExt}; use hyper::{Body, Request, Response}; use primitives::{ campaign_validator::Validator, @@ -28,7 +29,6 @@ use slog::error; use std::cmp::{max, Ordering}; use thiserror::Error; use tokio_postgres::error::SqlState; -use futures::{future::try_join_all, TryFutureExt}; #[derive(Debug, Error)] pub enum Error { @@ -115,15 +115,20 @@ pub async fn fetch_campaign_ids_for_channel( if total_pages < 2 { Ok(campaign_ids) } else { - let pages_skip: Vec = (1..total_pages).map(|i| { - i.checked_mul(limit.into()).ok_or_else(|| { - ResponseError::FailedValidation("Calculating skip while fetching campaign ids results in an overflow".to_string()) + let pages_skip: Vec = (1..total_pages) + .map(|i| { + i.checked_mul(limit.into()).ok_or_else(|| { + ResponseError::FailedValidation( + "Calculating skip while fetching campaign ids results in an overflow" + .to_string(), + ) + }) }) - }).collect::>()?; - + .collect::>()?; let other_pages = try_join_all(pages_skip.into_iter().map(|skip| { - get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip).map_err(|e| ResponseError::BadRequest(e.to_string())) + get_campaign_ids_by_channel(pool, &channel_id, limit.into(), skip) + .map_err(|e| ResponseError::BadRequest(e.to_string())) })) .await?; diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 476442780..abe37956a 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -479,7 +479,9 @@ pub async fn channel_payout( fetch_spendable(app.pool.clone(), &spender, &channel_context.context.id()) .await .map_err(|err| ResponseError::BadRequest(err.to_string()))? - .ok_or(ResponseError::BadRequest("There is no spendable amount for the spender in this Channel.".to_string()))?; + .ok_or(ResponseError::BadRequest( + "There is no spendable amount for the spender in this Channel".to_string(), + ))?; let total_deposited = latest_spendable.deposit.total; let available_for_payout = total_deposited @@ -1014,17 +1016,25 @@ mod test { .body(body) .expect("Should build Request") }; + let mut payouts = UnifiedMap::default(); + payouts.insert(*PUBLISHER, UnifiedNum::from_u64(500)); - // TODO: Add Body for this test // Testing before Accounting/Spendable are inserted - // let res = - // channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - // assert!( - // res.is_err(), - // "Should return an error when there is no Accounting/Spendable" - // ); - // let err = res.unwrap_err(); - // assert_eq!(err, ResponseError::FailedValidation("Request has empty payouts".to_string()), "Failed validation because payouts are empty"); + { + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + res.is_err(), + "Should return an error when there is no Accounting/Spendable" + ); + let err = res.unwrap_err(); + assert_eq!( + err, + ResponseError::BadRequest( + "There is no spendable amount for the spender in this Channel".to_string() + ), + "Failed validation because payouts are empty" + ); + } // Add accounting for spender -> 500 update_accounting( @@ -1065,32 +1075,37 @@ mod test { .expect("Should update spendable"); // Test with no body - let res = - channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - assert!( - res.is_err(), - "Should return an error when payouts are empty" - ); - let err = res.unwrap_err(); - assert_eq!(err, ResponseError::FailedValidation("Request has empty payouts".to_string()), "Failed validation because payouts are empty"); - // make a normal request and get accounting to see if its as expected - - let mut payouts = UnifiedMap::default(); - payouts.insert(*PUBLISHER, UnifiedNum::from_u64(500)); + { + let res = + channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; + assert!( + res.is_err(), + "Should return an error when payouts are empty" + ); + let err = res.unwrap_err(); + assert_eq!( + err, + ResponseError::FailedValidation("Request has empty payouts".to_string()), + "Failed validation because payouts are empty" + ); + } - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!(res.is_ok(), "This request shouldn't result in an error"); + // make a normal request and get accounting to see if its as expected + { + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!(res.is_ok(), "This request shouldn't result in an error"); - let json = hyper::body::to_bytes(res.unwrap().into_body()) - .await - .expect("Should get json"); + let json = hyper::body::to_bytes(res.unwrap().into_body()) + .await + .expect("Should get json"); - let res: SuccessResponse = serde_json::from_slice(&json).expect("Should get SuccessResponse"); - assert!( - matches!(res, - SuccessResponse { success: true }), - "Request with JSON body with one address and no errors triggered on purpose" - ); + let res: SuccessResponse = + serde_json::from_slice(&json).expect("Should get SuccessResponse"); + assert!( + matches!(res, SuccessResponse { success: true }), + "Request with JSON body with one address and no errors triggered on purpose" + ); + } // Checking if Earner/Spender balances have been updated by looking up the Accounting in the database { @@ -1120,53 +1135,77 @@ mod test { .expect("Should be some"); assert_eq!( accounting.amount, - UnifiedNum::from_u64(500), + UnifiedNum::from_u64(1000), // 500 initial + 500 for earners "Accounting is updated to reflect the amount deducted from the spender" ); } - // make a request where total - spent + earned will be a negative balance resulting in an error - update_accounting( - app.pool.clone(), - channel_context.context.id(), - auth.uid.to_address(), - Side::Spender, - UnifiedNum::from_u64(1000), - ) - .await - .expect("should update"); // total spent: 500 + 1000 - - let res = - channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - assert!( - res.is_err(), - "Should return err when available_for_payout is negative" - ); - let err = res.unwrap_err(); - assert_eq!(err, ResponseError::FailedValidation("a".to_string()), "Failed validation because available_for_payout is negative"); // make a request where "total_to_pay" will exceed available - payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); + { + payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!( - res.is_err(), - "Should return an error when total_to_pay > available_for_payout" - ); - let err = res.unwrap_err(); - assert_eq!(err, ResponseError::FailedValidation("b".to_string()), "Failed validation because total_to_pay > available_for_payout"); - // make a request where campaigns will have available remaining - campaign_remaining - .increase_by(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(1000)) + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + res.is_err(), + "Should return an error when total_to_pay > available_for_payout" + ); + let err = res.unwrap_err(); + assert_eq!( + err, + ResponseError::FailedValidation( + "The total requested payout amount exceeds the available payout".to_string() + ), + "Failed validation because total_to_pay > available_for_payout" + ); + } + + // make a request where total - spent + earned will be a negative balance resulting in an error + { + update_accounting( + app.pool.clone(), + channel_context.context.id(), + auth.uid.to_address(), + Side::Spender, + UnifiedNum::from_u64(1000), + ) .await - .expect("Should set value in redis"); + .expect("should update"); // total spent: 500 + 1000 - let res = - channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - assert!( - res.is_err(), - "Should return an error when a campaign has remaining funds" - ); - let err = res.unwrap_err(); - assert_eq!(err, ResponseError::FailedValidation("c".to_string()), "Failed validation because the campaign has remaining funds"); + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + res.is_err(), + "Should return err when available_for_payout is negative" + ); + let err = res.unwrap_err(); + assert_eq!( + err, + ResponseError::FailedValidation( + "Underflow while calculating available for payout".to_string() + ), + "Failed validation because available_for_payout is negative" + ); + } + + // make a request where campaigns will have available remaining + { + campaign_remaining + .increase_by(DUMMY_CAMPAIGN.id, UnifiedNum::from_u64(1000)) + .await + .expect("Should set value in redis"); + + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; + assert!( + res.is_err(), + "Should return an error when a campaign has remaining funds" + ); + let err = res.unwrap_err(); + assert_eq!( + err, + ResponseError::FailedValidation( + "All campaigns should be closed or have no budget left".to_string() + ), + "Failed validation because the campaign has remaining funds" + ); + } } } From d6fdfe9fea058a9916f2d118c91f7b210bb852d6 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Wed, 23 Mar 2022 13:53:17 +0200 Subject: [PATCH 13/13] sentry - routes -channel - clean up tests --- sentry/src/routes/channel.rs | 97 ++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 53 deletions(-) diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index abe37956a..f40d4c299 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -1021,14 +1021,12 @@ mod test { // Testing before Accounting/Spendable are inserted { - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!( - res.is_err(), - "Should return an error when there is no Accounting/Spendable" - ); - let err = res.unwrap_err(); + let err_response = + channel_payout(build_request(&channel_context, payouts.clone()), &app) + .await + .expect_err("Should return an error when there is no Accounting/Spendable"); assert_eq!( - err, + err_response, ResponseError::BadRequest( "There is no spendable amount for the spender in this Channel".to_string() ), @@ -1036,7 +1034,7 @@ mod test { ); } - // Add accounting for spender -> 500 + // Add accounting for spender = 500 update_accounting( app.pool.clone(), channel_context.context.id(), @@ -1047,7 +1045,7 @@ mod test { .await .expect("should update"); - // Add spendable for the channel_context where total deposit -> 1000 + // Add spendable for the channel_context where total deposit = 1000 let spendable = Spendable { spender: auth.uid.to_address(), channel: channel_context.context, @@ -1057,7 +1055,8 @@ mod test { }, }; - // Add accounting for earner -> 100 + // Add accounting for earner = 100 + // available for return will be = 600 update_accounting( app.pool.clone(), channel_context.context.id(), @@ -1067,7 +1066,6 @@ mod test { ) .await .expect("should update"); - // available for return is now 600 // Updating spendable so that we have a value for total_deposited update_spendable(app.pool.clone(), &spendable) @@ -1076,15 +1074,12 @@ mod test { // Test with no body { - let res = - channel_payout(build_request(&channel_context, UnifiedMap::default()), &app).await; - assert!( - res.is_err(), - "Should return an error when payouts are empty" - ); - let err = res.unwrap_err(); + let err_response = + channel_payout(build_request(&channel_context, UnifiedMap::default()), &app) + .await + .expect_err("Should return an error when payouts are empty"); assert_eq!( - err, + err_response, ResponseError::FailedValidation("Request has empty payouts".to_string()), "Failed validation because payouts are empty" ); @@ -1092,24 +1087,26 @@ mod test { // make a normal request and get accounting to see if its as expected { - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!(res.is_ok(), "This request shouldn't result in an error"); + let res = channel_payout(build_request(&channel_context, payouts.clone()), &app) + .await + .expect("This request shouldn't result in an error"); - let json = hyper::body::to_bytes(res.unwrap().into_body()) + let json = hyper::body::to_bytes(res.into_body()) .await .expect("Should get json"); - let res: SuccessResponse = - serde_json::from_slice(&json).expect("Should get SuccessResponse"); - assert!( - matches!(res, SuccessResponse { success: true }), + let success_response: SuccessResponse = + serde_json::from_slice(&json).expect("Should deserialize SuccessResponse"); + assert_eq!( + SuccessResponse { success: true }, + success_response, "Request with JSON body with one address and no errors triggered on purpose" ); } // Checking if Earner/Spender balances have been updated by looking up the Accounting in the database { - let accounting = get_accounting( + let earner_accounting = get_accounting( app.pool.clone(), channel_context.context.id(), *PUBLISHER, @@ -1117,14 +1114,14 @@ mod test { ) .await .expect("should get accounting") - .expect("Should be some"); + .expect("Should have value, i.e. Some"); assert_eq!( - accounting.amount, + earner_accounting.amount, UnifiedNum::from_u64(500), "Accounting is updated to reflect the publisher's earnings" ); - let accounting = get_accounting( + let spender_accounting = get_accounting( app.pool.clone(), channel_context.context.id(), auth.uid.to_address(), @@ -1132,9 +1129,9 @@ mod test { ) .await .expect("should get accounting") - .expect("Should be some"); + .expect("Should have value, i.e. Some"); assert_eq!( - accounting.amount, + spender_accounting.amount, UnifiedNum::from_u64(1000), // 500 initial + 500 for earners "Accounting is updated to reflect the amount deducted from the spender" ); @@ -1144,17 +1141,15 @@ mod test { { payouts.insert(*CREATOR, UnifiedNum::from_u64(1000)); - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!( - res.is_err(), - "Should return an error when total_to_pay > available_for_payout" - ); - let err = res.unwrap_err(); + let response_error = + channel_payout(build_request(&channel_context, payouts.clone()), &app) + .await + .expect_err("Should return an error when total_to_pay > available_for_payout"); assert_eq!( - err, ResponseError::FailedValidation( "The total requested payout amount exceeds the available payout".to_string() ), + response_error, "Failed validation because total_to_pay > available_for_payout" ); } @@ -1171,17 +1166,15 @@ mod test { .await .expect("should update"); // total spent: 500 + 1000 - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!( - res.is_err(), - "Should return err when available_for_payout is negative" - ); - let err = res.unwrap_err(); + let response_error = + channel_payout(build_request(&channel_context, payouts.clone()), &app) + .await + .expect_err("Should return err when available_for_payout is negative"); assert_eq!( - err, ResponseError::FailedValidation( "Underflow while calculating available for payout".to_string() ), + response_error, "Failed validation because available_for_payout is negative" ); } @@ -1193,17 +1186,15 @@ mod test { .await .expect("Should set value in redis"); - let res = channel_payout(build_request(&channel_context, payouts.clone()), &app).await; - assert!( - res.is_err(), - "Should return an error when a campaign has remaining funds" - ); - let err = res.unwrap_err(); + let response_error = + channel_payout(build_request(&channel_context, payouts.clone()), &app) + .await + .expect_err("Should return an error when a campaign has remaining funds"); assert_eq!( - err, ResponseError::FailedValidation( "All campaigns should be closed or have no budget left".to_string() ), + response_error, "Failed validation because the campaign has remaining funds" ); }