-
Notifications
You must be signed in to change notification settings - Fork 16
feat sum #65
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
base: main
Are you sure you want to change the base?
feat sum #65
Changes from 4 commits
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 | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||||||||||||||||||||||||||||||||||||||||
use crate::common::{ | ||||||||||||||||||||||||||||||||||||||||||||
account::{Account, AccountField}, | ||||||||||||||||||||||||||||||||||||||||||||
chain::{Chain, ChainOrRpc}, | ||||||||||||||||||||||||||||||||||||||||||||
ens::NameOrAddress, | ||||||||||||||||||||||||||||||||||||||||||||
query_result::{ExpressionResult, SumQueryRes}, | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
use alloy::{ | ||||||||||||||||||||||||||||||||||||||||||||
primitives::Address, | ||||||||||||||||||||||||||||||||||||||||||||
providers::{Provider, ProviderBuilder, RootProvider}, | ||||||||||||||||||||||||||||||||||||||||||||
transports::http::{Client, Http}, | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
use anyhow::Result; | ||||||||||||||||||||||||||||||||||||||||||||
use futures::future::try_join_all; | ||||||||||||||||||||||||||||||||||||||||||||
use serde::{Deserialize, Serialize}; | ||||||||||||||||||||||||||||||||||||||||||||
use std::sync::Arc; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
#[derive(Debug, Serialize, Deserialize, thiserror::Error)] | ||||||||||||||||||||||||||||||||||||||||||||
pub enum AccountResolverErrors { | ||||||||||||||||||||||||||||||||||||||||||||
#[error("Mismatch between Entity and EntityId, {0} can't be resolved as a account id")] | ||||||||||||||||||||||||||||||||||||||||||||
MismatchEntityAndEntityId(String), | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+17
to
+21
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. 🛠️ Refactor suggestion Utilize the error enum for error handling. The Apply this diff to add error handling: pub fn resolve_sum_query(exp: &ExpressionResult) -> Result<ExpressionResult> {
+ match exp {
+ ExpressionResult::Account(_) => {
+ // Handle account summation
+ let res = ExpressionResult::Sum(vec![SumQueryRes::default()]);
+ Ok(res)
+ }
+ _ => Err(anyhow::anyhow!(AccountResolverErrors::MismatchEntityAndEntityId(
+ "Cannot sum non-account entities".to_string(),
+ ))),
+ }
- let res = ExpressionResult::Sum(vec![SumQueryRes::default()]);
- Ok(res)
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/// Resolve the query to get accounts after receiving an account entity expression | ||||||||||||||||||||||||||||||||||||||||||||
/// Iterate through entity_ids and map them to a futures list. Execute all futures concurrently and collect the results. | ||||||||||||||||||||||||||||||||||||||||||||
pub fn resolve_sum_query( | ||||||||||||||||||||||||||||||||||||||||||||
exp: &ExpressionResult, | ||||||||||||||||||||||||||||||||||||||||||||
) -> Result<ExpressionResult>{ | ||||||||||||||||||||||||||||||||||||||||||||
|
pub fn resolve_sum_query( | |
exp: &ExpressionResult, | |
) -> Result<ExpressionResult>{ | |
/// Resolve the query to get accounts after receiving an account entity expression | |
/// Iterate through entity_ids and map them to a futures list. Execute all futures concurrently and collect the results. | |
/// | |
/// # Arguments | |
/// * `exp` - The expression result to process | |
/// | |
/// # Returns | |
/// * `Result<ExpressionResult>` - The sum query result or an error | |
pub fn resolve_sum_query( | |
exp: &ExpressionResult, | |
) -> Result<ExpressionResult> { | |
// Validate input | |
match exp { | |
ExpressionResult::Account(_) => (), | |
_ => return Err(anyhow::anyhow!("Expected Account expression result")), | |
} | |
// TODO: Implement the actual sum query resolution logic | |
} |
Outdated
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.
Implement the commented-out code.
The function currently returns a default result without implementing the actual summation logic. The commented-out code should be properly implemented to handle multiple chains and account IDs concurrently.
Would you like me to help implement the complete solution for handling multiple chains and account IDs concurrently?
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.
Make constructor public and add input validation.
The constructor should be public to allow creating instances from other modules.