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

feat(katana): modify DevApi trait with new endpoints #2490

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5347bc3
Modify DevApi trait with new endpoints
fabrobles92 Oct 1, 2024
75155c0
Implementation of account_balance method
fabrobles92 Oct 3, 2024
e2612e3
clippy.sh and rust_fmt.sh
fabrobles92 Oct 3, 2024
9902cfe
Replace RPC call with querying the storage directly
fabrobles92 Oct 11, 2024
b2e3ad4
-Re implement proxy_get_request.rs
fabrobles92 Oct 22, 2024
3947795
cargo fmt
fabrobles92 Oct 22, 2024
c23d9eb
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Oct 23, 2024
5a3865c
update cargo.lock
fabrobles92 Oct 30, 2024
8c4a8b8
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Oct 30, 2024
8e08862
Keep ProxyGetRequestLayer for / (health) endpoint
fabrobles92 Oct 30, 2024
e4666bc
Enable query param contract_address for endpoint account_balance
fabrobles92 Oct 31, 2024
0bf530d
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Oct 31, 2024
3ec9a5e
cargo fmt
fabrobles92 Oct 31, 2024
378ae06
Remove unused files
fabrobles92 Dec 7, 2024
9411eb5
Update cargo
fabrobles92 Dec 7, 2024
23cd841
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Dec 7, 2024
8e61aa4
- Accepting unit param
fabrobles92 Dec 13, 2024
0f2380d
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Dec 13, 2024
01f69c2
Fix cargo, removing duplicates
fabrobles92 Dec 13, 2024
78c2180
Implement Devnet Proxy Layer in new implementation
fabrobles92 Dec 13, 2024
1aafb25
Merge branch 'main' into feat/expose_dev_endpoints
fabrobles92 Jan 3, 2025
c8d2444
Clean unused code
fabrobles92 Jan 3, 2025
2cb90f1
clippy.sh
fabrobles92 Jan 3, 2025
0c14c03
rust_fmt.sh
fabrobles92 Jan 4, 2025
3d73532
Fix default case in match and early return when invalid route provided
fabrobles92 Jan 4, 2025
17a53b5
Fix error when returning response when route is not known in DevnetPr…
fabrobles92 Jan 4, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

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

13 changes: 13 additions & 0 deletions crates/katana/primitives/src/fee.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::str::FromStr;

#[derive(Debug, Clone, PartialEq, Eq, Default)]
#[cfg_attr(feature = "arbitrary", derive(::arbitrary::Arbitrary))]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down Expand Up @@ -29,6 +31,17 @@
Fri,
}

impl FromStr for PriceUnit {
type Err = ();
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"WEI" => Ok(PriceUnit::Wei),
"FRI" => Ok(PriceUnit::Fri),
_ => Err(()), // Return an error for unknown units

Check warning on line 40 in crates/katana/primitives/src/fee.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/fee.rs#L36-L40

Added lines #L36 - L40 were not covered by tests
}
}

Check warning on line 42 in crates/katana/primitives/src/fee.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/fee.rs#L42

Added line #L42 was not covered by tests
}

/// Information regarding the fee and gas usages of a transaction.
#[derive(Debug, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "arbitrary", derive(::arbitrary::Arbitrary))]
Expand Down
11 changes: 11 additions & 0 deletions crates/katana/primitives/src/genesis/constant.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use anyhow::Error;
use lazy_static::lazy_static;
use starknet::core::utils::get_storage_var_address;
use starknet::macros::felt;

use crate::class::{ClassHash, CompiledClass, CompiledClassHash, ContractClass};
use crate::contract::{ContractAddress, StorageKey};
use crate::fee::PriceUnit;
use crate::utils::class::{
parse_compiled_class, parse_deprecated_compiled_class, parse_sierra_class,
};
Expand Down Expand Up @@ -117,6 +119,15 @@
ContractClass::Legacy(class)
}

pub fn get_erc20_address(unit: &PriceUnit) -> Result<ContractAddress, Error> {
let erc20_contract_address = match unit {
PriceUnit::Wei => DEFAULT_ETH_FEE_TOKEN_ADDRESS,
PriceUnit::Fri => DEFAULT_STRK_FEE_TOKEN_ADDRESS,

Check warning on line 125 in crates/katana/primitives/src/genesis/constant.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/genesis/constant.rs#L122-L125

Added lines #L122 - L125 were not covered by tests
};

Ok(ContractAddress::new(erc20_contract_address.into()))
}

Check warning on line 129 in crates/katana/primitives/src/genesis/constant.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/genesis/constant.rs#L128-L129

Added lines #L128 - L129 were not covered by tests

