diff --git a/Cargo.lock b/Cargo.lock index 75b85ae8d..369d12e06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -149,15 +149,6 @@ version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "983cd8b9d4b02a6dc6ffa557262eb5858a27a0038ffffe21a0f133eaa819a164" -[[package]] -name = "array-init" -version = "0.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23589ecb866b460d3a0f1278834750268c607e8e28a1b982c907219f3178cd72" -dependencies = [ - "nodrop", -] - [[package]] name = "array-init" version = "2.0.1" @@ -860,7 +851,7 @@ dependencies = [ "phf 0.8.0", "proc-macro2", "quote", - "smallvec 1.10.0", + "smallvec", "syn", ] @@ -2056,12 +2047,6 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73cbba799671b762df5a175adf59ce145165747bb891505c43d09aefbbf38beb" -[[package]] -name = "maybe-uninit" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00" - [[package]] name = "md-5" version = "0.10.5" @@ -2518,7 +2503,7 @@ dependencies = [ "cfg-if", "libc", "redox_syscall", - "smallvec 1.10.0", + "smallvec", "windows-sys", ] @@ -2879,7 +2864,7 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73d946ec7d256b04dfadc4e6a3292324e6f417124750fc5c0950f981b703a0f1" dependencies = [ - "array-init 2.0.1", + "array-init", "bytes", "chrono", "fallible-iterator", @@ -2959,7 +2944,6 @@ dependencies = [ "parse-display", "pretty_assertions", "serde", - "serde-hex", "serde_json", "serde_millis", "serde_qs 0.10.1", @@ -3416,7 +3400,7 @@ dependencies = [ "html5ever", "matches", "selectors", - "smallvec 1.10.0", + "smallvec", "tendril", ] @@ -3518,7 +3502,7 @@ dependencies = [ "phf_codegen 0.8.0", "precomputed-hash", "servo_arc", - "smallvec 1.10.0", + "smallvec", "thin-slice", ] @@ -3574,17 +3558,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "serde-hex" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca37e3e4d1b39afd7ff11ee4e947efae85adfddf4841787bfa47c470e96dc26d" -dependencies = [ - "array-init 0.0.4", - "serde", - "smallvec 0.6.14", -] - [[package]] name = "serde_derive" version = "1.0.145" @@ -3837,15 +3810,6 @@ dependencies = [ "deunicode", ] -[[package]] -name = "smallvec" -version = "0.6.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97fcaeba89edba30f044a10c6a3cc39df9c3f17d7cd829dd1446cab35f890e0" -dependencies = [ - "maybe-uninit", -] - [[package]] name = "smallvec" version = "1.10.0" diff --git a/adapter/src/ethereum/client.rs b/adapter/src/ethereum/client.rs index f1f2d3c94..6ede76817 100644 --- a/adapter/src/ethereum/client.rs +++ b/adapter/src/ethereum/client.rs @@ -10,7 +10,7 @@ use ethsign::{KeyFile, Signature}; use primitives::{Address, BigNum, Chain, ChainId, ChainOf, Channel, Config, ValidatorId}; use super::{ - error::{Error, EwtSigningError, KeystoreError, VerifyError}, + error::{Error, KeystoreError, SigningError, VerifyError}, ewt::{self, Payload}, to_ethereum_signed, Electrum, LockedWallet, UnlockedWallet, WalletState, IDENTITY_ABI, OUTPACE_ABI, @@ -275,15 +275,14 @@ impl Locked for Ethereum { #[async_trait] impl Unlocked for Ethereum { fn sign(&self, state_root: &str) -> Result { - let state_root = hex::decode(state_root).map_err(VerifyError::StateRootDecoding)?; + let state_root = hex::decode(state_root).map_err(SigningError::StateRootDecoding)?; let message = to_ethereum_signed(&state_root); let wallet_sign = self .state .wallet .sign(&message) - // TODO: This is not entirely true, we do not sign an Ethereum Web Token but Outpace state_root - .map_err(|err| EwtSigningError::SigningMessage(err.to_string()))?; + .map_err(|err| SigningError::SignStateRoot(err.to_string()))?; Ok(format!("0x{}", hex::encode(wallet_sign.to_electrum()))) } @@ -298,7 +297,7 @@ impl Unlocked for Ethereum { chain_id: for_chain, }; - let token = ewt::Token::sign(&self.state.wallet, payload).map_err(Error::SignMessage)?; + let token = ewt::Token::sign(&self.state.wallet, payload).map_err(Error::TokenSign)?; Ok(token.to_string()) } diff --git a/adapter/src/ethereum/error.rs b/adapter/src/ethereum/error.rs index a7a8b610a..307924bee 100644 --- a/adapter/src/ethereum/error.rs +++ b/adapter/src/ethereum/error.rs @@ -1,4 +1,5 @@ use crate::Error as AdapterError; +use hex::FromHexError; use primitives::{ address::Error as AddressError, big_num::ParseBigIntError, ChainId, ChannelId, ValidatorId, }; @@ -17,11 +18,12 @@ impl From for AdapterError { err @ Error::ChainNotWhitelisted(..) => AdapterError::adapter(err), err @ Error::InvalidDepositAsset(..) => AdapterError::adapter(err), err @ Error::BigNumParsing(..) => AdapterError::adapter(err), - err @ Error::SignMessage(..) => AdapterError::adapter(err), + err @ Error::TokenSign(..) => AdapterError::adapter(err), err @ Error::VerifyMessage(..) => AdapterError::adapter(err), err @ Error::ContractInitialization(..) => AdapterError::adapter(err), err @ Error::ContractQuerying(..) => AdapterError::adapter(err), err @ Error::VerifyAddress(..) => AdapterError::adapter(err), + err @ Error::SigningMessage(..) => AdapterError::adapter(err), err @ Error::AuthenticationTokenNotIntendedForUs { .. } => { AdapterError::authentication(err) } @@ -51,7 +53,7 @@ pub enum Error { ChannelInactive(ChannelId), /// Signing of the message failed #[error("Signing message: {0}")] - SignMessage(#[from] EwtSigningError), + TokenSign(#[from] EwtSigningError), #[error("Verifying message: {0}")] VerifyMessage(#[from] EwtVerifyError), #[error("Contract initialization: {0}")] @@ -74,6 +76,8 @@ pub enum Error { }, #[error("Insufficient privilege")] InsufficientAuthorizationPrivilege, + #[error("Signing message error: {0}")] + SigningMessage(#[from] SigningError), } #[derive(Debug, Error)] @@ -123,6 +127,14 @@ pub enum EwtSigningError { DecodingHexSignature(#[from] hex::FromHexError), } +#[derive(Debug, Error)] +pub enum SigningError { + #[error("Error while signing message: {0}")] + SignStateRoot(String), + #[error("Error while decoding StateRoot from hex")] + StateRootDecoding(#[from] FromHexError), +} + #[derive(Debug, Error)] pub enum EwtVerifyError { #[error("The Ethereum Web Token header is invalid")] @@ -142,12 +154,18 @@ pub enum EwtVerifyError { /// or if Signature V component is not in "Electrum" notation (`< 27`). #[error("Error when decoding token signature")] InvalidSignature, + #[error(transparent)] + Payload(#[from] PayloadError), +} + +#[derive(Debug, Error)] +pub enum PayloadError { #[error("Payload decoding: {0}")] - PayloadDecoding(#[source] base64::DecodeError), + Decoding(#[source] base64::DecodeError), #[error("Payload deserialization: {0}")] - PayloadDeserialization(#[from] serde_json::Error), + Deserialization(#[from] serde_json::Error), #[error("Payload is not a valid UTF-8 string: {0}")] - PayloadUtf8(#[from] std::str::Utf8Error), + Utf8(#[from] std::str::Utf8Error), } #[cfg(test)] diff --git a/adapter/src/ethereum/ewt.rs b/adapter/src/ethereum/ewt.rs index c729a705f..83d99ab6a 100644 --- a/adapter/src/ethereum/ewt.rs +++ b/adapter/src/ethereum/ewt.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use web3::signing::keccak256; use super::{ - error::{EwtSigningError, EwtVerifyError}, + error::{EwtSigningError, EwtVerifyError, PayloadError}, to_ethereum_signed, Electrum, }; @@ -54,14 +54,13 @@ pub struct Payload { impl Payload { /// Decodes the [`Payload`] from a base64 encoded json string - // TODO: replace with own error? - pub fn base64_decode(encoded_json: &str) -> Result { + pub fn base64_decode(encoded_json: &str) -> Result { let base64_decode = base64::decode_config(encoded_json, base64::URL_SAFE_NO_PAD) - .map_err(EwtVerifyError::PayloadDecoding)?; + .map_err(PayloadError::Decoding)?; - let json = std::str::from_utf8(&base64_decode).map_err(EwtVerifyError::PayloadUtf8)?; + let json = std::str::from_utf8(&base64_decode).map_err(PayloadError::Utf8)?; - serde_json::from_str(json).map_err(EwtVerifyError::PayloadDeserialization) + serde_json::from_str(json).map_err(PayloadError::Deserialization) } } diff --git a/adview-manager/serve/src/routes.rs b/adview-manager/serve/src/routes.rs index 1c3891d98..23909b67d 100644 --- a/adview-manager/serve/src/routes.rs +++ b/adview-manager/serve/src/routes.rs @@ -51,8 +51,7 @@ pub async fn get_preview_ad(Extension(state): Extension>) -> Html>) -> Html< // All passed tokens must be of the same price and decimals, so that the amounts can be accurately compared whitelisted_tokens, size: Some(Size::new(728, 90)), - // TODO: Check this value - navigator_language: Some("bg".into()), + navigator_language: Some("bg-BG".into()), /// Defaulted disabled_video, disabled_sticky: false, diff --git a/adview-manager/src/helpers.rs b/adview-manager/src/helpers.rs index 5e0103d4b..9a670ce09 100644 --- a/adview-manager/src/helpers.rs +++ b/adview-manager/src/helpers.rs @@ -156,7 +156,6 @@ pub fn get_unit_html_with_events( let body = serde_json::to_string(&events_body).expect("It should always serialize EventBody"); - // TODO: check whether the JSON body with `''` quotes executes correctly! let fetch_opts = format!("var fetchOpts = {{ method: 'POST', headers: {{ 'content-type': 'application/json' }}, body: {body} }};"); let validators: String = validators @@ -371,7 +370,7 @@ mod test { // All passed tokens must be of the same price and decimals, so that the amounts can be accurately compared whitelisted_tokens, size: Some(Size::new(300, 100)), - navigator_language: Some("bg".into()), + navigator_language: Some("bg-BG".into()), disabled_video: false, disabled_sticky: false, validators: vec![validator_1_url, validator_2_url], diff --git a/adview-manager/src/manager.rs b/adview-manager/src/manager.rs index 9da2f9534..5f2aa4843 100644 --- a/adview-manager/src/manager.rs +++ b/adview-manager/src/manager.rs @@ -399,7 +399,7 @@ impl Manager { None } }) - // TODO: Check what should happen here if we don't find the Validator + // There should always be validators since we are finding the campaign from the campaigns where we obtained the campaign id in the first place .unwrap(); let html = get_unit_html_with_events( @@ -468,7 +468,7 @@ mod test { // All passed tokens must be of the same price and decimals, so that the amounts can be accurately compared whitelisted_tokens, size: Some(Size::new(300, 100)), - navigator_language: Some("bg".into()), + navigator_language: Some("bg-BG".into()), disabled_video: false, disabled_sticky: false, validators: vec![validator_1_url, validator_2_url, validator_3_url], @@ -676,7 +676,6 @@ mod test { assert!(res.is_some()); - // TODO: Here we modify the campaign manually, verify that such a scenario is possible let campaign = Campaign { campaign: DUMMY_CAMPAIGN.clone(), units_with_price: vec![UnitsWithPrice { diff --git a/lib/protocol-eth b/lib/protocol-eth index 4b4bfe380..d85fa46ac 160000 --- a/lib/protocol-eth +++ b/lib/protocol-eth @@ -1 +1 @@ -Subproject commit 4b4bfe380ae696b241513d8e1cdfb10cc12df332 +Subproject commit d85fa46ac2b605ea92df63b5fbacbe83c1419f75 diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index 4363ccc87..85a7122b6 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -83,8 +83,6 @@ required-features = ["test-util"] # (De)Serialization serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -# TODO: Remove once we change `ChannelId` Serialize impl -serde-hex = "0.1" serde_millis = "0.1" # Used prefixes on field for targeting::Input, and `campaign::Active` serde_with = "2" diff --git a/primitives/src/analytics/query.rs b/primitives/src/analytics/query.rs index b3d7a4b56..40d8d2770 100644 --- a/primitives/src/analytics/query.rs +++ b/primitives/src/analytics/query.rs @@ -99,7 +99,6 @@ pub struct Time { /// - a timestamp in milliseconds /// **Note:** [`DateHour`] rules should be uphold, this means that passed values should always be rounded to hours /// And it should not contain **minutes**, **seconds** or **nanoseconds** - // TODO: Either Timestamp (number) or DateTime (string) de/serialization pub start: DateHour, /// The End [`DateHour`] which will fetch `analytics_time <= end` and should be after Start [`DateHour`]! pub end: Option>, diff --git a/primitives/src/channel.rs b/primitives/src/channel.rs index 684a6784b..2c804c1f7 100644 --- a/primitives/src/channel.rs +++ b/primitives/src/channel.rs @@ -2,22 +2,14 @@ use std::{fmt, ops::Deref, str::FromStr}; use ethereum_types::U256; -use serde::{Deserialize, Deserializer, Serialize}; -use serde_hex::{SerHex, StrictPfx}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use hex::{FromHex, FromHexError}; -use crate::{Address, Validator, ValidatorId}; +use crate::{Address, ToHex, Validator, ValidatorId}; -#[derive(Serialize, Deserialize, PartialEq, Eq, Copy, Clone, Hash)] -#[serde(transparent)] -pub struct ChannelId( - #[serde( - deserialize_with = "deserialize_channel_id", - serialize_with = "SerHex::::serialize" - )] - [u8; 32], -); +#[derive(PartialEq, Eq, Copy, Clone, Hash)] +pub struct ChannelId([u8; 32]); impl ChannelId { pub fn as_bytes(&self) -> &[u8; 32] { @@ -39,6 +31,24 @@ where validate_channel_id(&channel_id).map_err(serde::de::Error::custom) } +impl<'de> Deserialize<'de> for ChannelId { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserialize_channel_id(deserializer).map(ChannelId) + } +} + +impl Serialize for ChannelId { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.0.to_hex_prefixed()) + } +} + fn validate_channel_id(s: &str) -> Result<[u8; 32], FromHexError> { // strip `0x` prefix let hex = s.strip_prefix("0x").unwrap_or(s); @@ -332,6 +342,7 @@ mod postgres { #[cfg(test)] mod test { use crate::{channel::Nonce, postgres::POSTGRES_POOL}; + #[tokio::test] async fn nonce_to_from_sql() { let client = POSTGRES_POOL.get().await.unwrap(); diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index 1c8fd9e4d..32fbc9c56 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -2,8 +2,6 @@ #![deny(clippy::all)] #![deny(rustdoc::broken_intra_doc_links)] #![cfg_attr(docsrs, feature(doc_cfg))] -// TODO: Remove once stabled and upstream num::Integer::div_floor(...) is fixed -#![allow(unstable_name_collisions)] use std::{error, fmt}; #[doc(inline)] diff --git a/primitives/src/targeting/eval.rs b/primitives/src/targeting/eval.rs index c507c1fb9..98cf0be5a 100644 --- a/primitives/src/targeting/eval.rs +++ b/primitives/src/targeting/eval.rs @@ -205,7 +205,6 @@ impl From for SerdeValue { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] #[serde(rename_all = "camelCase")] -// TODO: https://github.com/AdExNetwork/adex-validator-stack-rust/issues/296 pub enum Function { /// Multiplies first two values and then divides product by third value MulDiv(Box, Box, Box), diff --git a/primitives/src/targeting/eval_test.rs b/primitives/src/targeting/eval_test.rs index da2c943ec..5557d0d5d 100644 --- a/primitives/src/targeting/eval_test.rs +++ b/primitives/src/targeting/eval_test.rs @@ -15,7 +15,7 @@ fn get_default_input() -> Input { ad_view: Some(input::AdView { seconds_since_campaign_impression: Some(10), has_custom_preferences: false, - navigator_language: "bg".to_string(), + navigator_language: "bg-BG".to_string(), }), global: input::Global { ad_slot_id: DUMMY_IPFS[0], diff --git a/primitives/src/targeting/input.rs b/primitives/src/targeting/input.rs index abd2e9ea2..9645eb172 100644 --- a/primitives/src/targeting/input.rs +++ b/primitives/src/targeting/input.rs @@ -350,19 +350,6 @@ pub mod balances { .. }) => Some(Value::UnifiedNum(*campaign_total_spent)), }, - // Leave the default of `0` if the publisher is not found in the balances. - field::Balances::PublisherEarnedFromCampaign => { - Some(Value::UnifiedNum(match self { - Get::Getter(Getter { - balances, - publisher_id, - }) => balances.get(publisher_id).cloned().unwrap_or_default(), - Get::Value(Values { - publisher_earned_from_campaign, - .. - }) => *publisher_earned_from_campaign, - })) - } } } } @@ -388,7 +375,7 @@ mod test { // Global scope, accessible everywhere "adView.secondsSinceCampaignImpression": 10, "adView.hasCustomPreferences": true, - "adView.navigatorLanguage": "en", + "adView.navigatorLanguage": "en-US", "adSlotId": "QmcUVX7fvoLMM93uN2bD3wGTH8MXSxeL8hojYfL2Lhp7mR", "adSlotType": "legacy_300x100", "publisherId": "0xE882ebF439207a70dDcCb39E13CA8506c9F45fD9", @@ -402,13 +389,11 @@ mod test { "adUnitId": "Qmasg8FrbuSQpjFu3kRnZF9beg8rEBFrqgi1uXDRwCbX5f", "advertiserId": "0xaCBaDA2d5830d1875ae3D2de207A1363B316Df2F", "campaignId": "0x936da01f9abd4d9d80c702af85c822a8", - "campaignTotalSpent": "40", "campaignSecondsActive": 40633521, "campaignSecondsDuration": 2509030800_u64, "campaignBudget": "100000000000", "eventMinPrice": "1", "eventMaxPrice": "10", - "publisherEarnedFromCampaign": "30", // adSlot scope, accessible on Supermarket and AdView "adSlot.categories": ["IAB3", "IAB13-7", "IAB5"], "adSlot.hostname": "adex.network", @@ -424,7 +409,7 @@ mod test { ad_view: Some(AdView { seconds_since_campaign_impression: Some(10), has_custom_preferences: true, - navigator_language: "en".into(), + navigator_language: "en-US".into(), }), global: Global { ad_slot_id: IPFS[0], diff --git a/primitives/src/targeting/input/field.rs b/primitives/src/targeting/input/field.rs index 4bb650883..8ac4542cf 100644 --- a/primitives/src/targeting/input/field.rs +++ b/primitives/src/targeting/input/field.rs @@ -4,7 +4,7 @@ use std::str::FromStr; use crate::targeting::Error; -pub const FIELDS: [Field; 23] = [ +pub const FIELDS: [Field; 22] = [ // AdView scope, accessible only on the AdView Field::AdView(AdView::SecondsSinceCampaignImpression), Field::AdView(AdView::HasCustomPreferences), @@ -31,7 +31,6 @@ pub const FIELDS: [Field; 23] = [ Field::Campaign(Campaign::EventMaxPrice), // Balances Field::Balances(Balances::CampaignTotalSpent), - Field::Balances(Balances::PublisherEarnedFromCampaign), // AdSlot scope, accessible on Supermarket and AdView Field::AdSlot(AdSlot::Categories), Field::AdSlot(AdSlot::Hostname), @@ -147,8 +146,6 @@ impl From for String { #[display(style = "camelCase")] pub enum Balances { CampaignTotalSpent, - // TODO: AIP#61 Should be dropped since we can't know - PublisherEarnedFromCampaign, } impl TryFrom for Balances { @@ -319,10 +316,6 @@ mod test { Field::Campaign(Campaign::CampaignBudget), SerdeValue::String("campaignBudget".into()), ); - test_field( - Field::Balances(Balances::PublisherEarnedFromCampaign), - SerdeValue::String("publisherEarnedFromCampaign".into()), - ); test_field( Field::AdSlot(AdSlot::Hostname), SerdeValue::String("adSlot.hostname".into()), diff --git a/primitives/src/unified_num.rs b/primitives/src/unified_num.rs index 244080415..bc1cb32c2 100644 --- a/primitives/src/unified_num.rs +++ b/primitives/src/unified_num.rs @@ -309,27 +309,22 @@ impl Integer for UnifiedNum { UnifiedNum::div_floor(self, other) } - // TODO: Check math and write tests fn mod_floor(&self, other: &Self) -> Self { self.0.mod_floor(&other.0).into() } - // TODO: Check math and write tests fn gcd(&self, other: &Self) -> Self { self.0.gcd(&other.0).into() } - // TODO: Check math and write tests fn lcm(&self, other: &Self) -> Self { self.0.lcm(&other.0).into() } - // TODO: Check math and write tests fn divides(&self, other: &Self) -> bool { self.0.divides(&other.0) } - // TODO: Check math and write tests fn is_multiple_of(&self, other: &Self) -> bool { self.0.is_multiple_of(&other.0) } @@ -342,7 +337,6 @@ impl Integer for UnifiedNum { !self.is_even() } - // TODO: Check math and write tests fn div_rem(&self, other: &Self) -> (Self, Self) { let (quotient, remainder) = self.0.div_rem(&other.0); @@ -961,6 +955,69 @@ mod test { ); } } + + #[test] + fn test_unified_num_mod_floor_gcd_lcm_divides_is_multiple_of_div_rem() { + // Mod floor + { + assert_eq!( + (UnifiedNum::from_u64(8)).mod_floor(&UnifiedNum::from_u64(3)), + UnifiedNum::from_u64(2) + ); + assert_eq!( + (UnifiedNum::from_u64(1)).mod_floor(&UnifiedNum::from_u64(2)), + UnifiedNum::from_u64(1) + ); + } + + // GCD + { + assert_eq!( + UnifiedNum::from_u64(6).gcd(&UnifiedNum::from_u64(8)), + UnifiedNum::from_u64(2) + ); + assert_eq!( + UnifiedNum::from_u64(7).gcd(&UnifiedNum::from_u64(3)), + UnifiedNum::from_u64(1) + ); + } + + // LCM + { + assert_eq!( + UnifiedNum::from_u64(7).lcm(&UnifiedNum::from_u64(3)), + UnifiedNum::from_u64(21) + ); + assert_eq!( + UnifiedNum::from_u64(2).lcm(&UnifiedNum::from_u64(4)), + UnifiedNum::from_u64(4) + ); + } + + // Is multiple of + { + assert_eq!( + UnifiedNum::from_u64(9).is_multiple_of(&UnifiedNum::from_u64(3)), + true + ); + assert_eq!( + UnifiedNum::from_u64(3).is_multiple_of(&UnifiedNum::from_u64(9)), + false + ); + } + + // Div rem + { + assert_eq!( + (UnifiedNum::from_u64(8)).div_rem(&UnifiedNum::from_u64(3)), + (UnifiedNum::from_u64(2), UnifiedNum::from_u64(2)) + ); + assert_eq!( + (UnifiedNum::from_u64(1)).div_rem(&UnifiedNum::from_u64(2)), + (UnifiedNum::from_u64(0), UnifiedNum::from_u64(1)) + ); + } + } } #[cfg(feature = "postgres")] diff --git a/sentry/src/db.rs b/sentry/src/db.rs index 1a128a4f1..790e904d5 100644 --- a/sentry/src/db.rs +++ b/sentry/src/db.rs @@ -347,7 +347,6 @@ pub mod tests_postgres { let manager = deadpool_postgres::Manager::from_config(config, NoTls, self.manager_config.clone()); - // TODO: Fix error mapping let pool = deadpool_postgres::Pool::builder(manager) .max_size(15) .build() diff --git a/sentry/src/db/analytics.rs b/sentry/src/db/analytics.rs index 3767dd2c6..78645196e 100644 --- a/sentry/src/db/analytics.rs +++ b/sentry/src/db/analytics.rs @@ -299,6 +299,7 @@ mod test { let ad_unit = DUMMY_AD_UNITS[0].clone(); let ad_unit_2 = DUMMY_AD_UNITS[1].clone(); let ad_slot_ipfs = DUMMY_IPFS[0]; + let ad_slot_ipfs_2 = DUMMY_IPFS[1]; setup_test_migrations(database.pool.clone()) .await @@ -311,9 +312,7 @@ mod test { let hours = 0..=23_u32; generate_analytics_for_december_2021(&database.pool, &ad_unit, ad_slot_ipfs).await; - // - // - // TODO: throw a few more analytics in specific DateHours w/ unique Query and check if it filters the analytics correctly + let other_analytics: HashMap<&str, Analytics> = { // 5.12.2021 16:00 let mut click_germany = make_click_analytics(&ad_unit, ad_slot_ipfs, 5, 16); @@ -336,16 +335,31 @@ mod test { .await .expect("Should update"); + // 8.12.2021 23:00 + let click_ad_slot_2 = make_click_analytics(&ad_unit, ad_slot_ipfs_2, 8, 23); + let click_ad_slot_2 = update_analytics(&database.pool, click_ad_slot_2) + .await + .expect("Should update"); + + // 9.12.2021 00:00 + let mut impression_new_slot_type = + make_impression_analytics(&ad_unit, ad_slot_ipfs_2, 9, 0); + impression_new_slot_type.ad_slot_type = Some("legacy_500x500".to_string()); + let impression_new_slot_type = + update_analytics(&database.pool, impression_new_slot_type) + .await + .expect("Should update"); + vec![ ("click_germany", click_germany), ("impression_ad_unit_2", impression_ad_unit_2), ("impression_publisher_2", impression_publisher_2), + ("click_ad_slot_2", click_ad_slot_2), + ("impression_new_slot_type", impression_new_slot_type), ] .into_iter() .collect() }; - // - // let amount_per_day: UnifiedNum = hours .clone() @@ -843,6 +857,114 @@ mod test { "It's count should be == to the hour" ) } + + // Filter by ad_slot == AdSlot2 + // + // Type: Click + // DateHour: 8.12.2021 23:00 + // We should get 1 analytics entry + { + let ad_slot_2_query = AnalyticsQuery { + limit: 1000, + event_type: CLICK, + metric: Metric::Count, + segment_by: Some(AllowedKey::Country), + time: Time { + timeframe: Timeframe::Day, + start: start_date, + end: Some(end_date), + }, + campaign_id: None, + ad_unit: None, + ad_slot: Some(ad_slot_ipfs_2), + ad_slot_type: None, + advertiser: None, + publisher: None, + hostname: None, + country: None, + os_name: None, + chains: vec![], + }; + + let count_clicks = fetch_analytics( + &database.pool, + ad_slot_2_query.clone(), + ALLOWED_KEYS.clone(), + None, + ad_slot_2_query.limit, + ) + .await + .expect("Should fetch"); + + assert_eq!(1, count_clicks.len(), "Only single analytics is expected"); + let fetched = count_clicks.get(0).expect("Should have index 0"); + assert_eq!( + 23, + fetched.value.get_count().expect("Should be Metric::Count"), + "The fetched count should be the same as the hour of the Analytics" + ); + assert_eq!( + other_analytics["click_ad_slot_2"].time.to_datetime(), + fetched.time, + "The fetched analytics date should be the same as the inserted Analytics, but with no hour because Timeframe::Day" + ); + } + + // Filter by ad_slot_type + // + // Type: Impression + // DateHour: 9.12.2021 00:00 + // We should get 1 analytics entry + { + let ad_slot_type_query = AnalyticsQuery { + limit: 1000, + event_type: IMPRESSION, + metric: Metric::Count, + segment_by: Some(AllowedKey::Country), + time: Time { + timeframe: Timeframe::Day, + start: start_date, + end: Some(end_date), + }, + campaign_id: None, + ad_unit: None, + ad_slot: None, + ad_slot_type: Some("legacy_500x500".to_string()), + advertiser: None, + publisher: None, + hostname: None, + country: None, + os_name: None, + chains: vec![], + }; + + let count_impressions = fetch_analytics( + &database.pool, + ad_slot_type_query.clone(), + ALLOWED_KEYS.clone(), + None, + ad_slot_type_query.limit, + ) + .await + .expect("Should fetch"); + + assert_eq!( + 1, + count_impressions.len(), + "Only single analytics is expected" + ); + let fetched = count_impressions.get(0).expect("Should have index 0"); + assert_eq!( + 0, + fetched.value.get_count().expect("Should be Metric::Count"), + "The fetched count should be the same as the hour of the Analytics" + ); + assert_eq!( + other_analytics["impression_new_slot_type"].time.to_datetime(), + fetched.time, + "The fetched analytics date should be the same as the inserted Analytics, but with no hour because Timeframe::Day" + ); + } } /// Makes an [`IMPRESSION`] [`UpdateAnalytics`] for testing. diff --git a/sentry/src/db/campaign.rs b/sentry/src/db/campaign.rs index 47aa1473e..cae8de205 100644 --- a/sentry/src/db/campaign.rs +++ b/sentry/src/db/campaign.rs @@ -62,7 +62,6 @@ pub async fn fetch_campaign( campaign: &CampaignId, ) -> Result, PoolError> { let client = pool.get().await?; - // TODO: Check and update let statement = client.prepare("SELECT campaigns.id, creator, budget, validators, title, pricing_bounds, event_submission, ad_units, targeting_rules, campaigns.created, active_from, active_to, channels.leader, channels.follower, channels.guardian, channels.token, channels.nonce FROM campaigns INNER JOIN channels ON campaigns.channel_id=channels.id WHERE campaigns.id = $1").await?; diff --git a/validator_worker/src/channel.rs b/validator_worker/src/channel.rs index 0fbd9339f..21dd906d4 100644 --- a/validator_worker/src/channel.rs +++ b/validator_worker/src/channel.rs @@ -20,7 +20,7 @@ pub async fn channel_tick( let tick = channel_context .context .find_validator(adapter.whoami()) - .ok_or(Error::ChannelNotIntendedForUs)?; + .ok_or_else(|| Error::ChannelNotIntendedForUs(channel.id(), adapter.whoami()))?; // 1. `GET /channel/:id/spender/all` let all_spenders = sentry.get_all_spenders(&channel_context).await?; diff --git a/validator_worker/src/error.rs b/validator_worker/src/error.rs index 29e0eb200..a8976c79b 100644 --- a/validator_worker/src/error.rs +++ b/validator_worker/src/error.rs @@ -1,4 +1,4 @@ -use primitives::ChannelId; +use primitives::{ChannelId, ValidatorId}; use std::fmt; use thiserror::Error; @@ -27,9 +27,8 @@ pub enum Error { FollowerTick(ChannelId, TickError), #[error("Placeholder for Validation errors")] Validation, - #[error("Whoami is neither a Leader or follower in channel")] - // TODO: Add channel, validatorId, etc. - ChannelNotIntendedForUs, + #[error("Whoami: {1} is neither a Leader or follower in channel: {0}")] + ChannelNotIntendedForUs(ChannelId, ValidatorId), #[error("Channel token is not whitelisted")] ChannelTokenNotWhitelisted, } diff --git a/validator_worker/src/follower.rs b/validator_worker/src/follower.rs index fd48ac543..29f875f21 100644 --- a/validator_worker/src/follower.rs +++ b/validator_worker/src/follower.rs @@ -20,8 +20,8 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum Error { - #[error("overflow placeholder")] - Overflow, + #[error("Overflow error: {0}")] + Overflow(Overflow), #[error("The Channel's Token is not whitelisted")] TokenNotWhitelisted, #[error("Couldn't get state root hash of the proposed balances")] @@ -38,7 +38,7 @@ pub enum Error { pub enum InvalidNewState { RootHash, Signature, - Transition, + Transition(Transition), Health(Health), } @@ -48,13 +48,45 @@ pub enum Health { Spenders(u64), } +#[derive(Debug)] +pub enum Transition { + PayoutMismatch, + Earners, + Spenders, +} + +#[derive(Debug)] +pub enum Overflow { + AllSpendersSum, + ProposedBalancesCheck, + SpenderBalances, + EarnerBalances, + SpenderHealth, + EarnerHealth, +} + +impl fmt::Display for Overflow { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let string = match self { + Overflow::AllSpendersSum => "AllSpendersSum", + Overflow::ProposedBalancesCheck => "ProposedBalancesCheck", + _ => "Overflow", + }; + + write!(f, "{}", string) + } +} + impl fmt::Display for InvalidNewState { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let string = match self { InvalidNewState::RootHash => "InvalidRootHash", InvalidNewState::Signature => "InvalidSignature", - InvalidNewState::Transition => "InvalidTransition", - // TODO: Should we use health value? + InvalidNewState::Transition(t) => match t { + Transition::PayoutMismatch => "InvalidTransitionPayoutMismatch", + Transition::Earners => "InvalidTransitionEarners", + Transition::Spenders => "InvalidTransitionSpenders", + }, InvalidNewState::Health(health) => match health { Health::Earners(_health) => "TooLowHealthEarners", Health::Spenders(_health) => "TooLowHealthSpenders", @@ -91,12 +123,11 @@ pub async fn tick( let from = channel_context.context.leader; let channel_id = channel_context.context.id(); - // TODO: Context for All spender sum Error when overflow occurs let all_spenders_sum = all_spenders .values() .map(|spender| &spender.total_deposited) .sum::>() - .ok_or(Error::Overflow)?; + .ok_or(Error::Overflow(Overflow::AllSpendersSum))?; // if we don't have a `NewState` return `None` let new_msg = sentry @@ -154,18 +185,16 @@ async fn on_new_state<'a, C: Unlocked + 'static>( let proposed_balances = match new_state.balances.clone().check() { Ok(balances) => balances, - // TODO: Should we show the Payout Mismatch between Spent & Earned? Err(balances::Error::PayoutMismatch { .. }) => { return on_error( sentry, channel_context, new_state, - InvalidNewState::Transition, + InvalidNewState::Transition(Transition::PayoutMismatch), ) .await; } - // TODO: Add context for `proposed_balances.check()` overflow error - Err(_) => return Err(Error::Overflow), + Err(_) => return Err(Error::Overflow(Overflow::ProposedBalancesCheck)), }; let proposed_state_root = new_state.state_root.clone(); @@ -204,13 +233,12 @@ async fn on_new_state<'a, C: Unlocked + 'static>( { Ok(Some(previous_balances)) => previous_balances, Ok(None) => Default::default(), - // TODO: Add Context for Transition error Err(_err) => { return on_error( sentry, channel_context, new_state, - InvalidNewState::Transition, + InvalidNewState::Transition(Transition::PayoutMismatch), ) .await; } @@ -226,14 +254,13 @@ async fn on_new_state<'a, C: Unlocked + 'static>( &prev_balances.spenders, &proposed_balances.spenders, ) - .ok_or(Error::Overflow)? + .ok_or(Error::Overflow(Overflow::SpenderBalances))? { - // TODO: Add context for error in Spenders transition return on_error( sentry, channel_context, new_state, - InvalidNewState::Transition, + InvalidNewState::Transition(Transition::Spenders), ) .await; } @@ -248,14 +275,13 @@ async fn on_new_state<'a, C: Unlocked + 'static>( &prev_balances.earners, &proposed_balances.earners, ) - .ok_or(Error::Overflow)? + .ok_or(Error::Overflow(Overflow::EarnerBalances))? { - // TODO: Add context for error in Earners transition return on_error( sentry, channel_context, new_state, - InvalidNewState::Transition, + InvalidNewState::Transition(Transition::Earners), ) .await; } @@ -265,7 +291,7 @@ async fn on_new_state<'a, C: Unlocked + 'static>( &accounting_balances.earners, &proposed_balances.earners, ) - .ok_or(Error::Overflow)?; + .ok_or(Error::Overflow(Overflow::EarnerHealth))?; if health_earners < u64::from(sentry.config.worker.health_unsignable_promilles) { return on_error( sentry, @@ -281,7 +307,7 @@ async fn on_new_state<'a, C: Unlocked + 'static>( &accounting_balances.spenders, &proposed_balances.spenders, ) - .ok_or(Error::Overflow)?; + .ok_or(Error::Overflow(Overflow::SpenderHealth))?; if health_spenders < u64::from(sentry.config.worker.health_unsignable_promilles) { return on_error( sentry, @@ -667,7 +693,7 @@ mod test { ) .await; assert!( - matches!(tick_res, Err(Error::Overflow)), + matches!(tick_res, Err(Error::Overflow(Overflow::AllSpendersSum))), "Returns an Overflow error" ); } @@ -695,7 +721,7 @@ mod test { assert!(matches!( res, ApproveStateResult::RejectedState { - reason: InvalidNewState::Transition, + reason: InvalidNewState::Transition(Transition::PayoutMismatch), .. } ), "InvalidNewState::Transition is the rejection reason when there is a payout mismatch"); @@ -802,7 +828,7 @@ mod test { assert!(matches!( res, ApproveStateResult::RejectedState { - reason: InvalidNewState::Transition, + reason: InvalidNewState::Transition(Transition::PayoutMismatch), .. } ), "InvalidNewState::Transition is the rejection reason last_approved.new_state.balances have a payout mismatch"); @@ -851,7 +877,7 @@ mod test { assert!(matches!( res, ApproveStateResult::RejectedState { - reason: InvalidNewState::Transition, + reason: InvalidNewState::Transition(Transition::Spenders), .. } ), "InvalidNewState::Transition is the rejection reason when proposed balances are lower than the previous balances"); diff --git a/validator_worker/src/lib.rs b/validator_worker/src/lib.rs index 98a55958d..75e9467d7 100644 --- a/validator_worker/src/lib.rs +++ b/validator_worker/src/lib.rs @@ -85,7 +85,6 @@ mod test { }; #[test] - // TODO: Double check this test - encoded value! after introducing `spenders` ("spender", address, amount) fn get_state_root_hash_returns_correct_hash() { let channel = Channel { leader: IDS[&LEADER],