From 3661ca225a11de77d259ac95d16fe914d71d84ec Mon Sep 17 00:00:00 2001 From: simzzz Date: Fri, 28 May 2021 18:36:05 +0300 Subject: [PATCH 01/31] committing current progress --- sentry/src/db/campaign.rs | 50 +++++++++++++ sentry/src/lib.rs | 11 +++ sentry/src/routes/campaign.rs | 137 +++++++++++++++++++++++++++++----- 3 files changed, 178 insertions(+), 20 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index c3660d68e..edab437a4 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -49,6 +49,51 @@ pub async fn fetch_campaign(pool: DbPool, campaign: &Campaign) -> Result Result, PoolError> { + let client = pool.get().await?; + let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE channel_id = $1").await?; + + let row = client.query(&statement, &[&campaign.channel.id()]).await?; + + let campaigns = row.into_iter().for_each(|c| Campaign::from(c)).collect(); + Ok(campaigns); +} + +pub async fn campaign_exists(pool: &DbPool, campaign: &Campaign) -> Result { + let client = pool.get().await?; + let statement = client + .prepare("SELECT EXISTS(SELECT 1 FROM campaigns WHERE id = $1)") + .await?; + + let row = client.execute(&statement, &[&campaign.id]).await?; + + let exists = row == 1; + Ok(exists) +} + +pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { + let client = pool.get().await?; + let statement = client + .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") + .await?; + + let row = client + .execute(&statement, &[ + &campaign.budget, + &campaign.validators, + &campaign.title, + &campaign.pricing_bounds, + &campaign.event_submission, + &campaign.ad_units, + &campaign.targeting_rules, + &campaign.id, + ]) + .await?; + + let exists = row == 1; + Ok(exists) +} + #[cfg(test)] mod test { use primitives::{ @@ -77,6 +122,11 @@ mod test { assert!(is_inserted); + let exists = campaign_exists(&db_pool.clone(), campaign: &campaign_for_testing) + .await + .expect("Should succeed"); + asser!(exists); + let fetched_campaign: Campaign = fetch_campaign(db_pool.clone(), &campaign_for_testing) .await .expect("Should fetch successfully"); diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 403df405a..455d8833c 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -21,6 +21,7 @@ use primitives::{Config, ValidatorId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; +use routes::campaign::create_campaign; use routes::cfg::config; use routes::channel::{ channel_list, channel_validate, create_channel, create_validator_messages, insert_events, @@ -150,6 +151,16 @@ impl Application { publisher_analytics(req, &self).await } + ("/campaign.create", &Method::POST) => { + let req = match AuthRequired.call(req, &self).await { + Ok(req) => req, + Err(error) => { + return map_response_error(error); + } + }; + + create_campaign(req, &self).await + } (route, _) if route.starts_with("/analytics") => analytics_router(req, &self).await, // This is important becuase it prevents us from doing // expensive regex matching for routes without /channel diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 94076e34b..58747bfad 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,7 +1,21 @@ -use crate::{success_response, Application, Auth, ResponseError, RouteParams, Session}; +use crate::{ + success_response, Application, Auth, ResponseError, RouteParams, Session, + event_aggregate::latest_new_state, + db::{ + spendable::fetch_spendable, + campaign::{campaign_exists, update_campaign, insert_campaign, get_campaigns_for_channel}, + } +}; use hyper::{Body, Request, Response}; -use primitives::{adapter::Adapter, sentry::{ - campaign_create::CreateCampaign,SuccessResponse}}; +use primitives::{ + adapter::Adapter, + sentry::{ + campaign_create::CreateCampaign, + SuccessResponse + }, + Campaign +}; +use redis::aio::MultiplexedConnection; pub async fn create_campaign( req: Request, @@ -20,25 +34,108 @@ pub async fn create_campaign( let error_response = ResponseError::BadRequest("err occurred; please try again later".into()); // insert Campaign - - // match insert_campaign(&app.pool, &campaign).await { - // Err(error) => { - // error!(&app.logger, "{}", &error; "module" => "create_channel"); - - // match error { - // PoolError::Backend(error) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) => { - // Err(ResponseError::Conflict( - // "channel already exists".to_string(), - // )) - // } - // _ => Err(error_response), - // } - // } - // Ok(false) => Err(error_response), - // _ => Ok(()), - // }?; + + match insert_or_modify_campaign(&app.pool, &campaign, &app.redis).await { + Err(error) => { + error!(&app.logger, "{}", &error; "module" => "create_channel"); + + match error { + PoolError::Backend(error) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) => { + Err(ResponseError::Conflict( + "channel already exists".to_string(), + )) + } + _ => Err(error_response), + } + } + Ok(false) => Err(error_response), + _ => Ok(()), + }?; let create_response = SuccessResponse { success: true }; Ok(success_response(serde_json::to_string(&campaign)?)) } + +// TODO: Double check redis calls +async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { + let key = format!("adexCampaign:campaignSpent:{}", id) + // campaignSpent tracks the portion of the budget which has already been spent + let campaign_spent = match redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await? + { + Some(spent) => UnifiedNum::from(spent), + // TODO: Double check if this is true + // If the campaign is just being inserted, there would be no entry therefore no funds would be spent + None => 0 + }; +} + +async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + // update a key in Redis for the remaining spendable amount + let key = format!("adexCampaign:remainingSpendable:{}", id) + redis::cmd("SET") + .arg(&key) + .arg(amount) + .query_async(&mut redis.clone()) + .await? +} + +async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { + let key = format!("adexChannel:remaining:{}", id) + redis::cmd("SET") + .arg(&key) + .arg(amount) + .query_async(&mut redis.clone()) + .await? +} + +async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaign: &Campaign) -> Result { + let campaigns_for_channel = get_campaigns_for_channel(&campaign).await?; + let sum_of_campaigns_remaining = campaigns_for_channel + .map(|c| { + let spent = get_spent_for_campaign(&redis, c.id).await?; + let remaining = c.budget - spent; + remaining + }) + .sum(); +} + +pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { + let campaign_spent = get_spent_for_campaign(&redis, campaign.id()).await?; + + // Check if we haven't exceeded the budget yet + if (campaign.budget <= campaign_spent) { + ResponseError::FailedValidation("No more budget available for spending") + } + + let remaining_spendable_campaign = campaign.budget - campaign_spent; + update_remaining_for_campaign(&redis, campaign.id, remaining_spendable_campaign).await?; + + + // Getting the latest new state from Postgres + let latest_new_state = latest_new_state(&pool, &campaign.channel, "").await?; + // Gets the latest Spendable for this (spender, channelId) pair + let latest_spendable = fetch_spendable(pool, campaign.creator, campaign.channel.id()).await?; + + let total_deposited = latest_spendable.deposit.total; + let total_spent = latest_new_state.spenders[campaign.creator]; + let total_remaining = total_deposited - total_spent; + + update_remaining_for_channel(&redis, campaign.channel.id(), total_remaining).await?; + + if (campaign_exists(&pool, &campaign)) { + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaign).await?; + if campaigns_remaining_sum > total_remaining { + ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel") + } + update_campaign(&pool, &campaign).await? + } + insert_campaign(&pool, &campaign).await? + + // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent + // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] + // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 +} From 8c4d68721ac8da55cd1295d71d80b7c6d3a99594 Mon Sep 17 00:00:00 2001 From: simzzz Date: Mon, 31 May 2021 12:32:04 +0300 Subject: [PATCH 02/31] fixed some more errors --- sentry/src/db.rs | 2 +- sentry/src/db/campaign.rs | 2 +- sentry/src/lib.rs | 1 + sentry/src/routes/campaign.rs | 36 ++++++++++++++++------------------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/sentry/src/db.rs b/sentry/src/db.rs index 5fb148aeb..12d631afa 100644 --- a/sentry/src/db.rs +++ b/sentry/src/db.rs @@ -6,7 +6,7 @@ use tokio_postgres::NoTls; use lazy_static::lazy_static; pub mod analytics; -mod campaign; +pub mod campaign; mod channel; pub mod event_aggregate; pub mod spendable; diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 4002bb7bc..0df31ba16 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -56,7 +56,7 @@ pub async fn get_campaigns_for_channel(pool: DbPool, campaign: &Campaign) -> Res let row = client.query(&statement, &[&campaign.channel.id()]).await?; let campaigns = row.into_iter().for_each(|c| Campaign::from(c)).collect(); - Ok(campaigns); + Ok(campaigns) } pub async fn campaign_exists(pool: &DbPool, campaign: &Campaign) -> Result { diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 455d8833c..226644902 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -35,6 +35,7 @@ pub mod routes { pub mod analytics; pub mod cfg; pub mod channel; + pub mod campaign; pub mod event_aggregate; pub mod validator_message; } diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 58747bfad..626ddba7b 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,10 +1,10 @@ use crate::{ success_response, Application, Auth, ResponseError, RouteParams, Session, - event_aggregate::latest_new_state, db::{ spendable::fetch_spendable, - campaign::{campaign_exists, update_campaign, insert_campaign, get_campaigns_for_channel}, - } + event_aggregate::latest_new_state, + DbPool + }, }; use hyper::{Body, Request, Response}; use primitives::{ @@ -13,9 +13,13 @@ use primitives::{ campaign_create::CreateCampaign, SuccessResponse }, - Campaign + Campaign, CampaignId, UnifiedNum, ChannelId }; use redis::aio::MultiplexedConnection; +use deadpool_postgres::PoolError; +use slog::error; +use tokio_postgres::error::SqlState; +use crate::db::campaign::{campaign_exists, update_campaign, insert_campaign, get_campaigns_for_channel}; pub async fn create_campaign( req: Request, @@ -37,16 +41,8 @@ pub async fn create_campaign( match insert_or_modify_campaign(&app.pool, &campaign, &app.redis).await { Err(error) => { - error!(&app.logger, "{}", &error; "module" => "create_channel"); - - match error { - PoolError::Backend(error) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) => { - Err(ResponseError::Conflict( - "channel already exists".to_string(), - )) - } - _ => Err(error_response), - } + // error!(&app.logger, "{}", &error; "module" => "create_channel"); + Err(ResponseError::Conflict("channel already exists".to_string())) } Ok(false) => Err(error_response), _ => Ok(()), @@ -59,7 +55,7 @@ pub async fn create_campaign( // TODO: Double check redis calls async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { - let key = format!("adexCampaign:campaignSpent:{}", id) + let key = format!("adexCampaign:campaignSpent:{}", id); // campaignSpent tracks the portion of the budget which has already been spent let campaign_spent = match redis::cmd("GET") .arg(&key) @@ -75,7 +71,7 @@ async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) - async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { // update a key in Redis for the remaining spendable amount - let key = format!("adexCampaign:remainingSpendable:{}", id) + let key = format!("adexCampaign:remainingSpendable:{}", id); redis::cmd("SET") .arg(&key) .arg(amount) @@ -84,7 +80,7 @@ async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: Campai } async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { - let key = format!("adexChannel:remaining:{}", id) + let key = format!("adexChannel:remaining:{}", id); redis::cmd("SET") .arg(&key) .arg(amount) @@ -104,11 +100,11 @@ async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaign: &C } pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { - let campaign_spent = get_spent_for_campaign(&redis, campaign.id()).await?; + let campaign_spent = get_spent_for_campaign(&redis, campaign.id).await?; // Check if we haven't exceeded the budget yet if (campaign.budget <= campaign_spent) { - ResponseError::FailedValidation("No more budget available for spending") + ResponseError::FailedValidation("No more budget available for spending".into()) } let remaining_spendable_campaign = campaign.budget - campaign_spent; @@ -118,7 +114,7 @@ pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis // Getting the latest new state from Postgres let latest_new_state = latest_new_state(&pool, &campaign.channel, "").await?; // Gets the latest Spendable for this (spender, channelId) pair - let latest_spendable = fetch_spendable(pool, campaign.creator, campaign.channel.id()).await?; + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &*campaign.channel.id()).await?; let total_deposited = latest_spendable.deposit.total; let total_spent = latest_new_state.spenders[campaign.creator]; From 58c61aa79b43b1eab728461bed0e135545cc8227 Mon Sep 17 00:00:00 2001 From: simzzz Date: Mon, 31 May 2021 13:52:18 +0300 Subject: [PATCH 03/31] more error fixes --- sentry/src/db/campaign.rs | 4 +-- sentry/src/db/event_aggregate.rs | 27 ++++++++++++++++- sentry/src/routes/campaign.rs | 52 +++++++++++++++++++------------- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 0df31ba16..5ba83e882 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -49,13 +49,13 @@ pub async fn fetch_campaign(pool: DbPool, campaign: &Campaign) -> Result Result, PoolError> { +pub async fn get_campaigns_for_channel(pool: &DbPool, campaign: &Campaign) -> Result, PoolError> { let client = pool.get().await?; let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE channel_id = $1").await?; let row = client.query(&statement, &[&campaign.channel.id()]).await?; - let campaigns = row.into_iter().for_each(|c| Campaign::from(c)).collect(); + let campaigns = row.into_iter().map(|c| Campaign::from(c)).collect(); Ok(campaigns) } diff --git a/sentry/src/db/event_aggregate.rs b/sentry/src/db/event_aggregate.rs index 73b73d138..c74c6cda9 100644 --- a/sentry/src/db/event_aggregate.rs +++ b/sentry/src/db/event_aggregate.rs @@ -3,7 +3,7 @@ use futures::pin_mut; use primitives::{ sentry::{EventAggregate, MessageResponse}, validator::{ApproveState, Heartbeat, NewState}, - Address, BigNum, Channel, ChannelId, ValidatorId, + Address, BigNum, Channel, ChannelId, ValidatorId, channel_v5::Channel as ChannelV5, }; use std::{convert::TryFrom, ops::Add}; use tokio_postgres::{ @@ -58,6 +58,31 @@ pub async fn latest_new_state( .map_err(PoolError::Backend) } +pub async fn latest_new_state_v5( + pool: &DbPool, + channel: &ChannelV5, + state_root: &str, +) -> Result>, PoolError> { + let client = pool.get().await?; + + let select = client.prepare("SELECT \"from\", msg, received FROM validator_messages WHERE channel_id = $1 AND \"from\" = $2 AND msg ->> 'type' = 'NewState' AND msg->> 'stateRoot' = $3 ORDER BY received DESC LIMIT 1").await?; + let rows = client + .query( + &select, + &[ + &channel.id(), + &channel.leader, + &state_root, + ], + ) + .await?; + + rows.get(0) + .map(MessageResponse::::try_from) + .transpose() + .map_err(PoolError::Backend) +} + pub async fn latest_heartbeats( pool: &DbPool, channel_id: &ChannelId, diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 626ddba7b..4ae2741e4 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -2,7 +2,7 @@ use crate::{ success_response, Application, Auth, ResponseError, RouteParams, Session, db::{ spendable::fetch_spendable, - event_aggregate::latest_new_state, + event_aggregate::latest_new_state_v5, DbPool }, }; @@ -42,7 +42,7 @@ pub async fn create_campaign( match insert_or_modify_campaign(&app.pool, &campaign, &app.redis).await { Err(error) => { // error!(&app.logger, "{}", &error; "module" => "create_channel"); - Err(ResponseError::Conflict("channel already exists".to_string())) + return Err(ResponseError::Conflict("channel already exists".to_string())); } Ok(false) => Err(error_response), _ => Ok(()), @@ -65,8 +65,9 @@ async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) - Some(spent) => UnifiedNum::from(spent), // TODO: Double check if this is true // If the campaign is just being inserted, there would be no entry therefore no funds would be spent - None => 0 + None => UnifiedNum::from(0) }; + Ok(campaign_spent) } async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { @@ -74,37 +75,41 @@ async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: Campai let key = format!("adexCampaign:remainingSpendable:{}", id); redis::cmd("SET") .arg(&key) - .arg(amount) + .arg(amount.to_u64()) .query_async(&mut redis.clone()) - .await? + .await?; + Ok(true) } async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { let key = format!("adexChannel:remaining:{}", id); redis::cmd("SET") .arg(&key) - .arg(amount) + .arg(amount.to_u64()) .query_async(&mut redis.clone()) - .await? + .await?; + Ok(true) } -async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaign: &Campaign) -> Result { - let campaigns_for_channel = get_campaigns_for_channel(&campaign).await?; - let sum_of_campaigns_remaining = campaigns_for_channel - .map(|c| { +async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { + let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; + let sum_of_campaigns_remaining = campaigns_for_channel + .into_iter() + .map(async |c| { let spent = get_spent_for_campaign(&redis, c.id).await?; let remaining = c.budget - spent; remaining }) .sum(); + Ok(sum_of_campaigns_remaining) } pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { let campaign_spent = get_spent_for_campaign(&redis, campaign.id).await?; // Check if we haven't exceeded the budget yet - if (campaign.budget <= campaign_spent) { - ResponseError::FailedValidation("No more budget available for spending".into()) + if campaign.budget <= campaign_spent { + return Err(ResponseError::FailedValidation("No more budget available for spending".into())); } let remaining_spendable_campaign = campaign.budget - campaign_spent; @@ -112,24 +117,29 @@ pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis // Getting the latest new state from Postgres - let latest_new_state = latest_new_state(&pool, &campaign.channel, "").await?; + let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; // Gets the latest Spendable for this (spender, channelId) pair - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &*campaign.channel.id()).await?; + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; let total_deposited = latest_spendable.deposit.total; - let total_spent = latest_new_state.spenders[campaign.creator]; + let total_spent = if let Some(lns) = latest_new_state { + lns.msg.into_inner().spenders[campaign.creator] + } else { + 0 + }; + let total_remaining = total_deposited - total_spent; update_remaining_for_channel(&redis, campaign.channel.id(), total_remaining).await?; - if (campaign_exists(&pool, &campaign)) { - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaign).await?; + if campaign_exists(&pool, &campaign).await? { + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaign).await?; if campaigns_remaining_sum > total_remaining { - ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel") + return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".into())); } - update_campaign(&pool, &campaign).await? + return update_campaign(&pool, &campaign).await? } - insert_campaign(&pool, &campaign).await? + return insert_campaign(&pool, &campaign).await?; // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] From 70ff87c198210120ebcffe1a3445f10c698930af Mon Sep 17 00:00:00 2001 From: simzzz Date: Tue, 8 Jun 2021 19:12:05 +0300 Subject: [PATCH 04/31] Progress w/out tests --- primitives/src/sentry.rs | 12 +++ sentry/src/db/campaign.rs | 12 +-- sentry/src/lib.rs | 16 ++- sentry/src/routes/campaign.rs | 177 +++++++++++++++++++++++----------- 4 files changed, 149 insertions(+), 68 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index da6bbacf2..166f1a117 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -379,6 +379,18 @@ pub mod campaign_create { } } } + + // All editable fields stored in one place, used for checking when a budget is changed + // #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] + // pub struct CampaignState { + // pub budget: UnifiedNum, + // pub validators: Validators, + // pub title: Option, + // pub pricing_bounds: Option, + // pub event_submission: Option, + // pub ad_units: Vec, + // pub targeting_rules: Rules, + // } } #[cfg(feature = "postgres")] diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 5ba83e882..62c2dd8fb 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -2,8 +2,6 @@ use crate::db::{DbPool, PoolError}; use primitives::Campaign; use tokio_postgres::types::Json; -// TODO: Remove once we use this fn -#[allow(dead_code)] pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let ad_units = Json(campaign.ad_units.clone()); @@ -38,9 +36,7 @@ pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result Result { +pub async fn fetch_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE id = $1").await?; @@ -55,7 +51,7 @@ pub async fn get_campaigns_for_channel(pool: &DbPool, campaign: &Campaign) -> Re let row = client.query(&statement, &[&campaign.channel.id()]).await?; - let campaigns = row.into_iter().map(|c| Campaign::from(c)).collect(); + let campaigns = row.into_iter().map(|c| Campaign::from(&c)).collect(); Ok(campaigns) } @@ -71,7 +67,7 @@ pub async fn campaign_exists(pool: &DbPool, campaign: &Campaign) -> Result Result { +pub async fn update_campaign_in_db(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let statement = client .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") @@ -122,7 +118,7 @@ mod test { .expect("Should succeed"); asser!(exists); - let fetched_campaign: Campaign = fetch_campaign(db_pool.clone(), &campaign_for_testing) + let fetched_campaign: Campaign = fetch_campaign(&db_pool, &campaign_for_testing) .await .expect("Should fetch successfully"); diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 226644902..993e1eed4 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -21,7 +21,7 @@ use primitives::{Config, ValidatorId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; -use routes::campaign::create_campaign; +use routes::campaign::{create_campaign, update_campaign}; use routes::cfg::config; use routes::channel::{ channel_list, channel_validate, create_channel, create_validator_messages, insert_events, @@ -152,7 +152,8 @@ impl Application { publisher_analytics(req, &self).await } - ("/campaign.create", &Method::POST) => { + // For creating campaigns + ("/campaign", &Method::POST) => { let req = match AuthRequired.call(req, &self).await { Ok(req) => req, Err(error) => { @@ -162,6 +163,17 @@ impl Application { create_campaign(req, &self).await } + // For editing campaigns + ("/campaign/:id", &Method::POST) => { + let req = match AuthRequired.call(req, &self).await { + Ok(req) => req, + Err(error) => { + return map_response_error(error); + } + }; + + update_campaign(req, &self).await + } (route, _) if route.starts_with("/analytics") => analytics_router(req, &self).await, // This is important becuase it prevents us from doing // expensive regex matching for routes without /channel diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 4ae2741e4..415e38aff 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -13,13 +13,14 @@ use primitives::{ campaign_create::CreateCampaign, SuccessResponse }, - Campaign, CampaignId, UnifiedNum, ChannelId + Campaign, CampaignId, UnifiedNum, ChannelId, BigNum }; use redis::aio::MultiplexedConnection; use deadpool_postgres::PoolError; use slog::error; use tokio_postgres::error::SqlState; -use crate::db::campaign::{campaign_exists, update_campaign, insert_campaign, get_campaigns_for_channel}; +use crate::db::campaign::{campaign_exists, update_campaign_in_db, insert_campaign, get_campaigns_for_channel, fetch_campaign}; +use std::{convert::TryFrom, fs, str::FromStr}; pub async fn create_campaign( req: Request, @@ -35,14 +36,23 @@ pub async fn create_campaign( // TODO AIP#61: Validate Campaign - let error_response = ResponseError::BadRequest("err occurred; please try again later".into()); + let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); - // insert Campaign + // Checking if there is enough remaining deposit + let remaining_for_channel = get_total_remaining_for_channel(&app.redis, &app.pool, &campaign).await?; + + if campaign.budget > remaining_for_channel { + return Err(ResponseError::Conflict("Not Enough budget for campaign".to_string())); + } + + // If the channel is being created, the amount spent is 0, therefore remaining = budget + update_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await?; - match insert_or_modify_campaign(&app.pool, &campaign, &app.redis).await { + // insert Campaign + match insert_campaign(&app.pool, &campaign).await { Err(error) => { - // error!(&app.logger, "{}", &error; "module" => "create_channel"); - return Err(ResponseError::Conflict("channel already exists".to_string())); + error!(&app.logger, "{}", &error; "module" => "create_campaign"); + return Err(ResponseError::Conflict("campaign already exists".to_string())); } Ok(false) => Err(error_response), _ => Ok(()), @@ -53,93 +63,144 @@ pub async fn create_campaign( Ok(success_response(serde_json::to_string(&campaign)?)) } +pub async fn update_campaign( + req: Request, + app: &Application, +) -> Result, ResponseError> { + let body = hyper::body::to_bytes(req.into_body()).await?; + + let campaign = serde_json::from_slice::(&body) + .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; + + let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); + + // modify Campaign + + match modify_campaign(&app.pool, &campaign, &app.redis).await { + Err(error) => { + error!(&app.logger, "{:?}", &error; "module" => "update_campaign"); + return Err(ResponseError::Conflict("Error modifying campaign".to_string())); + } + Ok(false) => Err(error_response), + _ => Ok(()), + }?; + + let update_response = SuccessResponse { success: true }; + + Ok(success_response(serde_json::to_string(&campaign)?)) +} + // TODO: Double check redis calls -async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { +async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { let key = format!("adexCampaign:campaignSpent:{}", id); // campaignSpent tracks the portion of the budget which has already been spent let campaign_spent = match redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) - .await? - { - Some(spent) => UnifiedNum::from(spent), - // TODO: Double check if this is true - // If the campaign is just being inserted, there would be no entry therefore no funds would be spent - None => UnifiedNum::from(0) - }; + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + .map_err(|_| ResponseError::Conflict("Error getting campaignSpent for current campaign".to_string()))?{ + Some(spent) => { + // TODO: Fix unwraps + let res = BigNum::from_str(&spent).unwrap(); + let res = res.to_u64().unwrap(); + UnifiedNum::from_u64(res) + }, + None => UnifiedNum::from_u64(0) + }; + Ok(campaign_spent) } -async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { +async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { // update a key in Redis for the remaining spendable amount let key = format!("adexCampaign:remainingSpendable:{}", id); redis::cmd("SET") .arg(&key) .arg(amount.to_u64()) .query_async(&mut redis.clone()) - .await?; - Ok(true) + .await + .map_err(|_| ResponseError::Conflict("Error updating remainingSpendable for current campaign".to_string()))? } -async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { - let key = format!("adexChannel:remaining:{}", id); - redis::cmd("SET") - .arg(&key) - .arg(amount.to_u64()) - .query_async(&mut redis.clone()) - .await?; - Ok(true) +async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { + // Getting the latest new state from Postgres + let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; + + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + + let total_deposited = latest_spendable.deposit.total; + + let latest_new_state = latest_new_state.ok_or_else(|| ResponseError::Conflict("Error getting latest new state message".to_string()))?; + let msg = latest_new_state.msg; + let total_spent = msg.balances.get(&campaign.creator); + let zero = BigNum::from(0); + let total_spent = if let Some(spent) = total_spent { + spent + } else { + &zero + }; + + // TODO: total_spent is BigNum, is it safe to just convert it to UnifiedNum like this? + let total_spent = total_spent.to_u64().ok_or_else(|| { + ResponseError::Conflict("Error while converting total_spent to u64".to_string()) + }); + let total_remaining = total_deposited.checked_sub(&UnifiedNum::from_u64(total_spent)).ok_or_else(|| { + ResponseError::Conflict("Error while calculating the total remaining amount".to_string()) + })?; + Ok(total_remaining) } -async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { +// async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { +// let key = format!("adexChannel:remaining:{}", id); +// redis::cmd("SET") +// .arg(&key) +// .arg(amount.to_u64()) +// .query_async(&mut redis.clone()) +// .await?; +// Ok(true) +// } + +async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; let sum_of_campaigns_remaining = campaigns_for_channel .into_iter() - .map(async |c| { + .map(|c| async move { let spent = get_spent_for_campaign(&redis, c.id).await?; - let remaining = c.budget - spent; + let remaining = c.budget.checked_sub(&spent).ok_or_else(|| { + ResponseError::Conflict("Error while calculating remaining for campaign".to_string()) + }); remaining }) - .sum(); - Ok(sum_of_campaigns_remaining) + .fold(0u64, |sum: u64, val: u64| sum.checked_add(val).ok_or_else(|| { + return ResponseError::Conflict("Error while calculating total remaining for all campaigns".to_string()); + })); + Ok(UnifiedNum::from_u64(sum_of_campaigns_remaining)) } -pub async fn insert_or_modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { +pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { let campaign_spent = get_spent_for_campaign(&redis, campaign.id).await?; - // Check if we haven't exceeded the budget yet - if campaign.budget <= campaign_spent { + // Check if we have reached the budget + if campaign_spent >= campaign.budget { return Err(ResponseError::FailedValidation("No more budget available for spending".into())); } - let remaining_spendable_campaign = campaign.budget - campaign_spent; + let remaining_spendable_campaign = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { + ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) + })?; update_remaining_for_campaign(&redis, campaign.id, remaining_spendable_campaign).await?; - // Getting the latest new state from Postgres - let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; - // Gets the latest Spendable for this (spender, channelId) pair - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - let total_deposited = latest_spendable.deposit.total; - let total_spent = if let Some(lns) = latest_new_state { - lns.msg.into_inner().spenders[campaign.creator] - } else { - 0 - }; - - let total_remaining = total_deposited - total_spent; - - update_remaining_for_channel(&redis, campaign.channel.id(), total_remaining).await?; + // Gets the latest Spendable for this (spender, channelId) pair + let total_remaining = get_total_remaining_for_channel(&redis, &pool, &campaign).await?; - if campaign_exists(&pool, &campaign).await? { - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaign).await?; - if campaigns_remaining_sum > total_remaining { - return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".into())); - } - return update_campaign(&pool, &campaign).await? + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaign).await?; + if campaigns_remaining_sum > total_remaining { + return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".to_string())); } - return insert_campaign(&pool, &campaign).await?; + + update_campaign_in_db(&pool, &campaign).await.map_err(|e| ResponseError::Conflict(e.to_string())) // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] From f7dae5b60acc516d5b03ca86d869d5d8e03026aa Mon Sep 17 00:00:00 2001 From: simzzz Date: Thu, 10 Jun 2021 10:17:23 +0300 Subject: [PATCH 05/31] fixed typo --- sentry/src/db/campaign.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 62c2dd8fb..5f9c1211b 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -116,7 +116,7 @@ mod test { let exists = campaign_exists(&db_pool.clone(), campaign: &campaign_for_testing) .await .expect("Should succeed"); - asser!(exists); + assert!(exists); let fetched_campaign: Campaign = fetch_campaign(&db_pool, &campaign_for_testing) .await From 0f400160669c70b75b130bd7f7a7bbf5adf6eaf1 Mon Sep 17 00:00:00 2001 From: simzzz Date: Thu, 10 Jun 2021 16:42:46 +0300 Subject: [PATCH 06/31] Refactoring + changes to the way the remaining for all campaigns is received --- sentry/src/db/campaign.rs | 2 +- sentry/src/routes/campaign.rs | 76 ++++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 5f9c1211b..43d29d507 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -113,7 +113,7 @@ mod test { assert!(is_inserted); - let exists = campaign_exists(&db_pool.clone(), campaign: &campaign_for_testing) + let exists = campaign_exists(&db_pool.clone(), &campaign_for_testing) .await .expect("Should succeed"); assert!(exists); diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 415e38aff..7a24dbfeb 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,5 +1,5 @@ use crate::{ - success_response, Application, Auth, ResponseError, RouteParams, Session, + success_response, Application, ResponseError, db::{ spendable::fetch_spendable, event_aggregate::latest_new_state_v5, @@ -11,16 +11,17 @@ use primitives::{ adapter::Adapter, sentry::{ campaign_create::CreateCampaign, - SuccessResponse + SuccessResponse, + MessageResponse }, - Campaign, CampaignId, UnifiedNum, ChannelId, BigNum + spender::Spendable, + validator::NewState, + Campaign, CampaignId, UnifiedNum, BigNum }; use redis::aio::MultiplexedConnection; -use deadpool_postgres::PoolError; use slog::error; -use tokio_postgres::error::SqlState; -use crate::db::campaign::{campaign_exists, update_campaign_in_db, insert_campaign, get_campaigns_for_channel, fetch_campaign}; -use std::{convert::TryFrom, fs, str::FromStr}; +use crate::db::campaign::{update_campaign_in_db, insert_campaign, get_campaigns_for_channel}; +use std::str::FromStr; pub async fn create_campaign( req: Request, @@ -39,7 +40,12 @@ pub async fn create_campaign( let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); // Checking if there is enough remaining deposit - let remaining_for_channel = get_total_remaining_for_channel(&app.redis, &app.pool, &campaign).await?; + // TODO: Switch with Accounting once it's ready + let latest_new_state = latest_new_state_v5(&app.pool, &campaign.channel, "").await?; + + // TODO: AIP#61: Update when changes to Spendable are ready + let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let remaining_for_channel = get_total_remaining_for_channel(&app.redis, &app.pool, &campaign, &latest_new_state, &latest_spendable).await?; if campaign.budget > remaining_for_channel { return Err(ResponseError::Conflict("Not Enough budget for campaign".to_string())); @@ -122,16 +128,11 @@ async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: Campai .map_err(|_| ResponseError::Conflict("Error updating remainingSpendable for current campaign".to_string()))? } -async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { - // Getting the latest new state from Postgres - let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; - - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - +async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign, latest_new_state: &Option>, latest_spendable: &Spendable) -> Result { let total_deposited = latest_spendable.deposit.total; - let latest_new_state = latest_new_state.ok_or_else(|| ResponseError::Conflict("Error getting latest new state message".to_string()))?; - let msg = latest_new_state.msg; + let latest_new_state = latest_new_state.as_ref().ok_or_else(|| ResponseError::Conflict("Error getting latest new state message".to_string()))?; + let msg = &latest_new_state.msg; let total_spent = msg.balances.get(&campaign.creator); let zero = BigNum::from(0); let total_spent = if let Some(spent) = total_spent { @@ -143,7 +144,7 @@ async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &D // TODO: total_spent is BigNum, is it safe to just convert it to UnifiedNum like this? let total_spent = total_spent.to_u64().ok_or_else(|| { ResponseError::Conflict("Error while converting total_spent to u64".to_string()) - }); + })?; let total_remaining = total_deposited.checked_sub(&UnifiedNum::from_u64(total_spent)).ok_or_else(|| { ResponseError::Conflict("Error while calculating the total remaining amount".to_string()) })?; @@ -160,26 +161,37 @@ async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &D // Ok(true) // } -async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign) -> Result { - let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; - let sum_of_campaigns_remaining = campaigns_for_channel +async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaigns: &Vec, mutated_campaign: &Campaign) -> Result { + let other_campaigns_remaining: Vec = campaigns .into_iter() + .filter(|c| c.id != mutated_campaign.id) .map(|c| async move { let spent = get_spent_for_campaign(&redis, c.id).await?; - let remaining = c.budget.checked_sub(&spent).ok_or_else(|| { - ResponseError::Conflict("Error while calculating remaining for campaign".to_string()) - }); - remaining + let remaining = c.budget.checked_sub(&spent)?; + Ok(remaining) }) - .fold(0u64, |sum: u64, val: u64| sum.checked_add(val).ok_or_else(|| { - return ResponseError::Conflict("Error while calculating total remaining for all campaigns".to_string()); - })); - Ok(UnifiedNum::from_u64(sum_of_campaigns_remaining)) + .collect(); + // TODO: Fix Unwrap + let sum_of_campaigns_remaining = other_campaigns_remaining + .into_iter() + .fold(UnifiedNum::from_u64(0), |mut sum, val| sum.checked_add(&val).unwrap()); + // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB + let spent_for_mutated_campaign = get_spent_for_campaign(&redis, mutated_campaign.id).await?; + let remaining_for_mutated_campaign = mutated_campaign.budget.checked_sub(&spent_for_mutated_campaign).ok_or_else(|| { + ResponseError::Conflict("Error while calculating remaining for mutated campaign".to_string()) + })?; + sum_of_campaigns_remaining.checked_add(&remaining_for_mutated_campaign).ok_or_else(|| { + ResponseError::Conflict("Error while calculating sum for all campaigns".to_string()) + }); + Ok(sum_of_campaigns_remaining) } pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { let campaign_spent = get_spent_for_campaign(&redis, campaign.id).await?; + // Getting the latest new state from Postgres + let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; // Check if we have reached the budget if campaign_spent >= campaign.budget { return Err(ResponseError::FailedValidation("No more budget available for spending".into())); @@ -190,12 +202,10 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl })?; update_remaining_for_campaign(&redis, campaign.id, remaining_spendable_campaign).await?; - - // Gets the latest Spendable for this (spender, channelId) pair - let total_remaining = get_total_remaining_for_channel(&redis, &pool, &campaign).await?; - - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaign).await?; + let total_remaining = get_total_remaining_for_channel(&redis, &pool, &campaign, &latest_new_state, &latest_spendable).await?; + let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaigns_for_channel, &campaign).await?; if campaigns_remaining_sum > total_remaining { return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".to_string())); } From 33f0e3ea33bbaea9c0b6486d3b21aac61b25558b Mon Sep 17 00:00:00 2001 From: simzzz Date: Thu, 10 Jun 2021 16:58:30 +0300 Subject: [PATCH 07/31] added CampaignCreateResponse --- sentry/src/routes/campaign.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 7a24dbfeb..b99551d39 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -12,7 +12,7 @@ use primitives::{ sentry::{ campaign_create::CreateCampaign, SuccessResponse, - MessageResponse + MessageResponse, }, spender::Spendable, validator::NewState, @@ -27,6 +27,12 @@ pub async fn create_campaign( req: Request, app: &Application, ) -> Result, ResponseError> { + use serde::Serialize; + #[derive(Serialize)] + struct CreateCampaignResponse<'a> { + campaign: &'a Campaign, + } + let body = hyper::body::to_bytes(req.into_body()).await?; let campaign = serde_json::from_slice::(&body) @@ -64,9 +70,9 @@ pub async fn create_campaign( _ => Ok(()), }?; - let create_response = SuccessResponse { success: true }; + let create_response = CreateCampaignResponse { campaign: &campaign }; - Ok(success_response(serde_json::to_string(&campaign)?)) + Ok(success_response(serde_json::to_string(&create_response)?)) } pub async fn update_campaign( From 475b8db256ffe9047fd53d420f7301f6e959284b Mon Sep 17 00:00:00 2001 From: simzzz Date: Fri, 11 Jun 2021 16:42:12 +0300 Subject: [PATCH 08/31] More changes to budget/redis operations + fixed problems from PR review --- sentry/src/db/campaign.rs | 7 ++-- sentry/src/routes/campaign.rs | 71 +++++++++++++++++++++++------------ 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index fa49dcce5..eba32944f 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -49,9 +49,9 @@ pub async fn get_campaigns_for_channel(pool: &DbPool, campaign: &Campaign) -> Re let client = pool.get().await?; let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE channel_id = $1").await?; - let row = client.query(&statement, &[&campaign.channel.id()]).await?; + let rows = client.query(&statement, &[&campaign.channel.id()]).await?; - let campaigns = row.into_iter().map(|c| Campaign::from(&c)).collect(); + let campaigns = rows.iter().map(Campaign::from).collect(); Ok(campaigns) } @@ -67,7 +67,8 @@ pub async fn campaign_exists(pool: &DbPool, campaign: &Campaign) -> Result Result { +// TODO: Test for campaign ad_units +pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let statement = client .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index b99551d39..647f89bae 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -20,19 +20,17 @@ use primitives::{ }; use redis::aio::MultiplexedConnection; use slog::error; -use crate::db::campaign::{update_campaign_in_db, insert_campaign, get_campaigns_for_channel}; -use std::str::FromStr; +use crate::db::campaign::{update_campaign as update_campaign_db, insert_campaign, get_campaigns_for_channel}; +use std::{ + cmp::max, + str::FromStr, +}; +use futures::future::join_all; pub async fn create_campaign( req: Request, app: &Application, ) -> Result, ResponseError> { - use serde::Serialize; - #[derive(Serialize)] - struct CreateCampaignResponse<'a> { - campaign: &'a Campaign, - } - let body = hyper::body::to_bytes(req.into_body()).await?; let campaign = serde_json::from_slice::(&body) @@ -70,9 +68,7 @@ pub async fn create_campaign( _ => Ok(()), }?; - let create_response = CreateCampaignResponse { campaign: &campaign }; - - Ok(success_response(serde_json::to_string(&create_response)?)) + Ok(success_response(serde_json::to_string(&campaign)?)) } pub async fn update_campaign( @@ -104,34 +100,51 @@ pub async fn update_campaign( // TODO: Double check redis calls async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { - let key = format!("adexCampaign:campaignSpent:{}", id); + let key = format!("spent:{}", id); // campaignSpent tracks the portion of the budget which has already been spent let campaign_spent = match redis::cmd("GET") .arg(&key) .query_async::<_, Option>(&mut redis.clone()) .await - .map_err(|_| ResponseError::Conflict("Error getting campaignSpent for current campaign".to_string()))?{ - Some(spent) => { - // TODO: Fix unwraps + { + Ok(Some(spent)) => { let res = BigNum::from_str(&spent).unwrap(); let res = res.to_u64().unwrap(); UnifiedNum::from_u64(res) }, - None => UnifiedNum::from_u64(0) + _ => UnifiedNum::from_u64(0) }; Ok(campaign_spent) } +async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { + let key = format!("remaining:{}", id); + let remaining = match redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + { + Ok(Some(remaining)) => { + let res = BigNum::from_str(&remaining).unwrap(); + let res = res.to_u64().unwrap(); + UnifiedNum::from_u64(res) + }, + _ => UnifiedNum::from_u64(0) + }; + Ok(remaining) +} + async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { // update a key in Redis for the remaining spendable amount - let key = format!("adexCampaign:remainingSpendable:{}", id); - redis::cmd("SET") + let key = format!("remaining:{}", id); + redis::cmd("INCRBY") .arg(&key) .arg(amount.to_u64()) .query_async(&mut redis.clone()) .await - .map_err(|_| ResponseError::Conflict("Error updating remainingSpendable for current campaign".to_string()))? + .map_err(|_| ResponseError::Conflict("Error updating remainingSpendable for current campaign".to_string()))?; + Ok(true) } async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign, latest_new_state: &Option>, latest_spendable: &Spendable) -> Result { @@ -168,7 +181,7 @@ async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &D // } async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaigns: &Vec, mutated_campaign: &Campaign) -> Result { - let other_campaigns_remaining: Vec = campaigns + let other_campaigns_remaining = campaigns .into_iter() .filter(|c| c.id != mutated_campaign.id) .map(|c| async move { @@ -176,7 +189,8 @@ async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPoo let remaining = c.budget.checked_sub(&spent)?; Ok(remaining) }) - .collect(); + .collect::>(); + let other_campaigns_remaining = join_all(other_campaigns_remaining).await; // TODO: Fix Unwrap let sum_of_campaigns_remaining = other_campaigns_remaining .into_iter() @@ -203,10 +217,19 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl return Err(ResponseError::FailedValidation("No more budget available for spending".into())); } - let remaining_spendable_campaign = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { + let old_spendable = get_remaining_for_campaign(&redis, campaign.id).await?; + + let new_spendable = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) })?; - update_remaining_for_campaign(&redis, campaign.id, remaining_spendable_campaign).await?; + + let diff_in_remaining = new_spendable.checked_sub(&old_spendable).ok_or_else(|| { + ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) + })?; + + let diff_in_remaining = max(UnifiedNum::from_u64(0), diff_in_remaining); + + update_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; // Gets the latest Spendable for this (spender, channelId) pair let total_remaining = get_total_remaining_for_channel(&redis, &pool, &campaign, &latest_new_state, &latest_spendable).await?; @@ -216,7 +239,7 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".to_string())); } - update_campaign_in_db(&pool, &campaign).await.map_err(|e| ResponseError::Conflict(e.to_string())) + update_campaign_db(&pool, &campaign).await.map_err(|e| ResponseError::Conflict(e.to_string())) // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] From 60efa4a6b3cfe2fb39d29c634c1dba353db5486c Mon Sep 17 00:00:00 2001 From: simzzz Date: Fri, 11 Jun 2021 17:27:01 +0300 Subject: [PATCH 09/31] more changes to updating remaining --- sentry/src/routes/campaign.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 647f89bae..4a10d9621 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -217,18 +217,17 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl return Err(ResponseError::FailedValidation("No more budget available for spending".into())); } - let old_spendable = get_remaining_for_campaign(&redis, campaign.id).await?; + let old_remaining = get_remaining_for_campaign(&redis, campaign.id).await?; + let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); - let new_spendable = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { + let new_remaining = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) })?; - let diff_in_remaining = new_spendable.checked_sub(&old_spendable).ok_or_else(|| { + let diff_in_remaining = new_remaining.checked_sub(&old_remaining).ok_or_else(|| { ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) })?; - let diff_in_remaining = max(UnifiedNum::from_u64(0), diff_in_remaining); - update_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; // Gets the latest Spendable for this (spender, channelId) pair From 1b8cd75edf0d9cbddf418cf4dc2d6541d37409c0 Mon Sep 17 00:00:00 2001 From: simzzz Date: Mon, 14 Jun 2021 16:10:53 +0300 Subject: [PATCH 10/31] added regex for campaign update route --- sentry/src/lib.rs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 993e1eed4..b2dbd2634 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -60,6 +60,8 @@ lazy_static! { static ref ADVERTISER_ANALYTICS_BY_CHANNEL_ID: Regex = Regex::new(r"^/analytics/for-advertiser/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); static ref PUBLISHER_ANALYTICS_BY_CHANNEL_ID: Regex = Regex::new(r"^/analytics/for-publisher/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); static ref CREATE_EVENTS_BY_CHANNEL_ID: Regex = Regex::new(r"^/channel/0x([a-zA-Z0-9]{64})/events/?$").expect("The regex should be valid"); + static ref CAMPAIGN_UPDATE_BY_ID: Regex = + Regex::new(r"^/channel/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); } #[derive(Debug)] @@ -163,21 +165,11 @@ impl Application { create_campaign(req, &self).await } - // For editing campaigns - ("/campaign/:id", &Method::POST) => { - let req = match AuthRequired.call(req, &self).await { - Ok(req) => req, - Err(error) => { - return map_response_error(error); - } - }; - - update_campaign(req, &self).await - } (route, _) if route.starts_with("/analytics") => analytics_router(req, &self).await, // This is important becuase it prevents us from doing // expensive regex matching for routes without /channel (path, _) if path.starts_with("/channel") => channels_router(req, &self).await, + (path, _) if path.starts_with("/campaign") => campaigns_router(req, &self).await, _ => Err(ResponseError::NotFound), } .unwrap_or_else(map_response_error); @@ -332,6 +324,28 @@ async fn channels_router( } } +async fn campaigns_router( + mut req: Request, + app: &Application, +) -> Result, ResponseError> { + req = AuthRequired.call(req, app).await?; + + let (path, method) = (req.uri().path().to_owned(), req.method()); + + // regex matching for routes with params + if let (Some(caps), &Method::POST) = (CAMPAIGN_UPDATE_BY_ID.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); + + update_campaign(req, app).await + } else { + Err(ResponseError::NotFound) + } +} + #[derive(Debug)] pub enum ResponseError { NotFound, From 57116cc38237dd02c0dae2e34df0aabd3aadb2c7 Mon Sep 17 00:00:00 2001 From: simzzz Date: Tue, 15 Jun 2021 15:42:53 +0300 Subject: [PATCH 11/31] Accounting messages + First integration tests --- primitives/src/sentry/accounting.rs | 22 ++-- sentry/src/routes/campaign.rs | 160 +++++++++++++++++++++++----- 2 files changed, 146 insertions(+), 36 deletions(-) diff --git a/primitives/src/sentry/accounting.rs b/primitives/src/sentry/accounting.rs index 6c5b1b88d..9896db703 100644 --- a/primitives/src/sentry/accounting.rs +++ b/primitives/src/sentry/accounting.rs @@ -1,9 +1,6 @@ -use std::{ - convert::TryFrom, - marker::PhantomData, -}; +use std::{convert::TryFrom, marker::PhantomData}; -use crate::{balances_map::UnifiedMap, Address, channel_v5::Channel, UnifiedNum}; +use crate::{balances_map::UnifiedMap, channel_v5::Channel, Address, UnifiedNum}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Deserializer, Serialize}; use thiserror::Error; @@ -130,7 +127,8 @@ mod de { Ok(Self { channel: de_acc.channel, - balances: Balances::::try_from(de_acc.balances).map_err(serde::de::Error::custom)?, + balances: Balances::::try_from(de_acc.balances) + .map_err(serde::de::Error::custom)?, created: de_acc.created, updated: de_acc.updated, }) @@ -146,7 +144,10 @@ mod de { Ok(Self { channel: unchecked_acc.channel, - balances: unchecked_acc.balances.check().map_err(serde::de::Error::custom)?, + balances: unchecked_acc + .balances + .check() + .map_err(serde::de::Error::custom)?, created: unchecked_acc.created, updated: unchecked_acc.updated, }) @@ -204,13 +205,14 @@ mod postgres { impl TryFrom<&Row> for Accounting { type Error = Error; - + fn try_from(row: &Row) -> Result { let balances = Balances:: { earners: row.get::<_, Json<_>>("earners").0, spenders: row.get::<_, Json<_>>("spenders").0, state: PhantomData::default(), - }.check()?; + } + .check()?; Ok(Self { channel: row.get("channel"), @@ -220,4 +222,4 @@ mod postgres { }) } } -} \ No newline at end of file +} diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 4a10d9621..afd603aac 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -2,7 +2,7 @@ use crate::{ success_response, Application, ResponseError, db::{ spendable::fetch_spendable, - event_aggregate::latest_new_state_v5, + accounting::get_accounting_spent, DbPool }, }; @@ -12,11 +12,11 @@ use primitives::{ sentry::{ campaign_create::CreateCampaign, SuccessResponse, - MessageResponse, + accounting::Accounting, }, spender::Spendable, validator::NewState, - Campaign, CampaignId, UnifiedNum, BigNum + Address, Campaign, CampaignId, UnifiedNum, BigNum }; use redis::aio::MultiplexedConnection; use slog::error; @@ -43,13 +43,11 @@ pub async fn create_campaign( let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); - // Checking if there is enough remaining deposit - // TODO: Switch with Accounting once it's ready - let latest_new_state = latest_new_state_v5(&app.pool, &campaign.channel, "").await?; + let accounting_spent = get_accounting_spent(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; // TODO: AIP#61: Update when changes to Spendable are ready let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - let remaining_for_channel = get_total_remaining_for_channel(&app.redis, &app.pool, &campaign, &latest_new_state, &latest_spendable).await?; + let remaining_for_channel = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable)?; if campaign.budget > remaining_for_channel { return Err(ResponseError::Conflict("Not Enough budget for campaign".to_string())); @@ -147,24 +145,10 @@ async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: Campai Ok(true) } -async fn get_total_remaining_for_channel(redis: &MultiplexedConnection, pool: &DbPool, campaign: &Campaign, latest_new_state: &Option>, latest_spendable: &Spendable) -> Result { +fn get_total_remaining_for_channel(creator: &Address, accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Result { let total_deposited = latest_spendable.deposit.total; - let latest_new_state = latest_new_state.as_ref().ok_or_else(|| ResponseError::Conflict("Error getting latest new state message".to_string()))?; - let msg = &latest_new_state.msg; - let total_spent = msg.balances.get(&campaign.creator); - let zero = BigNum::from(0); - let total_spent = if let Some(spent) = total_spent { - spent - } else { - &zero - }; - - // TODO: total_spent is BigNum, is it safe to just convert it to UnifiedNum like this? - let total_spent = total_spent.to_u64().ok_or_else(|| { - ResponseError::Conflict("Error while converting total_spent to u64".to_string()) - })?; - let total_remaining = total_deposited.checked_sub(&UnifiedNum::from_u64(total_spent)).ok_or_else(|| { + let total_remaining = total_deposited.checked_sub(&accounting_spent).ok_or_else(|| { ResponseError::Conflict("Error while calculating the total remaining amount".to_string()) })?; Ok(total_remaining) @@ -208,8 +192,7 @@ async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPoo pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { let campaign_spent = get_spent_for_campaign(&redis, campaign.id).await?; - // Getting the latest new state from Postgres - let latest_new_state = latest_new_state_v5(&pool, &campaign.channel, "").await?; + let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; // Check if we have reached the budget @@ -231,7 +214,7 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl update_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; // Gets the latest Spendable for this (spender, channelId) pair - let total_remaining = get_total_remaining_for_channel(&redis, &pool, &campaign, &latest_new_state, &latest_spendable).await?; + let total_remaining = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable)?; let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaigns_for_channel, &campaign).await?; if campaigns_remaining_sum > total_remaining { @@ -244,3 +227,128 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 } + + + +#[cfg(test)] +mod test { + use primitives::{ + util::tests::prep_db::{DUMMY_CAMPAIGN, DUMMY_CHANNEL}, + Deposit + }; + + use deadpool::managed::Object; + + use crate::{ + db::redis_pool::{Manager, TESTS_POOL}, + }; + + use super::*; + + async fn get_redis() -> Object { + let connection = TESTS_POOL.get().await.expect("Should return Object"); + connection + } + + fn get_campaign() -> Campaign { + DUMMY_CAMPAIGN.clone() + } + + fn get_dummy_spendable(spender: Address) -> Spendable { + Spendable { + spender, + channel: DUMMY_CHANNEL.clone(), + deposit: Deposit { + total: UnifiedNum::from_u64(1_000_000), + still_on_create2: UnifiedNum::from_u64(0), + }, + } + } + + #[tokio::test] + async fn does_it_get_total_remaianing() { + let campaign = get_campaign(); + let accounting_spent = UnifiedNum::from_u64(100_000); + let latest_spendable = get_dummy_spendable(campaign.creator); + + let total_remaining = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable); + + assert_eq!(total_remaining, UnifiedNum::from_u64(900_000)); + } + + #[tokio::test] + async fn does_it_update_remaining() { + let redis = get_redis().await; + let campaign = get_campaign(); + let key = format!("remaining:{}", campaign.id); + + // Setting the redis base variable + redis::cmd("SET") + .arg(&key) + .arg(100u64) + .query_async(&mut redis.connection) + .await + .expect("should set"); + + // 2 async calls at once, should be 500 after them + futures::future::join( + update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), + update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)) + ).await; + + let remaining = redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + .expect("should get remaining"); + + assert_eq!(remaining, UnifiedNum::from_u64(500)); + + update_remaining_for_campaign(&redis, campaign.id, campaign.budget).await.expect("should increase"); + + let remaining = redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + .expect("should get remaining"); + + let should_be_remaining = UnifiedNum::from_u64(500).checked_add(&campaign.budget).expect("should add"); + assert_eq!(remaining, should_be_remaining); + + update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)).await.expect("should work"); + + let remaining = redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + .expect("should get remaining"); + + assert_eq!(remaining, should_be_remaining); + + update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(-500)).await.expect("should work"); + + let should_be_remaining = should_be_remaining.checked_sub(500).expect("should work"); + + let remaining = redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await + .expect("should get remaining"); + + assert_eq!(remaining, should_be_remaining); + } + + #[tokio::test] + async fn update_remaining_before_it_is_set() { + let redis = get_redis().await; + let campaign = get_campaign(); + let key = format!("remaining:{}", campaign.id); + + let remaining = redis::cmd("GET") + .arg(&key) + .query_async::<_, Option>(&mut redis.clone()) + .await; + + assert_eq!(remaining, Err(ResponseError::Conflict)) + } +} \ No newline at end of file From f4e0949edc0f4a2d400549738c2563b23ba65ca1 Mon Sep 17 00:00:00 2001 From: simzzz Date: Wed, 16 Jun 2021 12:03:48 +0300 Subject: [PATCH 12/31] fixed some unwraps --- sentry/src/routes/campaign.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index afd603aac..f48ff3d36 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -106,14 +106,16 @@ async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) - .await { Ok(Some(spent)) => { - let res = BigNum::from_str(&spent).unwrap(); - let res = res.to_u64().unwrap(); - UnifiedNum::from_u64(res) + let res = BigNum::from_str(&spent)?; + let res = res.to_u64().ok_or_else(|| { + ResponseError::Conflict("Error while converting BigNum to u64".to_string()) + })?; + Ok(UnifiedNum::from_u64(res)) }, - _ => UnifiedNum::from_u64(0) + _ => Ok(UnifiedNum::from_u64(0)) }; - Ok(campaign_spent) + campaign_spent } async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { @@ -124,13 +126,15 @@ async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignI .await { Ok(Some(remaining)) => { - let res = BigNum::from_str(&remaining).unwrap(); - let res = res.to_u64().unwrap(); - UnifiedNum::from_u64(res) + let res = BigNum::from_str(&remaining)?; + let res = res.to_u64().ok_or_else(|| { + ResponseError::Conflict("Error while calculating the total remaining amount".to_string()) + })?; + Ok(UnifiedNum::from_u64(res)) }, - _ => UnifiedNum::from_u64(0) + _ => Ok(UnifiedNum::from_u64(0)) }; - Ok(remaining) + remaining } async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { From 825d6cbeacee1c126de1f198e6a96350e55b2353 Mon Sep 17 00:00:00 2001 From: simzzz Date: Tue, 22 Jun 2021 11:06:31 +0300 Subject: [PATCH 13/31] more fixes --- sentry/src/db/campaign.rs | 33 ++++++---- sentry/src/db/event_aggregate.rs | 12 +--- sentry/src/lib.rs | 2 +- sentry/src/routes/campaign.rs | 109 +++++++++++++++---------------- 4 files changed, 78 insertions(+), 78 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index eba32944f..0d4ecaf8a 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -45,13 +45,17 @@ pub async fn fetch_campaign(pool: &DbPool, campaign: &Campaign) -> Result Result, PoolError> { +pub async fn get_campaigns_for_channel( + pool: &DbPool, + campaign: &Campaign, +) -> Result, PoolError> { let client = pool.get().await?; let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE channel_id = $1").await?; let rows = client.query(&statement, &[&campaign.channel.id()]).await?; let campaigns = rows.iter().map(Campaign::from).collect(); + Ok(campaigns) } @@ -75,16 +79,19 @@ pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result> 'type' = 'NewState' AND msg->> 'stateRoot' = $3 ORDER BY received DESC LIMIT 1").await?; let rows = client - .query( - &select, - &[ - &channel.id(), - &channel.leader, - &state_root, - ], - ) + .query(&select, &[&channel.id(), &channel.leader, &state_root]) .await?; rows.get(0) diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index b2dbd2634..bcfa5d940 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -33,9 +33,9 @@ use std::collections::HashMap; pub mod middleware; pub mod routes { pub mod analytics; + pub mod campaign; pub mod cfg; pub mod channel; - pub mod campaign; pub mod event_aggregate; pub mod validator_message; } diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index f48ff3d36..a8c2bca7d 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -11,12 +11,9 @@ use primitives::{ adapter::Adapter, sentry::{ campaign_create::CreateCampaign, - SuccessResponse, - accounting::Accounting, }, spender::Spendable, - validator::NewState, - Address, Campaign, CampaignId, UnifiedNum, BigNum + Campaign, CampaignId, UnifiedNum, BigNum }; use redis::aio::MultiplexedConnection; use slog::error; @@ -47,7 +44,7 @@ pub async fn create_campaign( // TODO: AIP#61: Update when changes to Spendable are ready let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - let remaining_for_channel = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable)?; + let remaining_for_channel = get_total_remaining_for_channel(&accounting_spent, &latest_spendable)?; if campaign.budget > remaining_for_channel { return Err(ResponseError::Conflict("Not Enough budget for campaign".to_string())); @@ -91,8 +88,6 @@ pub async fn update_campaign( _ => Ok(()), }?; - let update_response = SuccessResponse { success: true }; - Ok(success_response(serde_json::to_string(&campaign)?)) } @@ -118,7 +113,7 @@ async fn get_spent_for_campaign(redis: &MultiplexedConnection, id: CampaignId) - campaign_spent } -async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result { +async fn get_remaining_for_campaign_from_redis(redis: &MultiplexedConnection, id: CampaignId) -> Result { let key = format!("remaining:{}", id); let remaining = match redis::cmd("GET") .arg(&key) @@ -137,6 +132,7 @@ async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignI remaining } +// tested async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { // update a key in Redis for the remaining spendable amount let key = format!("remaining:{}", id); @@ -149,7 +145,8 @@ async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: Campai Ok(true) } -fn get_total_remaining_for_channel(creator: &Address, accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Result { +// tested +fn get_total_remaining_for_channel(accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Result { let total_deposited = latest_spendable.deposit.total; let total_remaining = total_deposited.checked_sub(&accounting_spent).ok_or_else(|| { @@ -158,31 +155,29 @@ fn get_total_remaining_for_channel(creator: &Address, accounting_spent: &Unified Ok(total_remaining) } -// async fn update_remaining_for_channel(redis: &MultiplexedConnection, id: ChannelId, amount: UnifiedNum) -> Result { -// let key = format!("adexChannel:remaining:{}", id); -// redis::cmd("SET") -// .arg(&key) -// .arg(amount.to_u64()) -// .query_async(&mut redis.clone()) -// .await?; -// Ok(true) -// } - -async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPool, campaigns: &Vec, mutated_campaign: &Campaign) -> Result { +async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign_id: CampaignId) -> Result, ResponseError> { let other_campaigns_remaining = campaigns .into_iter() - .filter(|c| c.id != mutated_campaign.id) + .filter(|c| c.id != mutated_campaign_id) .map(|c| async move { let spent = get_spent_for_campaign(&redis, c.id).await?; - let remaining = c.budget.checked_sub(&spent)?; + let remaining = c.budget.checked_sub(&spent).ok_or_else(|| { + ResponseError::Conflict("Error while calculating remaining for mutated campaign".to_string()) + })?; Ok(remaining) }) .collect::>(); let other_campaigns_remaining = join_all(other_campaigns_remaining).await; - // TODO: Fix Unwrap + let other_campaigns_remaining: Result, _> = other_campaigns_remaining.into_iter().collect(); + other_campaigns_remaining +} + + +async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: &Campaign) -> Result { + let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns, mutated_campaign.id).await?; let sum_of_campaigns_remaining = other_campaigns_remaining .into_iter() - .fold(UnifiedNum::from_u64(0), |mut sum, val| sum.checked_add(&val).unwrap()); + .try_fold(UnifiedNum::from_u64(0), |sum, val| sum.checked_add(&val).ok_or(ResponseError::Conflict("Couldn't sum remaining for campaigns".to_string())))?; // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB let spent_for_mutated_campaign = get_spent_for_campaign(&redis, mutated_campaign.id).await?; let remaining_for_mutated_campaign = mutated_campaign.budget.checked_sub(&spent_for_mutated_campaign).ok_or_else(|| { @@ -190,7 +185,7 @@ async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, pool: &DbPoo })?; sum_of_campaigns_remaining.checked_add(&remaining_for_mutated_campaign).ok_or_else(|| { ResponseError::Conflict("Error while calculating sum for all campaigns".to_string()) - }); + })?; Ok(sum_of_campaigns_remaining) } @@ -204,7 +199,7 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl return Err(ResponseError::FailedValidation("No more budget available for spending".into())); } - let old_remaining = get_remaining_for_campaign(&redis, campaign.id).await?; + let old_remaining = get_remaining_for_campaign_from_redis(&redis, campaign.id).await?; let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); let new_remaining = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { @@ -218,9 +213,9 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl update_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; // Gets the latest Spendable for this (spender, channelId) pair - let total_remaining = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable)?; + let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable)?; let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &pool, &campaigns_for_channel, &campaign).await?; + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, &campaign).await?; if campaigns_remaining_sum > total_remaining { return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".to_string())); } @@ -237,16 +232,15 @@ pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &Multipl #[cfg(test)] mod test { use primitives::{ - util::tests::prep_db::{DUMMY_CAMPAIGN, DUMMY_CHANNEL}, - Deposit + util::tests::prep_db::{DUMMY_CAMPAIGN}, + spender::Deposit, + Address }; - use deadpool::managed::Object; - use crate::{ db::redis_pool::{Manager, TESTS_POOL}, }; - + use std::convert::TryFrom; use super::*; async fn get_redis() -> Object { @@ -261,7 +255,7 @@ mod test { fn get_dummy_spendable(spender: Address) -> Spendable { Spendable { spender, - channel: DUMMY_CHANNEL.clone(), + channel: DUMMY_CAMPAIGN.channel.clone(), deposit: Deposit { total: UnifiedNum::from_u64(1_000_000), still_on_create2: UnifiedNum::from_u64(0), @@ -275,14 +269,14 @@ mod test { let accounting_spent = UnifiedNum::from_u64(100_000); let latest_spendable = get_dummy_spendable(campaign.creator); - let total_remaining = get_total_remaining_for_channel(&campaign.creator, &accounting_spent, &latest_spendable); + let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).expect("should calculate"); assert_eq!(total_remaining, UnifiedNum::from_u64(900_000)); } #[tokio::test] async fn does_it_update_remaining() { - let redis = get_redis().await; + let mut redis = get_redis().await; let campaign = get_campaign(); let key = format!("remaining:{}", campaign.id); @@ -290,7 +284,7 @@ mod test { redis::cmd("SET") .arg(&key) .arg(100u64) - .query_async(&mut redis.connection) + .query_async::<_, ()>(&mut redis.connection) .await .expect("should set"); @@ -302,20 +296,26 @@ mod test { let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); + assert_eq!(remaining.is_some(), true); + let remaining = remaining.expect("should get remaining"); + let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); assert_eq!(remaining, UnifiedNum::from_u64(500)); update_remaining_for_campaign(&redis, campaign.id, campaign.budget).await.expect("should increase"); let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); + assert_eq!(remaining.is_some(), true); + let remaining = remaining.expect("should get remaining"); + let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); let should_be_remaining = UnifiedNum::from_u64(500).checked_add(&campaign.budget).expect("should add"); assert_eq!(remaining, should_be_remaining); @@ -323,36 +323,35 @@ mod test { let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) - .await - .expect("should get remaining"); - - assert_eq!(remaining, should_be_remaining); - - update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(-500)).await.expect("should work"); - - let should_be_remaining = should_be_remaining.checked_sub(500).expect("should work"); - - let remaining = redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); + assert_eq!(remaining.is_some(), true); + let remaining = remaining.expect("should get remaining"); + let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); assert_eq!(remaining, should_be_remaining); } #[tokio::test] async fn update_remaining_before_it_is_set() { - let redis = get_redis().await; + let mut redis = get_redis().await; let campaign = get_campaign(); let key = format!("remaining:{}", campaign.id); let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) + .query_async::<_, Option>(&mut redis.connection) .await; - assert_eq!(remaining, Err(ResponseError::Conflict)) + assert_eq!(remaining.is_err(), true) } + + // test get_campaigns_remaining_sum + + // test get_remaining_for_multiple_campaigns + + // test get_remaining_for_campaign_from_redis + + // test get_spent_for_campaign } \ No newline at end of file From 067b3afff269be3044fcc2c3ae7d9ad01f7f169c Mon Sep 17 00:00:00 2001 From: simzzz Date: Tue, 29 Jun 2021 18:18:17 +0300 Subject: [PATCH 14/31] fixed remaining issues in PR --- primitives/src/sentry.rs | 37 +++-- sentry/src/db/campaign.rs | 41 +++-- sentry/src/db/event_aggregate.rs | 19 --- sentry/src/lib.rs | 24 ++- sentry/src/routes/campaign.rs | 263 ++++++++++++++++++------------- sentry/src/routes/channel.rs | 4 +- 6 files changed, 218 insertions(+), 170 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index 0a5edf6e7..cb2fe99c2 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -1,7 +1,6 @@ use crate::{ - targeting::Rules, validator::{ApproveState, Heartbeat, MessageTypes, NewState, Type as MessageType}, - Address, BalancesMap, BigNum, Channel, ChannelId, ValidatorId, IPFS, + Address, BigNum, Channel, ChannelId, ValidatorId, IPFS, }; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; @@ -367,16 +366,30 @@ pub mod campaign_create { } // All editable fields stored in one place, used for checking when a budget is changed - // #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] - // pub struct CampaignState { - // pub budget: UnifiedNum, - // pub validators: Validators, - // pub title: Option, - // pub pricing_bounds: Option, - // pub event_submission: Option, - // pub ad_units: Vec, - // pub targeting_rules: Rules, - // } + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] + pub struct ModifyCampaign { + pub budget: Option, + pub validators: Option, + pub title: Option, + pub pricing_bounds: Option, + pub event_submission: Option, + pub ad_units: Option>, + pub targeting_rules: Option, + } + + impl ModifyCampaign { + pub fn from_campaign(campaign: Campaign) -> Self { + ModifyCampaign { + budget: Some(campaign.budget), + validators: Some(campaign.validators), + title: campaign.title, + pricing_bounds: campaign.pricing_bounds, + event_submission: campaign.event_submission, + ad_units: Some(campaign.ad_units), + targeting_rules: Some(campaign.targeting_rules), + } + } + } } #[cfg(feature = "postgres")] diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 49f2dca11..4f4817e72 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -1,5 +1,5 @@ use crate::db::{DbPool, PoolError}; -use primitives::{Campaign, CampaignId}; +use primitives::{ChannelId, CampaignId, Campaign}; use tokio_postgres::types::Json; pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { @@ -48,32 +48,20 @@ pub async fn fetch_campaign( Ok(row.as_ref().map(Campaign::from)) } -pub async fn get_campaigns_for_channel( +pub async fn get_campaigns_by_channel( pool: &DbPool, - campaign: &Campaign, + channel_id: &ChannelId, ) -> Result, PoolError> { let client = pool.get().await?; let statement = client.prepare("SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to FROM campaigns WHERE channel_id = $1").await?; - let rows = client.query(&statement, &[&campaign.channel.id()]).await?; + let rows = client.query(&statement, &[&channel_id]).await?; let campaigns = rows.iter().map(Campaign::from).collect(); Ok(campaigns) } -pub async fn campaign_exists(pool: &DbPool, campaign: &Campaign) -> Result { - let client = pool.get().await?; - let statement = client - .prepare("SELECT EXISTS(SELECT 1 FROM campaigns WHERE id = $1)") - .await?; - - let row = client.execute(&statement, &[&campaign.id]).await?; - - let exists = row == 1; - Ok(exists) -} - // TODO: Test for campaign ad_units pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; @@ -104,8 +92,12 @@ pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { + assert!(true); + } + _ => assert!(false), + } let fetched_campaign = fetch_campaign(database.pool.clone(), &campaign_for_testing.id) .await diff --git a/sentry/src/db/event_aggregate.rs b/sentry/src/db/event_aggregate.rs index 106a47ccc..73b73d138 100644 --- a/sentry/src/db/event_aggregate.rs +++ b/sentry/src/db/event_aggregate.rs @@ -1,7 +1,6 @@ use chrono::{DateTime, Utc}; use futures::pin_mut; use primitives::{ - channel_v5::Channel as ChannelV5, sentry::{EventAggregate, MessageResponse}, validator::{ApproveState, Heartbeat, NewState}, Address, BigNum, Channel, ChannelId, ValidatorId, @@ -59,24 +58,6 @@ pub async fn latest_new_state( .map_err(PoolError::Backend) } -pub async fn latest_new_state_v5( - pool: &DbPool, - channel: &ChannelV5, - state_root: &str, -) -> Result>, PoolError> { - let client = pool.get().await?; - - let select = client.prepare("SELECT \"from\", msg, received FROM validator_messages WHERE channel_id = $1 AND \"from\" = $2 AND msg ->> 'type' = 'NewState' AND msg->> 'stateRoot' = $3 ORDER BY received DESC LIMIT 1").await?; - let rows = client - .query(&select, &[&channel.id(), &channel.leader, &state_root]) - .await?; - - rows.get(0) - .map(MessageResponse::::try_from) - .transpose() - .map_err(PoolError::Backend) -} - pub async fn latest_heartbeats( pool: &DbPool, channel_id: &ChannelId, diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index d9e13d387..842799c5d 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -1,7 +1,7 @@ #![deny(clippy::all)] #![deny(rust_2018_idioms)] -use crate::db::DbPool; +use crate::db::{DbPool, fetch_campaign}; use crate::routes::campaign; use crate::routes::channel::channel_status; use crate::routes::event_aggregate::list_channel_event_aggregates; @@ -18,17 +18,18 @@ use middleware::{campaign::CampaignLoad, Chain, Middleware}; use once_cell::sync::Lazy; use primitives::adapter::Adapter; use primitives::sentry::ValidationErrorResponse; -use primitives::{Config, ValidatorId}; +use primitives::{Config, ValidatorId, CampaignId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; -use routes::campaign::{create_campaign, update_campaign}; +use routes::campaign::{create_campaign, update_campaign::handle_route}; use routes::cfg::config; use routes::channel::{ channel_list, channel_validate, create_channel, create_validator_messages, last_approved, }; use slog::Logger; use std::collections::HashMap; +use std::str::FromStr; pub mod middleware; pub mod routes { @@ -63,7 +64,7 @@ lazy_static! { static ref PUBLISHER_ANALYTICS_BY_CHANNEL_ID: Regex = Regex::new(r"^/analytics/for-publisher/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); static ref CREATE_EVENTS_BY_CHANNEL_ID: Regex = Regex::new(r"^/channel/0x([a-zA-Z0-9]{64})/events/?$").expect("The regex should be valid"); static ref CAMPAIGN_UPDATE_BY_ID: Regex = - Regex::new(r"^/channel/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); + Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/?$").expect("The regex should be valid"); } static INSERT_EVENTS_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { @@ -194,7 +195,20 @@ async fn campaigns_router( let (path, method) = (req.uri().path(), req.method()); // create events - if let (Some(caps), &Method::POST) = (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) { + if let (Some(caps), &Method::POST) = (CAMPAIGN_UPDATE_BY_ID.captures(&path), method) { + let param = RouteParams(vec![caps + .get(1) + .map_or("".to_string(), |m| m.as_str().to_string())]); + + // Should be safe to access indice here + let campaign_id = param.get(0).ok_or_else(|| ResponseError::BadRequest("No CampaignId".to_string()))?; + let campaign_id = CampaignId::from_str(&campaign_id).map_err(|_| ResponseError::BadRequest("Bad CampaignId".to_string()))?; + + let campaign = fetch_campaign(app.pool.clone(), &campaign_id).await.map_err(|_| ResponseError::NotFound)?; + + req.extensions_mut().insert(campaign); + handle_route(req, app).await + } else if let (Some(caps), &Method::POST) = (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) { let param = RouteParams(vec![caps .get(1) .map_or("".to_string(), |m| m.as_str().to_string())]); diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index e396200f6..f7e08f8e0 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -3,40 +3,56 @@ use crate::{ db::{ spendable::fetch_spendable, accounting::get_accounting_spent, + campaign::{update_campaign, insert_campaign, get_campaigns_by_channel}, DbPool }, + routes::campaign::update_campaign::increase_remaining_for_campaign, + access::{self, check_access}, + Auth, Session }; + use hyper::{Body, Request, Response}; use primitives::{ adapter::Adapter, sentry::{ - campaign_create::CreateCampaign, + campaign_create::{CreateCampaign, ModifyCampaign}, + Event, SuccessResponse, }, spender::Spendable, Campaign, CampaignId, UnifiedNum, BigNum }; use redis::aio::MultiplexedConnection; use slog::error; -use crate::db::campaign::{update_campaign as update_campaign_db, insert_campaign, get_campaigns_for_channel}; use std::{ cmp::max, str::FromStr, }; use futures::future::join_all; use std::collections::HashMap; +use deadpool_postgres::PoolError; +use tokio_postgres::error::SqlState; +use redis::RedisError; -use crate::{ - access::{self, check_access}, - success_response, Application, Auth, ResponseError, Session, -}; use chrono::Utc; -use hyper::{Body, Request, Response}; -use primitives::{ - adapter::Adapter, - sentry::{campaign_create::CreateCampaign, Event, SuccessResponse}, - Campaign, -}; -use crate::routes::campaign::modify_campaign::{get_remaining_for_campaign_from_redis, get_campaigns_remaining_sum}; + +#[derive(Debug, PartialEq, Eq)] +pub enum CampaignError { + FailedUpdate(String), + CalculationError, + BudgetExceeded, +} + +impl From for CampaignError { + fn from(err: RedisError) -> Self { + CampaignError::FailedUpdate(err.to_string()) + } +} + +impl From for CampaignError { + fn from(err: PoolError) -> Self { + CampaignError::FailedUpdate(err.to_string()) + } +} pub async fn create_campaign( req: Request, @@ -57,136 +73,166 @@ pub async fn create_campaign( // TODO: AIP#61: Update when changes to Spendable are ready let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - let remaining_for_channel = get_total_remaining_for_channel(&accounting_spent, &latest_spendable)?; + let remaining_for_channel = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).ok_or(ResponseError::BadRequest("couldn't get total remaining for channel".to_string()))?; if campaign.budget > remaining_for_channel { - return Err(ResponseError::Conflict("Not Enough budget for campaign".to_string())); + return Err(ResponseError::BadRequest("Not Enough budget for campaign".to_string())); } // If the channel is being created, the amount spent is 0, therefore remaining = budget - update_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await?; + increase_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; // insert Campaign match insert_campaign(&app.pool, &campaign).await { Err(error) => { error!(&app.logger, "{}", &error; "module" => "create_campaign"); - return Err(ResponseError::Conflict("campaign already exists".to_string())); + match error { + PoolError::Backend(error) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) => { + Err(ResponseError::Conflict( + "Campaign already exists".to_string(), + )) + } + _ => Err(error_response), + } } - Ok(false) => Err(error_response), + Ok(false) => Err(ResponseError::BadRequest("Encountered error while creating Campaign; please try again".to_string())), _ => Ok(()), }?; Ok(success_response(serde_json::to_string(&campaign)?)) } -pub async fn update_campaign( - req: Request, - app: &Application, -) -> Result, ResponseError> { - let body = hyper::body::to_bytes(req.into_body()).await?; - let campaign = serde_json::from_slice::(&body) - .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; +// tested +fn get_total_remaining_for_channel(accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Option { + let total_deposited = latest_spendable.deposit.total; - let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); + let total_remaining = total_deposited.checked_sub(&accounting_spent); + total_remaining +} - // modify Campaign +pub mod update_campaign { + use super::*; + use lazy_static::lazy_static; - match modify_campaign(&app.pool, &campaign, &app.redis).await { - Err(error) => { - error!(&app.logger, "{:?}", &error; "module" => "update_campaign"); - return Err(ResponseError::Conflict("Error modifying campaign".to_string())); - } - Ok(false) => Err(error_response), - _ => Ok(()), - }?; + lazy_static!{ + pub static ref CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; + } - Ok(success_response(serde_json::to_string(&campaign)?)) -} + pub async fn increase_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); + redis::cmd("INCRBY") + .arg(&key) + .arg(amount.to_u64()) + .query_async(&mut redis.clone()) + .await?; + Ok(true) + } -pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, redis: &MultiplexedConnection) -> Result { - let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + pub async fn decrease_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); + redis::cmd("DECRBY") + .arg(&key) + .arg(amount.to_u64()) + .query_async(&mut redis.clone()) + .await?; + Ok(true) + } - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + pub async fn handle_route( + req: Request, + app: &Application, + ) -> Result, ResponseError> { + let campaign = req.extensions().get::().expect("We must have a campaign in extensions"); - let old_remaining = get_remaining_for_campaign_from_redis(&redis, campaign.id).await?; - let campaign_spent = campaign.budget.checked_sub(&old_remaining)?; + let modified_campaign = ModifyCampaign::from_campaign(campaign.clone()); - let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); + let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); - let new_remaining = campaign.budget.checked_sub(&campaign_spent).ok_or_else(|| { - ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) - })?; + // modify Campaign + modify_campaign(&app.pool, &campaign, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; - let diff_in_remaining = new_remaining.checked_sub(&old_remaining).ok_or_else(|| { - ResponseError::Conflict("Error while subtracting campaign_spent from budget".to_string()) - })?; + Ok(success_response(serde_json::to_string(&campaign)?)) + } - update_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; + pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, modified_campaign: &ModifyCampaign, redis: &MultiplexedConnection) -> Result { + // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent + // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] + // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 + let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - // Gets the latest Spendable for this (spender, channelId) pair - let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable)?; - let campaigns_for_channel = get_campaigns_for_channel(&pool, &campaign).await?; - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, &campaign).await?; - if campaigns_remaining_sum > total_remaining { - return Err(ResponseError::Conflict("Remaining for campaigns exceeds total remaining for channel".to_string())); - } + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - update_campaign_db(&pool, &campaign).await.map_err(|e| ResponseError::Conflict(e.to_string())) + let old_remaining = get_remaining_for_campaign_from_redis(&redis, campaign.id).await.ok_or(CampaignError::FailedUpdate("Couldn't get remaining for campaign".to_string()))?; - // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent - // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] - // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 -} + let campaign_spent = campaign.budget.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; + if campaign_spent >= campaign.budget { + return Err(CampaignError::BudgetExceeded); + } -// tested -async fn update_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { - // update a key in Redis for the remaining spendable amount - let key = format!("remaining:{}", id); - redis::cmd("INCRBY") - .arg(&key) - .arg(amount.to_u64()) - .query_async(&mut redis.clone()) - .await - .map_err(|_| ResponseError::Conflict("Error updating remainingSpendable for current campaign".to_string()))?; - Ok(true) -} + let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); -// tested -fn get_total_remaining_for_channel(accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Option { - let total_deposited = latest_spendable.deposit.total; + let new_remaining = campaign.budget.checked_sub(&campaign_spent).ok_or(CampaignError::CalculationError)?; - let total_remaining = total_deposited.checked_sub(&accounting_spent); - total_remaining -} + if new_remaining >= old_remaining { + let diff_in_remaining = new_remaining.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; + increase_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; + } else { + let diff_in_remaining = old_remaining.checked_sub(&new_remaining).ok_or(CampaignError::CalculationError)?; + decrease_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; + } -mod modify_campaign { - use super::*; - pub async fn get_remaining_for_campaign_from_redis(redis: &MultiplexedConnection, id: CampaignId) -> Result { - let key = format!("remaining:{}", id); + + // Gets the latest Spendable for this (spender, channelId) pair + let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).ok_or(CampaignError::FailedUpdate("Could not get total remaining for channel".to_string()))?; + let campaigns_for_channel = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, &campaign).await.map_err(|_| CampaignError::CalculationError)?; + if campaigns_remaining_sum > total_remaining { + return Err(CampaignError::BudgetExceeded); + } + + update_campaign(&pool, &campaign).await?; + + Ok(campaign.clone()) + } + + // TODO: #382 Remove after #412 is merged + fn get_unified_num_from_string(value: &str) -> Option { + let value_as_big_num: Option = BigNum::from_str(value).ok(); + let value_as_u64 = match value_as_big_num { + Some(num) => num.to_u64(), + _ => None, + }; + let value_as_unified = match value_as_u64 { + Some(num) => Some(UnifiedNum::from_u64(num)), + _ => None + }; + value_as_unified + } + + pub async fn get_remaining_for_campaign_from_redis(redis: &MultiplexedConnection, id: CampaignId) -> Option { + let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); let remaining = match redis::cmd("GET") .arg(&key) .query_async::<_, Option>(&mut redis.clone()) .await { Ok(Some(remaining)) => { - let res = BigNum::from_str(&remaining)?; - let res = res.to_u64().ok_or_else(|| { - ResponseError::Conflict("Error while calculating the total remaining amount".to_string()) - })?; - Ok(UnifiedNum::from_u64(res)) + // TODO: #382 Just parse from string once #412 is merged + get_unified_num_from_string(&remaining) }, - _ => Ok(UnifiedNum::from_u64(0)) + _ => None }; remaining } - async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign_id: CampaignId) -> Result, ResponseError> { + async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign_id: CampaignId) -> Result, CampaignError> { let other_campaigns_remaining = campaigns .into_iter() .filter(|c| c.id != mutated_campaign_id) + // TODO: Do 1 call with MGET .map(|c| async move { - let remaining = get_remaining_for_campaign_from_redis(&redis, c.id).await?; + let remaining = get_remaining_for_campaign_from_redis(&redis, c.id).await.ok_or(CampaignError::FailedUpdate("Couldn't get remaining for campaign".to_string()))?; Ok(remaining) }) .collect::>(); @@ -195,21 +241,17 @@ mod modify_campaign { other_campaigns_remaining } - pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: &Campaign) -> Result { + pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: &Campaign) -> Result { let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns, mutated_campaign.id).await?; let sum_of_campaigns_remaining = other_campaigns_remaining .into_iter() - .try_fold(UnifiedNum::from_u64(0), |sum, val| sum.checked_add(&val).ok_or(ResponseError::Conflict("Couldn't sum remaining for campaigns".to_string())))?; + .try_fold(UnifiedNum::from_u64(0), |sum, val| sum.checked_add(&val).ok_or(CampaignError::CalculationError))?; // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB - let old_remaining_for_mutated_campaign = get_remaining_for_campaign_from_redis(&redis, mutated_campaign.id); - let spent_for_mutated_campaign = mutated_campaign.budget.checked_sub(old_remaining_for_mutated_campaign); - let new_remaining_for_mutated_campaign = mutated_campaign.budget.checked_sub(&spent_for_mutated_campaign).ok_or_else(|| { - ResponseError::Conflict("Error while calculating remaining for mutated campaign".to_string()) - })?; - sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or_else(|| { - ResponseError::Conflict("Error while calculating sum for all campaigns".to_string()) - })?; + let old_remaining_for_mutated_campaign = get_remaining_for_campaign_from_redis(&redis, mutated_campaign.id).await.ok_or(CampaignError::CalculationError)?; + let spent_for_mutated_campaign = mutated_campaign.budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; + let new_remaining_for_mutated_campaign = mutated_campaign.budget.checked_sub(&spent_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; + sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; Ok(sum_of_campaigns_remaining) } } @@ -224,6 +266,7 @@ mod test { use deadpool::managed::Object; use crate::{ db::redis_pool::{Manager, TESTS_POOL}, + campaign::update_campaign::CAMPAIGN_REMAINING_KEY, }; use std::convert::TryFrom; use super::*; @@ -260,10 +303,10 @@ mod test { } #[tokio::test] - async fn does_it_update_remaining() { + async fn does_it_increase_remaining() { let mut redis = get_redis().await; let campaign = get_campaign(); - let key = format!("remaining:{}", campaign.id); + let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, campaign.id); // Setting the redis base variable redis::cmd("SET") @@ -275,8 +318,8 @@ mod test { // 2 async calls at once, should be 500 after them futures::future::join( - update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), - update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)) + increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), + increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)) ).await; let remaining = redis::cmd("GET") @@ -290,7 +333,7 @@ mod test { let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); assert_eq!(remaining, UnifiedNum::from_u64(500)); - update_remaining_for_campaign(&redis, campaign.id, campaign.budget).await.expect("should increase"); + increase_remaining_for_campaign(&redis, campaign.id, campaign.budget).await.expect("should increase"); let remaining = redis::cmd("GET") .arg(&key) @@ -304,7 +347,7 @@ mod test { let should_be_remaining = UnifiedNum::from_u64(500).checked_add(&campaign.budget).expect("should add"); assert_eq!(remaining, should_be_remaining); - update_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)).await.expect("should work"); + increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)).await.expect("should work"); let remaining = redis::cmd("GET") .arg(&key) @@ -322,7 +365,7 @@ mod test { async fn update_remaining_before_it_is_set() { let mut redis = get_redis().await; let campaign = get_campaign(); - let key = format!("remaining:{}", campaign.id); + let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, campaign.id); let remaining = redis::cmd("GET") .arg(&key) @@ -337,8 +380,6 @@ mod test { // test get_remaining_for_multiple_campaigns // test get_remaining_for_campaign_from_redis - - // test get_spent_for_campaign } pub async fn insert_events( req: Request, diff --git a/sentry/src/routes/channel.rs b/sentry/src/routes/channel.rs index 304bd3432..ca337b6f7 100644 --- a/sentry/src/routes/channel.rs +++ b/sentry/src/routes/channel.rs @@ -3,7 +3,7 @@ use crate::db::{ get_channel_by_id, insert_channel, insert_validator_messages, list_channels, update_exhausted_channel, PoolError, }; -use crate::{success_response, Application, Auth, ResponseError, RouteParams, Session}; +use crate::{success_response, Application, Auth, ResponseError, RouteParams}; use futures::future::try_join_all; use hex::FromHex; use hyper::{Body, Request, Response}; @@ -11,7 +11,7 @@ use primitives::{ adapter::Adapter, sentry::{ channel_list::{ChannelListQuery, LastApprovedQuery}, - Event, LastApproved, LastApprovedResponse, SuccessResponse, + LastApproved, LastApprovedResponse, SuccessResponse, }, validator::MessageTypes, Channel, ChannelId, From 9fd4f4add7f77a49bb8915ab7c58609709e4827e Mon Sep 17 00:00:00 2001 From: simzzz Date: Wed, 30 Jun 2021 11:43:21 +0300 Subject: [PATCH 15/31] Included MGET for getting multiple remainings at once --- sentry/src/routes/campaign.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index f7e08f8e0..5895a7035 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -227,18 +227,20 @@ pub mod update_campaign { } async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign_id: CampaignId) -> Result, CampaignError> { - let other_campaigns_remaining = campaigns + let keys: Vec = campaigns.into_iter().map(|c| format!("{}:{}", *CAMPAIGN_REMAINING_KEY, c.id)).collect(); + let remainings = redis::cmd("MGET") + .arg(keys) + .query_async::<_, Vec>>(&mut redis.clone()) + .await?; + + let remainings = remainings .into_iter() - .filter(|c| c.id != mutated_campaign_id) - // TODO: Do 1 call with MGET - .map(|c| async move { - let remaining = get_remaining_for_campaign_from_redis(&redis, c.id).await.ok_or(CampaignError::FailedUpdate("Couldn't get remaining for campaign".to_string()))?; - Ok(remaining) - }) - .collect::>(); - let other_campaigns_remaining = join_all(other_campaigns_remaining).await; - let other_campaigns_remaining: Result, _> = other_campaigns_remaining.into_iter().collect(); - other_campaigns_remaining + .flat_map(|r| r) + .map(|r| get_unified_num_from_string(&r)) + .flatten() + .collect(); + + Ok(remainings) } pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: &Campaign) -> Result { From 93800894d62f5ec8b5ae4c24b68bbd13d64e25e7 Mon Sep 17 00:00:00 2001 From: simzzz Date: Wed, 30 Jun 2021 12:03:15 +0300 Subject: [PATCH 16/31] fixed failing test --- sentry/src/routes/campaign.rs | 152 +++++++++++++++++----------------- 1 file changed, 77 insertions(+), 75 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 5895a7035..5b174f380 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -258,6 +258,79 @@ pub mod update_campaign { } } +pub async fn insert_events( + req: Request, + app: &Application, +) -> Result, ResponseError> { + let (req_head, req_body) = req.into_parts(); + + let auth = req_head.extensions.get::(); + let session = req_head + .extensions + .get::() + .expect("request should have session"); + + let campaign = req_head + .extensions + .get::() + .expect("request should have a Campaign loaded"); + + let body_bytes = hyper::body::to_bytes(req_body).await?; + let mut request_body = serde_json::from_slice::>>(&body_bytes)?; + + let events = request_body + .remove("events") + .ok_or_else(|| ResponseError::BadRequest("invalid request".to_string()))?; + + let processed = process_events(app, auth, session, campaign, events).await?; + + Ok(Response::builder() + .header("Content-type", "application/json") + .body(serde_json::to_string(&SuccessResponse { success: processed })?.into()) + .unwrap()) +} + +async fn process_events( + app: &Application, + auth: Option<&Auth>, + session: &Session, + campaign: &Campaign, + events: Vec, +) -> Result { + if &Utc::now() > &campaign.active.to { + return Err(ResponseError::BadRequest("Campaign is expired".into())); + } + + // + // TODO #381: AIP#61 Spender Aggregator should be called + // + + // handle events - check access + // handle events - Update targeting rules + // calculate payout + // distribute fees + // handle spending - Spender Aggregate + // handle events - aggregate Events and put into analytics + + check_access( + &app.redis, + session, + auth, + &app.config.ip_rate_limit, + &campaign, + &events, + ) + .await + .map_err(|e| match e { + access::Error::ForbiddenReferrer => ResponseError::Forbidden(e.to_string()), + access::Error::RulesError(error) => ResponseError::TooManyRequests(error), + access::Error::UnAuthenticated => ResponseError::Unauthorized, + _ => ResponseError::BadRequest(e.to_string()), + })?; + + Ok(true) +} + #[cfg(test)] mod test { use primitives::{ @@ -372,9 +445,10 @@ mod test { let remaining = redis::cmd("GET") .arg(&key) .query_async::<_, Option>(&mut redis.connection) - .await; + .await + .expect("should return None"); - assert_eq!(remaining.is_err(), true) + assert_eq!(remaining, None) } // test get_campaigns_remaining_sum @@ -382,76 +456,4 @@ mod test { // test get_remaining_for_multiple_campaigns // test get_remaining_for_campaign_from_redis -} -pub async fn insert_events( - req: Request, - app: &Application, -) -> Result, ResponseError> { - let (req_head, req_body) = req.into_parts(); - - let auth = req_head.extensions.get::(); - let session = req_head - .extensions - .get::() - .expect("request should have session"); - - let campaign = req_head - .extensions - .get::() - .expect("request should have a Campaign loaded"); - - let body_bytes = hyper::body::to_bytes(req_body).await?; - let mut request_body = serde_json::from_slice::>>(&body_bytes)?; - - let events = request_body - .remove("events") - .ok_or_else(|| ResponseError::BadRequest("invalid request".to_string()))?; - - let processed = process_events(app, auth, session, campaign, events).await?; - - Ok(Response::builder() - .header("Content-type", "application/json") - .body(serde_json::to_string(&SuccessResponse { success: processed })?.into()) - .unwrap()) -} - -async fn process_events( - app: &Application, - auth: Option<&Auth>, - session: &Session, - campaign: &Campaign, - events: Vec, -) -> Result { - if &Utc::now() > &campaign.active.to { - return Err(ResponseError::BadRequest("Campaign is expired".into())); - } - - // - // TODO #381: AIP#61 Spender Aggregator should be called - // - - // handle events - check access - // handle events - Update targeting rules - // calculate payout - // distribute fees - // handle spending - Spender Aggregate - // handle events - aggregate Events and put into analytics - - check_access( - &app.redis, - session, - auth, - &app.config.ip_rate_limit, - &campaign, - &events, - ) - .await - .map_err(|e| match e { - access::Error::ForbiddenReferrer => ResponseError::Forbidden(e.to_string()), - access::Error::RulesError(error) => ResponseError::TooManyRequests(error), - access::Error::UnAuthenticated => ResponseError::Unauthorized, - _ => ResponseError::BadRequest(e.to_string()), - })?; - - Ok(true) -} +} \ No newline at end of file From 219e3f01dd3989ab04dd12fa82c70d81ca0ba0a5 Mon Sep 17 00:00:00 2001 From: simzzz Date: Wed, 30 Jun 2021 14:24:55 +0300 Subject: [PATCH 17/31] cargofmt + some refactoring + used MutatedCampaign object in more places --- sentry/src/db/campaign.rs | 7 +++---- sentry/src/lib.rs | 26 +++++++++++++++----------- sentry/src/routes/campaign.rs | 33 +++++++++++++++++---------------- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 4f4817e72..3b143cd0e 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -1,5 +1,5 @@ use crate::db::{DbPool, PoolError}; -use primitives::{ChannelId, CampaignId, Campaign}; +use primitives::{Campaign, CampaignId, ChannelId}; use tokio_postgres::types::Json; pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { @@ -96,7 +96,7 @@ mod test { use crate::{ db::tests_postgres::{setup_test_migrations, DATABASE_POOL}, - ResponseError + ResponseError, }; use super::*; @@ -123,8 +123,7 @@ mod test { assert!(is_inserted); - let is_duplicate_inserted = insert_campaign(&database.pool, &campaign_for_testing) - .await; + let is_duplicate_inserted = insert_campaign(&database.pool, &campaign_for_testing).await; assert!(is_duplicate_inserted.is_err()); let insertion_error = is_duplicate_inserted.err().expect("should get error"); diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 842799c5d..69d203d7a 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -1,7 +1,7 @@ #![deny(clippy::all)] #![deny(rust_2018_idioms)] -use crate::db::{DbPool, fetch_campaign}; +use crate::db::{fetch_campaign, DbPool}; use crate::routes::campaign; use crate::routes::channel::channel_status; use crate::routes::event_aggregate::list_channel_event_aggregates; @@ -18,7 +18,7 @@ use middleware::{campaign::CampaignLoad, Chain, Middleware}; use once_cell::sync::Lazy; use primitives::adapter::Adapter; use primitives::sentry::ValidationErrorResponse; -use primitives::{Config, ValidatorId, CampaignId}; +use primitives::{CampaignId, Config, ValidatorId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; @@ -201,14 +201,21 @@ async fn campaigns_router( .map_or("".to_string(), |m| m.as_str().to_string())]); // Should be safe to access indice here - let campaign_id = param.get(0).ok_or_else(|| ResponseError::BadRequest("No CampaignId".to_string()))?; - let campaign_id = CampaignId::from_str(&campaign_id).map_err(|_| ResponseError::BadRequest("Bad CampaignId".to_string()))?; + let campaign_id = param + .get(0) + .ok_or_else(|| ResponseError::BadRequest("No CampaignId".to_string()))?; + let campaign_id = CampaignId::from_str(&campaign_id) + .map_err(|_| ResponseError::BadRequest("Bad CampaignId".to_string()))?; - let campaign = fetch_campaign(app.pool.clone(), &campaign_id).await.map_err(|_| ResponseError::NotFound)?; + let campaign = fetch_campaign(app.pool.clone(), &campaign_id) + .await + .map_err(|_| ResponseError::NotFound)?; req.extensions_mut().insert(campaign); handle_route(req, app).await - } else if let (Some(caps), &Method::POST) = (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) { + } else if let (Some(caps), &Method::POST) = + (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) + { let param = RouteParams(vec![caps .get(1) .map_or("".to_string(), |m| m.as_str().to_string())]); @@ -245,8 +252,6 @@ async fn analytics_router( ) -> Result, ResponseError> { let (route, method) = (req.uri().path(), req.method()); - - // TODO AIP#61: Add routes for: // - POST /channel/:id/pay // #[serde(rename_all = "camelCase")] @@ -323,9 +328,8 @@ async fn channels_router( req.extensions_mut().insert(param); insert_events(req, app).await - } else */ - if let (Some(caps), &Method::GET) = (LAST_APPROVED_BY_CHANNEL_ID.captures(&path), method) - { + } else */ + if let (Some(caps), &Method::GET) = (LAST_APPROVED_BY_CHANNEL_ID.captures(&path), method) { let param = RouteParams(vec![caps .get(1) .map_or("".to_string(), |m| m.as_str().to_string())]); diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 5b174f380..79dd84bfb 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -21,17 +21,18 @@ use primitives::{ spender::Spendable, Campaign, CampaignId, UnifiedNum, BigNum }; -use redis::aio::MultiplexedConnection; +use redis::{ + aio::MultiplexedConnection, + RedisError, +}; use slog::error; use std::{ cmp::max, str::FromStr, + collections::HashMap, }; -use futures::future::join_all; -use std::collections::HashMap; use deadpool_postgres::PoolError; use tokio_postgres::error::SqlState; -use redis::RedisError; use chrono::Utc; @@ -147,8 +148,6 @@ pub mod update_campaign { let modified_campaign = ModifyCampaign::from_campaign(campaign.clone()); - let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); - // modify Campaign modify_campaign(&app.pool, &campaign, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; @@ -159,20 +158,22 @@ pub mod update_campaign { // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 + + let new_budget = modified_campaign.budget.ok_or(CampaignError::FailedUpdate("Couldn't get new budget".to_string()))?; let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; let old_remaining = get_remaining_for_campaign_from_redis(&redis, campaign.id).await.ok_or(CampaignError::FailedUpdate("Couldn't get remaining for campaign".to_string()))?; - let campaign_spent = campaign.budget.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; - if campaign_spent >= campaign.budget { + let campaign_spent = new_budget.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; + if campaign_spent >= new_budget { return Err(CampaignError::BudgetExceeded); } let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); - let new_remaining = campaign.budget.checked_sub(&campaign_spent).ok_or(CampaignError::CalculationError)?; + let new_remaining = new_budget.checked_sub(&campaign_spent).ok_or(CampaignError::CalculationError)?; if new_remaining >= old_remaining { let diff_in_remaining = new_remaining.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; @@ -186,7 +187,7 @@ pub mod update_campaign { // Gets the latest Spendable for this (spender, channelId) pair let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).ok_or(CampaignError::FailedUpdate("Could not get total remaining for channel".to_string()))?; let campaigns_for_channel = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, &campaign).await.map_err(|_| CampaignError::CalculationError)?; + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, campaign.id, &new_budget).await.map_err(|_| CampaignError::CalculationError)?; if campaigns_remaining_sum > total_remaining { return Err(CampaignError::BudgetExceeded); } @@ -226,7 +227,7 @@ pub mod update_campaign { remaining } - async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign_id: CampaignId) -> Result, CampaignError> { + async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign]) -> Result, CampaignError> { let keys: Vec = campaigns.into_iter().map(|c| format!("{}:{}", *CAMPAIGN_REMAINING_KEY, c.id)).collect(); let remainings = redis::cmd("MGET") .arg(keys) @@ -243,16 +244,16 @@ pub mod update_campaign { Ok(remainings) } - pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: &Campaign) -> Result { - let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns, mutated_campaign.id).await?; + pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: CampaignId, new_budget: &UnifiedNum) -> Result { + let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns).await?; let sum_of_campaigns_remaining = other_campaigns_remaining .into_iter() .try_fold(UnifiedNum::from_u64(0), |sum, val| sum.checked_add(&val).ok_or(CampaignError::CalculationError))?; // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB - let old_remaining_for_mutated_campaign = get_remaining_for_campaign_from_redis(&redis, mutated_campaign.id).await.ok_or(CampaignError::CalculationError)?; - let spent_for_mutated_campaign = mutated_campaign.budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; - let new_remaining_for_mutated_campaign = mutated_campaign.budget.checked_sub(&spent_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; + let old_remaining_for_mutated_campaign = get_remaining_for_campaign_from_redis(&redis, mutated_campaign).await.ok_or(CampaignError::CalculationError)?; + let spent_for_mutated_campaign = new_budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; + let new_remaining_for_mutated_campaign = new_budget.checked_sub(&spent_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; Ok(sum_of_campaigns_remaining) } From ad2d585eec05dc6c37ed3a20ec16db005e6d89c3 Mon Sep 17 00:00:00 2001 From: simzzz Date: Tue, 6 Jul 2021 18:36:21 +0300 Subject: [PATCH 18/31] Finished with PR changes requested --- sentry/src/db/campaign.rs | 118 +++++++++++--- sentry/src/lib.rs | 10 +- sentry/src/routes/campaign.rs | 297 ++++++++++++++++------------------ 3 files changed, 239 insertions(+), 186 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 3b143cd0e..28b04a41e 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -1,12 +1,12 @@ use crate::db::{DbPool, PoolError}; -use primitives::{Campaign, CampaignId, ChannelId}; +use primitives::{sentry::campaign_create::ModifyCampaign, Campaign, CampaignId, ChannelId}; use tokio_postgres::types::Json; pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let ad_units = Json(campaign.ad_units.clone()); let stmt = client.prepare("INSERT INTO campaigns (id, channel_id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to) values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)").await?; - let row = client + let inserted = client .execute( &stmt, &[ @@ -28,7 +28,7 @@ pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result Result { +pub async fn update_campaign( + pool: &DbPool, + campaign: &Campaign, + modified_campaign: &ModifyCampaign, +) -> Result { let client = pool.get().await?; let statement = client .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") .await?; - let row = client + let ad_units = &modified_campaign + .ad_units + .as_ref() + .unwrap_or(&campaign.ad_units); + let ad_units = Json(ad_units); + + let updated_rows = client .execute( &statement, &[ - &campaign.budget, - &campaign.validators, - &campaign.title, - &campaign.pricing_bounds, - &campaign.event_submission, - &campaign.ad_units, - &campaign.targeting_rules, + &modified_campaign.budget.unwrap_or(campaign.budget), + &modified_campaign + .validators + .as_ref() + .unwrap_or(&campaign.validators), + &modified_campaign + .title + .as_ref() + .ok_or_else(|| &campaign.title) + .unwrap(), + &modified_campaign + .pricing_bounds + .as_ref() + .ok_or_else(|| &campaign.title) + .unwrap(), + &modified_campaign + .event_submission + .as_ref() + .ok_or_else(|| &campaign.title) + .unwrap(), + &ad_units, + &modified_campaign + .targeting_rules + .as_ref() + .unwrap_or(&campaign.targeting_rules), &campaign.id, ], ) .await?; - let exists = row == 1; + let exists = updated_rows == 1; Ok(exists) } #[cfg(test)] mod test { - use primitives::util::tests::prep_db::DUMMY_CAMPAIGN; + use primitives::{ + util::tests::prep_db::{DUMMY_CAMPAIGN, DUMMY_AD_UNITS}, + event_submission::{Rule, RateLimit}, + targeting::Rules, + UnifiedNum, EventSubmission, + }; + use primitives::campaign; + use std::time::Duration; use tokio_postgres::error::SqlState; use crate::{ @@ -125,14 +159,10 @@ mod test { let is_duplicate_inserted = insert_campaign(&database.pool, &campaign_for_testing).await; - assert!(is_duplicate_inserted.is_err()); - let insertion_error = is_duplicate_inserted.err().expect("should get error"); - match insertion_error { - PoolError::Backend(error) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) => { - assert!(true); - } - _ => assert!(false), - } + assert!(matches!( + is_duplicate_inserted, + Err(PoolError::Backend(error)) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) + )); let fetched_campaign = fetch_campaign(database.pool.clone(), &campaign_for_testing.id) .await @@ -140,4 +170,46 @@ mod test { assert_eq!(Some(campaign_for_testing), fetched_campaign); } + + #[tokio::test] + async fn it_updates_campaign() { + let database = DATABASE_POOL.get().await.expect("Should get a DB pool"); + + setup_test_migrations(database.pool.clone()) + .await + .expect("Migrations should succeed"); + + let campaign_for_testing = DUMMY_CAMPAIGN.clone(); + + let is_inserted = insert_campaign(&database.pool, &campaign_for_testing) + .await + .expect("Should succeed"); + + assert!(is_inserted); + + let rule = Rule { + uids: None, + rate_limit: Some(RateLimit { + limit_type: "sid".to_string(), + time_frame: Duration::from_millis(20_000), + }), + }; + let new_budget = campaign_for_testing.budget + UnifiedNum::from_u64(1_000_000_000); + let modified_campaign = ModifyCampaign { + // pub budget: Option, + budget: Some(new_budget), + validators: None, + title: Some("Modified Campaign".to_string()), + pricing_bounds: Some(campaign::PricingBounds { + impression: Some(campaign::Pricing { min: 1.into(), max: 10.into()}), + click: Some(campaign::Pricing { min: 0.into(), max: 0.into()}) + }), + event_submission: Some(EventSubmission { allow: vec![rule] }), + ad_units: Some(DUMMY_AD_UNITS.to_vec()), + targeting_rules: Some(Rules::new()), + }; + + let is_campaign_updated = update_campaign(&database.pool, &campaign_for_testing, &modified_campaign).await.expect("should update"); + assert!(is_campaign_updated); + } } diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index 69d203d7a..f98886b5a 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -22,7 +22,7 @@ use primitives::{CampaignId, Config, ValidatorId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; -use routes::campaign::{create_campaign, update_campaign::handle_route}; +use routes::campaign::{create_campaign, update_campaign}; use routes::cfg::config; use routes::channel::{ channel_list, channel_validate, create_channel, create_validator_messages, last_approved, @@ -63,8 +63,6 @@ lazy_static! { static ref ADVERTISER_ANALYTICS_BY_CHANNEL_ID: Regex = Regex::new(r"^/analytics/for-advertiser/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); static ref PUBLISHER_ANALYTICS_BY_CHANNEL_ID: Regex = Regex::new(r"^/analytics/for-publisher/0x([a-zA-Z0-9]{64})/?$").expect("The regex should be valid"); static ref CREATE_EVENTS_BY_CHANNEL_ID: Regex = Regex::new(r"^/channel/0x([a-zA-Z0-9]{64})/events/?$").expect("The regex should be valid"); - static ref CAMPAIGN_UPDATE_BY_ID: Regex = - Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/?$").expect("The regex should be valid"); } static INSERT_EVENTS_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { @@ -73,6 +71,9 @@ static INSERT_EVENTS_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { static CLOSE_CAMPAIGN_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/close/?$").expect("The regex should be valid") }); +static CAMPAIGN_UPDATE_BY_ID: Lazy = Lazy::new(|| { + Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/?$").expect("The regex should be valid") +}); #[derive(Debug, Clone)] pub struct RouteParams(pub Vec); @@ -194,7 +195,6 @@ async fn campaigns_router( ) -> Result, ResponseError> { let (path, method) = (req.uri().path(), req.method()); - // create events if let (Some(caps), &Method::POST) = (CAMPAIGN_UPDATE_BY_ID.captures(&path), method) { let param = RouteParams(vec![caps .get(1) @@ -212,7 +212,7 @@ async fn campaigns_router( .map_err(|_| ResponseError::NotFound)?; req.extensions_mut().insert(campaign); - handle_route(req, app).await + update_campaign::handle_route(req, app).await } else if let (Some(caps), &Method::POST) = (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) { diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 79dd84bfb..8b0e5867a 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -6,7 +6,7 @@ use crate::{ campaign::{update_campaign, insert_campaign, get_campaigns_by_channel}, DbPool }, - routes::campaign::update_campaign::increase_remaining_for_campaign, + routes::campaign::update_campaign::set_initial_remaining_for_campaign, access::{self, check_access}, Auth, Session }; @@ -18,8 +18,8 @@ use primitives::{ campaign_create::{CreateCampaign, ModifyCampaign}, Event, SuccessResponse, }, - spender::Spendable, - Campaign, CampaignId, UnifiedNum, BigNum + campaign_validator::Validator, + Address, Campaign, CampaignId, UnifiedNum }; use redis::{ aio::MultiplexedConnection, @@ -27,38 +27,42 @@ use redis::{ }; use slog::error; use std::{ - cmp::max, - str::FromStr, + cmp::{max, Ordering}, collections::HashMap, + convert::TryInto, }; use deadpool_postgres::PoolError; use tokio_postgres::error::SqlState; use chrono::Utc; +use thiserror::Error; -#[derive(Debug, PartialEq, Eq)] -pub enum CampaignError { +#[derive(Debug, Error)] +pub enum Error { + #[error("Error while updating campaign: {0}")] FailedUpdate(String), - CalculationError, + #[error("Error while performing calculations")] + Calculation, + #[error("Error: Budget has been exceeded")] BudgetExceeded, -} - -impl From for CampaignError { - fn from(err: RedisError) -> Self { - CampaignError::FailedUpdate(err.to_string()) - } -} - -impl From for CampaignError { - fn from(err: PoolError) -> Self { - CampaignError::FailedUpdate(err.to_string()) - } + #[error("Error with new budget: {0}")] + NewBudget(String), + #[error("Redis error: {0}")] + Redis(#[from] RedisError), + #[error("DB Pool error: {0}")] + Pool(#[from] PoolError) } pub async fn create_campaign( req: Request, app: &Application, ) -> Result, ResponseError> { + let session = req + .extensions() + .get::() + .expect("request should have session") + .to_owned(); + let body = hyper::body::to_bytes(req.into_body()).await?; let campaign = serde_json::from_slice::(&body) @@ -66,7 +70,12 @@ pub async fn create_campaign( // create the actual `Campaign` with random `CampaignId` .into_campaign(); - // TODO AIP#61: Validate Campaign + campaign.validate(&app.config, &app.adapter.whoami()).map_err(|_| ResponseError::FailedValidation("couldn't valdiate campaign".to_string()))?; + + // TODO: Just use session.uid once it's address + if Address::from_bytes(session.uid.as_bytes()) != campaign.creator { + return Err(ResponseError::Forbidden("Request not sent by campaign creator".to_string())) + } let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); @@ -74,14 +83,16 @@ pub async fn create_campaign( // TODO: AIP#61: Update when changes to Spendable are ready let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - let remaining_for_channel = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).ok_or(ResponseError::BadRequest("couldn't get total remaining for channel".to_string()))?; + let total_deposited = latest_spendable.deposit.total; + + let remaining_for_channel = total_deposited.checked_sub(&accounting_spent).ok_or(ResponseError::FailedValidation("couldn't calculate remaining for channel".to_string()))?; if campaign.budget > remaining_for_channel { return Err(ResponseError::BadRequest("Not Enough budget for campaign".to_string())); } - // If the channel is being created, the amount spent is 0, therefore remaining = budget - increase_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; + // If the campaign is being created, the amount spent is 0, therefore remaining = budget + set_initial_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; // insert Campaign match insert_campaign(&app.pool, &campaign).await { @@ -103,26 +114,15 @@ pub async fn create_campaign( Ok(success_response(serde_json::to_string(&campaign)?)) } - -// tested -fn get_total_remaining_for_channel(accounting_spent: &UnifiedNum, latest_spendable: &Spendable) -> Option { - let total_deposited = latest_spendable.deposit.total; - - let total_remaining = total_deposited.checked_sub(&accounting_spent); - total_remaining -} - pub mod update_campaign { use super::*; - use lazy_static::lazy_static; + use crate::fetch_campaign; - lazy_static!{ - pub static ref CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; - } + pub const CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; - pub async fn increase_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { - let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); - redis::cmd("INCRBY") + pub async fn set_initial_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); + redis::cmd("SETNX") .arg(&key) .arg(amount.to_u64()) .query_async(&mut redis.clone()) @@ -130,9 +130,9 @@ pub mod update_campaign { Ok(true) } - pub async fn decrease_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { - let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); - redis::cmd("DECRBY") + pub async fn increase_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); + redis::cmd("INCRBY") .arg(&key) .arg(amount.to_u64()) .query_async(&mut redis.clone()) @@ -140,121 +140,143 @@ pub mod update_campaign { Ok(true) } + pub async fn decrease_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Option { + let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); + let value = match redis::cmd("DECRBY") + .arg(&key) + .arg(amount.to_u64()) + .query_async::<_, Option>(&mut redis.clone()) + .await + { + Ok(Some(remaining)) => { + // Can't be less than 0 due to max() + Some(UnifiedNum::from_u64(max(0, remaining).try_into().unwrap())) + }, + _ => None + }; + value + } + pub async fn handle_route( req: Request, app: &Application, ) -> Result, ResponseError> { let campaign = req.extensions().get::().expect("We must have a campaign in extensions"); - let modified_campaign = ModifyCampaign::from_campaign(campaign.clone()); + // Getting the campaign as it was before the update operation, should exist + let campaign_being_mutated = fetch_campaign(app.pool.clone(), &campaign.id).await?.ok_or(ResponseError::NotFound)?; + // modify Campaign - modify_campaign(&app.pool, &campaign, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; + modify_campaign(&app.pool, &campaign_being_mutated, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; Ok(success_response(serde_json::to_string(&campaign)?)) } - pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, modified_campaign: &ModifyCampaign, redis: &MultiplexedConnection) -> Result { + pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, modified_campaign: &ModifyCampaign, redis: &MultiplexedConnection) -> Result { // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 - let new_budget = modified_campaign.budget.ok_or(CampaignError::FailedUpdate("Couldn't get new budget".to_string()))?; - let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - - let old_remaining = get_remaining_for_campaign_from_redis(&redis, campaign.id).await.ok_or(CampaignError::FailedUpdate("Couldn't get remaining for campaign".to_string()))?; + if let Some(new_budget) = modified_campaign.budget { + let old_remaining = get_remaining_for_campaign(&redis, campaign.id).await?.ok_or(Error::FailedUpdate("No remaining entry for campaign".to_string()))?; - let campaign_spent = new_budget.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; - if campaign_spent >= new_budget { - return Err(CampaignError::BudgetExceeded); - } + let campaign_spent = campaign.budget.checked_sub(&old_remaining).ok_or(Error::Calculation)?; + if campaign_spent >= new_budget { + return Err(Error::NewBudget("New budget should be greater than the spent amount".to_string())); + } - let old_remaining = UnifiedNum::from_u64(max(0, old_remaining.to_u64())); + // Separate variable for clarity + let old_budget = campaign.budget; - let new_remaining = new_budget.checked_sub(&campaign_spent).ok_or(CampaignError::CalculationError)?; + match new_budget.cmp(&old_budget) { + Ordering::Greater | Ordering::Equal => { + let new_remaining = old_remaining.checked_add(&new_budget.checked_sub(&old_budget).ok_or(Error::Calculation)?).ok_or(Error::Calculation)?; + let amount_to_incr = new_remaining.checked_sub(&old_remaining).ok_or(Error::Calculation)?; + increase_remaining_for_campaign(&redis, campaign.id, amount_to_incr).await?; + }, + Ordering::Less => { + let new_remaining = old_remaining.checked_add(&old_budget.checked_sub(&new_budget).ok_or(Error::Calculation)?).ok_or(Error::Calculation)?; + let amount_to_decr = new_remaining.checked_sub(&old_remaining).ok_or(Error::Calculation)?; + let decreased_remaining = decrease_remaining_for_campaign(&redis, campaign.id, amount_to_decr).await.ok_or(Error::FailedUpdate("Could't decrease remaining".to_string()))?; + // If it goes below 0 it will still return 0 + if decreased_remaining.eq(&UnifiedNum::from_u64(0)) { + return Err(Error::NewBudget("No budget remaining after decreasing".to_string())); + } + } + } + }; - if new_remaining >= old_remaining { - let diff_in_remaining = new_remaining.checked_sub(&old_remaining).ok_or(CampaignError::CalculationError)?; - increase_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; - } else { - let diff_in_remaining = old_remaining.checked_sub(&new_remaining).ok_or(CampaignError::CalculationError)?; - decrease_remaining_for_campaign(&redis, campaign.id, diff_in_remaining).await?; - } + let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; // Gets the latest Spendable for this (spender, channelId) pair - let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).ok_or(CampaignError::FailedUpdate("Could not get total remaining for channel".to_string()))?; + let total_deposited = latest_spendable.deposit.total; + + let total_remaining = total_deposited.checked_sub(&accounting_spent).ok_or(Error::Calculation)?; let campaigns_for_channel = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, campaign.id, &new_budget).await.map_err(|_| CampaignError::CalculationError)?; + // campaign.budget should 100% exist, therefore it should be safe to just unwrap? + let current_campaign_budget = modified_campaign.budget.ok_or(campaign.budget).unwrap(); + let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, campaign.id, ¤t_campaign_budget).await.map_err(|_| Error::Calculation)?; if campaigns_remaining_sum > total_remaining { - return Err(CampaignError::BudgetExceeded); + return Err(Error::BudgetExceeded); } - update_campaign(&pool, &campaign).await?; + update_campaign(&pool, &campaign, &modified_campaign).await?; Ok(campaign.clone()) } - // TODO: #382 Remove after #412 is merged - fn get_unified_num_from_string(value: &str) -> Option { - let value_as_big_num: Option = BigNum::from_str(value).ok(); - let value_as_u64 = match value_as_big_num { - Some(num) => num.to_u64(), - _ => None, - }; - let value_as_unified = match value_as_u64 { - Some(num) => Some(UnifiedNum::from_u64(num)), - _ => None - }; - value_as_unified - } - - pub async fn get_remaining_for_campaign_from_redis(redis: &MultiplexedConnection, id: CampaignId) -> Option { - let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, id); + pub async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result, RedisError> { + let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); let remaining = match redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) - .await - { + .query_async::<_, Option>(&mut redis.clone()) + .await { Ok(Some(remaining)) => { - // TODO: #382 Just parse from string once #412 is merged - get_unified_num_from_string(&remaining) + // Can't be negative due to max() + Some(UnifiedNum::from_u64(max(0, remaining).try_into().unwrap())) }, - _ => None + Ok(None) => None, + Err(e) => return Err(e), }; - remaining + Ok(remaining) } - async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign]) -> Result, CampaignError> { - let keys: Vec = campaigns.into_iter().map(|c| format!("{}:{}", *CAMPAIGN_REMAINING_KEY, c.id)).collect(); + async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign]) -> Result, Error> { + let keys: Vec = campaigns.iter().map(|c| format!("{}:{}", CAMPAIGN_REMAINING_KEY, c.id)).collect(); let remainings = redis::cmd("MGET") .arg(keys) - .query_async::<_, Vec>>(&mut redis.clone()) + .query_async::<_, Vec>>(&mut redis.clone()) .await?; let remainings = remainings .into_iter() - .flat_map(|r| r) - .map(|r| get_unified_num_from_string(&r)) - .flatten() + .map(|r| { + match r { + // Can't be negative due to max() + Some(remaining) => UnifiedNum::from_u64(max(0, remaining).try_into().unwrap()), + None => UnifiedNum::from_u64(0) + } + }) .collect(); Ok(remainings) } - pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: CampaignId, new_budget: &UnifiedNum) -> Result { + pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: CampaignId, new_budget: &UnifiedNum) -> Result { let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns).await?; let sum_of_campaigns_remaining = other_campaigns_remaining - .into_iter() - .try_fold(UnifiedNum::from_u64(0), |sum, val| sum.checked_add(&val).ok_or(CampaignError::CalculationError))?; + .iter() + .sum::>() + .ok_or(Error::Calculation)?; // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB - let old_remaining_for_mutated_campaign = get_remaining_for_campaign_from_redis(&redis, mutated_campaign).await.ok_or(CampaignError::CalculationError)?; - let spent_for_mutated_campaign = new_budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; - let new_remaining_for_mutated_campaign = new_budget.checked_sub(&spent_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; - sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or(CampaignError::CalculationError)?; + let old_remaining_for_mutated_campaign = get_remaining_for_campaign(&redis, mutated_campaign).await?.ok_or(Error::FailedUpdate("No remaining entry for campaign".to_string()))?; + let spent_for_mutated_campaign = new_budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(Error::Calculation)?; + let new_remaining_for_mutated_campaign = new_budget.checked_sub(&spent_for_mutated_campaign).ok_or(Error::Calculation)?; + sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or(Error::Calculation)?; Ok(sum_of_campaigns_remaining) } } @@ -336,30 +358,21 @@ async fn process_events( mod test { use primitives::{ util::tests::prep_db::{DUMMY_CAMPAIGN}, - spender::Deposit, + spender::{Deposit, Spendable}, Address }; use deadpool::managed::Object; use crate::{ db::redis_pool::{Manager, TESTS_POOL}, - campaign::update_campaign::CAMPAIGN_REMAINING_KEY, + campaign::update_campaign::{CAMPAIGN_REMAINING_KEY, increase_remaining_for_campaign}, }; use std::convert::TryFrom; use super::*; - async fn get_redis() -> Object { - let connection = TESTS_POOL.get().await.expect("Should return Object"); - connection - } - - fn get_campaign() -> Campaign { - DUMMY_CAMPAIGN.clone() - } - - fn get_dummy_spendable(spender: Address) -> Spendable { + fn get_dummy_spendable(spender: Address, campaign: Campaign) -> Spendable { Spendable { spender, - channel: DUMMY_CAMPAIGN.channel.clone(), + channel: campaign.channel.clone(), deposit: Deposit { total: UnifiedNum::from_u64(1_000_000), still_on_create2: UnifiedNum::from_u64(0), @@ -367,22 +380,11 @@ mod test { } } - #[tokio::test] - async fn does_it_get_total_remaianing() { - let campaign = get_campaign(); - let accounting_spent = UnifiedNum::from_u64(100_000); - let latest_spendable = get_dummy_spendable(campaign.creator); - - let total_remaining = get_total_remaining_for_channel(&accounting_spent, &latest_spendable).expect("should calculate"); - - assert_eq!(total_remaining, UnifiedNum::from_u64(900_000)); - } - #[tokio::test] async fn does_it_increase_remaining() { - let mut redis = get_redis().await; - let campaign = get_campaign(); - let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, campaign.id); + let mut redis = TESTS_POOL.get().await.expect("Should return Object"); + let campaign = DUMMY_CAMPAIGN.clone(); + let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, campaign.id); // Setting the redis base variable redis::cmd("SET") @@ -413,15 +415,15 @@ mod test { let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.connection) + // Directly parsing to u64 as we know it will be >0 + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); assert_eq!(remaining.is_some(), true); - let remaining = remaining.expect("should get remaining"); - let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); - let should_be_remaining = UnifiedNum::from_u64(500).checked_add(&campaign.budget).expect("should add"); - assert_eq!(remaining, should_be_remaining); + let remaining = remaining.expect("should get result out of the option"); + let should_be_remaining = UnifiedNum::from_u64(500) + campaign.budget; + assert_eq!(UnifiedNum::from_u64(remaining), should_be_remaining); increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)).await.expect("should work"); @@ -436,25 +438,4 @@ mod test { let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); assert_eq!(remaining, should_be_remaining); } - - #[tokio::test] - async fn update_remaining_before_it_is_set() { - let mut redis = get_redis().await; - let campaign = get_campaign(); - let key = format!("{}:{}", *CAMPAIGN_REMAINING_KEY, campaign.id); - - let remaining = redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.connection) - .await - .expect("should return None"); - - assert_eq!(remaining, None) - } - - // test get_campaigns_remaining_sum - - // test get_remaining_for_multiple_campaigns - - // test get_remaining_for_campaign_from_redis } \ No newline at end of file From 4f238614917f18355cd06c6c54f26eece0203d44 Mon Sep 17 00:00:00 2001 From: simzzz Date: Fri, 9 Jul 2021 21:23:58 +0300 Subject: [PATCH 19/31] PR changes --- primitives/src/sentry.rs | 18 ++++++++++ sentry/src/db/campaign.rs | 47 ++++++++---------------- sentry/src/lib.rs | 29 ++++----------- sentry/src/routes/campaign.rs | 68 +++++++++++++++++------------------ 4 files changed, 72 insertions(+), 90 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index cb2fe99c2..862a1eaef 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -389,6 +389,24 @@ pub mod campaign_create { targeting_rules: Some(campaign.targeting_rules), } } + + pub fn apply(&self, campaign: &Campaign) -> Campaign { + let campaign = campaign.clone(); + Campaign { + id: campaign.id, + channel: campaign.channel, + creator: campaign.creator, + budget: self.budget.unwrap_or(campaign.budget), + validators: self.validators.clone().unwrap_or(campaign.validators), + title: self.title.clone().or(campaign.title), + pricing_bounds: self.pricing_bounds.clone().or(campaign.pricing_bounds), + event_submission: self.event_submission.clone().or(campaign.event_submission), + ad_units: self.ad_units.clone().unwrap_or(campaign.ad_units), + targeting_rules: self.targeting_rules.clone().unwrap_or(campaign.targeting_rules), + created: campaign.created, + active: campaign.active, + } + } } } diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 28b04a41e..75989a420 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -1,5 +1,5 @@ use crate::db::{DbPool, PoolError}; -use primitives::{sentry::campaign_create::ModifyCampaign, Campaign, CampaignId, ChannelId}; +use primitives::{Campaign, CampaignId, ChannelId}; use tokio_postgres::types::Json; pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { @@ -48,6 +48,7 @@ pub async fn fetch_campaign( Ok(row.as_ref().map(Campaign::from)) } +// TODO: We might need to use LIMIT to implement pagination pub async fn get_campaigns_by_channel( pool: &DbPool, channel_id: &ChannelId, @@ -65,48 +66,26 @@ pub async fn get_campaigns_by_channel( pub async fn update_campaign( pool: &DbPool, campaign: &Campaign, - modified_campaign: &ModifyCampaign, ) -> Result { let client = pool.get().await?; let statement = client .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") .await?; - let ad_units = &modified_campaign - .ad_units - .as_ref() - .unwrap_or(&campaign.ad_units); - let ad_units = Json(ad_units); + + let ad_units = Json(&campaign.ad_units); let updated_rows = client .execute( &statement, &[ - &modified_campaign.budget.unwrap_or(campaign.budget), - &modified_campaign - .validators - .as_ref() - .unwrap_or(&campaign.validators), - &modified_campaign - .title - .as_ref() - .ok_or_else(|| &campaign.title) - .unwrap(), - &modified_campaign - .pricing_bounds - .as_ref() - .ok_or_else(|| &campaign.title) - .unwrap(), - &modified_campaign - .event_submission - .as_ref() - .ok_or_else(|| &campaign.title) - .unwrap(), + &campaign.budget, + &campaign.validators, + &campaign.title, + &campaign.pricing_bounds, + &campaign.event_submission, &ad_units, - &modified_campaign - .targeting_rules - .as_ref() - .unwrap_or(&campaign.targeting_rules), + &campaign.targeting_rules, &campaign.id, ], ) @@ -121,6 +100,7 @@ mod test { use primitives::{ util::tests::prep_db::{DUMMY_CAMPAIGN, DUMMY_AD_UNITS}, event_submission::{Rule, RateLimit}, + sentry::campaign_create::ModifyCampaign, targeting::Rules, UnifiedNum, EventSubmission, }; @@ -130,7 +110,6 @@ mod test { use crate::{ db::tests_postgres::{setup_test_migrations, DATABASE_POOL}, - ResponseError, }; use super::*; @@ -209,7 +188,9 @@ mod test { targeting_rules: Some(Rules::new()), }; - let is_campaign_updated = update_campaign(&database.pool, &campaign_for_testing, &modified_campaign).await.expect("should update"); + let applied_campaign = modified_campaign.apply(&campaign_for_testing); + + let is_campaign_updated = update_campaign(&database.pool, &applied_campaign).await.expect("should update"); assert!(is_campaign_updated); } } diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index f98886b5a..e16be4ac3 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -1,7 +1,7 @@ #![deny(clippy::all)] #![deny(rust_2018_idioms)] -use crate::db::{fetch_campaign, DbPool}; +use crate::db::{DbPool}; use crate::routes::campaign; use crate::routes::channel::channel_status; use crate::routes::event_aggregate::list_channel_event_aggregates; @@ -18,7 +18,7 @@ use middleware::{campaign::CampaignLoad, Chain, Middleware}; use once_cell::sync::Lazy; use primitives::adapter::Adapter; use primitives::sentry::ValidationErrorResponse; -use primitives::{CampaignId, Config, ValidatorId}; +use primitives::{Config, ValidatorId}; use redis::aio::MultiplexedConnection; use regex::Regex; use routes::analytics::{advanced_analytics, advertiser_analytics, analytics, publisher_analytics}; @@ -29,7 +29,6 @@ use routes::channel::{ }; use slog::Logger; use std::collections::HashMap; -use std::str::FromStr; pub mod middleware; pub mod routes { @@ -69,10 +68,10 @@ static INSERT_EVENTS_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/events/?$").expect("The regex should be valid") }); static CLOSE_CAMPAIGN_BY_CAMPAIGN_ID: Lazy = Lazy::new(|| { - Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/close/?$").expect("The regex should be valid") + Regex::new(r"^/v5/campaign/0x([a-zA-Z0-9]{32})/close/?$").expect("The regex should be valid") }); static CAMPAIGN_UPDATE_BY_ID: Lazy = Lazy::new(|| { - Regex::new(r"^/campaign/0x([a-zA-Z0-9]{32})/?$").expect("The regex should be valid") + Regex::new(r"^/v5/campaign/0x([a-zA-Z0-9]{32})/?$").expect("The regex should be valid") }); #[derive(Debug, Clone)] @@ -164,7 +163,7 @@ impl Application { publisher_analytics(req, &self).await } // For creating campaigns - ("/campaign", &Method::POST) => { + ("/v5/campaign", &Method::POST) => { let req = match AuthRequired.call(req, &self).await { Ok(req) => req, Err(error) => { @@ -195,23 +194,9 @@ async fn campaigns_router( ) -> Result, ResponseError> { let (path, method) = (req.uri().path(), req.method()); - if let (Some(caps), &Method::POST) = (CAMPAIGN_UPDATE_BY_ID.captures(&path), method) { - let param = RouteParams(vec![caps - .get(1) - .map_or("".to_string(), |m| m.as_str().to_string())]); - - // Should be safe to access indice here - let campaign_id = param - .get(0) - .ok_or_else(|| ResponseError::BadRequest("No CampaignId".to_string()))?; - let campaign_id = CampaignId::from_str(&campaign_id) - .map_err(|_| ResponseError::BadRequest("Bad CampaignId".to_string()))?; - - let campaign = fetch_campaign(app.pool.clone(), &campaign_id) - .await - .map_err(|_| ResponseError::NotFound)?; + if let (Some(_caps), &Method::POST) = (CAMPAIGN_UPDATE_BY_ID.captures(&path), method) { + let req = CampaignLoad.call(req, app).await?; - req.extensions_mut().insert(campaign); update_campaign::handle_route(req, app).await } else if let (Some(caps), &Method::POST) = (INSERT_EVENTS_BY_CAMPAIGN_ID.captures(&path), method) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 8b0e5867a..e37502c91 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -19,7 +19,7 @@ use primitives::{ Event, SuccessResponse, }, campaign_validator::Validator, - Address, Campaign, CampaignId, UnifiedNum + Campaign, CampaignId, UnifiedNum }; use redis::{ aio::MultiplexedConnection, @@ -57,7 +57,7 @@ pub async fn create_campaign( req: Request, app: &Application, ) -> Result, ResponseError> { - let session = req + let auth = req .extensions() .get::() .expect("request should have session") @@ -72,8 +72,7 @@ pub async fn create_campaign( campaign.validate(&app.config, &app.adapter.whoami()).map_err(|_| ResponseError::FailedValidation("couldn't valdiate campaign".to_string()))?; - // TODO: Just use session.uid once it's address - if Address::from_bytes(session.uid.as_bytes()) != campaign.creator { + if auth.uid.to_address() != campaign.creator { return Err(ResponseError::Forbidden("Request not sent by campaign creator".to_string())) } @@ -85,14 +84,14 @@ pub async fn create_campaign( let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; let total_deposited = latest_spendable.deposit.total; - let remaining_for_channel = total_deposited.checked_sub(&accounting_spent).ok_or(ResponseError::FailedValidation("couldn't calculate remaining for channel".to_string()))?; + let remaining_for_channel = total_deposited.checked_sub(&accounting_spent).ok_or(ResponseError::FailedValidation("No more budget remaining".to_string()))?; if campaign.budget > remaining_for_channel { - return Err(ResponseError::BadRequest("Not Enough budget for campaign".to_string())); + return Err(ResponseError::BadRequest("Not enough deposit left for the new campaign budget".to_string())); } // If the campaign is being created, the amount spent is 0, therefore remaining = budget - set_initial_remaining_for_campaign(&app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; + set_initial_remaining_for_campaign(&mut app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; // insert Campaign match insert_campaign(&app.pool, &campaign).await { @@ -116,16 +115,15 @@ pub async fn create_campaign( pub mod update_campaign { use super::*; - use crate::fetch_campaign; pub const CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; - pub async fn set_initial_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + pub async fn set_initial_remaining_for_campaign(redis: &mut MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); redis::cmd("SETNX") .arg(&key) .arg(amount.to_u64()) - .query_async(&mut redis.clone()) + .query_async(redis) .await?; Ok(true) } @@ -161,16 +159,19 @@ pub mod update_campaign { req: Request, app: &Application, ) -> Result, ResponseError> { - let campaign = req.extensions().get::().expect("We must have a campaign in extensions"); - let modified_campaign = ModifyCampaign::from_campaign(campaign.clone()); + let campaign_being_mutated = req.extensions().get::().expect("We must have a campaign in extensions").to_owned(); + + let body = hyper::body::to_bytes(req.into_body()).await?; + + let modified_campaign = serde_json::from_slice::(&body) + .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; - // Getting the campaign as it was before the update operation, should exist - let campaign_being_mutated = fetch_campaign(app.pool.clone(), &campaign.id).await?.ok_or(ResponseError::NotFound)?; + let modified_campaign = ModifyCampaign::from_campaign(modified_campaign.clone()); // modify Campaign modify_campaign(&app.pool, &campaign_being_mutated, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; - Ok(success_response(serde_json::to_string(&campaign)?)) + Ok(success_response(serde_json::to_string(&modified_campaign)?)) } pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, modified_campaign: &ModifyCampaign, redis: &MultiplexedConnection) -> Result { @@ -190,7 +191,8 @@ pub mod update_campaign { let old_budget = campaign.budget; match new_budget.cmp(&old_budget) { - Ordering::Greater | Ordering::Equal => { + Ordering::Equal => (), + Ordering::Greater => { let new_remaining = old_remaining.checked_add(&new_budget.checked_sub(&old_budget).ok_or(Error::Calculation)?).ok_or(Error::Calculation)?; let amount_to_incr = new_remaining.checked_sub(&old_remaining).ok_or(Error::Calculation)?; increase_remaining_for_campaign(&redis, campaign.id, amount_to_incr).await?; @@ -216,15 +218,13 @@ pub mod update_campaign { let total_remaining = total_deposited.checked_sub(&accounting_spent).ok_or(Error::Calculation)?; let campaigns_for_channel = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; - // campaign.budget should 100% exist, therefore it should be safe to just unwrap? - let current_campaign_budget = modified_campaign.budget.ok_or(campaign.budget).unwrap(); + let current_campaign_budget = modified_campaign.budget.unwrap_or(campaign.budget); let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, campaign.id, ¤t_campaign_budget).await.map_err(|_| Error::Calculation)?; - if campaigns_remaining_sum > total_remaining { - return Err(Error::BudgetExceeded); + if campaigns_remaining_sum <= total_remaining { + let campaign_with_updates = modified_campaign.apply(campaign); + update_campaign(&pool, &campaign_with_updates).await?; } - update_campaign(&pool, &campaign, &modified_campaign).await?; - Ok(campaign.clone()) } @@ -361,24 +361,22 @@ mod test { spender::{Deposit, Spendable}, Address }; - use deadpool::managed::Object; use crate::{ - db::redis_pool::{Manager, TESTS_POOL}, + db::redis_pool::TESTS_POOL, campaign::update_campaign::{CAMPAIGN_REMAINING_KEY, increase_remaining_for_campaign}, }; - use std::convert::TryFrom; use super::*; - fn get_dummy_spendable(spender: Address, campaign: Campaign) -> Spendable { - Spendable { - spender, - channel: campaign.channel.clone(), - deposit: Deposit { - total: UnifiedNum::from_u64(1_000_000), - still_on_create2: UnifiedNum::from_u64(0), - }, - } - } + // fn get_dummy_spendable(spender: Address, campaign: Campaign) -> Spendable { + // Spendable { + // spender, + // channel: campaign.channel.clone(), + // deposit: Deposit { + // total: UnifiedNum::from_u64(1_000_000), + // still_on_create2: UnifiedNum::from_u64(0), + // }, + // } + // } #[tokio::test] async fn does_it_increase_remaining() { From 8fcef1313747034bdb867167d6b11b6008a9ba9c Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Tue, 13 Jul 2021 14:02:27 +0300 Subject: [PATCH 20/31] sentry - routes - modify campaign --- primitives/src/sentry.rs | 45 ++-- sentry/src/db/campaign.rs | 131 +++++------ sentry/src/db/spendable.rs | 8 +- sentry/src/lib.rs | 4 +- sentry/src/routes/campaign.rs | 432 +++++++++++++++++++++------------- 5 files changed, 368 insertions(+), 252 deletions(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index b9c9c8f0c..6018dc235 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -386,22 +386,37 @@ pub mod campaign_create { } } - pub fn apply(&self, campaign: &Campaign) -> Campaign { - let campaign = campaign.clone(); - Campaign { - id: campaign.id, - channel: campaign.channel, - creator: campaign.creator, - budget: self.budget.unwrap_or(campaign.budget), - validators: self.validators.clone().unwrap_or(campaign.validators), - title: self.title.clone().or(campaign.title), - pricing_bounds: self.pricing_bounds.clone().or(campaign.pricing_bounds), - event_submission: self.event_submission.clone().or(campaign.event_submission), - ad_units: self.ad_units.clone().unwrap_or(campaign.ad_units), - targeting_rules: self.targeting_rules.clone().unwrap_or(campaign.targeting_rules), - created: campaign.created, - active: campaign.active, + pub fn apply(self, mut campaign: Campaign) -> Campaign { + if let Some(new_budget) = self.budget { + campaign.budget = new_budget; + } + + if let Some(new_validators) = self.validators { + campaign.validators = new_validators; + } + + // check if it was passed otherwise not sending a Title will result in clearing of the current one + if let Some(new_title) = self.title { + campaign.title = Some(new_title); + } + + if let Some(new_pricing_bounds) = self.pricing_bounds { + campaign.pricing_bounds = Some(new_pricing_bounds); } + + if let Some(new_event_submission) = self.event_submission { + campaign.event_submission = Some(new_event_submission); + } + + if let Some(new_ad_units) = self.ad_units { + campaign.ad_units = new_ad_units; + } + + if let Some(new_targeting_rules) = self.targeting_rules { + campaign.targeting_rules = new_targeting_rules; + } + + campaign } } } diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 75989a420..0c00b78c9 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -63,20 +63,21 @@ pub async fn get_campaigns_by_channel( Ok(campaigns) } -pub async fn update_campaign( - pool: &DbPool, - campaign: &Campaign, -) -> Result { +/// ```text +/// UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 +/// WHERE id = $8 +/// RETURNING id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to +/// +pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let statement = client - .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8") + .prepare("UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 WHERE id = $8 RETURNING id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to") .await?; - let ad_units = Json(&campaign.ad_units); - let updated_rows = client - .execute( + let updated_row = client + .query_one( &statement, &[ &campaign.budget, @@ -91,106 +92,100 @@ pub async fn update_campaign( ) .await?; - let exists = updated_rows == 1; - Ok(exists) + Ok(Campaign::from(&updated_row)) } #[cfg(test)] mod test { use primitives::{ - util::tests::prep_db::{DUMMY_CAMPAIGN, DUMMY_AD_UNITS}, - event_submission::{Rule, RateLimit}, + campaign, + event_submission::{RateLimit, Rule}, sentry::campaign_create::ModifyCampaign, targeting::Rules, - UnifiedNum, EventSubmission, + util::tests::prep_db::{DUMMY_AD_UNITS, DUMMY_CAMPAIGN}, + EventSubmission, UnifiedNum, }; - use primitives::campaign; use std::time::Duration; use tokio_postgres::error::SqlState; - use crate::{ - db::tests_postgres::{setup_test_migrations, DATABASE_POOL}, - }; + use crate::db::tests_postgres::{setup_test_migrations, DATABASE_POOL}; use super::*; #[tokio::test] - async fn it_inserts_and_fetches_campaign() { + async fn it_inserts_fetches_and_updates_a_campaign() { let database = DATABASE_POOL.get().await.expect("Should get a DB pool"); setup_test_migrations(database.pool.clone()) .await .expect("Migrations should succeed"); - let campaign_for_testing = DUMMY_CAMPAIGN.clone(); + let campaign = DUMMY_CAMPAIGN.clone(); - let non_existent_campaign = fetch_campaign(database.pool.clone(), &campaign_for_testing.id) + let non_existent_campaign = fetch_campaign(database.pool.clone(), &campaign.id) .await .expect("Should fetch successfully"); assert_eq!(None, non_existent_campaign); - let is_inserted = insert_campaign(&database.pool, &campaign_for_testing) + let is_inserted = insert_campaign(&database.pool, &campaign) .await .expect("Should succeed"); assert!(is_inserted); - let is_duplicate_inserted = insert_campaign(&database.pool, &campaign_for_testing).await; + let is_duplicate_inserted = insert_campaign(&database.pool, &campaign).await; assert!(matches!( is_duplicate_inserted, Err(PoolError::Backend(error)) if error.code() == Some(&SqlState::UNIQUE_VIOLATION) )); - let fetched_campaign = fetch_campaign(database.pool.clone(), &campaign_for_testing.id) + let fetched_campaign = fetch_campaign(database.pool.clone(), &campaign.id) .await .expect("Should fetch successfully"); - assert_eq!(Some(campaign_for_testing), fetched_campaign); - } - - #[tokio::test] - async fn it_updates_campaign() { - let database = DATABASE_POOL.get().await.expect("Should get a DB pool"); - - setup_test_migrations(database.pool.clone()) - .await - .expect("Migrations should succeed"); - - let campaign_for_testing = DUMMY_CAMPAIGN.clone(); - - let is_inserted = insert_campaign(&database.pool, &campaign_for_testing) - .await - .expect("Should succeed"); - - assert!(is_inserted); - - let rule = Rule { - uids: None, - rate_limit: Some(RateLimit { - limit_type: "sid".to_string(), - time_frame: Duration::from_millis(20_000), - }), - }; - let new_budget = campaign_for_testing.budget + UnifiedNum::from_u64(1_000_000_000); - let modified_campaign = ModifyCampaign { - // pub budget: Option, - budget: Some(new_budget), - validators: None, - title: Some("Modified Campaign".to_string()), - pricing_bounds: Some(campaign::PricingBounds { - impression: Some(campaign::Pricing { min: 1.into(), max: 10.into()}), - click: Some(campaign::Pricing { min: 0.into(), max: 0.into()}) - }), - event_submission: Some(EventSubmission { allow: vec![rule] }), - ad_units: Some(DUMMY_AD_UNITS.to_vec()), - targeting_rules: Some(Rules::new()), - }; - - let applied_campaign = modified_campaign.apply(&campaign_for_testing); - - let is_campaign_updated = update_campaign(&database.pool, &applied_campaign).await.expect("should update"); - assert!(is_campaign_updated); + assert_eq!(Some(campaign.clone()), fetched_campaign); + + // Update campaign + { + let rule = Rule { + uids: None, + rate_limit: Some(RateLimit { + limit_type: "sid".to_string(), + time_frame: Duration::from_millis(20_000), + }), + }; + let new_budget = campaign.budget + UnifiedNum::from_u64(1_000_000_000); + let modified_campaign = ModifyCampaign { + budget: Some(new_budget), + validators: None, + title: Some("Modified Campaign".to_string()), + pricing_bounds: Some(campaign::PricingBounds { + impression: Some(campaign::Pricing { + min: 1.into(), + max: 10.into(), + }), + click: Some(campaign::Pricing { + min: 0.into(), + max: 0.into(), + }), + }), + event_submission: Some(EventSubmission { allow: vec![rule] }), + ad_units: Some(DUMMY_AD_UNITS.to_vec()), + targeting_rules: Some(Rules::new()), + }; + + let applied_campaign = modified_campaign.apply(campaign.clone()); + + let updated_campaign = update_campaign(&database.pool, &applied_campaign) + .await + .expect("should update"); + + assert_eq!( + applied_campaign, updated_campaign, + "Postgres should update all modified fields" + ); + } } } diff --git a/sentry/src/db/spendable.rs b/sentry/src/db/spendable.rs index 67dc8fcb2..a6c340c2f 100644 --- a/sentry/src/db/spendable.rs +++ b/sentry/src/db/spendable.rs @@ -37,13 +37,13 @@ pub async fn fetch_spendable( pool: DbPool, spender: &Address, channel_id: &ChannelId, -) -> Result { +) -> Result, PoolError> { let client = pool.get().await?; let statement = client.prepare("SELECT spender, channel_id, channel, total, still_on_create2 FROM spendable WHERE spender = $1 AND channel_id = $2").await?; - let row = client.query_one(&statement, &[spender, channel_id]).await?; + let row = client.query_opt(&statement, &[spender, channel_id]).await?; - Ok(Spendable::try_from(row)?) + Ok(row.map(Spendable::try_from).transpose()?) } #[cfg(test)] @@ -88,6 +88,6 @@ mod test { .await .expect("Should fetch successfully"); - assert_eq!(spendable, fetched_spendable); + assert_eq!(Some(spendable), fetched_spendable); } } diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index e16be4ac3..c5ff2ef83 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -1,7 +1,7 @@ #![deny(clippy::all)] #![deny(rust_2018_idioms)] -use crate::db::{DbPool}; +use crate::db::DbPool; use crate::routes::campaign; use crate::routes::channel::channel_status; use crate::routes::event_aggregate::list_channel_event_aggregates; @@ -437,7 +437,7 @@ pub fn bad_response(response_body: String, status_code: StatusCode) -> Response< let mut error_response = HashMap::new(); error_response.insert("message", response_body); - let body = Body::from(serde_json::to_string(&error_response).expect("serialise err response")); + let body = Body::from(serde_json::to_string(&error_response).expect("serialize err response")); let mut response = Response::new(body); response diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index e37502c91..f7612dd04 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,41 +1,34 @@ use crate::{ - success_response, Application, ResponseError, + access::{self, check_access}, db::{ - spendable::fetch_spendable, accounting::get_accounting_spent, - campaign::{update_campaign, insert_campaign, get_campaigns_by_channel}, - DbPool + campaign::{get_campaigns_by_channel, insert_campaign, update_campaign}, + spendable::fetch_spendable, + DbPool, }, routes::campaign::update_campaign::set_initial_remaining_for_campaign, - access::{self, check_access}, - Auth, Session + success_response, Application, Auth, ResponseError, Session, }; - +use chrono::Utc; +use deadpool_postgres::PoolError; use hyper::{Body, Request, Response}; use primitives::{ adapter::Adapter, + campaign_validator::Validator, sentry::{ campaign_create::{CreateCampaign, ModifyCampaign}, Event, SuccessResponse, }, - campaign_validator::Validator, - Campaign, CampaignId, UnifiedNum -}; -use redis::{ - aio::MultiplexedConnection, - RedisError, + Address, Campaign, CampaignId, UnifiedNum, }; +use redis::{aio::MultiplexedConnection, RedisError}; use slog::error; use std::{ cmp::{max, Ordering}, collections::HashMap, - convert::TryInto, }; -use deadpool_postgres::PoolError; -use tokio_postgres::error::SqlState; - -use chrono::Utc; use thiserror::Error; +use tokio_postgres::error::SqlState; #[derive(Debug, Error)] pub enum Error { @@ -47,10 +40,14 @@ pub enum Error { BudgetExceeded, #[error("Error with new budget: {0}")] NewBudget(String), + #[error("Spendable amount for campaign creator {0} not found")] + SpenderNotFound(Address), + #[error("Campaign was not modified because of spending constraints")] + CampaignNotModified, #[error("Redis error: {0}")] Redis(#[from] RedisError), #[error("DB Pool error: {0}")] - Pool(#[from] PoolError) + Pool(#[from] PoolError), } pub async fn create_campaign( @@ -70,28 +67,51 @@ pub async fn create_campaign( // create the actual `Campaign` with random `CampaignId` .into_campaign(); - campaign.validate(&app.config, &app.adapter.whoami()).map_err(|_| ResponseError::FailedValidation("couldn't valdiate campaign".to_string()))?; + campaign + .validate(&app.config, &app.adapter.whoami()) + .map_err(|_| ResponseError::FailedValidation("couldn't valdiate campaign".to_string()))?; if auth.uid.to_address() != campaign.creator { - return Err(ResponseError::Forbidden("Request not sent by campaign creator".to_string())) + return Err(ResponseError::Forbidden( + "Request not sent by campaign creator".to_string(), + )); } - let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); + let error_response = + ResponseError::BadRequest("err occurred; please try again later".to_string()); - let accounting_spent = get_accounting_spent(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let accounting_spent = + get_accounting_spent(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; - // TODO: AIP#61: Update when changes to Spendable are ready - let latest_spendable = fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let latest_spendable = + fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()) + .await? + .ok_or(ResponseError::BadRequest( + "No spendable amount found for the Campaign creator".to_string(), + ))?; let total_deposited = latest_spendable.deposit.total; - let remaining_for_channel = total_deposited.checked_sub(&accounting_spent).ok_or(ResponseError::FailedValidation("No more budget remaining".to_string()))?; + let remaining_for_channel = + total_deposited + .checked_sub(&accounting_spent) + .ok_or(ResponseError::FailedValidation( + "No more budget remaining".to_string(), + ))?; if campaign.budget > remaining_for_channel { - return Err(ResponseError::BadRequest("Not enough deposit left for the new campaign budget".to_string())); + return Err(ResponseError::BadRequest( + "Not enough deposit left for the new campaign budget".to_string(), + )); } // If the campaign is being created, the amount spent is 0, therefore remaining = budget - set_initial_remaining_for_campaign(&mut app.redis.clone(), campaign.id, campaign.budget).await.map_err(|_| ResponseError::BadRequest("Couldn't update remaining while creating campaign".to_string()))?; + set_initial_remaining_for_campaign(&app.redis, campaign.id, campaign.budget) + .await + .map_err(|_| { + ResponseError::BadRequest( + "Couldn't update remaining while creating campaign".to_string(), + ) + })?; // insert Campaign match insert_campaign(&app.pool, &campaign).await { @@ -106,7 +126,9 @@ pub async fn create_campaign( _ => Err(error_response), } } - Ok(false) => Err(ResponseError::BadRequest("Encountered error while creating Campaign; please try again".to_string())), + Ok(false) => Err(ResponseError::BadRequest( + "Encountered error while creating Campaign; please try again".to_string(), + )), _ => Ok(()), }?; @@ -118,167 +140,266 @@ pub mod update_campaign { pub const CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; - pub async fn set_initial_remaining_for_campaign(redis: &mut MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + pub async fn set_initial_remaining_for_campaign( + redis: &MultiplexedConnection, + id: CampaignId, + amount: UnifiedNum, + ) -> Result { let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); redis::cmd("SETNX") .arg(&key) .arg(amount.to_u64()) - .query_async(redis) + .query_async(&mut redis.clone()) .await?; Ok(true) } - pub async fn increase_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Result { + pub async fn increase_remaining_for_campaign( + redis: &MultiplexedConnection, + id: CampaignId, + amount: UnifiedNum, + ) -> Result { let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); redis::cmd("INCRBY") .arg(&key) .arg(amount.to_u64()) - .query_async(&mut redis.clone()) - .await?; - Ok(true) + .query_async::<_, u64>(&mut redis.clone()) + .await + .map(UnifiedNum::from) } - pub async fn decrease_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId, amount: UnifiedNum) -> Option { + pub async fn decrease_remaining_for_campaign( + redis: &MultiplexedConnection, + id: CampaignId, + amount: UnifiedNum, + ) -> Result { let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - let value = match redis::cmd("DECRBY") + redis::cmd("DECRBY") .arg(&key) .arg(amount.to_u64()) - .query_async::<_, Option>(&mut redis.clone()) + .query_async::<_, i64>(&mut redis.clone()) .await - { - Ok(Some(remaining)) => { - // Can't be less than 0 due to max() - Some(UnifiedNum::from_u64(max(0, remaining).try_into().unwrap())) - }, - _ => None - }; - value } pub async fn handle_route( req: Request, app: &Application, ) -> Result, ResponseError> { - let campaign_being_mutated = req.extensions().get::().expect("We must have a campaign in extensions").to_owned(); + let campaign_being_mutated = req + .extensions() + .get::() + .expect("We must have a campaign in extensions") + .to_owned(); let body = hyper::body::to_bytes(req.into_body()).await?; - let modified_campaign = serde_json::from_slice::(&body) + let modify_campaign_fields = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; - let modified_campaign = ModifyCampaign::from_campaign(modified_campaign.clone()); - // modify Campaign - modify_campaign(&app.pool, &campaign_being_mutated, &modified_campaign, &app.redis).await.map_err(|_| ResponseError::BadRequest("Failed to update campaign".to_string()))?; + let modified_campaign = modify_campaign( + &app.pool, + &mut app.redis.clone(), + campaign_being_mutated, + modify_campaign_fields, + ) + .await + .map_err(|err| ResponseError::BadRequest(err.to_string()))?; Ok(success_response(serde_json::to_string(&modified_campaign)?)) } - pub async fn modify_campaign(pool: &DbPool, campaign: &Campaign, modified_campaign: &ModifyCampaign, redis: &MultiplexedConnection) -> Result { + pub async fn modify_campaign( + pool: &DbPool, + redis: &MultiplexedConnection, + campaign: Campaign, + modify_campaign: ModifyCampaign, + ) -> Result { // *NOTE*: When updating campaigns make sure sum(campaigns.map(getRemaining)) <= totalDepoisted - totalspent // !WARNING!: totalSpent != sum(campaign.map(c => c.spending)) therefore we must always calculate remaining funds based on total_deposit - lastApprovedNewState.spenders[user] // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 - if let Some(new_budget) = modified_campaign.budget { - let old_remaining = get_remaining_for_campaign(&redis, campaign.id).await?.ok_or(Error::FailedUpdate("No remaining entry for campaign".to_string()))?; + let delta_budget = if let Some(new_budget) = modify_campaign.budget { + get_delta_budget(redis, &campaign, new_budget).await? + } else { + None + }; + + // if we are going to update the budget + // validate the totalDeposit - totalSpent for all campaign + // sum(AllChannelCampaigns.map(getRemaining)) + DeltaBudgetForMutatedCampaign <= totalDeposited - totalSpent + // sum(AllChannelCampaigns.map(getRemaining)) - DeltaBudgetForMutatedCampaign <= totalDeposited - totalSpent + if let Some(delta_budget) = delta_budget { + let accounting_spent = + get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()) + .await?; + + let latest_spendable = + fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()) + .await? + .ok_or(Error::SpenderNotFound(campaign.creator))?; + + // Gets the latest Spendable for this (spender, channelId) pair + let total_deposited = latest_spendable.deposit.total; + + let total_remaining = total_deposited + .checked_sub(&accounting_spent) + .ok_or(Error::Calculation)?; + let channel_campaigns = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; + + // this will include the Campaign we are currently modifying + let campaigns_current_remaining_sum = + get_remaining_for_multiple_campaigns(&redis, &channel_campaigns) + .await? + .iter() + .sum::>() + .ok_or(Error::Calculation)?; + + // apply the delta_budget to the sum + let new_campaigns_remaining = match delta_budget { + DeltaBudget::Increase(increase_by) => { + campaigns_current_remaining_sum.checked_add(&increase_by) + } + DeltaBudget::Decrease(decrease_by) => { + campaigns_current_remaining_sum.checked_sub(&decrease_by) + } + } + .ok_or(Error::Calculation)?; - let campaign_spent = campaign.budget.checked_sub(&old_remaining).ok_or(Error::Calculation)?; - if campaign_spent >= new_budget { - return Err(Error::NewBudget("New budget should be greater than the spent amount".to_string())); + if !(new_campaigns_remaining <= total_remaining) { + return Err(Error::CampaignNotModified); } - // Separate variable for clarity - let old_budget = campaign.budget; - - match new_budget.cmp(&old_budget) { - Ordering::Equal => (), - Ordering::Greater => { - let new_remaining = old_remaining.checked_add(&new_budget.checked_sub(&old_budget).ok_or(Error::Calculation)?).ok_or(Error::Calculation)?; - let amount_to_incr = new_remaining.checked_sub(&old_remaining).ok_or(Error::Calculation)?; - increase_remaining_for_campaign(&redis, campaign.id, amount_to_incr).await?; - }, - Ordering::Less => { - let new_remaining = old_remaining.checked_add(&old_budget.checked_sub(&new_budget).ok_or(Error::Calculation)?).ok_or(Error::Calculation)?; - let amount_to_decr = new_remaining.checked_sub(&old_remaining).ok_or(Error::Calculation)?; - let decreased_remaining = decrease_remaining_for_campaign(&redis, campaign.id, amount_to_decr).await.ok_or(Error::FailedUpdate("Could't decrease remaining".to_string()))?; - // If it goes below 0 it will still return 0 - if decreased_remaining.eq(&UnifiedNum::from_u64(0)) { - return Err(Error::NewBudget("No budget remaining after decreasing".to_string())); + // if the value is not positive it will return an error because of UnifiedNum + let _campaign_remaining = match delta_budget { + // should always be positive + DeltaBudget::Increase(increase_by) => { + increase_remaining_for_campaign(redis, campaign.id, increase_by).await? + } + // there is a chance that an even lowered the remaining and it's no longer positive + // check if positive and create an UnifiedNum, or return an error + DeltaBudget::Decrease(decrease_by) => { + match decrease_remaining_for_campaign(redis, campaign.id, decrease_by).await? { + remaining if remaining >= 0 => UnifiedNum::from(remaining.unsigned_abs()), + _ => UnifiedNum::from(0), } } - } - }; + }; + } - let accounting_spent = get_accounting_spent(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let modified_campaign = modify_campaign.apply(campaign); + update_campaign(&pool, &modified_campaign).await?; - let latest_spendable = fetch_spendable(pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + Ok(modified_campaign) + } - // Gets the latest Spendable for this (spender, channelId) pair - let total_deposited = latest_spendable.deposit.total; + /// Delta Budget describes the difference between the New and Old budget + /// It is used to decrease or increase the remaining budget instead of setting it up directly + /// This way if a new event alters the remaining budget in Redis while the modification of campaign hasn't finished + /// it will correctly update the remaining using an atomic redis operation with `INCRBY` or `DECRBY` instead of using `SET` + enum DeltaBudget { + Increase(T), + Decrease(T), + } - let total_remaining = total_deposited.checked_sub(&accounting_spent).ok_or(Error::Calculation)?; - let campaigns_for_channel = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; - let current_campaign_budget = modified_campaign.budget.unwrap_or(campaign.budget); - let campaigns_remaining_sum = get_campaigns_remaining_sum(&redis, &campaigns_for_channel, campaign.id, ¤t_campaign_budget).await.map_err(|_| Error::Calculation)?; - if campaigns_remaining_sum <= total_remaining { - let campaign_with_updates = modified_campaign.apply(campaign); - update_campaign(&pool, &campaign_with_updates).await?; + async fn get_delta_budget( + redis: &MultiplexedConnection, + campaign: &Campaign, + new_budget: UnifiedNum, + ) -> Result>, Error> { + let current_budget = campaign.budget; + + let budget_action = match new_budget.cmp(¤t_budget) { + // if there is no difference in budgets - no action needed + Ordering::Equal => return Ok(None), + Ordering::Greater => DeltaBudget::Increase(()), + Ordering::Less => DeltaBudget::Decrease(()), + }; + + let old_remaining = get_remaining_for_campaign(redis, campaign.id) + .await? + .ok_or(Error::FailedUpdate( + "No remaining entry for campaign".to_string(), + ))?; + + let campaign_spent = campaign + .budget + .checked_sub(&old_remaining) + .ok_or(Error::Calculation)?; + + if campaign_spent >= new_budget { + return Err(Error::NewBudget( + "New budget should be greater than the spent amount".to_string(), + )); } - Ok(campaign.clone()) + let budget = match budget_action { + DeltaBudget::Increase(()) => { + // delta budget = New budget - Old budget ( the difference between the new and old when New > Old) + let new_remaining = new_budget + .checked_sub(¤t_budget) + .and_then(|delta_budget| old_remaining.checked_add(&delta_budget)) + .ok_or(Error::Calculation)?; + let increase_by = new_remaining + .checked_sub(&old_remaining) + .ok_or(Error::Calculation)?; + + DeltaBudget::Increase(increase_by) + } + DeltaBudget::Decrease(()) => { + // delta budget = New budget - Old budget ( the difference between the new and old when New > Old) + let new_remaining = ¤t_budget + .checked_sub(&new_budget) + .and_then(|delta_budget| old_remaining.checked_add(&delta_budget)) + .ok_or(Error::Calculation)?; + let decrease_by = new_remaining + .checked_sub(&old_remaining) + .ok_or(Error::Calculation)?; + + DeltaBudget::Decrease(decrease_by) + } + }; + + Ok(Some(budget)) } - pub async fn get_remaining_for_campaign(redis: &MultiplexedConnection, id: CampaignId) -> Result, RedisError> { + pub async fn get_remaining_for_campaign( + redis: &MultiplexedConnection, + id: CampaignId, + ) -> Result, RedisError> { let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - let remaining = match redis::cmd("GET") + + let remaining = redis::cmd("GET") .arg(&key) .query_async::<_, Option>(&mut redis.clone()) - .await { - Ok(Some(remaining)) => { - // Can't be negative due to max() - Some(UnifiedNum::from_u64(max(0, remaining).try_into().unwrap())) - }, - Ok(None) => None, - Err(e) => return Err(e), - }; + .await? + .map(|remaining| UnifiedNum::from(max(0, remaining).unsigned_abs())); + Ok(remaining) } - async fn get_remaining_for_multiple_campaigns(redis: &MultiplexedConnection, campaigns: &[Campaign]) -> Result, Error> { - let keys: Vec = campaigns.iter().map(|c| format!("{}:{}", CAMPAIGN_REMAINING_KEY, c.id)).collect(); + async fn get_remaining_for_multiple_campaigns( + redis: &MultiplexedConnection, + campaigns: &[Campaign], + ) -> Result, Error> { + let keys: Vec = campaigns + .iter() + .map(|c| format!("{}:{}", CAMPAIGN_REMAINING_KEY, c.id)) + .collect(); + let remainings = redis::cmd("MGET") .arg(keys) .query_async::<_, Vec>>(&mut redis.clone()) - .await?; - - let remainings = remainings + .await? .into_iter() - .map(|r| { - match r { - // Can't be negative due to max() - Some(remaining) => UnifiedNum::from_u64(max(0, remaining).try_into().unwrap()), - None => UnifiedNum::from_u64(0) - } + .map(|remaining| match remaining { + Some(remaining) => UnifiedNum::from_u64(max(0, remaining).unsigned_abs()), + None => UnifiedNum::from_u64(0), }) .collect(); Ok(remainings) } - - pub async fn get_campaigns_remaining_sum(redis: &MultiplexedConnection, campaigns: &[Campaign], mutated_campaign: CampaignId, new_budget: &UnifiedNum) -> Result { - let other_campaigns_remaining = get_remaining_for_multiple_campaigns(&redis, &campaigns).await?; - let sum_of_campaigns_remaining = other_campaigns_remaining - .iter() - .sum::>() - .ok_or(Error::Calculation)?; - - // Necessary to do it explicitly for current campaign as its budget is not yet updated in DB - let old_remaining_for_mutated_campaign = get_remaining_for_campaign(&redis, mutated_campaign).await?.ok_or(Error::FailedUpdate("No remaining entry for campaign".to_string()))?; - let spent_for_mutated_campaign = new_budget.checked_sub(&old_remaining_for_mutated_campaign).ok_or(Error::Calculation)?; - let new_remaining_for_mutated_campaign = new_budget.checked_sub(&spent_for_mutated_campaign).ok_or(Error::Calculation)?; - sum_of_campaigns_remaining.checked_add(&new_remaining_for_mutated_campaign).ok_or(Error::Calculation)?; - Ok(sum_of_campaigns_remaining) - } } pub async fn insert_events( @@ -356,27 +477,12 @@ async fn process_events( #[cfg(test)] mod test { - use primitives::{ - util::tests::prep_db::{DUMMY_CAMPAIGN}, - spender::{Deposit, Spendable}, - Address - }; + use super::*; use crate::{ + campaign::update_campaign::{increase_remaining_for_campaign, CAMPAIGN_REMAINING_KEY}, db::redis_pool::TESTS_POOL, - campaign::update_campaign::{CAMPAIGN_REMAINING_KEY, increase_remaining_for_campaign}, }; - use super::*; - - // fn get_dummy_spendable(spender: Address, campaign: Campaign) -> Spendable { - // Spendable { - // spender, - // channel: campaign.channel.clone(), - // deposit: Deposit { - // total: UnifiedNum::from_u64(1_000_000), - // still_on_create2: UnifiedNum::from_u64(0), - // }, - // } - // } + use primitives::util::tests::prep_db::DUMMY_CAMPAIGN; #[tokio::test] async fn does_it_increase_remaining() { @@ -387,29 +493,32 @@ mod test { // Setting the redis base variable redis::cmd("SET") .arg(&key) - .arg(100u64) + .arg(100_u64) .query_async::<_, ()>(&mut redis.connection) .await .expect("should set"); // 2 async calls at once, should be 500 after them - futures::future::join( + futures::future::try_join_all([ + increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), - increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)) - ).await; + ]) + .await + .expect("Should increase remaining twice"); let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.connection) + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); - assert_eq!(remaining.is_some(), true); - - let remaining = remaining.expect("should get remaining"); - let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); - assert_eq!(remaining, UnifiedNum::from_u64(500)); + assert_eq!( + remaining.map(UnifiedNum::from_u64), + Some(UnifiedNum::from_u64(500)) + ); - increase_remaining_for_campaign(&redis, campaign.id, campaign.budget).await.expect("should increase"); + increase_remaining_for_campaign(&redis, campaign.id, campaign.budget) + .await + .expect("should increase"); let remaining = redis::cmd("GET") .arg(&key) @@ -417,23 +526,20 @@ mod test { .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); - assert_eq!(remaining.is_some(), true); - let remaining = remaining.expect("should get result out of the option"); let should_be_remaining = UnifiedNum::from_u64(500) + campaign.budget; - assert_eq!(UnifiedNum::from_u64(remaining), should_be_remaining); + assert_eq!(remaining.map(UnifiedNum::from), Some(should_be_remaining)); - increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)).await.expect("should work"); + increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)) + .await + .expect("should increase remaining"); let remaining = redis::cmd("GET") .arg(&key) - .query_async::<_, Option>(&mut redis.connection) + .query_async::<_, Option>(&mut redis.connection) .await .expect("should get remaining"); - assert_eq!(remaining.is_some(), true); - let remaining = remaining.expect("should get remaining"); - let remaining = UnifiedNum::from_u64(remaining.parse::().expect("should parse")); - assert_eq!(remaining, should_be_remaining); + assert_eq!(remaining.map(UnifiedNum::from), Some(should_be_remaining)); } -} \ No newline at end of file +} From b664dd1924894ae32191e7f978a6aa9912e0ac7c Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Tue, 13 Jul 2021 14:04:24 +0300 Subject: [PATCH 21/31] sentry - db - campaign - fix doc comment --- sentry/src/db/campaign.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 0c00b78c9..fed600754 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -67,7 +67,7 @@ pub async fn get_campaigns_by_channel( /// UPDATE campaigns SET budget = $1, validators = $2, title = $3, pricing_bounds = $4, event_submission = $5, ad_units = $6, targeting_rules = $7 /// WHERE id = $8 /// RETURNING id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to -/// +/// ``` pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let statement = client From 10deba9789a5bc324ca30222c490b0233e4850ea Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Wed, 4 Aug 2021 17:35:09 +0300 Subject: [PATCH 22/31] sentry - mod test_util - setup_dummy_app helper fn --- sentry/src/lib.rs | 47 +++++++++++++++++++++++++++++++ sentry/src/middleware/campaign.rs | 45 ++--------------------------- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index c5ff2ef83..ca5bde526 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -500,3 +500,50 @@ pub struct Auth { pub era: i64, pub uid: ValidatorId, } + +#[cfg(test)] +pub mod test_util { + use adapter::DummyAdapter; + use primitives::{ + adapter::DummyAdapterOptions, + config::configuration, + util::tests::{ + discard_logger, + prep_db::{IDS}, + }, + }; + + use crate::{Application, db::{ + redis_pool::TESTS_POOL, + tests_postgres::{setup_test_migrations, DATABASE_POOL}, + }}; + + pub async fn setup_dummy_app() -> Application { + let config = configuration("development", None).expect("Should get Config"); + let adapter = DummyAdapter::init( + DummyAdapterOptions { + dummy_identity: IDS["leader"], + dummy_auth: Default::default(), + dummy_auth_tokens: Default::default(), + }, + &config, + ); + + let redis = TESTS_POOL.get().await.expect("Should return Object"); + let database = DATABASE_POOL.get().await.expect("Should get a DB pool"); + + setup_test_migrations(database.pool.clone()) + .await + .expect("Migrations should succeed"); + + let app = Application::new( + adapter, + config, + discard_logger(), + redis.connection.clone(), + database.pool.clone(), + ); + + app + } +} diff --git a/sentry/src/middleware/campaign.rs b/sentry/src/middleware/campaign.rs index 42d16b281..024dc1400 100644 --- a/sentry/src/middleware/campaign.rs +++ b/sentry/src/middleware/campaign.rs @@ -37,57 +37,18 @@ impl Middleware for CampaignLoad { #[cfg(test)] mod test { - use adapter::DummyAdapter; use primitives::{ - adapter::DummyAdapterOptions, - config::configuration, - util::tests::{ - discard_logger, - prep_db::{DUMMY_CAMPAIGN, IDS}, - }, + util::tests::prep_db::{DUMMY_CAMPAIGN, IDS}, Campaign, }; - use crate::db::{ - insert_campaign, - redis_pool::TESTS_POOL, - tests_postgres::{setup_test_migrations, DATABASE_POOL}, - }; + use crate::{db::insert_campaign, test_util::setup_dummy_app}; use super::*; - async fn setup_app() -> Application { - let config = configuration("development", None).expect("Should get Config"); - let adapter = DummyAdapter::init( - DummyAdapterOptions { - dummy_identity: IDS["leader"], - dummy_auth: Default::default(), - dummy_auth_tokens: Default::default(), - }, - &config, - ); - - let redis = TESTS_POOL.get().await.expect("Should return Object"); - let database = DATABASE_POOL.get().await.expect("Should get a DB pool"); - - setup_test_migrations(database.pool.clone()) - .await - .expect("Migrations should succeed"); - - let app = Application::new( - adapter, - config, - discard_logger(), - redis.connection.clone(), - database.pool.clone(), - ); - - app - } - #[tokio::test] async fn campaign_loading() { - let app = setup_app().await; + let app = setup_dummy_app().await; let build_request = |params: RouteParams| { Request::builder() From 88c324331be45e4ee463487f3d8e036f7882a3c6 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Wed, 4 Aug 2021 17:36:46 +0300 Subject: [PATCH 23/31] sentry - db - campaign - CampaignRemaining redis struct --- sentry/src/db/campaign.rs | 251 +++++++++++++++++++++++++++++++++- sentry/src/routes/campaign.rs | 233 ++++++++----------------------- 2 files changed, 307 insertions(+), 177 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index fed600754..9c5287a20 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -2,10 +2,16 @@ use crate::db::{DbPool, PoolError}; use primitives::{Campaign, CampaignId, ChannelId}; use tokio_postgres::types::Json; +pub use campaign_remaining::CampaignRemaining; + +/// ```text +/// INSERT INTO campaigns (id, channel_id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to) +/// VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) +/// ``` pub async fn insert_campaign(pool: &DbPool, campaign: &Campaign) -> Result { let client = pool.get().await?; let ad_units = Json(campaign.ad_units.clone()); - let stmt = client.prepare("INSERT INTO campaigns (id, channel_id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to) values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)").await?; + let stmt = client.prepare("INSERT INTO campaigns (id, channel_id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)").await?; let inserted = client .execute( &stmt, @@ -49,6 +55,10 @@ pub async fn fetch_campaign( } // TODO: We might need to use LIMIT to implement pagination +/// ```text +/// SELECT id, channel, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, created, active_from, active_to +/// FROM campaigns WHERE channel_id = $1 +/// ``` pub async fn get_campaigns_by_channel( pool: &DbPool, channel_id: &ChannelId, @@ -95,6 +105,245 @@ pub async fn update_campaign(pool: &DbPool, campaign: &Campaign) -> Result String { + format!("{}:{}", Self::CAMPAIGN_REMAINING_KEY, campaign) + } + + pub fn new(redis: MultiplexedConnection) -> Self { + Self { redis } + } + + pub async fn set_initial( + &self, + campaign: CampaignId, + amount: UnifiedNum, + ) -> Result { + redis::cmd("SETNX") + .arg(&Self::get_key(campaign)) + .arg(amount.to_u64()) + .query_async(&mut self.redis.clone()) + .await + } + + pub async fn get_remaining_opt( + &self, + campaign: CampaignId, + ) -> Result, RedisError> { + redis::cmd("GET") + .arg(&Self::get_key(campaign)) + .query_async::<_, Option>(&mut self.redis.clone()) + .await + } + + /// This method uses `max(0, value)` to clamp the value of a campaign, which can be negative and uses `i64`. + /// In addition, it defaults the campaign keys that were not found to `0`. + pub async fn get_multiple( + &self, + campaigns: &[CampaignId], + ) -> Result, RedisError> { + let keys: Vec = campaigns + .iter() + .map(|campaign| Self::get_key(*campaign)) + .collect(); + + let campaigns_remaining = redis::cmd("MGET") + .arg(keys) + .query_async::<_, Vec>>(&mut self.redis.clone()) + .await? + .into_iter() + .map(|remaining| match remaining { + Some(remaining) => UnifiedNum::from_u64(remaining.max(0).unsigned_abs()), + None => UnifiedNum::from_u64(0), + }) + .collect(); + + Ok(campaigns_remaining) + } + + pub async fn increase_by( + &self, + campaign: CampaignId, + amount: UnifiedNum, + ) -> Result { + let key = Self::get_key(campaign); + redis::cmd("INCRBY") + .arg(&key) + .arg(amount.to_u64()) + .query_async(&mut self.redis.clone()) + .await + } + + pub async fn decrease_by( + &self, + campaign: CampaignId, + amount: UnifiedNum, + ) -> Result { + let key = Self::get_key(campaign); + redis::cmd("DECRBY") + .arg(&key) + .arg(amount.to_u64()) + .query_async(&mut self.redis.clone()) + .await + } + } + + #[cfg(test)] + mod test { + use primitives::util::tests::prep_db::DUMMY_CAMPAIGN; + + use crate::db::redis_pool::TESTS_POOL; + + use super::*; + + #[tokio::test] + async fn it_sets_initial_increases_and_decreases_remaining_for_campaign() { + let redis = TESTS_POOL.get().await.expect("Should return Object"); + + let campaign = DUMMY_CAMPAIGN.id; + let campaign_remaining = CampaignRemaining::new(redis.connection.clone()); + + // Get remaining on a key which was not set + { + let get_remaining = campaign_remaining + .get_remaining_opt(campaign) + .await + .expect("Should get None"); + + assert_eq!(None, get_remaining); + } + + // Set Initial amount on that key + { + let initial_amount = UnifiedNum::from(1_000_u64); + let set_initial = campaign_remaining + .set_initial(campaign, initial_amount) + .await + .expect("Should set value in redis"); + assert!(set_initial); + + // get the remaining of that key, should be the initial value + let get_remaining = campaign_remaining + .get_remaining_opt(campaign) + .await + .expect("Should get None"); + + assert_eq!( + Some(1_000_i64), + get_remaining, + "should return the initial value that was set" + ); + } + + // Set initial on already existing key, should return `false` + { + let set_failing_initial = campaign_remaining + .set_initial(campaign, UnifiedNum::from(999_u64)) + .await + .expect("Should set value in redis"); + assert!(!set_failing_initial); + } + + // Decrease by amount + { + let decrease_amount = UnifiedNum::from(64); + let decrease_by = campaign_remaining + .decrease_by(campaign, decrease_amount) + .await + .expect("Should decrease remaining amount"); + + assert_eq!(936_i64, decrease_by); + } + + // Increase by amount + { + let increase_amount = UnifiedNum::from(1064); + let increase_by = campaign_remaining + .increase_by(campaign, increase_amount) + .await + .expect("Should increase remaining amount"); + + assert_eq!(2_000_i64, increase_by); + } + + let get_remaining = campaign_remaining + .get_remaining_opt(campaign) + .await + .expect("Should get remaining"); + + assert_eq!(Some(2_000_i64), get_remaining); + + // Decrease by amount > than currently set + { + let decrease_amount = UnifiedNum::from(5_000); + let decrease_by = campaign_remaining + .decrease_by(campaign, decrease_amount) + .await + .expect("Should decrease remaining amount"); + + assert_eq!(-3_000_i64, decrease_by); + } + + // Increase the negative value without going > 0 + { + let increase_amount = UnifiedNum::from(1000); + let increase_by = campaign_remaining + .increase_by(campaign, increase_amount) + .await + .expect("Should increase remaining amount"); + + assert_eq!(-2_000_i64, increase_by); + } + } + + #[tokio::test] + async fn it_gets_multiple_campaigns_remaining() { + let redis = TESTS_POOL.get().await.expect("Should return Object"); + let campaign_remaining = CampaignRemaining::new(redis.connection.clone()); + + let campaigns = (CampaignId::new(), CampaignId::new(), CampaignId::new()); + + // set initial amounts + { + assert!(campaign_remaining + .set_initial(campaigns.0, UnifiedNum::from(100)) + .await + .expect("Should set value in redis")); + + assert!(campaign_remaining + .set_initial(campaigns.1, UnifiedNum::from(200)) + .await + .expect("Should set value in redis")); + + assert!(campaign_remaining + .set_initial(campaigns.2, UnifiedNum::from(300)) + .await + .expect("Should set value in redis")); + } + + // set campaigns.1 to negative value, should return `0` because of `max(value, 0)` + assert_eq!(-300_i64, campaign_remaining.decrease_by(campaigns.1, UnifiedNum::from(500)).await.expect("Should decrease remaining")); + + let multiple = campaign_remaining.get_multiple(&[campaigns.0, campaigns.1, campaigns.2]).await.expect("Should get multiple"); + + assert_eq!(vec![UnifiedNum::from(100), UnifiedNum::from(0), UnifiedNum::from(300)], multiple); + } + } +} + #[cfg(test)] mod test { use primitives::{ diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index f7612dd04..6d57607b0 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,14 +1,4 @@ -use crate::{ - access::{self, check_access}, - db::{ - accounting::get_accounting_spent, - campaign::{get_campaigns_by_channel, insert_campaign, update_campaign}, - spendable::fetch_spendable, - DbPool, - }, - routes::campaign::update_campaign::set_initial_remaining_for_campaign, - success_response, Application, Auth, ResponseError, Session, -}; +use crate::{Application, Auth, ResponseError, Session, access::{self, check_access}, db::{CampaignRemaining, DbPool, RedisError, accounting::get_accounting_spent, campaign::{get_campaigns_by_channel, insert_campaign, update_campaign}, spendable::fetch_spendable}, success_response}; use chrono::Utc; use deadpool_postgres::PoolError; use hyper::{Body, Request, Response}; @@ -19,9 +9,8 @@ use primitives::{ campaign_create::{CreateCampaign, ModifyCampaign}, Event, SuccessResponse, }, - Address, Campaign, CampaignId, UnifiedNum, + Address, Campaign, UnifiedNum, }; -use redis::{aio::MultiplexedConnection, RedisError}; use slog::error; use std::{ cmp::{max, Ordering}, @@ -69,7 +58,7 @@ pub async fn create_campaign( campaign .validate(&app.config, &app.adapter.whoami()) - .map_err(|_| ResponseError::FailedValidation("couldn't valdiate campaign".to_string()))?; + .map_err(|err| ResponseError::FailedValidation(err.to_string()))?; if auth.uid.to_address() != campaign.creator { return Err(ResponseError::Forbidden( @@ -105,14 +94,22 @@ pub async fn create_campaign( } // If the campaign is being created, the amount spent is 0, therefore remaining = budget - set_initial_remaining_for_campaign(&app.redis, campaign.id, campaign.budget) + let remaining_set = CampaignRemaining::new(app.redis.clone()).set_initial(campaign.id, campaign.budget) .await .map_err(|_| { ResponseError::BadRequest( - "Couldn't update remaining while creating campaign".to_string(), + "Couldn't set remaining while creating campaign".to_string(), ) })?; + // If for some reason the randomly generated `CampaignId` exists in Redis + // This should **NOT** happen! + if !remaining_set { + return Err(ResponseError::Conflict( + "The generated CampaignId already exists, please repeat the request".to_string(), + )) + } + // insert Campaign match insert_campaign(&app.pool, &campaign).await { Err(error) => { @@ -136,50 +133,9 @@ pub async fn create_campaign( } pub mod update_campaign { - use super::*; - - pub const CAMPAIGN_REMAINING_KEY: &'static str = "campaignRemaining"; - - pub async fn set_initial_remaining_for_campaign( - redis: &MultiplexedConnection, - id: CampaignId, - amount: UnifiedNum, - ) -> Result { - let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - redis::cmd("SETNX") - .arg(&key) - .arg(amount.to_u64()) - .query_async(&mut redis.clone()) - .await?; - Ok(true) - } - - pub async fn increase_remaining_for_campaign( - redis: &MultiplexedConnection, - id: CampaignId, - amount: UnifiedNum, - ) -> Result { - let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - redis::cmd("INCRBY") - .arg(&key) - .arg(amount.to_u64()) - .query_async::<_, u64>(&mut redis.clone()) - .await - .map(UnifiedNum::from) - } + use crate::db::CampaignRemaining; - pub async fn decrease_remaining_for_campaign( - redis: &MultiplexedConnection, - id: CampaignId, - amount: UnifiedNum, - ) -> Result { - let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - redis::cmd("DECRBY") - .arg(&key) - .arg(amount.to_u64()) - .query_async::<_, i64>(&mut redis.clone()) - .await - } + use super::*; pub async fn handle_route( req: Request, @@ -196,10 +152,12 @@ pub mod update_campaign { let modify_campaign_fields = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; + let campaign_remaining = CampaignRemaining::new(app.redis.clone()); + // modify Campaign let modified_campaign = modify_campaign( &app.pool, - &mut app.redis.clone(), + campaign_remaining, campaign_being_mutated, modify_campaign_fields, ) @@ -211,7 +169,7 @@ pub mod update_campaign { pub async fn modify_campaign( pool: &DbPool, - redis: &MultiplexedConnection, + campaign_remaining: CampaignRemaining, campaign: Campaign, modify_campaign: ModifyCampaign, ) -> Result { @@ -220,7 +178,7 @@ pub mod update_campaign { // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 let delta_budget = if let Some(new_budget) = modify_campaign.budget { - get_delta_budget(redis, &campaign, new_budget).await? + get_delta_budget(&campaign_remaining, &campaign, new_budget).await? } else { None }; @@ -245,15 +203,19 @@ pub mod update_campaign { let total_remaining = total_deposited .checked_sub(&accounting_spent) .ok_or(Error::Calculation)?; - let channel_campaigns = get_campaigns_by_channel(&pool, &campaign.channel.id()).await?; + let channel_campaigns = get_campaigns_by_channel(&pool, &campaign.channel.id()) + .await? + .iter() + .map(|c| c.id) + .collect::>(); // this will include the Campaign we are currently modifying - let campaigns_current_remaining_sum = - get_remaining_for_multiple_campaigns(&redis, &channel_campaigns) - .await? - .iter() - .sum::>() - .ok_or(Error::Calculation)?; + let campaigns_current_remaining_sum = campaign_remaining + .get_multiple(&channel_campaigns) + .await? + .iter() + .sum::>() + .ok_or(Error::Calculation)?; // apply the delta_budget to the sum let new_campaigns_remaining = match delta_budget { @@ -270,19 +232,18 @@ pub mod update_campaign { return Err(Error::CampaignNotModified); } - // if the value is not positive it will return an error because of UnifiedNum + // there is a chance that the new remaining will be negative even when increasing the budget + // We don't currently use this value but can be used to perform additional checks or return messages accordingly let _campaign_remaining = match delta_budget { - // should always be positive DeltaBudget::Increase(increase_by) => { - increase_remaining_for_campaign(redis, campaign.id, increase_by).await? + campaign_remaining + .increase_by(campaign.id, increase_by) + .await? } - // there is a chance that an even lowered the remaining and it's no longer positive - // check if positive and create an UnifiedNum, or return an error DeltaBudget::Decrease(decrease_by) => { - match decrease_remaining_for_campaign(redis, campaign.id, decrease_by).await? { - remaining if remaining >= 0 => UnifiedNum::from(remaining.unsigned_abs()), - _ => UnifiedNum::from(0), - } + campaign_remaining + .decrease_by(campaign.id, decrease_by) + .await? } }; } @@ -303,7 +264,7 @@ pub mod update_campaign { } async fn get_delta_budget( - redis: &MultiplexedConnection, + campaign_remaining: &CampaignRemaining, campaign: &Campaign, new_budget: UnifiedNum, ) -> Result>, Error> { @@ -316,8 +277,10 @@ pub mod update_campaign { Ordering::Less => DeltaBudget::Decrease(()), }; - let old_remaining = get_remaining_for_campaign(redis, campaign.id) + let old_remaining = campaign_remaining + .get_remaining_opt(campaign.id) .await? + .map(|remaining| UnifiedNum::from(max(0, remaining).unsigned_abs())) .ok_or(Error::FailedUpdate( "No remaining entry for campaign".to_string(), ))?; @@ -362,44 +325,6 @@ pub mod update_campaign { Ok(Some(budget)) } - - pub async fn get_remaining_for_campaign( - redis: &MultiplexedConnection, - id: CampaignId, - ) -> Result, RedisError> { - let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, id); - - let remaining = redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.clone()) - .await? - .map(|remaining| UnifiedNum::from(max(0, remaining).unsigned_abs())); - - Ok(remaining) - } - - async fn get_remaining_for_multiple_campaigns( - redis: &MultiplexedConnection, - campaigns: &[Campaign], - ) -> Result, Error> { - let keys: Vec = campaigns - .iter() - .map(|c| format!("{}:{}", CAMPAIGN_REMAINING_KEY, c.id)) - .collect(); - - let remainings = redis::cmd("MGET") - .arg(keys) - .query_async::<_, Vec>>(&mut redis.clone()) - .await? - .into_iter() - .map(|remaining| match remaining { - Some(remaining) => UnifiedNum::from_u64(max(0, remaining).unsigned_abs()), - None => UnifiedNum::from_u64(0), - }) - .collect(); - - Ok(remainings) - } } pub async fn insert_events( @@ -479,67 +404,23 @@ async fn process_events( mod test { use super::*; use crate::{ - campaign::update_campaign::{increase_remaining_for_campaign, CAMPAIGN_REMAINING_KEY}, - db::redis_pool::TESTS_POOL, + db::{redis_pool::TESTS_POOL, tests_postgres::DATABASE_POOL}, }; + use adapter::DummyAdapter; use primitives::util::tests::prep_db::DUMMY_CAMPAIGN; #[tokio::test] - async fn does_it_increase_remaining() { - let mut redis = TESTS_POOL.get().await.expect("Should return Object"); - let campaign = DUMMY_CAMPAIGN.clone(); - let key = format!("{}:{}", CAMPAIGN_REMAINING_KEY, campaign.id); - - // Setting the redis base variable - redis::cmd("SET") - .arg(&key) - .arg(100_u64) - .query_async::<_, ()>(&mut redis.connection) - .await - .expect("should set"); - - // 2 async calls at once, should be 500 after them - futures::future::try_join_all([ - increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), - increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(200)), - ]) - .await - .expect("Should increase remaining twice"); - - let remaining = redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.connection) - .await - .expect("should get remaining"); - assert_eq!( - remaining.map(UnifiedNum::from_u64), - Some(UnifiedNum::from_u64(500)) - ); - - increase_remaining_for_campaign(&redis, campaign.id, campaign.budget) - .await - .expect("should increase"); - - let remaining = redis::cmd("GET") - .arg(&key) - // Directly parsing to u64 as we know it will be >0 - .query_async::<_, Option>(&mut redis.connection) - .await - .expect("should get remaining"); - - let should_be_remaining = UnifiedNum::from_u64(500) + campaign.budget; - assert_eq!(remaining.map(UnifiedNum::from), Some(should_be_remaining)); - - increase_remaining_for_campaign(&redis, campaign.id, UnifiedNum::from_u64(0)) - .await - .expect("should increase remaining"); - - let remaining = redis::cmd("GET") - .arg(&key) - .query_async::<_, Option>(&mut redis.connection) - .await - .expect("should get remaining"); - - assert_eq!(remaining.map(UnifiedNum::from), Some(should_be_remaining)); + /// Test single campaign creation and modification + // & + /// Test with multiple campaigns (because of Budget) a modification of campaign + async fn create_and_modify_with_multiple_campaigns() { + todo!() + // let redis = TESTS_POOL.get().await.expect("Should get redis"); + // let postgres = DATABASE_POOL.get().await.expect("Should get postgres"); + // let campaign = DUMMY_CAMPAIGN.clone(); + + // let app = Application::new() + + } } From 3ca6ab56264fb5ca6c23b149706ebe6adc652ed6 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Wed, 4 Aug 2021 18:41:40 +0300 Subject: [PATCH 24/31] primitives - sentry - impl From for CreateCampaign --- primitives/src/sentry.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/primitives/src/sentry.rs b/primitives/src/sentry.rs index 6018dc235..ba07b4035 100644 --- a/primitives/src/sentry.rs +++ b/primitives/src/sentry.rs @@ -361,6 +361,26 @@ pub mod campaign_create { } } + /// This implementation helps with test setup + /// **NOTE:** It erases the CampaignId, since the creation of the campaign gives it's CampaignId + impl From for CreateCampaign { + fn from(campaign: Campaign) -> Self { + Self { + channel: campaign.channel, + creator: campaign.creator, + budget: campaign.budget, + validators: campaign.validators, + title: campaign.title, + pricing_bounds: campaign.pricing_bounds, + event_submission: campaign.event_submission, + ad_units: campaign.ad_units, + targeting_rules: campaign.targeting_rules, + created: campaign.created, + active: campaign.active, + } + } + } + // All editable fields stored in one place, used for checking when a budget is changed #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct ModifyCampaign { @@ -403,7 +423,7 @@ pub mod campaign_create { if let Some(new_pricing_bounds) = self.pricing_bounds { campaign.pricing_bounds = Some(new_pricing_bounds); } - + if let Some(new_event_submission) = self.event_submission { campaign.event_submission = Some(new_event_submission); } From 11dc71564d960796801c967aa92014d7ba0cbff2 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Wed, 4 Aug 2021 18:42:05 +0300 Subject: [PATCH 25/31] primitives - validator - From
for ValidatorID --- primitives/src/validator.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/primitives/src/validator.rs b/primitives/src/validator.rs index 81452b5c9..72b89f7a1 100644 --- a/primitives/src/validator.rs +++ b/primitives/src/validator.rs @@ -39,6 +39,12 @@ impl From<&Address> for ValidatorId { } } +impl From
for ValidatorId { + fn from(address: Address) -> Self { + Self(address) + } +} + impl From<&[u8; 20]> for ValidatorId { fn from(bytes: &[u8; 20]) -> Self { Self(Address::from(bytes)) From 07d9c74c969b243060800d0f883800e3799bb46d Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 11:12:25 +0300 Subject: [PATCH 26/31] primitives - sentry - accounting - Balances - add spender/earner --- primitives/src/sentry/accounting.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/primitives/src/sentry/accounting.rs b/primitives/src/sentry/accounting.rs index 4b9256cec..e04167eb9 100644 --- a/primitives/src/sentry/accounting.rs +++ b/primitives/src/sentry/accounting.rs @@ -68,6 +68,16 @@ impl Balances { Ok(()) } + + /// Adds the spender to the Balances with `UnifiedNum::from(0)` if he does not exist + pub fn add_spender(&mut self, spender: Address) { + self.spenders.entry(spender).or_insert(UnifiedNum::from(0)); + } + + /// Adds the earner to the Balances with `UnifiedNum::from(0)` if he does not exist + pub fn add_earner(&mut self, earner: Address) { + self.earners.entry(earner).or_insert(UnifiedNum::from(0)); + } } #[derive(Debug)] From d80e109c61fbefe392f091f32cd883b5bca81ea6 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 11:13:31 +0300 Subject: [PATCH 27/31] sentry - Applicaiton - add CampaignRemaining to app --- sentry/src/lib.rs | 17 +++++++++++------ sentry/src/main.rs | 7 ++++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/sentry/src/lib.rs b/sentry/src/lib.rs index ca5bde526..0d2126a76 100644 --- a/sentry/src/lib.rs +++ b/sentry/src/lib.rs @@ -7,6 +7,7 @@ use crate::routes::channel::channel_status; use crate::routes::event_aggregate::list_channel_event_aggregates; use crate::routes::validator_message::{extract_params, list_validator_messages}; use chrono::Utc; +use db::CampaignRemaining; use hyper::{Body, Method, Request, Response, StatusCode}; use lazy_static::lazy_static; use middleware::{ @@ -90,10 +91,11 @@ impl RouteParams { #[derive(Clone)] pub struct Application { pub adapter: A, + pub config: Config, pub logger: Logger, pub redis: MultiplexedConnection, pub pool: DbPool, - pub config: Config, + pub campaign_remaining: CampaignRemaining, } impl Application { @@ -103,6 +105,7 @@ impl Application { logger: Logger, redis: MultiplexedConnection, pool: DbPool, + campaign_remaining: CampaignRemaining ) -> Self { Self { adapter, @@ -110,6 +113,7 @@ impl Application { logger, redis, pool, + campaign_remaining, } } @@ -513,13 +517,11 @@ pub mod test_util { }, }; - use crate::{Application, db::{ - redis_pool::TESTS_POOL, - tests_postgres::{setup_test_migrations, DATABASE_POOL}, - }}; + use crate::{Application, db::{CampaignRemaining, redis_pool::TESTS_POOL, tests_postgres::{setup_test_migrations, DATABASE_POOL}}}; + /// Uses production configuration to setup the correct Contract addresses for tokens. pub async fn setup_dummy_app() -> Application { - let config = configuration("development", None).expect("Should get Config"); + let config = configuration("production", None).expect("Should get Config"); let adapter = DummyAdapter::init( DummyAdapterOptions { dummy_identity: IDS["leader"], @@ -536,12 +538,15 @@ pub mod test_util { .await .expect("Migrations should succeed"); + let campaign_remaining = CampaignRemaining::new(redis.connection.clone()); + let app = Application::new( adapter, config, discard_logger(), redis.connection.clone(), database.pool.clone(), + campaign_remaining ); app diff --git a/sentry/src/main.rs b/sentry/src/main.rs index f8963f562..634831e5d 100644 --- a/sentry/src/main.rs +++ b/sentry/src/main.rs @@ -10,7 +10,7 @@ use primitives::adapter::{Adapter, DummyAdapterOptions, KeystoreOptions}; use primitives::config::configuration; use primitives::util::tests::prep_db::{AUTH, IDS}; use primitives::ValidatorId; -use sentry::db::{postgres_connection, redis_connection, setup_migrations}; +use sentry::db::{CampaignRemaining, postgres_connection, redis_connection, setup_migrations}; use sentry::Application; use slog::{error, info, Logger}; use std::{ @@ -115,18 +115,19 @@ async fn main() -> Result<(), Box> { // Check connection and setup migrations before setting up Postgres setup_migrations(&environment).await; let postgres = postgres_connection(42).await; + let campaign_remaining = CampaignRemaining::new(redis.clone()); match adapter { AdapterTypes::EthereumAdapter(adapter) => { run( - Application::new(*adapter, config, logger, redis, postgres), + Application::new(*adapter, config, logger, redis, postgres, campaign_remaining), socket_addr, ) .await } AdapterTypes::DummyAdapter(adapter) => { run( - Application::new(*adapter, config, logger, redis, postgres), + Application::new(*adapter, config, logger, redis, postgres, campaign_remaining), socket_addr, ) .await From e1a245ed20d7008f471a4cdca7462587b001ad39 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 11:14:41 +0300 Subject: [PATCH 28/31] sentry - db - CampaignRemaining - MGET guard against empty vec of campaigns --- sentry/src/db/campaign.rs | 54 +++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 9c5287a20..ea24ec6f7 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -155,6 +155,11 @@ mod campaign_remaining { &self, campaigns: &[CampaignId], ) -> Result, RedisError> { + // `MGET` fails on empty keys + if campaigns.is_empty() { + return Ok(vec![]); + } + let keys: Vec = campaigns .iter() .map(|campaign| Self::get_key(*campaign)) @@ -314,6 +319,17 @@ mod campaign_remaining { let redis = TESTS_POOL.get().await.expect("Should return Object"); let campaign_remaining = CampaignRemaining::new(redis.connection.clone()); + // get multiple with empty campaigns slice + // `MGET` throws error on an empty keys argument + assert!( + campaign_remaining + .get_multiple(&[]) + .await + .expect("Should get multiple") + .is_empty(), + "Should return an empty result" + ); + let campaigns = (CampaignId::new(), CampaignId::new(), CampaignId::new()); // set initial amounts @@ -323,23 +339,39 @@ mod campaign_remaining { .await .expect("Should set value in redis")); - assert!(campaign_remaining - .set_initial(campaigns.1, UnifiedNum::from(200)) - .await - .expect("Should set value in redis")); + assert!(campaign_remaining + .set_initial(campaigns.1, UnifiedNum::from(200)) + .await + .expect("Should set value in redis")); - assert!(campaign_remaining - .set_initial(campaigns.2, UnifiedNum::from(300)) - .await - .expect("Should set value in redis")); + assert!(campaign_remaining + .set_initial(campaigns.2, UnifiedNum::from(300)) + .await + .expect("Should set value in redis")); } // set campaigns.1 to negative value, should return `0` because of `max(value, 0)` - assert_eq!(-300_i64, campaign_remaining.decrease_by(campaigns.1, UnifiedNum::from(500)).await.expect("Should decrease remaining")); + assert_eq!( + -300_i64, + campaign_remaining + .decrease_by(campaigns.1, UnifiedNum::from(500)) + .await + .expect("Should decrease remaining") + ); - let multiple = campaign_remaining.get_multiple(&[campaigns.0, campaigns.1, campaigns.2]).await.expect("Should get multiple"); + let multiple = campaign_remaining + .get_multiple(&[campaigns.0, campaigns.1, campaigns.2]) + .await + .expect("Should get multiple"); - assert_eq!(vec![UnifiedNum::from(100), UnifiedNum::from(0), UnifiedNum::from(300)], multiple); + assert_eq!( + vec![ + UnifiedNum::from(100), + UnifiedNum::from(0), + UnifiedNum::from(300) + ], + multiple + ); } } } From 9a94c5f1fad1e8577c027c7c2ba7528b381c3db1 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 11:15:27 +0300 Subject: [PATCH 29/31] sentry - routes - campaign create/modify - more checks & tests --- sentry/src/routes/campaign.rs | 310 +++++++++++++++++++++++++++++----- 1 file changed, 269 insertions(+), 41 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 6d57607b0..7fd2b9b50 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -1,4 +1,13 @@ -use crate::{Application, Auth, ResponseError, Session, access::{self, check_access}, db::{CampaignRemaining, DbPool, RedisError, accounting::get_accounting_spent, campaign::{get_campaigns_by_channel, insert_campaign, update_campaign}, spendable::fetch_spendable}, success_response}; +use crate::{ + access::{self, check_access}, + db::{ + accounting::get_accounting_spent, + campaign::{get_campaigns_by_channel, insert_campaign, update_campaign}, + spendable::fetch_spendable, + CampaignRemaining, DbPool, RedisError, + }, + success_response, Application, Auth, ResponseError, Session, +}; use chrono::Utc; use deadpool_postgres::PoolError; use hyper::{Body, Request, Response}; @@ -69,37 +78,55 @@ pub async fn create_campaign( let error_response = ResponseError::BadRequest("err occurred; please try again later".to_string()); - let accounting_spent = - get_accounting_spent(app.pool.clone(), &campaign.creator, &campaign.channel.id()).await?; + let total_remaining = + { + let accounting_spent = + get_accounting_spent(app.pool.clone(), &campaign.creator, &campaign.channel.id()) + .await?; - let latest_spendable = - fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()) - .await? - .ok_or(ResponseError::BadRequest( - "No spendable amount found for the Campaign creator".to_string(), - ))?; - let total_deposited = latest_spendable.deposit.total; + let latest_spendable = + fetch_spendable(app.pool.clone(), &campaign.creator, &campaign.channel.id()) + .await? + .ok_or(ResponseError::BadRequest( + "No spendable amount found for the Campaign creator".to_string(), + ))?; + // Gets the latest Spendable for this (spender, channelId) pair + let total_deposited = latest_spendable.deposit.total; - let remaining_for_channel = - total_deposited - .checked_sub(&accounting_spent) - .ok_or(ResponseError::FailedValidation( - "No more budget remaining".to_string(), - ))?; + total_deposited.checked_sub(&accounting_spent).ok_or( + ResponseError::FailedValidation("No more budget remaining".to_string()), + )? + }; - if campaign.budget > remaining_for_channel { + let channel_campaigns = get_campaigns_by_channel(&app.pool, &campaign.channel.id()) + .await? + .iter() + .map(|c| c.id) + .collect::>(); + + let campaigns_remaining_sum = app + .campaign_remaining + .get_multiple(&channel_campaigns) + .await? + .iter() + .sum::>() + .ok_or(Error::Calculation)? + // DO NOT FORGET to add the Campaign being created right now! + .checked_add(&campaign.budget) + .ok_or(Error::Calculation)?; + + if !(campaigns_remaining_sum <= total_remaining) || campaign.budget > total_remaining { return Err(ResponseError::BadRequest( - "Not enough deposit left for the new campaign budget".to_string(), + "Not enough deposit left for the new campaign's budget".to_string(), )); } // If the campaign is being created, the amount spent is 0, therefore remaining = budget - let remaining_set = CampaignRemaining::new(app.redis.clone()).set_initial(campaign.id, campaign.budget) + let remaining_set = CampaignRemaining::new(app.redis.clone()) + .set_initial(campaign.id, campaign.budget) .await .map_err(|_| { - ResponseError::BadRequest( - "Couldn't set remaining while creating campaign".to_string(), - ) + ResponseError::BadRequest("Couldn't set remaining while creating campaign".to_string()) })?; // If for some reason the randomly generated `CampaignId` exists in Redis @@ -107,7 +134,7 @@ pub async fn create_campaign( if !remaining_set { return Err(ResponseError::Conflict( "The generated CampaignId already exists, please repeat the request".to_string(), - )) + )); } // insert Campaign @@ -152,12 +179,10 @@ pub mod update_campaign { let modify_campaign_fields = serde_json::from_slice::(&body) .map_err(|e| ResponseError::FailedValidation(e.to_string()))?; - let campaign_remaining = CampaignRemaining::new(app.redis.clone()); - // modify Campaign let modified_campaign = modify_campaign( &app.pool, - campaign_remaining, + &app.campaign_remaining, campaign_being_mutated, modify_campaign_fields, ) @@ -169,7 +194,7 @@ pub mod update_campaign { pub async fn modify_campaign( pool: &DbPool, - campaign_remaining: CampaignRemaining, + campaign_remaining: &CampaignRemaining, campaign: Campaign, modify_campaign: ModifyCampaign, ) -> Result { @@ -178,7 +203,7 @@ pub mod update_campaign { // *NOTE*: To close a campaign set campaignBudget to campaignSpent so that spendable == 0 let delta_budget = if let Some(new_budget) = modify_campaign.budget { - get_delta_budget(&campaign_remaining, &campaign, new_budget).await? + get_delta_budget(campaign_remaining, &campaign, new_budget).await? } else { None }; @@ -229,7 +254,9 @@ pub mod update_campaign { .ok_or(Error::Calculation)?; if !(new_campaigns_remaining <= total_remaining) { - return Err(Error::CampaignNotModified); + return Err(Error::NewBudget( + "Not enough deposit left for the campaign's new budget".to_string(), + )); } // there is a chance that the new remaining will be negative even when increasing the budget @@ -402,25 +429,226 @@ async fn process_events( #[cfg(test)] mod test { - use super::*; + use super::{update_campaign::modify_campaign, *}; use crate::{ - db::{redis_pool::TESTS_POOL, tests_postgres::DATABASE_POOL}, + db::{accounting::insert_accounting, spendable::insert_spendable}, + test_util::setup_dummy_app, + }; + use hyper::StatusCode; + use primitives::{ + sentry::accounting::{Balances, CheckedState}, + spender::{Deposit, Spendable}, + util::tests::prep_db::DUMMY_CAMPAIGN, + ValidatorId, }; - use adapter::DummyAdapter; - use primitives::util::tests::prep_db::DUMMY_CAMPAIGN; #[tokio::test] /// Test single campaign creation and modification // & /// Test with multiple campaigns (because of Budget) a modification of campaign async fn create_and_modify_with_multiple_campaigns() { - todo!() - // let redis = TESTS_POOL.get().await.expect("Should get redis"); - // let postgres = DATABASE_POOL.get().await.expect("Should get postgres"); - // let campaign = DUMMY_CAMPAIGN.clone(); - - // let app = Application::new() - - + let app = setup_dummy_app().await; + + let build_request = |create_campaign: CreateCampaign| -> Request { + let auth = Auth { + era: 0, + uid: ValidatorId::from(create_campaign.creator), + }; + + let body = + Body::from(serde_json::to_string(&create_campaign).expect("Should serialize")); + + Request::builder() + .extension(auth) + .body(body) + .expect("Should build Request") + }; + + let campaign: Campaign = { + // erases the CampaignId for the CreateCampaign request + let mut create = CreateCampaign::from(DUMMY_CAMPAIGN.clone()); + // 500.00000000 + create.budget = UnifiedNum::from(50_000_000_000); + + let spendable = Spendable { + spender: create.creator, + channel: create.channel.clone(), + deposit: Deposit { + // a deposit equal to double the Campaign Budget + total: UnifiedNum::from(200_000_000_000), + still_on_create2: UnifiedNum::from(0), + }, + }; + assert!(insert_spendable(app.pool.clone(), &spendable) + .await + .expect("Should insert Spendable for Campaign creator")); + + let mut balances = Balances::::default(); + balances.add_spender(create.creator); + + // TODO: Replace this once https://github.com/AdExNetwork/adex-validator-stack-rust/pull/413 is merged + let _accounting = insert_accounting(app.pool.clone(), create.channel.clone(), balances) + .await + .expect("Should create Accounting"); + + let create_response = create_campaign(build_request(create), &app) + .await + .expect("Should create campaign"); + + assert_eq!(StatusCode::OK, create_response.status()); + let json = hyper::body::to_bytes(create_response.into_body()) + .await + .expect("Should get json"); + + let campaign: Campaign = + serde_json::from_slice(&json).expect("Should get new Campaign"); + + assert_ne!(DUMMY_CAMPAIGN.id, campaign.id); + + let campaign_remaining = CampaignRemaining::new(app.redis.clone()); + + let remaining = campaign_remaining + .get_remaining_opt(campaign.id) + .await + .expect("Should get remaining from redis") + .expect("There should be value for the Campaign"); + + assert_eq!( + UnifiedNum::from(50_000_000_000), + UnifiedNum::from(remaining.unsigned_abs()) + ); + campaign + }; + + // modify campaign + let modified = { + // 1000.00000000 + let new_budget = UnifiedNum::from(100_000_000_000); + let modify = ModifyCampaign { + budget: Some(new_budget.clone()), + validators: None, + title: Some("Updated title".to_string()), + pricing_bounds: None, + event_submission: None, + ad_units: None, + targeting_rules: None, + }; + + let modified_campaign = + modify_campaign(&app.pool, &app.campaign_remaining, campaign.clone(), modify) + .await + .expect("Should modify campaign"); + + assert_eq!(new_budget, modified_campaign.budget); + assert_eq!(Some("Updated title".to_string()), modified_campaign.title); + + modified_campaign + }; + + // we have 1000 left from our deposit, so we are using half of it + let _second_campaign = { + // erases the CampaignId for the CreateCampaign request + let mut create_second = CreateCampaign::from(DUMMY_CAMPAIGN.clone()); + // 500.00000000 + create_second.budget = UnifiedNum::from(50_000_000_000); + + let create_response = create_campaign(build_request(create_second), &app) + .await + .expect("Should create campaign"); + + assert_eq!(StatusCode::OK, create_response.status()); + let json = hyper::body::to_bytes(create_response.into_body()) + .await + .expect("Should get json"); + + let second_campaign: Campaign = + serde_json::from_slice(&json).expect("Should get new Campaign"); + + second_campaign + }; + + // No budget left for new campaigns + // remaining: 500 + // new campaign budget: 600 + { + // erases the CampaignId for the CreateCampaign request + let mut create = CreateCampaign::from(DUMMY_CAMPAIGN.clone()); + // 600.00000000 + create.budget = UnifiedNum::from(60_000_000_000); + + let create_err = create_campaign(build_request(create), &app) + .await + .expect_err("Should return Error response"); + + assert_eq!(ResponseError::BadRequest("Not enough deposit left for the new campaign's budget".to_string()), create_err); + } + + // modify first campaign, by lowering the budget from 1000 to 900 + let modified = { + let lower_budget = UnifiedNum::from(90_000_000_000); + let modify = ModifyCampaign { + budget: Some(lower_budget.clone()), + validators: None, + title: None, + pricing_bounds: None, + event_submission: None, + ad_units: None, + targeting_rules: None, + }; + + let modified_campaign = + modify_campaign(&app.pool, &app.campaign_remaining, modified, modify) + .await + .expect("Should modify campaign"); + + assert_eq!(lower_budget, modified_campaign.budget); + + modified_campaign + }; + + // Just enough budget to create this Campaign + // remaining: 600 + // new campaign budget: 600 + { + // erases the CampaignId for the CreateCampaign request + let mut create = CreateCampaign::from(DUMMY_CAMPAIGN.clone()); + // 600.00000000 + create.budget = UnifiedNum::from(60_000_000_000); + + let create_response = create_campaign(build_request(create), &app) + .await + .expect("Should return create campaign"); + + let json = hyper::body::to_bytes(create_response.into_body()) + .await + .expect("Should get json"); + + let _campaign: Campaign = + serde_json::from_slice(&json).expect("Should get new Campaign"); + } + + // Modify a campaign without enough budget + // remaining: 0 + // new campaign budget: 1100 + // current campaign budget: 900 + { + let new_budget = UnifiedNum::from(110_000_000_000); + let modify = ModifyCampaign { + budget: Some(new_budget), + validators: None, + title: None, + pricing_bounds: None, + event_submission: None, + ad_units: None, + targeting_rules: None, + }; + + let modify_err = + modify_campaign(&app.pool, &app.campaign_remaining, modified, modify) + .await + .expect_err("Should return Error response"); + + assert!(matches!(modify_err, Error::NewBudget(string) if string == "Not enough deposit left for the campaign's new budget")); + } } } From 5177217cd7d24ddf261a855983259dabf52311f5 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 12:46:50 +0300 Subject: [PATCH 30/31] sentry - db - make pub insert_accounting --- sentry/src/db/accounting.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sentry/src/db/accounting.rs b/sentry/src/db/accounting.rs index ffc8e3db4..94fcae16e 100644 --- a/sentry/src/db/accounting.rs +++ b/sentry/src/db/accounting.rs @@ -38,9 +38,7 @@ pub async fn get_accounting_spent( Ok(row.get("spent")) } -// TODO This is still WIP -#[allow(dead_code)] -async fn insert_accounting( +pub async fn insert_accounting( pool: DbPool, channel: Channel, balances: Balances, From 75a5a5f63350f4a1991fa7c209329f1b8b142f1c Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 9 Aug 2021 15:46:53 +0300 Subject: [PATCH 31/31] sentry - routes - campaign - get_delta_budget - fix logic & naming --- sentry/src/routes/campaign.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sentry/src/routes/campaign.rs b/sentry/src/routes/campaign.rs index 7fd2b9b50..b3db3abe1 100644 --- a/sentry/src/routes/campaign.rs +++ b/sentry/src/routes/campaign.rs @@ -330,6 +330,7 @@ pub mod update_campaign { .checked_sub(¤t_budget) .and_then(|delta_budget| old_remaining.checked_add(&delta_budget)) .ok_or(Error::Calculation)?; + // new remaining > old remaining let increase_by = new_remaining .checked_sub(&old_remaining) .ok_or(Error::Calculation)?; @@ -337,13 +338,14 @@ pub mod update_campaign { DeltaBudget::Increase(increase_by) } DeltaBudget::Decrease(()) => { - // delta budget = New budget - Old budget ( the difference between the new and old when New > Old) + // delta budget = Old budget - New budget ( the difference between the new and old when New < Old) let new_remaining = ¤t_budget .checked_sub(&new_budget) - .and_then(|delta_budget| old_remaining.checked_add(&delta_budget)) + .and_then(|delta_budget| old_remaining.checked_sub(&delta_budget)) .ok_or(Error::Calculation)?; - let decrease_by = new_remaining - .checked_sub(&old_remaining) + // old remaining > new remaining + let decrease_by = old_remaining + .checked_sub(&new_remaining) .ok_or(Error::Calculation)?; DeltaBudget::Decrease(decrease_by)