#[cfg(test)]
mod tests {

Expand Down
6 changes: 6 additions & 0 deletions crates/katana/rpc/rpc-api/src/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ pub trait DevApi {

#[method(name = "predeployedAccounts")]
async fn predeployed_accounts(&self) -> RpcResult<Vec<Account>>;

#[method(name = "accountBalance")]
async fn account_balance(&self, address: String, unit: String) -> RpcResult<u128>;

#[method(name = "mint")]
async fn mint(&self) -> RpcResult<()>;
}
13 changes: 13 additions & 0 deletions crates/katana/rpc/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ repository.workspace = true
version.workspace = true

[dependencies]
jsonrpsee-core = { version = "0.16.3", features = [ "server", "soketto", "http-helpers" ] }
jsonrpsee-types = { version = "0.16.3"}
hyper.workspace = true
katana-core.workspace = true
katana-executor.workspace = true
katana-pool.workspace = true
Expand All @@ -31,6 +34,16 @@ tower.workspace = true
tower-http.workspace = true
tracing.workspace = true
url.workspace = true
serde.workspace = true
soketto = { version = "0.7.1", features = ["http"] }
futures-channel = { version = "0.3.14"}
futures-util = { version = "0.3.14", features = [
"io",
"async-await-macro",
]}
tokio-stream = { version = "0.1.7" }
tokio-util = { version = "0.7", features = ["compat"]}
starknet-crypto.workspace = true

[dev-dependencies]
katana-cairo.workspace = true
Expand Down
31 changes: 29 additions & 2 deletions crates/katana/rpc/rpc/src/dev.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
use std::str::FromStr;
use std::sync::Arc;

use jsonrpsee::core::{async_trait, Error};
use katana_core::backend::Backend;
use katana_core::service::block_producer::{BlockProducer, BlockProducerMode, PendingExecutor};
use katana_executor::ExecutorFactory;
use katana_primitives::Felt;
use katana_primitives::fee::PriceUnit;
use katana_primitives::genesis::constant::{
get_erc20_address, get_fee_token_balance_base_storage_address,
};
use katana_primitives::ContractAddress;
use katana_provider::traits::state::StateFactoryProvider;
use katana_rpc_api::dev::DevApiServer;
use katana_rpc_types::account::Account;
use katana_rpc_types::error::dev::DevApiError;
use starknet_crypto::Felt;

use crate::transport::http;

#[allow(missing_debug_implementations)]
pub struct DevApi<EF: ExecutorFactory> {
Expand Down Expand Up @@ -54,7 +63,6 @@

let mut block_context_generator = self.backend.block_context_generator.write();
block_context_generator.block_timestamp_offset += offset as i64;

Ok(())
}
}
Expand Down Expand Up @@ -92,6 +100,25 @@
Ok(())
}

async fn account_balance(&self, address: String, unit: String) -> Result<u128, Error> {
let account_address: ContractAddress = Felt::from_str(address.as_str()).unwrap().into();
let unit = PriceUnit::from_str(unit.to_uppercase().as_str()).unwrap_or(PriceUnit::Wei);

let erc20_address =
get_erc20_address(&unit).map_err(|_| http::response::internal_error()).unwrap();

let provider = self.backend.blockchain.provider();
let state = provider.latest().unwrap();
let storage_slot = get_fee_token_balance_base_storage_address(account_address);
let balance_felt = state.storage(erc20_address, storage_slot).unwrap().unwrap();
let balance: u128 = balance_felt.to_string().parse().unwrap();
Ok(balance)
}

Check warning on line 116 in crates/katana/rpc/rpc/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/dev.rs#L103-L116

Added lines #L103 - L116 were not covered by tests
Comment on lines +103 to +116
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ohayo sensei! Avoid panics on invalid address or unit.

Felt::from_str(address.as_str()).unwrap() and PriceUnit::from_str(...) can cause runtime panics if the user sends malformed data. Returning a structured error might be safer.

