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(torii-grpc): erc tokens and balances #2698

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
22 changes: 22 additions & 0 deletions crates/torii/core/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,28 @@
pub executed_at: DateTime<Utc>,
pub created_at: DateTime<Utc>,
}

#[derive(FromRow, Deserialize, Debug, Clone)]

Check warning on line 125 in crates/torii/core/src/types.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/types.rs#L125

Added line #L125 was not covered by tests
#[serde(rename_all = "camelCase")]
pub struct Token {
pub id: String,
pub contract_address: String,
pub name: String,
pub symbol: String,
pub decimals: u8,
pub metadata: String,
}

#[derive(FromRow, Deserialize, Debug, Clone)]

Check warning on line 136 in crates/torii/core/src/types.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/types.rs#L136

Added line #L136 was not covered by tests
#[serde(rename_all = "camelCase")]
pub struct TokenBalance {
pub id: String,
pub balance: String,
pub account_address: String,
pub contract_address: String,
pub token_id: String,
}

#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq)]
pub struct Contract {
pub address: Felt,
Expand Down
15 changes: 15 additions & 0 deletions crates/torii/grpc/proto/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,19 @@ enum ComparisonOperator {
GTE = 3;
LT = 4;
LTE = 5;
}

message Token {
string contract_address = 2;
string name = 3;
string symbol = 4;
uint32 decimals = 5;
string metadata = 6;
}

message TokenBalance {
string balance = 1;
string account_address = 2;
string contract_address = 3;
string token_id = 4;
}
30 changes: 30 additions & 0 deletions crates/torii/grpc/proto/world.proto
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@ service World {

// Subscribe to events
rpc SubscribeEvents (SubscribeEventsRequest) returns (stream SubscribeEventsResponse);

// Retrieve tokens
rpc RetrieveTokens (RetrieveTokensRequest) returns (RetrieveTokensResponse);

// Retrieve token balances
rpc RetrieveTokenBalances (RetrieveTokenBalancesRequest) returns (RetrieveTokenBalancesResponse);
}

// A request to retrieve tokens
message RetrieveTokensRequest {
// The list of contract addresses to retrieve tokens for
repeated bytes contract_addresses = 1;
}

// A response containing tokens
message RetrieveTokensResponse {
repeated types.Token tokens = 1;
}

// A request to retrieve token balances
message RetrieveTokenBalancesRequest {
// The account addresses to retrieve balances for
repeated bytes account_addresses = 1;
// The list of token contract addresses to retrieve balances for
repeated bytes contract_addresses = 2;
}

// A response containing token balances
message RetrieveTokenBalancesResponse {
repeated types.TokenBalance balances = 1;
}

// A request to subscribe to indexer updates.
Expand Down
100 changes: 96 additions & 4 deletions crates/torii/grpc/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, TokenBalance};
use tower_http::cors::{AllowOrigin, CorsLayer};

use self::subscriptions::entity::EntityManager;
Expand All @@ -53,10 +54,11 @@
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, RetrieveTokenBalancesRequest,
RetrieveTokenBalancesResponse, RetrieveTokensRequest, RetrieveTokensResponse,
SubscribeEntitiesRequest, SubscribeEntityResponse, SubscribeEventMessagesRequest,
SubscribeEventsResponse, SubscribeIndexerRequest, SubscribeIndexerResponse,
UpdateEventMessagesSubscriptionRequest, WorldMetadataRequest, WorldMetadataResponse,
};
use crate::proto::{self};
use crate::types::schema::SchemaError;
Expand Down Expand Up @@ -789,6 +791,57 @@
})
}

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(", ")
);

Check warning on line 805 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L794-L805

Added lines #L794 - L805 were not covered by tests

let tokens: Vec<Token> = sqlx::query_as(&query)
.fetch_all(&self.pool)
.await
.map_err(|e| Status::internal(e.to_string()))?;

Check warning on line 810 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L807-L810

Added lines #L807 - L810 were not covered by tests

let tokens = tokens.iter().map(|token| token.clone().into()).collect();
Ok(RetrieveTokensResponse { tokens })
}

Check warning on line 814 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L812-L814

Added lines #L812 - L814 were not covered by tests
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! Please address SQL injection and input validation concerns.

The implementation needs several security and robustness improvements:

  1. SQL Injection vulnerability: Using string formatting for SQL query construction
  2. No validation of input contract addresses
  3. No limit on number of addresses that can be queried

Consider this safer implementation:

