feat(stats): account abstraction wallets charts #1201
Conversation
WalkthroughThis pull request introduces comprehensive metrics for tracking Account Abstraction (AA) wallets in the Blockscout statistics system. The changes span multiple configuration and source files to implement four key metrics: new AA wallets, AA wallets growth, total AA wallets, and active AA wallets. The implementation involves creating new JSON configuration entries, updating runtime setup, adding chart source implementations, and extending test coverage. The modifications enable granular tracking of AA wallet metrics across daily, weekly, monthly, and yearly resolutions, providing a detailed view of account abstraction wallet adoption and activity. Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
bragov4ik
requested review from
lok52,
rimrakhimov,
k1rill-fedoseev and
sevenzing
bragov4ik
force-pushed
the
bragov4ik/stats-aa-charts
branch
from
9cbe3b4 to
1e101e5
Compare
bragov4ik
force-pushed
the
bragov4ik/stats-aa-charts
branch
from
1e101e5 to
3fcea7c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (1)
166-185: Consider adding documentation for type aliasesAdding documentation comments for the type aliases (e.g.,
NewAccountAbstractionWallets,NewAccountAbstractionWalletsWeekly, etc.) would enhance code readability and maintainability.You can add Rust doc comments to explain the purpose and usage of each alias.
stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs (1)
72-87: Consider enhancing test coverage.While the test structure is good, consider:
- Adding more diverse test data (e.g., multiple wallets per day, days with no activity)
- Adding edge cases (e.g., invalid dates, null values)
async fn update_active_account_abstraction_wallets() { simple_test_chart::<ActiveAccountAbstractionWallets>( "update_active_account_abstraction_wallets", vec![ ("2022-11-09", "1"), ("2022-11-10", "1"), ("2022-11-11", "1"), ("2022-11-12", "1"), ("2022-12-01", "1"), ("2023-02-01", "1"), + // Multiple wallets per day + ("2023-03-01", "3"), + // Gap in activity + ("2023-03-15", "2"), + // Recent date + ("2024-01-01", "5"), ], ) .await; }stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (1)
68-112: Consider enhancing test coverage.While the tests cover all time resolutions, consider adding more test cases to verify:
- Edge cases (e.g., date transitions)
- Different growth patterns
- Boundary conditions
Example test cases to add:
// Test date transition vec![ ("2022-12-31", "10"), ("2023-01-01", "15"), ], // Test rapid growth vec![ ("2022-11-09", "1"), ("2022-11-10", "100"), ("2022-11-11", "1000"), ], // Test zero growth vec![ ("2022-11-09", "10"), ("2022-11-10", "10"), ],stats/config/charts.json (2)
72-75: Consider adding the "enabled" flag for consistency.The counter definition looks good, but consider adding the "enabled" flag to match other counters (e.g., total_operational_txns, total_native_coin_holders).
"total_account_abstraction_wallets": { + "enabled": true, "title": "Total AA wallets", "description": "Number of account abstraction wallets (ERC-4337) that sent at least 1 user operation" }
178-189: Consider adding the "enabled" flag to new chart definitions.The line chart definitions are well-structured, but consider adding the "enabled" flag to maintain consistency with other charts (e.g., active_recurring_accounts_60_days).
"new_account_abstraction_wallets": { + "enabled": true, "title": "New AA wallets", "description": "Number of newly added account abstraction wallets (ERC-4337)" }, "account_abstraction_wallets_growth": { + "enabled": true, "title": "Number of AA wallets", "description": "Cumulative account abstraction wallets (ERC-4337) growth over time" }, "active_account_abstraction_wallets": { + "enabled": true, "title": "Active AA wallets", "description": "Active account abstraction wallets (ERC-4337) number per period" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
stats/config/charts.json(2 hunks)stats/config/layout.json(2 hunks)stats/config/update_groups.json(1 hunks)stats/stats-server/src/runtime_setup.rs(2 hunks)stats/stats-server/tests/it/chart_endpoints/counters.rs(1 hunks)stats/stats-server/tests/it/chart_endpoints/lines.rs(1 hunks)stats/stats/src/charts/counters/mod.rs(2 hunks)stats/stats/src/charts/counters/total_aa_wallets.rs(1 hunks)stats/stats/src/charts/lines/mod.rs(1 hunks)stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs(1 hunks)stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs(1 hunks)stats/stats/src/charts/lines/user_ops/mod.rs(1 hunks)stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs(1 hunks)stats/stats/src/update_groups.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (20)
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (3)
43-102: Implementation ofNewAccountAbstractionWalletsStatementlooks goodThe SQL statement is correctly constructed to retrieve new account abstraction wallets, ensuring accurate data collection and filtering based on the specified date range.
105-131:NewAccountAbstractionWalletsQueryBehaviouris properly implementedThe remote query behavior handles data retrieval efficiently, ensuring that results are sorted and trimmed according to the specified range.
187-231: Enhance test coverage and clarityThe test module includes tests for updating new account abstraction wallets across different resolutions. Ensure that these tests cover edge cases and validate the correctness of the data aggregation logic.
Do you want assistance in identifying additional test cases or improving test coverage?
stats/stats/src/charts/counters/total_aa_wallets.rs (2)
13-30:Propertiesstruct and implementations look solidThe
Propertiesstruct correctly implementsNamedandChartProperties, defining the chart characteristics such as resolution, chart type, and missing date policy.
32-34: Definition ofTotalAccountAbstractionWalletsis appropriateThe type alias appropriately sets up the chart source using the last point of the account abstraction wallets growth data.
stats/stats-server/tests/it/chart_endpoints/counters.rs (1)
29-29:totalAccountAbstractionWalletsadded to expected countersThe new counter
totalAccountAbstractionWalletsis properly included in the list of expected counters in the test, ensuring that the API returns the correct set of counters.stats/stats/src/charts/counters/mod.rs (1)
8-8: LGTM! Module and export follow established patterns.The new module
total_aa_walletsand its exportTotalAccountAbstractionWalletsare well-integrated and follow the existing module structure and naming conventions.Also applies to: 33-33
stats/stats/src/charts/lines/user_ops/mod.rs (2)
1-3: Great documentation! Clear explanation of the module's purpose.The documentation comments effectively explain the context of user operations and their relation to ERC 4337 (Account Abstraction).
4-8: LGTM! Well-organized module structure.The new AA wallet-related modules are logically organized alongside existing user operation modules.
stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs (1)
27-36: LGTM! Clean implementation of statement generation.The
ActiveAccountAbstractionWalletsStatementimplementation correctly uses the common counting function with appropriate filtering by sender.stats/stats-server/tests/it/chart_endpoints/lines.rs (1)
61-63: LGTM! Comprehensive test coverage for new metrics.The new line names are well-integrated into the test suite, ensuring API endpoint testing for all new AA wallet metrics.
stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (3)
1-18: Well-organized imports!The imports are logically grouped and follow Rust conventions. All necessary dependencies for chart functionality are properly included.
20-37: Properties implementation follows established patterns.The implementation correctly:
- Uses camelCase naming convention for chart name
- Sets appropriate chart type as Line
- Uses FillPrevious policy which is suitable for growth metrics
39-66: Type definitions are well-structured.The implementation properly defines chart sources for different time resolutions with appropriate batch sizes and data aggregation strategies.
stats/stats/src/charts/lines/mod.rs (1)
22-25: Exports are well-organized.The new AA wallet-related exports are properly grouped and follow the module's organization pattern.
Also applies to: 29-29, 33-36
stats/stats/src/update_groups.rs (1)
21-24: Update groups are properly structured.The new AA wallet-related groups are correctly integrated:
- ActiveAccountAbstractionWallets is added to singleton groups
- NewAccountAbstractionWalletsGroup includes all necessary charts for tracking AA wallets
Also applies to: 167-179
stats/stats-server/src/runtime_setup.rs (1)
306-306: Runtime setup properly integrates new groups.The AA wallet-related update groups are correctly added to the runtime setup in a logical order.
Also applies to: 342-342
stats/config/update_groups.json (1)
40-40: LGTM! Schedule configuration looks good.The new schedules for AA wallet metrics are well-positioned and properly staggered with existing user operation related tasks.
Also applies to: 43-43
stats/config/layout.json (2)
17-17: LGTM! Counter placement is appropriate.The total_account_abstraction_wallets counter is well-positioned after total_user_ops, maintaining logical grouping.
95-97: LGTM! Chart ordering follows best practices.The new AA wallet charts follow a logical progression (new → growth → active) and are properly grouped with other user operation metrics.
bragov4ik
force-pushed
the
bragov4ik/stats-aa-charts
branch
from
3fcea7c to
ad5e3fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (1)
68-111: Comprehensive test coverage with proper database handling.The test suite:
- Covers all time resolutions
- Properly marks database dependency with #[ignore]
- Uses consistent test data structure
However, consider adding more test cases with different dates and values to ensure robust handling of edge cases.
stats/config/charts.json (1)
178-189: Well-structured line chart configurations.The line charts for AA wallets:
- Follow consistent naming patterns
- Provide clear descriptions
- Cover all necessary metrics (new, growth, active)
Consider adding units field for consistency with other charts that have measurable values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
stats/config/charts.json(2 hunks)stats/config/layout.json(2 hunks)stats/config/update_groups.json(1 hunks)stats/stats-server/src/runtime_setup.rs(2 hunks)stats/stats-server/tests/it/chart_endpoints/counters.rs(1 hunks)stats/stats-server/tests/it/chart_endpoints/lines.rs(1 hunks)stats/stats/src/charts/counters/mod.rs(2 hunks)stats/stats/src/charts/counters/total_aa_wallets.rs(1 hunks)stats/stats/src/charts/lines/mod.rs(1 hunks)stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs(1 hunks)stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs(1 hunks)stats/stats/src/charts/lines/user_ops/mod.rs(1 hunks)stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs(1 hunks)stats/stats/src/update_groups.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- stats/stats-server/tests/it/chart_endpoints/counters.rs
- stats/stats/src/charts/counters/mod.rs
- stats/stats-server/tests/it/chart_endpoints/lines.rs
- stats/stats/src/update_groups.rs
- stats/stats-server/src/runtime_setup.rs
- stats/config/update_groups.json
- stats/config/layout.json
- stats/stats/src/charts/lines/mod.rs
- stats/stats/src/charts/counters/total_aa_wallets.rs
- stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs
- stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Linting / lint
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (5)
stats/stats/src/charts/lines/user_ops/mod.rs (2)
1-2: Documentation clearly explains the module's purpose.Good job on providing context about account abstraction and ERC 4337 in the documentation.
4-8: Well-organized module structure for AA wallet metrics.The module organization logically separates different aspects of AA wallet tracking:
- Growth metrics
- Active wallets tracking
- New wallets monitoring
stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (2)
20-37: LGTM! Properties implementation follows best practices.The implementation:
- Uses descriptive naming for the chart
- Correctly specifies line chart type
- Appropriately handles missing dates with FillPrevious policy
48-66: Well-structured type aliases for different time resolutions.Good job on:
- Using appropriate source types for different resolutions
- Implementing proper batching parameters
- Maintaining consistency across weekly, monthly, and yearly resolutions
stats/config/charts.json (1)
72-75: LGTM! Clear and consistent counter configuration.The total AA wallets counter is well-defined with:
- Clear title using consistent abbreviation (AA)
- Descriptive explanation referencing ERC-4337
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
stats/stats/src/charts/counters/pending_txns.rs (1)
Line range hint
1-102: Consider architectural implications for AA wallet metrics.Given that this PR introduces new AA wallet metrics and the author mentioned code duplication concerns, consider extracting common patterns from this counter implementation that could be reused for the new AA wallet counters.
The current implementation of
PendingTxns30mdemonstrates good practices that could be applied to AA wallet metrics:
- Clear separation of database query logic (
PendingTxnsStatement)- Flexible time range handling
- Strong typing with proper error handling
- Configurable properties through traits
Consider creating shared abstractions to reduce the mentioned code duplication in the AA wallet implementations.
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (1)
173-211: Consider enhancing test coverage.While the tests cover all time resolutions, consider adding:
- Multiple data points per test to verify aggregation
- Edge cases (e.g., empty ranges, boundary dates)
Example enhancement for the daily test:
simple_test_chart::<NewAccountAbstractionWallets>( "update_new_account_abstraction_wallets", - vec![("2022-11-09", "1")], + vec![ + ("2022-11-09", "1"), + ("2022-11-10", "2"), + ("2022-11-11", "0"), // Edge case: day with no new wallets + ], )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
stats/stats/src/charts/counters/pending_txns.rs(1 hunks)stats/stats/src/charts/counters/total_txns.rs(1 hunks)stats/stats/src/charts/lines/accounts/new_accounts.rs(1 hunks)stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- stats/stats/src/charts/lines/accounts/new_accounts.rs
- stats/stats/src/charts/counters/total_txns.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (7)
stats/stats/src/charts/counters/pending_txns.rs (2)
101-101: LGTM! Test parameters preserved.The test parameters remain unchanged, maintaining the same test coverage for the basic functionality.
96-96: Verify the test helper function change.The change from
simple_test_counter_with_migration_variantstosimple_test_countermight affect test coverage of migration scenarios.Let's verify the test helper function changes:
✅ Verification successful
The test helper change is appropriate and safe.
The codebase maintains robust migration testing coverage through various specialized helpers. The simpler
simple_test_counteris sufficient for pending transaction tests, while migration-specific testing is preserved where needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if migration-related test coverage is maintained elsewhere # Search for any remaining usage of the old test helper rg "simple_test_counter_with_migration_variants" -A 5 # Search for alternative migration testing rg "migration.*test" --type rustLength of output: 7042
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (5)
1-35: LGTM! Well-organized imports and clear documentation.The file structure is clean with logically grouped imports and clear documentation explaining its purpose.
43-48: LGTM! Smart approach to range filtering.The implementation correctly uses
min_timestampto consider all transactions from the beginning, ensuring accurate identification of new wallets by checking their first activity.
73-101: LGTM! Robust query behavior implementation.The implementation properly handles:
- Data querying with error handling
- Sorting of results
- Range trimming
111-155: LGTM! Well-structured type definitions following established patterns.The implementation maintains consistency with the
NewAccountspattern, providing clear type definitions for different time resolutions.
50-70: Consider adding an index for better query performance.The query uses
DISTINCT ONwith ordering bysenderandtimestamp. Adding a composite index on(sender, timestamp)could improve query performance.Let's check if the index already exists:
lots of copypasting; I was quite careful but hopefully didn't miss smth 🙈
closes #1183
Summary by CodeRabbit
Release Notes
New Features
Metrics Added
Scheduled Updates
Performance