PE-9103: Add block range filters to ID-based GraphQL queries#2139
Open
vilenarios wants to merge 4 commits into
Open
PE-9103: Add block range filters to ID-based GraphQL queries#2139vilenarios wants to merge 4 commits into
vilenarios wants to merge 4 commits into
Conversation
- remove ARIO on AO, ARIO on Base, and ARIO via ETH from all token selection lists (token groups, inline payment dropdown, payment method selector, old token selection view) - payment_method_selector dropdown now uses TokenGroup.allGroups instead of CryptoToken.values to avoid showing hidden tokens - fix excessive vertical white space in crypto topup modals by replacing Expanded with Flexible in content wrappers and removing the fixed 600dp height from the modal container - remove unused _BenefitChip widget Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
turbo-gateway.com's ClickHouse indexer scans ~2.4 rows per block per
tx ID. Without block filters, queries with 3+ IDs exceed the 10M row
limit and fail with TOO_MANY_ROWS.
Changes:
- LicenseAssertions, LicenseComposed: add block: { min, max } filter
- TransactionStatuses: add block: { max } filter (no min for pending txs)
- Estimate minBlockHeight from revision dates (~2min/block from genesis)
- Dynamic chunk sizing based on block range: tight range = bigger chunks
e.g. 200k range → 10 IDs/chunk, 50k range → 41 IDs/chunk
- TransactionStatuses: fixed chunk size of 2 (no min block available)
For 16k licenses from the last year (~200k block range), this means
~1600 queries instead of 8000 (or 16000 with chunk=1). For incremental
syncs (~5k block range), a single query handles up to 50 licenses.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 45 minutes and 10 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough
WalkthroughBlock-height bounds are added to three Arweave GraphQL queries and their service methods via optional ChangesArweave Block-Range Bounded Sync
ARIO Token Removal and Turbo UI Layout Fixes
Sequence Diagram(s)sequenceDiagram
participant SyncRepository
participant ArweaveService
participant GraphQL
rect rgba(30, 100, 200, 0.5)
Note over SyncRepository,GraphQL: License sync with block bounds
SyncRepository->>SyncRepository: _estimateMinBlock(earliestRevisionDate) → minBlockHeight
SyncRepository->>ArweaveService: getLicenseAssertions(ids, minBlockHeight, maxBlockHeight)
ArweaveService->>ArweaveService: _safeChunkSize(min, max) → chunkSize [1..50]
ArweaveService->>GraphQL: LicenseAssertions(ids_chunk, block:{min,max})
GraphQL-->>ArweaveService: assertion results
ArweaveService-->>SyncRepository: yield assertions
SyncRepository->>ArweaveService: getLicenseComposed(ids, minBlockHeight, maxBlockHeight)
ArweaveService->>GraphQL: LicenseComposed(ids_chunk, block:{min,max})
GraphQL-->>ArweaveService: composed results
ArweaveService-->>SyncRepository: yield composed
end
rect rgba(200, 80, 30, 0.5)
Note over SyncRepository,GraphQL: Tx confirmations with block bound
SyncRepository->>ArweaveService: getTransactionConfirmations(ids, maxBlockHeight)
ArweaveService->>GraphQL: TransactionStatuses(ids_chunk=2, block:{max})
GraphQL-->>ArweaveService: statuses
ArweaveService-->>SyncRepository: confirmations map
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/services/arweave/arweave_service.dart (2)
1238-1260: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not treat bounded misses as globally missing transactions.
With
maxBlockHeightapplied, a missing edge only proves the transaction was not returned within that bounded view. Because the map is prefilled with-1, downstream status updates can mark a transaction as failed even if it was mined aftermaxBlockHeight.Proposed fix
- final transactionConfirmations = { - for (final transactionId in transactionIds) transactionId: -1 - }; + final transactionConfirmations = <String?, int>{}; + if (maxBlockHeight == null) { + for (final transactionId in transactionIds) { + transactionConfirmations[transactionId] = -1; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/arweave/arweave_service.dart` around lines 1238 - 1260, The issue is that the transactionConfirmations map is prefilled with -1 for all transactionIds, which causes bounded misses (transactions not found within the maxBlockHeight range) to be treated as globally missing transactions. When processing the query results from the GraphQL retry with the maxBlockHeight filter applied, transactions that are missing from the result should not automatically be marked as failed since they may have been mined after maxBlockHeight. Instead of initializing all transactions with -1, only update the map with transaction confirmations that are actually returned from the query results, and leave transactions that aren't found in the bounded query unset so they can be properly distinguished from globally confirmed missing transactions.
1242-1277: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid fanning out every 2-ID status chunk concurrently.
For a 5000-ID page,
chunkSize = 2creates up to 2500 simultaneous GraphQL requests. That can overload the gateway/client and make the 5-second caller timeout much more likely.Proposed fix
- final confirmationFutures = <Future<void>>[]; - for (var i = 0; i < transactionIds.length; i += chunkSize) { - confirmationFutures.add(() async { - final chunkEnd = (i + chunkSize < transactionIds.length) - ? i + chunkSize - : transactionIds.length; + final chunkEnd = (i + chunkSize < transactionIds.length) + ? i + chunkSize + : transactionIds.length; + final chunk = transactionIds + .sublist(i, chunkEnd) + .whereType<String>() + .toList(growable: false); + + if (chunk.isEmpty) { + continue; + } - final query = await graphQLRetry.execute( - TransactionStatusesQuery( - variables: TransactionStatusesArguments( - transactionIds: - transactionIds.sublist(i, chunkEnd) as List<String>?, - maxBlockHeight: maxBlockHeight, - ), + final query = await graphQLRetry.execute( + TransactionStatusesQuery( + variables: TransactionStatusesArguments( + transactionIds: chunk, + maxBlockHeight: maxBlockHeight, ), - ); + ), + ); - final currentBlockHeight = query.data!.blocks.edges.first.node.height; + final currentBlockHeight = query.data!.blocks.edges.first.node.height; - for (final transaction - in query.data!.transactions.edges.map((e) => e.node)) { - if (transaction.block == null) { - transactionConfirmations[transaction.id] = 0; - continue; - } + for (final transaction + in query.data!.transactions.edges.map((e) => e.node)) { + if (transaction.block == null) { + transactionConfirmations[transaction.id] = 0; + continue; + } - transactionConfirmations[transaction.id] = - currentBlockHeight - transaction.block!.height + 1; - } - }()); - } - - try { - await Future.wait(confirmationFutures); - } catch (e) { - logger.e('Error getting transactions confirmations on exception', e); - rethrow; + transactionConfirmations[transaction.id] = + currentBlockHeight - transaction.block!.height + 1; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/arweave/arweave_service.dart` around lines 1242 - 1277, The code currently creates a Future for every 2-ID chunk and adds all of them to the confirmationFutures list without any concurrency control, resulting in up to 2500 simultaneous GraphQL requests for a 5000-ID page. Implement concurrency limiting by batching the futures instead of adding all at once—for example, create a smaller batch of futures (e.g., 10-50 concurrent requests), await them to completion, then process the next batch. This can be done by modifying the loop structure to either await futures in groups using Future.wait with a limit, or by adding futures conditionally and awaiting them when a concurrent limit is reached, keeping the graphQLRetry.execute calls within the TransactionStatusesQuery execution under control.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/sync/domain/repositories/sync_repository.dart`:
- Around line 1576-1579: The minBlockHeight calculated from _estimateMinBlock
can exceed the maxBlockHeight when dateCreated is in the future, resulting in an
invalid bounded query range. After calculating minBlockHeight using
_estimateMinBlock with earliestDate, clamp the value to ensure it does not
exceed the maxBlockHeight to keep the range valid before passing it to the
GraphQL query.
In `@lib/turbo/topup/views/crypto_topup/crypto_topup_modal.dart`:
- Around line 303-325: The icon and message block containing the
timer_off_outlined icon and text messages are currently left-aligned because
they reside in a Column with crossAxisAlignment set to CrossAxisAlignment.start,
despite the Text widgets using TextAlign.center. To fix this, apply a centering
wrapper (such as a Center widget) around the icon and message block to ensure
both the icon and text are horizontally centered. Apply the same centering
pattern to the corresponding icon/message block in the _ConcurrentSessionView
widget as well.
---
Outside diff comments:
In `@lib/services/arweave/arweave_service.dart`:
- Around line 1238-1260: The issue is that the transactionConfirmations map is
prefilled with -1 for all transactionIds, which causes bounded misses
(transactions not found within the maxBlockHeight range) to be treated as
globally missing transactions. When processing the query results from the
GraphQL retry with the maxBlockHeight filter applied, transactions that are
missing from the result should not automatically be marked as failed since they
may have been mined after maxBlockHeight. Instead of initializing all
transactions with -1, only update the map with transaction confirmations that
are actually returned from the query results, and leave transactions that aren't
found in the bounded query unset so they can be properly distinguished from
globally confirmed missing transactions.
- Around line 1242-1277: The code currently creates a Future for every 2-ID
chunk and adds all of them to the confirmationFutures list without any
concurrency control, resulting in up to 2500 simultaneous GraphQL requests for a
5000-ID page. Implement concurrency limiting by batching the futures instead of
adding all at once—for example, create a smaller batch of futures (e.g., 10-50
concurrent requests), await them to completion, then process the next batch.
This can be done by modifying the loop structure to either await futures in
groups using Future.wait with a limit, or by adding futures conditionally and
awaiting them when a concurrent limit is reached, keeping the
graphQLRetry.execute calls within the TransactionStatusesQuery execution under
control.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cffc497-a420-4037-8787-4e69a2e9ac0b
📒 Files selected for processing (13)
lib/services/arweave/arweave_service.dartlib/services/arweave/graphql/queries/LicenseAssertions.graphqllib/services/arweave/graphql/queries/LicenseDataBundled.graphqllib/services/arweave/graphql/queries/TransactionStatuses.graphqllib/sync/domain/repositories/sync_repository.dartlib/turbo/topup/components/payment_method_selector.dartlib/turbo/topup/models/crypto_token.dartlib/turbo/topup/views/crypto_topup/crypto_topup_modal.dartlib/turbo/topup/views/crypto_topup/token_selection_view.dartlib/turbo/topup/views/unified/crypto_confirmation_view.dartlib/turbo/topup/views/unified/crypto_processing_view.dartlib/turbo/topup/views/unified/inline_crypto_payment.dartlib/turbo/topup/views/unified/wallet_connection_view.dart
💤 Files with no reviewable changes (3)
- lib/turbo/topup/models/crypto_token.dart
- lib/turbo/topup/views/crypto_topup/token_selection_view.dart
- lib/turbo/topup/views/unified/inline_crypto_payment.dart
Comment on lines
+303
to
+325
| // Icon and message | ||
| Icon( | ||
| Icons.timer_off_outlined, | ||
| size: 64, | ||
| color: colors.themeWarningFg, | ||
| ), | ||
| const SizedBox(height: 24), | ||
| Text( | ||
| 'Your session has expired due to inactivity.', | ||
| style: typography.paragraphLarge( | ||
| color: colors.themeFgDefault, | ||
| ), | ||
| textAlign: TextAlign.center, | ||
| ), | ||
| const SizedBox(height: 8), | ||
| Text( | ||
| 'Please start a new payment to continue.', | ||
| style: typography.paragraphNormal( | ||
| color: colors.themeFgMuted, | ||
| ), | ||
| textAlign: TextAlign.center, | ||
| ), | ||
| const SizedBox(height: 24), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Center alignment regressed for status icon/message blocks.
After removing Center/Expanded, these widgets now sit in Column(crossAxisAlignment: CrossAxisAlignment.start), so the icon and message block render left-aligned even though text uses TextAlign.center.
Suggested fix
- Icon(
- Icons.timer_off_outlined,
- size: 64,
- color: colors.themeWarningFg,
- ),
- const SizedBox(height: 24),
- Text(
- 'Your session has expired due to inactivity.',
- ...
- textAlign: TextAlign.center,
- ),
- const SizedBox(height: 8),
- Text(
- 'Please start a new payment to continue.',
- ...
- textAlign: TextAlign.center,
- ),
+ Center(
+ child: Column(
+ children: [
+ Icon(
+ Icons.timer_off_outlined,
+ size: 64,
+ color: colors.themeWarningFg,
+ ),
+ const SizedBox(height: 24),
+ Text(
+ 'Your session has expired due to inactivity.',
+ ...
+ textAlign: TextAlign.center,
+ ),
+ const SizedBox(height: 8),
+ Text(
+ 'Please start a new payment to continue.',
+ ...
+ textAlign: TextAlign.center,
+ ),
+ ],
+ ),
+ ),Apply the same centering pattern to the _ConcurrentSessionView icon/message block.
Also applies to: 393-415
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/turbo/topup/views/crypto_topup/crypto_topup_modal.dart` around lines 303
- 325, The icon and message block containing the timer_off_outlined icon and
text messages are currently left-aligned because they reside in a Column with
crossAxisAlignment set to CrossAxisAlignment.start, despite the Text widgets
using TextAlign.center. To fix this, apply a centering wrapper (such as a Center
widget) around the icon and message block to ensure both the icon and text are
horizontally centered. Apply the same centering pattern to the corresponding
icon/message block in the _ConcurrentSessionView widget as well.
Pending transactions older than 1 day are now marked failed locally before any GraphQL queries fire. This prevents zombie pending txs from firing TransactionStatuses queries every sync — especially when the queries time out and the txs never get their status updated. Also adds early return when there are no pending transactions to avoid unnecessary work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverted two approaches that had data integrity risks: 1. Block height estimation from FileRevision.dateCreated — this is ArFS metadata (client-set), not an Arweave block timestamp. License assertion txs can be at different blocks than the file revision. An overestimated minBlockHeight would silently miss licenses. 2. Marking stale pending txs as failed without checking the chain — a successfully mined tx that took >1 day would be incorrectly marked as failed. Safe changes retained: - maxBlockHeight filter on all ID-based queries (accurate, from getCurrentBlockHeight) - Fixed chunk size of 2 IDs per query (tested safe on turbo-gateway) - Early return when no pending transactions exist - block filter params remain in .graphql schemas for future use when a reliable minBlockHeight source is available Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Visit the preview URL for this PR (updated for commit bcb65b0): https://ardrive-web--pr2139-pe-9103-graphql-bloc-42hfwiag.web.app (expires Tue, 30 Jun 2026 04:26:04 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
block: { min, max }filter parameters to avoid ClickHouse TOO_MANY_ROWS errors on turbo-gateway.comblock: { max }filter (no min, since these are pending txs)minBlockHeightfrom the earliestFileRevision.dateCreatedusing Arweave's ~2min block time, with a 5000-block safety marginProblem
turbo-gateway.com uses ClickHouse which scans ~2.4 rows per block per tx ID. Without block filters, queries with 3+ IDs exceed the 10M row limit:
With block range filters:
Impact
License sync runs in the background (fire-and-forget) so query count doesn't block sync completion.
Test plan
flutter analyze— no issues🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Style