-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Pagination queries for balances
endpoint
#2490
base: master
Are you sure you want to change the base?
Conversation
…tion_for_balances
@@ -79,8 +79,6 @@ impl BalanceQuery { | |||
Ok(balance) | |||
} | |||
|
|||
// TODO: This API should be migrated to the indexer for better support and | |||
// discontinued within fuel-core. | |||
#[graphql(complexity = "query_costs().balance_query")] |
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.
#[graphql(complexity = "query_costs().balance_query")] | |
#[graphql(complexity = "query_costs().balance_query + query_costs().storage_iterator \ | |
+ (query_costs().storage_read + first.unwrap_or_default() as usize) * child_complexity \ | |
+ (query_costs().storage_read + last.unwrap_or_default() as usize) * child_complexity\")] |
We need to limit the number of requested balances=)
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.
It also makes this change breaking
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.
Updated in fd2b019
@@ -233,15 +233,48 @@ impl OffChainDatabase for OffChainIterableKeyValueView { | |||
} | |||
} | |||
|
|||
fn balances( | |||
&self, | |||
fn balances<'a>( |
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.
Yeah, it is a tricky function because of how we handle balances.
Hmm, do you remember why we decided to skip balances during iteration and add them at the begin?
I remember: if you don't have coins but have messages, they will not appear during the iteration.
The problem with the current solution is when start == base_asset_id
. The current code will return an incorrect iterator.
Either we need to handle it differently, or come up with another solution. It would be nice if we could insert base asset balance into the mid of the iterator. Maybe we could change the indexation and make it always add empy indexation for the base asset(like entry in the balances table with 0 amount just for balances)?
Then during iteration we call always fetch its value by using self.base_asset_balance
?
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.
…tion_for_balances
Converting to draft, some rework is needed here. |
…tion_for_balances
Closes #2023
TODO:
Description
This PR enables the paginated queries for balances, but only if the balance indexation is enabled. Otherwise, the attempt to send paginated query will result with
Can not use pagination when balances indexation is not available
error.Checklist
Before requesting review