Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleaning up remaining todo's in the code #572

Open
wants to merge 12 commits into
base: aip-61-adex-v5
Choose a base branch
from
46 changes: 5 additions & 41 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions adapter/src/ethereum/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -275,15 +275,14 @@ impl<S: WalletState> Locked for Ethereum<S> {
#[async_trait]
impl Unlocked for Ethereum<UnlockedWallet> {
fn sign(&self, state_root: &str) -> Result<String, Error> {
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())))
}
Expand All @@ -298,7 +297,7 @@ impl Unlocked for Ethereum<UnlockedWallet> {
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())
}
Expand Down
28 changes: 23 additions & 5 deletions adapter/src/ethereum/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::Error as AdapterError;
use hex::FromHexError;
use primitives::{
address::Error as AddressError, big_num::ParseBigIntError, ChainId, ChannelId, ValidatorId,
};
Expand All @@ -17,11 +18,12 @@ impl From<Error> 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)
}
Expand Down Expand Up @@ -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}")]
Expand All @@ -74,6 +76,8 @@ pub enum Error {
},
#[error("Insufficient privilege")]
InsufficientAuthorizationPrivilege,
#[error("Signing message error: {0}")]
SigningMessage(#[from] SigningError),
}

#[derive(Debug, Error)]
Expand Down Expand Up @@ -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")]
Expand All @@ -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)]
Expand Down
11 changes: 5 additions & 6 deletions adapter/src/ethereum/ewt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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<Self, EwtVerifyError> {
pub fn base64_decode(encoded_json: &str) -> Result<Self, PayloadError> {
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)
}
}

Expand Down
6 changes: 2 additions & 4 deletions adview-manager/serve/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ pub async fn get_preview_ad(Extension(state): Extension<Arc<State>>) -> Html<Str
// 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)),
// TODO: Check this value
navigator_language: Some("bg".into()),
navigator_language: Some("bg-BG".into()),
/// Defaulted
disabled_video,
disabled_sticky: false,
Expand Down Expand Up @@ -157,8 +156,7 @@ pub async fn get_preview_video(Extension(state): Extension<Arc<State>>) -> 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,
Expand Down
3 changes: 1 addition & 2 deletions adview-manager/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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],
Expand Down
5 changes: 2 additions & 3 deletions adview-manager/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion primitives/src/analytics/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Utc>,
/// The End [`DateHour`] which will fetch `analytics_time <= end` and should be after Start [`DateHour`]!
pub end: Option<DateHour<Utc>>,
Expand Down
35 changes: 23 additions & 12 deletions primitives/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<StrictPfx>::serialize"
)]
[u8; 32],
);
#[derive(PartialEq, Eq, Copy, Clone, Hash)]
pub struct ChannelId([u8; 32]);

impl ChannelId {
pub fn as_bytes(&self) -> &[u8; 32] {
Expand All @@ -39,6 +31,24 @@ where
validate_channel_id(&channel_id).map_err(serde::de::Error::custom)
}

impl<'de> Deserialize<'de> for ChannelId {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
deserialize_channel_id(deserializer).map(ChannelId)
}
}

impl Serialize for ChannelId {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
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);
Expand Down Expand Up @@ -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();
Expand Down
Loading