-
Notifications
You must be signed in to change notification settings - Fork 175
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(torii-grpc): erc tokens and balances #2698
base: main
Are you sure you want to change the base?
Changes from 3 commits
025d5a7
a19f1f2
a28545d
78a341d
4aece4a
e72bc05
cd0528b
a4b8a2a
52277da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -43,6 +43,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use torii_core::model::{build_sql_query, map_row_to_ty}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use torii_core::sql::cache::ModelCache; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use torii_core::sql::utils::sql_string_to_felts; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use torii_core::types::Token; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use tower_http::cors::{AllowOrigin, CorsLayer}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use self::subscriptions::entity::EntityManager; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -53,10 +54,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::proto::types::LogicalOperator; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::proto::world::world_server::WorldServer; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::proto::world::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
RetrieveEntitiesStreamingResponse, RetrieveEventMessagesRequest, SubscribeEntitiesRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SubscribeEntityResponse, SubscribeEventMessagesRequest, SubscribeEventsResponse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SubscribeIndexerRequest, SubscribeIndexerResponse, UpdateEventMessagesSubscriptionRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
WorldMetadataRequest, WorldMetadataResponse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
RetrieveEntitiesStreamingResponse, RetrieveEventMessagesRequest, RetrieveTokensRequest, RetrieveTokensResponse, SubscribeEntitiesRequest, SubscribeEntityResponse, SubscribeEventMessagesRequest, SubscribeEventsResponse, SubscribeIndexerRequest, SubscribeIndexerResponse, UpdateEventMessagesSubscriptionRequest, WorldMetadataRequest, WorldMetadataResponse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::proto::{self}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::types::schema::SchemaError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -789,6 +787,28 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async fn retrieve_tokens( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract_addresses: Vec<Felt>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) -> Result<RetrieveTokensResponse, Status> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let query = format!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"SELECT * FROM tokens WHERE contract_address IN ({})", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract_addresses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map(|address| format!("{:#x}", address)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.collect::<Vec<_>>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.join(", ") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tokens: Vec<Token> = sqlx::query_as(&query) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.fetch_all(&self.pool) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map_err(|e| Status::internal(e.to_string()))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tokens = tokens.iter().map(|token| token.clone().into()).collect(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(RetrieveTokensResponse { tokens }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohayo sensei! Please address potential SQL injection and performance concerns. The current implementation has several issues that need attention:
Consider this safer implementation: async fn retrieve_tokens(
&self,
contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokensResponse, Status> {
+ const MAX_ADDRESSES: usize = 100;
+ if contract_addresses.len() > MAX_ADDRESSES {
+ return Err(Status::invalid_argument(
+ format!("Too many addresses. Maximum allowed: {}", MAX_ADDRESSES)
+ ));
+ }
+
+ let placeholders = contract_addresses
+ .iter()
+ .map(|_| "?")
+ .collect::<Vec<_>>()
+ .join(", ");
+
+ let query = format!("SELECT * FROM tokens WHERE contract_address IN ({})", placeholders);
+
+ let mut query_builder = sqlx::query_as(&query);
+ for address in contract_addresses {
+ query_builder = query_builder.bind(format!("{:#x}", address));
+ }
- let query = format!(
- "SELECT * FROM tokens WHERE contract_address IN ({})",
- contract_addresses
- .iter()
- .map(|address| format!("{:#x}", address))
- .collect::<Vec<_>>()
- .join(", ")
- );
-
- let tokens: Vec<Token> = sqlx::query_as(&query)
+ let tokens: Vec<Token> = query_builder
.fetch_all(&self.pool)
.await
.map_err(|e| Status::internal(e.to_string()))?;
let tokens = tokens.iter().map(|token| token.clone().into()).collect();
Ok(RetrieveTokensResponse { tokens })
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async fn subscribe_indexer( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract_address: Felt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -1165,6 +1185,23 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(Response::new(WorldMetadataResponse { metadata })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async fn retrieve_tokens( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
request: Request<RetrieveTokensRequest>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) -> Result<Response<RetrieveTokensResponse>, Status> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let RetrieveTokensRequest { contract_addresses } = request.into_inner(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let contract_addresses = contract_addresses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map(|address| Felt::from_bytes_be_slice(address)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.collect::<Vec<_>>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1265
to
+1270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohayo sensei! Enhance error handling for The current implementation doesn't validate input contract addresses and lacks error handling for Apply this diff to improve error handling: let RetrieveTokensRequest { contract_addresses } = request.into_inner();
+if contract_addresses.is_empty() {
+ return Err(Status::invalid_argument("No contract addresses provided"));
+}
let contract_addresses = contract_addresses
.iter()
- .map(|address| Felt::from_bytes_be_slice(address))
- .collect::<Vec<_>>();
+ .map(|address| {
+ Felt::from_bytes_be_slice(address).map_err(|e| {
+ Status::invalid_argument(format!("Invalid contract address: {}", e))
+ })
+ })
+ .collect::<Result<Vec<_>, Status>>()?;
let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tokens = self | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.retrieve_tokens(contract_addresses) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map_err(|e| Status::internal(e.to_string()))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(Response::new(tokens)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1261
to
+1276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohayo sensei! Enhance error handling for the gRPC endpoint. The implementation needs better input validation and error handling:
Consider this improved implementation: async fn retrieve_tokens(
&self,
request: Request<RetrieveTokensRequest>,
) -> Result<Response<RetrieveTokensResponse>, Status> {
let RetrieveTokensRequest { contract_addresses } = request.into_inner();
+ if contract_addresses.is_empty() {
+ return Err(Status::invalid_argument("No contract addresses provided"));
+ }
+
let contract_addresses = contract_addresses
.iter()
- .map(|address| Felt::from_bytes_be_slice(address))
+ .map(|address| {
+ Felt::from_bytes_be_slice(address).map_err(|e| {
+ Status::invalid_argument(format!("Invalid contract address: {}", e))
+ })
+ })
+ .collect::<Result<Vec<_>, Status>>()?;
let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(tokens))
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1261
to
+1277
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance error handling for the gRPC endpoint. The implementation needs better input validation and error handling:
Consider this improved implementation: async fn retrieve_tokens(
&self,
request: Request<RetrieveTokensRequest>,
) -> Result<Response<RetrieveTokensResponse>, Status> {
let RetrieveTokensRequest { contract_addresses } = request.into_inner();
+ if contract_addresses.is_empty() {
+ return Err(Status::invalid_argument("No contract addresses provided"));
+ }
+
let contract_addresses = contract_addresses
.iter()
- .map(|address| Felt::from_bytes_be_slice(address))
+ .map(|address| {
+ Felt::from_bytes_be_slice(address).map_err(|e| {
+ Status::invalid_argument(format!("Invalid contract address: {}", e))
+ })
+ })
+ .collect::<Result<Vec<_>, Status>>()?;
let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(tokens))
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async fn subscribe_indexer( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
request: Request<SubscribeIndexerRequest>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,25 @@ | |
ContractStorageDiffItem, Felt, FromStrError, StateDiff, StateUpdate, StorageEntry, | ||
}; | ||
use strum_macros::{AsRefStr, EnumIter, FromRepr}; | ||
use torii_core::types::Token; | ||
|
||
use crate::proto::types::member_value; | ||
use crate::proto::{self}; | ||
|
||
pub mod schema; | ||
|
||
impl From<Token> for proto::types::Token { | ||
fn from(value: Token) -> Self { | ||
Self { | ||
contract_address: value.contract_address, | ||
name: value.name, | ||
symbol: value.symbol, | ||
decimals: value.decimals as u32, | ||
metadata: serde_json::to_string(&value.metadata).unwrap(), | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Handle serialization errors gracefully in Token conversion Ohayo sensei! Looking at the codebase, I can confirm this is a critical issue. The Here's a suggested improvement: impl From<Token> for proto::types::Token {
fn from(value: Token) -> Self {
+ // Serialize metadata, using empty JSON object as fallback
+ let metadata = serde_json::to_string(&value.metadata)
+ .unwrap_or_else(|_| "{}".to_string());
+
Self {
contract_address: value.contract_address,
name: value.name,
symbol: value.symbol,
decimals: value.decimals as u32,
- metadata: serde_json::to_string(&value.metadata).unwrap(),
+ metadata,
}
}
} The change ensures that even if metadata serialization fails, we'll get a valid JSON object rather than a panic. This is a safer approach for production code. 🔗 Analysis chainHandle potential serialization errors gracefully Ohayo sensei! While the implementation is generally good, the Here's a suggested improvement: impl From<Token> for proto::types::Token {
fn from(value: Token) -> Self {
+ // Serialize metadata, using empty JSON object as fallback
+ let metadata = serde_json::to_string(&value.metadata)
+ .unwrap_or_else(|_| "{}".to_string());
+
Self {
contract_address: value.contract_address,
name: value.name,
symbol: value.symbol,
decimals: value.decimals as u32,
- metadata: serde_json::to_string(&value.metadata).unwrap(),
+ metadata,
}
}
} Let's verify if there are similar unwrap patterns in other type conversions: Let me search for similar patterns in the codebase to verify if this is a common issue or an isolated case. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for potentially dangerous unwrap calls in type conversions
rg 'unwrap\(\)' --type rust
Length of output: 192327 Script: #!/bin/bash
# Search for serde_json::to_string unwrap patterns in type conversions
rg 'serde_json::to_string\(.*\)\.unwrap\(\)' --type rust
# Search for error handling patterns in similar type conversions
rg 'impl From<.*> for proto::types::.*' --type rust -A 10
Length of output: 13698 |
||
|
||
#[derive(Debug, Serialize, Deserialize, PartialEq, Hash, Eq, Clone)] | ||
pub struct IndexerUpdate { | ||
pub head: i64, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo sensei! Please address SQL injection and input validation concerns.
The implementation needs several security and robustness improvements:
Consider this safer implementation:
📝 Committable suggestion