- let account_address: ContractAddress = Felt::from_str(address.as_str()).unwrap().into();
- let unit = PriceUnit::from_str(unit.to_uppercase().as_str()).unwrap_or(PriceUnit::Wei);
+ let account_address: ContractAddress = Felt::from_str(address.as_str())
+     .map_err(|_| DevApiError::InvalidAddress(address.clone()))?
+     .into();
+ let unit = PriceUnit::from_str(unit.to_uppercase().as_str())
+     .map_err(|_| DevApiError::InvalidPriceUnit(unit.clone()))?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async fn account_balance(&self, address: String, unit: String) -> Result<u128, Error> {
let account_address: ContractAddress = Felt::from_str(address.as_str()).unwrap().into();
let unit = PriceUnit::from_str(unit.to_uppercase().as_str()).unwrap_or(PriceUnit::Wei);
let erc20_address =
get_erc20_address(&unit).map_err(|_| http::response::internal_error()).unwrap();
let provider = self.backend.blockchain.provider();
let state = provider.latest().unwrap();
let storage_slot = get_fee_token_balance_base_storage_address(account_address);
let balance_felt = state.storage(erc20_address, storage_slot).unwrap().unwrap();
let balance: u128 = balance_felt.to_string().parse().unwrap();
Ok(balance)
}
async fn account_balance(&self, address: String, unit: String) -> Result<u128, Error> {
let account_address: ContractAddress = Felt::from_str(address.as_str())
.map_err(|_| DevApiError::InvalidAddress(address.clone()))?
.into();
let unit = PriceUnit::from_str(unit.to_uppercase().as_str())
.map_err(|_| DevApiError::InvalidPriceUnit(unit.clone()))?;
let erc20_address =
get_erc20_address(&unit).map_err(|_| http::response::internal_error()).unwrap();
let provider = self.backend.blockchain.provider();
let state = provider.latest().unwrap();
let storage_slot = get_fee_token_balance_base_storage_address(account_address);
let balance_felt = state.storage(erc20_address, storage_slot).unwrap().unwrap();
let balance: u128 = balance_felt.to_string().parse().unwrap();
Ok(balance)
}


async fn mint(&self) -> Result<(), Error> {
Ok(())
}

Check warning on line 120 in crates/katana/rpc/rpc/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/dev.rs#L118-L120

Added lines #L118 - L120 were not covered by tests

async fn predeployed_accounts(&self) -> Result<Vec<Account>, Error> {
Ok(self.backend.chain_spec.genesis.accounts().map(|e| Account::new(*e.0, e.1)).collect())
}
Expand Down
10 changes: 5 additions & 5 deletions crates/katana/rpc/rpc/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
//! RPC implementations.

#![allow(clippy::blocks_in_conditions)]
#![cfg_attr(not(test), warn(unused_crate_dependencies))]

use std::net::SocketAddr;
use std::time::Duration;

use jsonrpsee::server::{AllowHosts, ServerBuilder, ServerHandle};
use jsonrpsee::RpcModule;
use proxy_get_request::DevnetProxyLayer;
use tower::ServiceBuilder;
use tracing::info;

pub mod cors;
pub mod dev;
pub mod health;
pub mod metrics;
pub mod proxy_get_request;
pub mod saya;
pub mod starknet;
pub mod torii;

mod transport;
mod utils;

