From 661a7ad244617af9e6d0b6fff409ea84a92bd94c Mon Sep 17 00:00:00 2001 From: Bryan Stitt Date: Thu, 11 Aug 2022 01:53:27 +0000 Subject: [PATCH] better redirect and jsonrpc handling --- TODO.md | 6 ++- web3_proxy/src/frontend/errors.rs | 4 +- web3_proxy/src/frontend/http_proxy.rs | 35 ++++++++------- web3_proxy/src/frontend/rate_limit.rs | 64 ++++++++++++++++----------- web3_proxy/src/frontend/users.rs | 27 ++++++----- web3_proxy/src/frontend/ws_proxy.rs | 43 ++++++++++-------- web3_proxy/src/jsonrpc.rs | 4 +- 7 files changed, 107 insertions(+), 76 deletions(-) diff --git a/TODO.md b/TODO.md index 3318a24a..1d0c07e9 100644 --- a/TODO.md +++ b/TODO.md @@ -69,6 +69,8 @@ - [x] Got warning: "WARN subscribe_new_heads:send_block: web3_proxy::connection: unable to get block from https://rpc.ethermine.org: Deserialization Error: expected value at line 1 column 1. Response: error code: 1015". this is cloudflare rate limiting on fetching a block, but this is a private rpc. why is there a block subscription? - [x] im seeing ethspam occasionally try to query a future block. something must be setting the head block too early - [x] we were sorting best block the wrong direction. i flipped a.cmp(b) to b.cmp(a) so that the largest would be first, but then i used 'max_by' which looks at the end of the list +- [x] HTTP GET to the websocket endpoints should redirect instead of giving an ugly error +- [ ] load the redirected page from config - [ ] basic request method stats - [ ] use siwe messages and signatures for sign up and login - [ ] active requests on /status is always 0 even when i'm running requests through @@ -76,7 +78,6 @@ - [ ] i think the server isn't following the spec. we need a context attached to this error so we know which one - [ ] maybe make jsonrpc an Option - [ ] "chain is forked" message is wrong. it includes nodes just being on different heights of the same chain. need a smarter check -- [ ] disable redis persistence in dev ## V1 @@ -242,4 +243,5 @@ in another repo: event subscriber eth_1 | 2022-08-10T23:26:06.377129Z WARN web3_proxy::connections: chain is forked! 261 possible heads. 1/2/5/5 rpcs have 0xd403…3c5d eth_1 | 2022-08-10T23:26:08.917603Z WARN web3_proxy::connections: chain is forked! 262 possible heads. 1/2/5/5 rpcs have 0x0538…bfff eth_1 | 2022-08-10T23:26:10.195014Z WARN web3_proxy::connections: chain is forked! 262 possible heads. 1/2/5/5 rpcs have 0x0538…bfff - eth_1 | 2022-08-10T23:26:10.195658Z WARN web3_proxy::connections: chain is forked! 262 possible heads. 2/3/5/5 rpcs have 0x0538…bfff \ No newline at end of file + eth_1 | 2022-08-10T23:26:10.195658Z WARN web3_proxy::connections: chain is forked! 262 possible heads. 2/3/5/5 rpcs have 0x0538…bfff +- [ ] disable redis persistence in dev diff --git a/web3_proxy/src/frontend/errors.rs b/web3_proxy/src/frontend/errors.rs index 89441f20..0d77091f 100644 --- a/web3_proxy/src/frontend/errors.rs +++ b/web3_proxy/src/frontend/errors.rs @@ -10,13 +10,13 @@ use crate::jsonrpc::JsonRpcForwardedResponse; pub async fn handler_404() -> Response { let err = anyhow::anyhow!("nothing to see here"); - handle_anyhow_error(Some(StatusCode::NOT_FOUND), None, err) + anyhow_error_into_response(Some(StatusCode::NOT_FOUND), None, err) } /// handle errors by converting them into something that implements `IntoResponse` /// TODO: use this. i can't get to work /// TODO: i think we want a custom result type instead. put the anyhow result inside. then `impl IntoResponse for CustomResult` -pub fn handle_anyhow_error( +pub fn anyhow_error_into_response( http_code: Option, id: Option>, err: anyhow::Error, diff --git a/web3_proxy/src/frontend/http_proxy.rs b/web3_proxy/src/frontend/http_proxy.rs index 2c3633d3..3ccc4436 100644 --- a/web3_proxy/src/frontend/http_proxy.rs +++ b/web3_proxy/src/frontend/http_proxy.rs @@ -5,8 +5,8 @@ use axum_client_ip::ClientIp; use std::sync::Arc; use uuid::Uuid; -use super::errors::handle_anyhow_error; -use super::rate_limit::handle_rate_limit_error_response; +use super::errors::anyhow_error_into_response; +use super::rate_limit::RateLimitResult; use crate::{app::Web3ProxyApp, jsonrpc::JsonRpcRequestEnum}; pub async fn public_proxy_web3_rpc( @@ -14,15 +14,18 @@ pub async fn public_proxy_web3_rpc( Extension(app): Extension>, ClientIp(ip): ClientIp, ) -> Response { - if let Some(err_response) = - handle_rate_limit_error_response(app.rate_limit_by_ip(&ip).await).await - { - return err_response.into_response(); - } + let _ip = match app.rate_limit_by_ip(ip).await { + Ok(x) => match x.try_into_response().await { + Ok(RateLimitResult::AllowedIp(x)) => x, + Err(err_response) => return err_response, + _ => unimplemented!(), + }, + Err(err) => return anyhow_error_into_response(None, None, err).into_response(), + }; match app.proxy_web3_rpc(payload).await { Ok(response) => (StatusCode::OK, Json(&response)).into_response(), - Err(err) => handle_anyhow_error(None, None, err).into_response(), + Err(err) => anyhow_error_into_response(None, None, err).into_response(), } } @@ -31,15 +34,17 @@ pub async fn user_proxy_web3_rpc( Extension(app): Extension>, Path(user_key): Path, ) -> Response { - // TODO: add a helper on this that turns RateLimitResult into error if its not allowed - if let Some(err_response) = - handle_rate_limit_error_response(app.rate_limit_by_key(user_key).await).await - { - return err_response.into_response(); - } + let _user_id = match app.rate_limit_by_key(user_key).await { + Ok(x) => match x.try_into_response().await { + Ok(RateLimitResult::AllowedUser(x)) => x, + Err(err_response) => return err_response, + _ => unimplemented!(), + }, + Err(err) => return anyhow_error_into_response(None, None, err).into_response(), + }; match app.proxy_web3_rpc(payload).await { Ok(response) => (StatusCode::OK, Json(&response)).into_response(), - Err(err) => handle_anyhow_error(None, None, err), + Err(err) => anyhow_error_into_response(None, None, err), } } diff --git a/web3_proxy/src/frontend/rate_limit.rs b/web3_proxy/src/frontend/rate_limit.rs index 89455e0e..559deb25 100644 --- a/web3_proxy/src/frontend/rate_limit.rs +++ b/web3_proxy/src/frontend/rate_limit.rs @@ -12,16 +12,46 @@ use uuid::Uuid; use crate::app::{UserCacheValue, Web3ProxyApp}; -use super::errors::handle_anyhow_error; +use super::errors::anyhow_error_into_response; pub enum RateLimitResult { - Allowed, - RateLimitExceeded, + AllowedIp(IpAddr), + AllowedUser(i64), + IpRateLimitExceeded(IpAddr), + UserRateLimitExceeded(i64), UnknownKey, } +impl RateLimitResult { + // TODO: i think this should be a function on RateLimitResult + pub async fn try_into_response(self) -> Result { + match self { + RateLimitResult::AllowedIp(_) => Ok(self), + RateLimitResult::AllowedUser(_) => Ok(self), + RateLimitResult::IpRateLimitExceeded(ip) => Err(anyhow_error_into_response( + Some(StatusCode::TOO_MANY_REQUESTS), + None, + // TODO: how can we attach context here? maybe add a request id tracing field? + anyhow::anyhow!(format!("rate limit exceeded for {}", ip)), + )), + RateLimitResult::UserRateLimitExceeded(user) => Err(anyhow_error_into_response( + Some(StatusCode::TOO_MANY_REQUESTS), + None, + // TODO: don't expose numeric ids. show the address instead + // TODO: how can we attach context here? maybe add a request id tracing field? + anyhow::anyhow!(format!("rate limit exceeded for user {}", user)), + )), + RateLimitResult::UnknownKey => Err(anyhow_error_into_response( + Some(StatusCode::FORBIDDEN), + None, + anyhow::anyhow!("unknown key"), + )), + } + } +} + impl Web3ProxyApp { - pub async fn rate_limit_by_ip(&self, ip: &IpAddr) -> anyhow::Result { + pub async fn rate_limit_by_ip(&self, ip: IpAddr) -> anyhow::Result { let rate_limiter_key = format!("ip-{}", ip); // TODO: dry this up with rate_limit_by_key @@ -35,7 +65,7 @@ impl Web3ProxyApp { // TODO: set headers so they know when they can retry debug!(?rate_limiter_key, "rate limit exceeded"); // this is too verbose, but a stat might be good // TODO: use their id if possible - return Ok(RateLimitResult::RateLimitExceeded); + return Ok(RateLimitResult::IpRateLimitExceeded(ip)); } Err(err) => { // internal error, not rate limit being hit @@ -48,7 +78,7 @@ impl Web3ProxyApp { warn!("no rate limiter!"); } - Ok(RateLimitResult::Allowed) + Ok(RateLimitResult::AllowedIp(ip)) } pub async fn rate_limit_by_key(&self, user_key: Uuid) -> anyhow::Result { @@ -142,26 +172,6 @@ impl Web3ProxyApp { unimplemented!("no redis. cannot rate limit") } - Ok(RateLimitResult::Allowed) - } -} - -pub async fn handle_rate_limit_error_response( - r: anyhow::Result, -) -> Option { - match r { - Ok(RateLimitResult::Allowed) => None, - Ok(RateLimitResult::RateLimitExceeded) => Some(handle_anyhow_error( - Some(StatusCode::TOO_MANY_REQUESTS), - None, - // TODO: how can we attach context here? maybe add a request id tracing field? - anyhow::anyhow!("rate limit exceeded"), - )), - Ok(RateLimitResult::UnknownKey) => Some(handle_anyhow_error( - Some(StatusCode::FORBIDDEN), - None, - anyhow::anyhow!("unknown key"), - )), - Err(err) => Some(handle_anyhow_error(None, None, err)), + Ok(RateLimitResult::AllowedUser(user_data.user_id)) } } diff --git a/web3_proxy/src/frontend/users.rs b/web3_proxy/src/frontend/users.rs index 9db3c1e9..b822faa2 100644 --- a/web3_proxy/src/frontend/users.rs +++ b/web3_proxy/src/frontend/users.rs @@ -7,17 +7,21 @@ // I wonder how we handle payment // probably have to do manual withdrawals -use axum::{response::IntoResponse, Extension, Json}; +use axum::{ + response::{IntoResponse, Response}, + Extension, Json, +}; use axum_client_ip::ClientIp; use entities::user; use ethers::{prelude::Address, types::Bytes}; +use reqwest::StatusCode; use sea_orm::ActiveModelTrait; use serde::Deserialize; use std::sync::Arc; use crate::app::Web3ProxyApp; -use super::rate_limit::handle_rate_limit_error_response; +use super::{rate_limit::RateLimitResult, errors::anyhow_error_into_response}; pub async fn create_user( // this argument tells axum to parse the request body @@ -25,12 +29,15 @@ pub async fn create_user( Json(payload): Json, Extension(app): Extension>, ClientIp(ip): ClientIp, -) -> impl IntoResponse { - if let Some(err_response) = - handle_rate_limit_error_response(app.rate_limit_by_ip(&ip).await).await - { - return err_response.into_response(); - } +) -> Response { + let _ip = match app.rate_limit_by_ip(ip).await { + Ok(x) => match x.try_into_response().await { + Ok(RateLimitResult::AllowedIp(x)) => x, + Err(err_response) => return err_response, + _ => unimplemented!(), + }, + Err(err) => return anyhow_error_into_response(None, None, err).into_response(), + }; // TODO: check invite_code against the app's config or database if payload.invite_code != "llam4n0des!" { @@ -58,8 +65,8 @@ pub async fn create_user( // TODO: proper error message let user = user.insert(db).await.unwrap(); - todo!("serialize and return the user: {:?}", user) - // (StatusCode::CREATED, Json(user)) + // TODO: do not expose user ids + (StatusCode::CREATED, Json(user)).into_response() } // the input to our `create_user` handler diff --git a/web3_proxy/src/frontend/ws_proxy.rs b/web3_proxy/src/frontend/ws_proxy.rs index 6aaff2fa..6e463f9d 100644 --- a/web3_proxy/src/frontend/ws_proxy.rs +++ b/web3_proxy/src/frontend/ws_proxy.rs @@ -1,10 +1,11 @@ use axum::{ extract::ws::{Message, WebSocket, WebSocketUpgrade}, extract::Path, - response::{IntoResponse, Response}, + response::{IntoResponse, Redirect, Response}, Extension, }; use axum_client_ip::ClientIp; +use fstrings::{format_args_f, format_f}; use futures::SinkExt; use futures::{ future::AbortHandle, @@ -22,18 +23,21 @@ use crate::{ jsonrpc::{JsonRpcForwardedResponse, JsonRpcForwardedResponseEnum, JsonRpcRequest}, }; -use super::rate_limit::handle_rate_limit_error_response; +use super::{errors::anyhow_error_into_response, rate_limit::RateLimitResult}; pub async fn public_websocket_handler( Extension(app): Extension>, ClientIp(ip): ClientIp, ws_upgrade: Option, ) -> Response { - if let Some(err_response) = - handle_rate_limit_error_response(app.rate_limit_by_ip(&ip).await).await - { - return err_response.into_response(); - } + let ip = match app.rate_limit_by_ip(ip).await { + Ok(x) => match x.try_into_response().await { + Ok(RateLimitResult::AllowedIp(x)) => x, + Err(err_response) => return err_response, + _ => unimplemented!(), + }, + Err(err) => return anyhow_error_into_response(None, None, err).into_response(), + }; match ws_upgrade { Some(ws) => ws @@ -41,9 +45,8 @@ pub async fn public_websocket_handler( .into_response(), None => { // this is not a websocket. give a friendly page. maybe redirect to the llama nodes home - // TODO: make a friendly page - // TODO: rate limit this? - "hello, world".into_response() + // TODO: redirect to a configurable url + Redirect::to(&format_f!("https://www.etherscan.io/#userip={ip}")).into_response() } } } @@ -53,19 +56,23 @@ pub async fn user_websocket_handler( Path(user_key): Path, ws_upgrade: Option, ) -> Response { - if let Some(err_response) = - handle_rate_limit_error_response(app.rate_limit_by_key(user_key).await).await - { - return err_response; - } + // TODO: dry this up. maybe a rate_limit_by_key_response function? + let user_id = match app.rate_limit_by_key(user_key).await { + Ok(x) => match x.try_into_response().await { + Ok(RateLimitResult::AllowedUser(x)) => x, + Err(err_response) => return err_response, + _ => unimplemented!(), + }, + Err(err) => return anyhow_error_into_response(None, None, err).into_response(), + }; match ws_upgrade { Some(ws_upgrade) => ws_upgrade.on_upgrade(|socket| proxy_web3_socket(app, socket)), None => { // this is not a websocket. give a friendly page with stats for this user - // TODO: make a friendly page - // TODO: rate limit this? - "hello, world".into_response() + // TODO: redirect to a configurable url + // TODO: should this use user_key instead? or user's address? + Redirect::to(&format_f!("https://www.etherscan.io/#userid={user_id}")).into_response() } } } diff --git a/web3_proxy/src/jsonrpc.rs b/web3_proxy/src/jsonrpc.rs index dc15acc7..a36e1f7a 100644 --- a/web3_proxy/src/jsonrpc.rs +++ b/web3_proxy/src/jsonrpc.rs @@ -8,8 +8,8 @@ use tracing::warn; #[derive(Clone, serde::Deserialize)] pub struct JsonRpcRequest { - // TODO: skip jsonrpc entireley? - pub jsonrpc: Box, + // TODO: skip jsonrpc entirely? its against spec to drop it, but some servers bad + pub jsonrpc: Option>, /// id could be a stricter type, but many rpcs do things against the spec pub id: Box, pub method: String,