async fn retrieve_tokens(
    &self,
    contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokensResponse, Status> {
+    const MAX_ADDRESSES: usize = 100;
+    if contract_addresses.is_empty() {
+        return Err(Status::invalid_argument("No contract addresses provided"));
+    }
+    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

‼️ 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 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 })
}
async fn retrieve_tokens(
&self,
contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokensResponse, Status> {
const MAX_ADDRESSES: usize = 100;
if contract_addresses.is_empty() {
return Err(Status::invalid_argument("No contract addresses provided"));
}
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 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 })
}


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! Please address potential SQL injection and performance concerns.

The current implementation has several issues that need attention:

  1. SQL Injection vulnerability: Using format! for SQL query construction is unsafe.
  2. No limit on the number of contract addresses that can be queried.

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

‼️ 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 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 })
}
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 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 })
}

async fn retrieve_token_balances(
&self,
account_addresses: Vec<Felt>,
contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokenBalancesResponse, Status> {
let query = format!(
"SELECT * FROM token_balances WHERE account_address IN ({}) AND contract_address IN \
({})",
account_addresses
.iter()
.map(|address| format!("{:#x}", address))
.collect::<Vec<_>>()
.join(", "),
contract_addresses
.iter()
.map(|address| format!("{:#x}", address))
.collect::<Vec<_>>()
.join(", ")
);

Check warning on line 834 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L816-L834

Added lines #L816 - L834 were not covered by tests

let balances: Vec<TokenBalance> = sqlx::query_as(&query)
.fetch_all(&self.pool)
.await
.map_err(|e| Status::internal(e.to_string()))?;

Check warning on line 839 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L836-L839

Added lines #L836 - L839 were not covered by tests

let balances = balances.iter().map(|balance| balance.clone().into()).collect();
Ok(RetrieveTokenBalancesResponse { balances })
}

Check warning on line 843 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L841-L843

Added lines #L841 - L843 were not covered by tests
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! Similar security concerns in token balances retrieval.

The implementation has the same security and robustness issues as retrieve_tokens:

  1. SQL Injection vulnerability
  2. No input validation
  3. No limits on address lists

Consider this safer implementation:

async fn retrieve_token_balances(
    &self,
    account_addresses: Vec<Felt>,
    contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokenBalancesResponse, Status> {
+    const MAX_ADDRESSES: usize = 100;
+    if account_addresses.is_empty() || contract_addresses.is_empty() {
+        return Err(Status::invalid_argument("Both account and contract addresses are required"));
+    }
+    if account_addresses.len() > MAX_ADDRESSES || contract_addresses.len() > MAX_ADDRESSES {
+        return Err(Status::invalid_argument(
+            format!("Too many addresses. Maximum allowed: {}", MAX_ADDRESSES)
+        ));
+    }
+
+    let account_placeholders = account_addresses.iter().map(|_| "?").collect::<Vec<_>>().join(", ");
+    let contract_placeholders = contract_addresses.iter().map(|_| "?").collect::<Vec<_>>().join(", ");
+
+    let query = format!(
+        "SELECT * FROM token_balances WHERE account_address IN ({}) AND contract_address IN ({})",
+        account_placeholders, contract_placeholders
+    );
+
+    let mut query_builder = sqlx::query_as(&query);
+    for address in account_addresses {
+        query_builder = query_builder.bind(format!("{:#x}", address));
+    }
+    for address in contract_addresses {
+        query_builder = query_builder.bind(format!("{:#x}", address));
+    }

-    let query = format!(
-        "SELECT * FROM token_balances WHERE account_address IN ({}) AND contract_address IN ({})",
-        account_addresses
-            .iter()
-            .map(|address| format!("{:#x}", address))
-            .collect::<Vec<_>>()
-            .join(", "),
-        contract_addresses
-            .iter()
-            .map(|address| format!("{:#x}", address))
-            .collect::<Vec<_>>()
-            .join(", ")
-    );
-
-    let balances: Vec<TokenBalance> = sqlx::query_as(&query)
+    let balances: Vec<TokenBalance> = query_builder
        .fetch_all(&self.pool)
        .await
        .map_err(|e| Status::internal(e.to_string()))?;

    let balances = balances.iter().map(|balance| balance.clone().into()).collect();
    Ok(RetrieveTokenBalancesResponse { balances })
}
📝 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 retrieve_token_balances(
&self,
account_addresses: Vec<Felt>,
contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokenBalancesResponse, Status> {
let query = format!(
"SELECT * FROM token_balances WHERE account_address IN ({}) AND contract_address IN \
({})",
account_addresses
.iter()
.map(|address| format!("{:#x}", address))
.collect::<Vec<_>>()
.join(", "),
contract_addresses
.iter()
.map(|address| format!("{:#x}", address))
.collect::<Vec<_>>()
.join(", ")
);
let balances: Vec<TokenBalance> = sqlx::query_as(&query)
.fetch_all(&self.pool)
.await
.map_err(|e| Status::internal(e.to_string()))?;
let balances = balances.iter().map(|balance| balance.clone().into()).collect();
Ok(RetrieveTokenBalancesResponse { balances })
}
async fn retrieve_token_balances(
&self,
account_addresses: Vec<Felt>,
contract_addresses: Vec<Felt>,
) -> Result<RetrieveTokenBalancesResponse, Status> {
const MAX_ADDRESSES: usize = 100;
if account_addresses.is_empty() || contract_addresses.is_empty() {
return Err(Status::invalid_argument("Both account and contract addresses are required"));
}
if account_addresses.len() > MAX_ADDRESSES || contract_addresses.len() > MAX_ADDRESSES {
return Err(Status::invalid_argument(
format!("Too many addresses. Maximum allowed: {}", MAX_ADDRESSES)
));
}
let account_placeholders = account_addresses.iter().map(|_| "?").collect::<Vec<_>>().join(", ");
let contract_placeholders = contract_addresses.iter().map(|_| "?").collect::<Vec<_>>().join(", ");
let query = format!(
"SELECT * FROM token_balances WHERE account_address IN ({}) AND contract_address IN ({})",
account_placeholders, contract_placeholders
);
let mut query_builder = sqlx::query_as(&query);
for address in account_addresses {
query_builder = query_builder.bind(format!("{:#x}", address));
}
for address in contract_addresses {
query_builder = query_builder.bind(format!("{:#x}", address));
}
let balances: Vec<TokenBalance> = query_builder
.fetch_all(&self.pool)
.await
.map_err(|e| Status::internal(e.to_string()))?;
let balances = balances.iter().map(|balance| balance.clone().into()).collect();
Ok(RetrieveTokenBalancesResponse { balances })
}


async fn subscribe_indexer(
&self,
contract_address: Felt,
Expand Down Expand Up @@ -1165,6 +1218,45 @@
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
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! Enhance error handling for retrieve_tokens endpoint.

The current implementation doesn't validate input contract addresses and lacks error handling for Felt conversions, which can cause silent failures.

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

‼️ 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
let RetrieveTokensRequest { contract_addresses } = request.into_inner();
let contract_addresses = contract_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
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_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()))?;

let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(tokens))
}