use cors::Cors;
Expand Down Expand Up @@ -116,6 +115,7 @@ impl RpcServer {
let middleware = ServiceBuilder::new()
.option_layer(self.cors.clone())
.option_layer(health_check_proxy)
.layer(DevnetProxyLayer::new()?)
.timeout(Duration::from_secs(20));

let builder = ServerBuilder::new()
Expand Down
162 changes: 162 additions & 0 deletions crates/katana/rpc/rpc/src/proxy_get_request.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
//! Middleware that proxies requests at a specified URI to internal
//! RPC method calls.
use std::collections::HashMap;
use std::error::Error;
use std::future::Future;
use std::pin::Pin;
use std::task::{Context, Poll};

use hyper::header::{ACCEPT, CONTENT_TYPE};
use hyper::http::HeaderValue;
use hyper::{Body, Method, Request, Response, Uri};
use jsonrpsee_core::error::Error as RpcError;
use jsonrpsee_core::JsonRawValue;
use jsonrpsee_types::{Id, RequestSer};
use tower::{Layer, Service};
use url::form_urlencoded;

use crate::transport::http;

/// Layer that applies [`DevnetProxy`] which proxies the `GET /path` requests to
/// specific RPC method calls and that strips the response.
///
/// See [`DevnetProxy`] for more details.
#[derive(Debug, Clone)]
pub struct DevnetProxyLayer {}

impl DevnetProxyLayer {
/// Creates a new [`DevnetProxyLayer`].
///
/// See [`DevnetProxy`] for more details.
pub fn new() -> Result<Self, RpcError> {
Ok(Self {})
}
}
impl<S> Layer<S> for DevnetProxyLayer {
type Service = DevnetProxy<S>;

fn layer(&self, inner: S) -> Self::Service {
DevnetProxy::new(inner).expect("Path already validated in DevnetProxyLayer; qed")
}
}

/// Proxy `GET /path` requests to the specified RPC method calls.
///
/// # Request
///
/// The `GET /path` requests are modified into valid `POST` requests for
/// calling the RPC method. This middleware adds appropriate headers to the
/// request, and completely modifies the request `BODY`.
///
/// # Response
///
/// The response of the RPC method is stripped down to contain only the method's
/// response, removing any RPC 2.0 spec logic regarding the response' body.
#[derive(Debug, Clone)]
pub struct DevnetProxy<S> {
inner: S,
}

impl<S> DevnetProxy<S> {
/// Creates a new [`DevnetProxy`].
///
/// The request `GET /path` is redirected to the provided method.
/// Fails if the path does not start with `/`.
pub fn new(inner: S) -> Result<Self, RpcError> {
Ok(Self { inner })
}
}

impl<S> Service<Request<Body>> for DevnetProxy<S>
where
S: Service<Request<Body>, Response = Response<Body>>,
S::Response: 'static,
S::Error: Into<Box<dyn Error + Send + Sync>> + 'static,
S::Future: Send + 'static,
{
type Response = S::Response;
type Error = Box<dyn Error + Send + Sync + 'static>;
type Future =
Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;

#[inline]
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
self.inner.poll_ready(cx).map_err(Into::into)
}

fn call(&mut self, mut req: Request<Body>) -> Self::Future {
let query = req.uri().query();
let path = req.uri().path();

let (params, method) = match path {
"/account_balance" => get_account_balance(query),
_ => (JsonRawValue::from_string("{}".to_string()).unwrap(), "".to_string()),
};

if !method.is_empty() {
// RPC methods are accessed with `POST`.
*req.method_mut() = Method::POST;
// Precautionary remove the URI.
*req.uri_mut() = Uri::from_static("/");

// Requests must have the following headers:
req.headers_mut().insert(CONTENT_TYPE, HeaderValue::from_static("application/json"));
req.headers_mut().insert(ACCEPT, HeaderValue::from_static("application/json"));

// Adjust the body to reflect the method call.
let body = Body::from(
serde_json::to_string(&RequestSer::borrowed(
&Id::Number(0),
&method,
Some(&params),
))
.expect("Valid request; qed"),
);
req = req.map(|_| body);

Check warning on line 115 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L97-L115

Added lines #L97 - L115 were not covered by tests
}

// Call the inner service and get a future that resolves to the response.
let fut = self.inner.call(req);

// Adjust the response if needed.
let res_fut = async move {
let res = fut.await.map_err(|err| err.into())?;

if method.is_empty() {
return Ok(res);
}

let body = res.into_body();
let bytes = hyper::body::to_bytes(body).await?;

Check warning on line 130 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L127-L130

Added lines #L127 - L130 were not covered by tests

#[derive(serde::Deserialize, Debug)]

Check warning on line 132 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L132

Added line #L132 was not covered by tests
struct RpcPayload<'a> {
#[serde(borrow)]
result: &'a serde_json::value::RawValue,
}

let response = if let Ok(payload) = serde_json::from_slice::<RpcPayload<'_>>(&bytes) {
http::response::ok_response(payload.result.to_string())

Check warning on line 139 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L138-L139

Added lines #L138 - L139 were not covered by tests
} else {
http::response::internal_error()

Check warning on line 141 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L141

Added line #L141 was not covered by tests
};

Ok(response)

Check warning on line 144 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L144

Added line #L144 was not covered by tests
};

Box::pin(res_fut)
}
}

fn get_account_balance(query: Option<&str>) -> (Box<JsonRawValue>, std::string::String) {
let default = String::new();

let query = query.unwrap_or(&default);
let params: HashMap<_, _> = form_urlencoded::parse(query.as_bytes()).into_owned().collect();

let address = params.get("contract_address").unwrap_or(&default);
let unit = params.get("unit").unwrap_or(&default);

let json_string = format!(r#"{{"address":"{}", "unit":"{}"}}"#, address, unit);
(JsonRawValue::from_string(json_string).unwrap(), "dev_accountBalance".to_string())
}

Check warning on line 162 in crates/katana/rpc/rpc/src/proxy_get_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/rpc/rpc/src/proxy_get_request.rs#L151-L162

Added lines #L151 - L162 were not covered by tests
Comment on lines +151 to +162
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ohayo sensei! Suggest graceful error handling for query extraction.

get_account_balance uses unwrap() for from_string. A malformed query string would panic. Consider returning a user-friendly error or defaulting gracefully.

 fn get_account_balance(query: Option<&str>) -> (Box<JsonRawValue>, std::string::String) {
     ...
-    (JsonRawValue::from_string(json_string).unwrap(), "dev_accountBalance".to_string())
+    let raw_value = match JsonRawValue::from_string(json_string) {
+        Ok(val) => val,
+        Err(_) => {
+            // fallback or error handling
+            JsonRawValue::from_string(r#"{}"#.to_string()).unwrap()
+        }
+    };
+    (raw_value, "dev_accountBalance".to_string())
 }

Committable suggestion skipped: line range outside the PR's diff.

Loading
Loading