From 1d6e1ef9afc4453b986c8a21c58eb9756445d43c Mon Sep 17 00:00:00 2001 From: Bryan Stitt Date: Sat, 24 Jun 2023 15:24:46 -0700 Subject: [PATCH] polish error logs --- Cargo.lock | 27 +++++++++++++ web3_proxy/Cargo.toml | 1 + web3_proxy/src/frontend/admin.rs | 51 ++++++++++-------------- web3_proxy/src/frontend/authorization.rs | 1 + web3_proxy/src/frontend/mod.rs | 4 +- web3_proxy/src/rpcs/request.rs | 9 ++--- web3_proxy/src/stats/influxdb_queries.rs | 2 +- web3_proxy/src/stats/mod.rs | 9 +---- 8 files changed, 59 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 978f4c36..53342169 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4752,6 +4752,32 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rstest" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de1bb486a691878cd320c2f0d319ba91eeaa2e894066d8b5f8f117c000e9d962" +dependencies = [ + "futures", + "futures-timer", + "rstest_macros", + "rustc_version", +] + +[[package]] +name = "rstest_macros" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "290ca1a1c8ca7edb7c3283bd44dc35dd54fdec6253a3912e201ba1072018fca8" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "rustc_version", + "syn 1.0.109", + "unicode-ident", +] + [[package]] name = "rust_decimal" version = "1.30.0" @@ -6948,6 +6974,7 @@ dependencies = [ "regex", "reqwest", "rmp-serde", + "rstest", "rust_decimal", "sentry", "sentry-tracing", diff --git a/web3_proxy/Cargo.toml b/web3_proxy/Cargo.toml index e884569a..3b32cae9 100644 --- a/web3_proxy/Cargo.toml +++ b/web3_proxy/Cargo.toml @@ -105,4 +105,5 @@ url = { version = "2.4.0", features = ["serde"] } uuid = { version = "1.3.4", default-features = false, features = ["fast-rng", "serde", "v4", "zerocopy"] } [dev-dependencies] +rstest = "0.17.0" tokio = { version = "1.28.2", features = ["full", "test-util"] } diff --git a/web3_proxy/src/frontend/admin.rs b/web3_proxy/src/frontend/admin.rs index a99c1b11..fa60e8b7 100644 --- a/web3_proxy/src/frontend/admin.rs +++ b/web3_proxy/src/frontend/admin.rs @@ -158,7 +158,7 @@ pub async fn admin_change_user_roles( /// - user_address that is to be logged in by /// We assume that the admin has already logged in, and has a bearer token ... #[debug_handler] -pub async fn admin_login_get( +pub async fn admin_imitate_login_get( Extension(app): Extension>, InsecureClientIp(ip): InsecureClientIp, Path(mut params): Path>, @@ -229,15 +229,11 @@ pub async fn admin_login_get( let db_conn = app.db_conn()?; let db_replica = app.db_replica()?; - // delete ALL expired rows. - let now = Utc::now(); - let delete_result = pending_login::Entity::delete_many() - .filter(pending_login::Column::ExpiresAt.lte(now)) - .exec(db_conn) - .await?; - - // TODO: emit a stat? if this is high something weird might be happening - debug!("cleared expired pending_logins: {:?}", delete_result); + let admin = user::Entity::find() + .filter(user::Column::Address.eq(admin_address.as_bytes())) + .one(db_replica.as_ref()) + .await? + .ok_or(Web3ProxyError::AccessDenied)?; // Get the user that we want to imitate from the read-only database (their id ...) // TODO: Only get the id, not the whole user object ... @@ -249,24 +245,25 @@ pub async fn admin_login_get( "Could not find user in db".into(), ))?; - // TODO: Gotta check if encoding messes up things maybe ... - info!("Admin address is: {:?}", admin_address); - let admin = user::Entity::find() - .filter(user::Column::Address.eq(admin_address.as_bytes())) - .one(db_replica.as_ref()) - .await? - .ok_or(Web3ProxyError::BadRequest( - "Could not find admin in db".into(), - ))?; + info!(admin=?admin.address, user=?user.address, "admin is imitating another user"); + + // delete ALL expired rows. + let now = Utc::now(); + let delete_result = pending_login::Entity::delete_many() + .filter(pending_login::Column::ExpiresAt.lte(now)) + .exec(db_conn) + .await?; + debug!("cleared expired pending_logins: {:?}", delete_result); // Note that the admin is trying to log in as this user let trail = admin_trail::ActiveModel { caller: sea_orm::Set(admin.id), imitating_user: sea_orm::Set(Some(user.id)), - endpoint: sea_orm::Set("admin_login_get".to_string()), + endpoint: sea_orm::Set("admin_imitate_login_get".to_string()), payload: sea_orm::Set(format!("{}", json!(params))), ..Default::default() }; + trail .save(db_conn) .await @@ -275,8 +272,6 @@ pub async fn admin_login_get( // Can there be two login-sessions at the same time? // I supposed if the user logs in, the admin would be logged out and vice versa - // massage types to fit in the database. sea-orm does not make this very elegant - let uuid = Uuid::from_u128(nonce.into()); // we add 1 to expire_seconds just to be sure the database has the key for the full expiration_time let expires_at = Utc .timestamp_opt(expiration_time.unix_timestamp() + 1, 0) @@ -286,7 +281,7 @@ pub async fn admin_login_get( // add a row to the database for this user let user_pending_login = pending_login::ActiveModel { id: sea_orm::NotSet, - nonce: sea_orm::Set(uuid), + nonce: sea_orm::Set(nonce.into()), message: sea_orm::Set(message.to_string()), expires_at: sea_orm::Set(expires_at), imitating_user: sea_orm::Set(Some(user.id)), @@ -316,9 +311,9 @@ pub async fn admin_login_get( Ok(message.into_response()) } -/// `POST /admin/login` - Register or login by posting a signed "siwe" message +/// `POST /admin/login` - Admin login by posting a signed "siwe" message /// It is recommended to save the returned bearer token in a cookie. -/// The bearer token can be used to authenticate other requests, such as getting user user's tats or modifying the user's profile +/// The bearer token can be used to authenticate other admin requests #[debug_handler] pub async fn admin_login_post( Extension(app): Extension>, @@ -372,12 +367,8 @@ pub async fn admin_login_post( // fetch the message we gave them from our database let db_replica = app.db_replica()?; - // massage type for the db - let login_nonce_uuid: Uuid = login_nonce.clone().into(); - - // TODO: Here we will need to re-find the parameter where the admin wants to log-in as the user ... let user_pending_login = pending_login::Entity::find() - .filter(pending_login::Column::Nonce.eq(login_nonce_uuid)) + .filter(pending_login::Column::Nonce.eq(Uuid::from(login_nonce.clone()))) .one(db_replica.as_ref()) .await .web3_context("database error while finding pending_login")? diff --git a/web3_proxy/src/frontend/authorization.rs b/web3_proxy/src/frontend/authorization.rs index 0226670f..f28ac8de 100644 --- a/web3_proxy/src/frontend/authorization.rs +++ b/web3_proxy/src/frontend/authorization.rs @@ -1340,6 +1340,7 @@ impl Web3ProxyApp { if balance.total_deposit < Decimal::from(10) || balance.remaining() < Decimal::new(1, 2) { + // TODO: include boolean to mark that the user is downgraded user_tier_model = user_tier::Entity::find_by_id(downgrade_user_tier) .one(db_replica.as_ref()) diff --git a/web3_proxy/src/frontend/mod.rs b/web3_proxy/src/frontend/mod.rs index 77ee9d70..33c80f01 100644 --- a/web3_proxy/src/frontend/mod.rs +++ b/web3_proxy/src/frontend/mod.rs @@ -217,11 +217,11 @@ pub async fn serve( .route("/admin/modify_role", post(admin::admin_change_user_roles)) .route( "/admin/imitate-login/:admin_address/:user_address", - get(admin::admin_login_get), + get(admin::admin_imitate_login_get), ) .route( "/admin/imitate-login/:admin_address/:user_address/:message_eip", - get(admin::admin_login_get), + get(admin::admin_imitate_login_get), ) .route("/admin/imitate-login", post(admin::admin_login_post)) .route("/admin/imitate-logout", post(admin::admin_logout_post)) diff --git a/web3_proxy/src/rpcs/request.rs b/web3_proxy/src/rpcs/request.rs index 2074ef2f..4c3a9b1c 100644 --- a/web3_proxy/src/rpcs/request.rs +++ b/web3_proxy/src/rpcs/request.rs @@ -1,5 +1,5 @@ use super::one::Web3Rpc; -use crate::errors::Web3ProxyResult; +use crate::errors::{Web3ProxyErrorContext, Web3ProxyResult}; use crate::frontend::authorization::{Authorization, AuthorizationType}; use crate::jsonrpc::{JsonRpcParams, JsonRpcResultData}; use anyhow::Context; @@ -118,11 +118,10 @@ impl Authorization { let rl = rl .save(db_conn) .await - .context("Failed saving new revert log")?; + .web3_context("Failed saving new revert log")?; - // TODO: what log level? - // TODO: better format - trace!("revert_log: {:?}", rl); + // TODO: what log level and format? + trace!(revert_log=?rl); // TODO: return something useful Ok(()) diff --git a/web3_proxy/src/stats/influxdb_queries.rs b/web3_proxy/src/stats/influxdb_queries.rs index c47f4dc4..7f452eaa 100644 --- a/web3_proxy/src/stats/influxdb_queries.rs +++ b/web3_proxy/src/stats/influxdb_queries.rs @@ -147,7 +147,7 @@ pub async fn query_user_stats<'a>( .config .influxdb_bucket .clone() - .context("No influxdb bucket was provided")?; // "web3_proxy"; + .context("No influxdb bucket was provided")?; trace!("Bucket is {:?}", bucket); let mut filter_chain_id = "".to_string(); diff --git a/web3_proxy/src/stats/mod.rs b/web3_proxy/src/stats/mod.rs index 1c9e8eac..db7ea861 100644 --- a/web3_proxy/src/stats/mod.rs +++ b/web3_proxy/src/stats/mod.rs @@ -736,22 +736,17 @@ impl BufferedRpcQueryStats { .field("sum_request_bytes", self.sum_request_bytes as i64) .field("sum_response_millis", self.sum_response_millis as i64) .field("sum_response_bytes", self.sum_response_bytes as i64) - // TODO: will this be enough of a range - // I guess Decimal can be a f64 - // TODO: This should prob be a float, i should change the query if we want float-precision for this (which would be important...) .field( "sum_credits_used", self.sum_credits_used .to_f64() - .context("number is really (too) large")?, + .context("sum_credits_used is really (too) large")?, ) .field( "balance", - remaining.to_f64().context("number is really (too) large")?, + remaining.to_f64().context("balance is really (too) large")?, ); - // .round() as i64 - builder = builder.timestamp(key.response_timestamp); let point = builder.build()?;