Check warning on line 1236 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1224-L1236

Added lines #L1224 - L1236 were not covered by tests
Comment on lines +1261 to +1276
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! Enhance error handling for the gRPC endpoint.

The implementation needs better input validation and error handling:

  1. No validation of input contract addresses
  2. Silent failures during Felt conversion

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

‼️ 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 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<_>>();
let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(tokens))
}
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_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))
}


Comment on lines +1261 to +1277
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance error handling for the gRPC endpoint.

The implementation needs better input validation and error handling:

  1. No validation of input contract addresses
  2. Silent failures during Felt conversion

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

‼️ 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 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<_>>();
let tokens = self
.retrieve_tokens(contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(tokens))
}
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_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))
}

async fn retrieve_token_balances(
&self,
request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
let account_addresses = account_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let contract_addresses = contract_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();

Comment on lines +1282 to +1292
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! Improve error handling for retrieve_token_balances endpoint.

There's a need to validate input addresses and handle potential conversion errors to prevent silent failures.

Apply this diff to enhance error handling:

 let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
     request.into_inner();
+if account_addresses.is_empty() || contract_addresses.is_empty() {
+    return Err(Status::invalid_argument("Both account and contract addresses are required"));
+}
 
 let account_addresses = account_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 account address: {}", e))
+        })
+    })
+    .collect::<Result<Vec<_>, Status>>()?;
 
 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 balances = self
     .retrieve_token_balances(account_addresses, contract_addresses)
     .await
     .map_err(|e| Status::internal(e.to_string()))?;
📝 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
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
let account_addresses = account_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let contract_addresses = contract_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
if account_addresses.is_empty() || contract_addresses.is_empty() {
return Err(Status::invalid_argument("Both account and contract addresses are required"));
}
let account_addresses = account_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid account address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;
let contract_addresses = contract_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid contract address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;

let balances = self
.retrieve_token_balances(account_addresses, contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(balances))
}

Check warning on line 1258 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1241-L1258

Added lines #L1241 - L1258 were not covered by tests
Comment on lines +1278 to +1298
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! Enhance error handling for address conversions.

The implementation needs better error handling for Felt conversions:

Consider this improved implementation:

async fn retrieve_token_balances(
    &self,
    request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
    let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
        request.into_inner();
+    if account_addresses.is_empty() || contract_addresses.is_empty() {
+        return Err(Status::invalid_argument("Both account and contract addresses are required"));
+    }
    let account_addresses = account_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 account address: {}", e))
+            })
+        })
+        .collect::<Result<Vec<_>, Status>>()?;
    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 balances = self
        .retrieve_token_balances(account_addresses, contract_addresses)
        .await
        .map_err(|e| Status::internal(e.to_string()))?;
    Ok(Response::new(balances))
}
📝 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 retrieve_token_balances(
&self,
request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
let account_addresses = account_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let contract_addresses = contract_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let balances = self
.retrieve_token_balances(account_addresses, contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(balances))
}
async fn retrieve_token_balances(
&self,
request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
if account_addresses.is_empty() || contract_addresses.is_empty() {
return Err(Status::invalid_argument("Both account and contract addresses are required"));
}
let account_addresses = account_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid account address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;
let contract_addresses = contract_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid contract address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;
let balances = self
.retrieve_token_balances(account_addresses, contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(balances))
}


Comment on lines +1278 to +1299
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! Enhance error handling for address conversions.

The implementation needs better error handling for Felt conversions:

Consider this improved implementation:

async fn retrieve_token_balances(
    &self,
    request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
    let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
        request.into_inner();
    let account_addresses = account_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 account address: {}", e))
+            })
+        })
+        .collect::<Result<Vec<_>, Status>>()?;
-        .collect::<Vec<_>>();
    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>>()?;
-        .collect::<Vec<_>>();

    let balances = self
        .retrieve_token_balances(account_addresses, contract_addresses)
        .await
        .map_err(|e| Status::internal(e.to_string()))?;
    Ok(Response::new(balances))
}
📝 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 retrieve_token_balances(
&self,
request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
let account_addresses = account_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let contract_addresses = contract_addresses
.iter()
.map(|address| Felt::from_bytes_be_slice(address))
.collect::<Vec<_>>();
let balances = self
.retrieve_token_balances(account_addresses, contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(balances))
}
async fn retrieve_token_balances(
&self,
request: Request<RetrieveTokenBalancesRequest>,
) -> Result<Response<RetrieveTokenBalancesResponse>, Status> {
let RetrieveTokenBalancesRequest { account_addresses, contract_addresses } =
request.into_inner();
let account_addresses = account_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid account address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;
let contract_addresses = contract_addresses
.iter()
.map(|address| {
Felt::from_bytes_be_slice(address).map_err(|e| {
Status::invalid_argument(format!("Invalid contract address: {}", e))
})
})
.collect::<Result<Vec<_>, Status>>()?;
let balances = self
.retrieve_token_balances(account_addresses, contract_addresses)
.await
.map_err(|e| Status::internal(e.to_string()))?;
Ok(Response::new(balances))
}

async fn subscribe_indexer(
&self,
request: Request<SubscribeIndexerRequest>,
Expand Down
24 changes: 24 additions & 0 deletions crates/torii/grpc/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,36 @@
ContractStorageDiffItem, Felt, FromStrError, StateDiff, StateUpdate, StorageEntry,
};
use strum_macros::{AsRefStr, EnumIter, FromRepr};
use torii_core::types::{Token, TokenBalance};

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(),
}
}

Check warning on line 29 in crates/torii/grpc/src/types/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/types/mod.rs#L21-L29

Added lines #L21 - L29 were not covered by tests
}
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Handle serialization errors gracefully in Token conversion

Ohayo sensei! Looking at the codebase, I can confirm this is a critical issue. The serde_json::to_string(&value.metadata).unwrap() call in the Token conversion could indeed panic. While there are other unwrap usages in the codebase, this one is particularly risky since it's in a type conversion that could be called frequently in production code.

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 chain

Handle potential serialization errors gracefully

Ohayo sensei! While the implementation is generally good, the unwrap() call on line 27 could panic if the metadata serialization fails. Consider handling this error case gracefully.

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 executed

The 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


impl From<TokenBalance> for proto::types::TokenBalance {
fn from(value: TokenBalance) -> Self {
Self {
balance: value.balance,
account_address: value.account_address,
contract_address: value.contract_address,
token_id: value.token_id,
}
}

Check warning on line 40 in crates/torii/grpc/src/types/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/types/mod.rs#L33-L40

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

#[derive(Debug, Serialize, Deserialize, PartialEq, Hash, Eq, Clone)]
pub struct IndexerUpdate {
pub head: i64,
Expand Down
Loading