feat(stats): user ops indexing status #1240
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request updates a dependency revision and overhauls how indexing statuses are managed throughout the codebase. The former single-indexing status enum has been replaced by a structured status object that distinguishes between blockscout indexing and user operations indexing. Several methods have been renamed, and new asynchronous functions have been added to check and process these statuses. The settings were updated to include additional toggles for user operations, and related tests and mocks have been revised to accommodate the new structure. A new module, along with a trait and dedicated enums for indexing statuses, was introduced, and corresponding import statements and re-exports were adjusted to maintain consistency. Changes in error handling and control flow, such as an indefinite sleep when checks are disabled, are also part of the update. 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 (
|
|
todo: change tag for blockscout-client to main |
![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/indexing_status.rs (1)
1-19: Minor spelling typo in method signature.
In fn most_restrictive_from(), the parameter is spelled “requrements” instead of “requirements.” Consider correcting it for clarity.- fn most_restrictive_from(requrements: impl Iterator<Item = Self> + Clone) -> Self; + fn most_restrictive_from(requirements: impl Iterator<Item = Self> + Clone) -> Self;stats/stats-server/src/blockscout_waiter.rs (3)
116-166: Consider aligning error tolerance for user_ops checks.
Unlike check_blockscout_status, the user_ops method does not track consecutive errors. If user_ops are mission-critical, consider adopting a similar retry or fallback mechanism to avoid indefinite partial failures.
168-171: Indefinite sleep is appropriate but might need caution in production.
For operational robustness, ensure this choice is well-documented. Stopping checks entirely is reasonable here, but keep watch for potential unintended stalls in the future.
326-345: Mocking logic is DRY and flexible.
Providing distinct responses for blockscout and optional user_ops helps test aggregator functionality thoroughly. Consider refactoring if you add more test endpoints, but for now, it looks good.stats/stats/src/charts/counters/yesterday_operational_txns.rs (1)
82-83: Consider adding unit tests that don't require database.While the integration test is appropriately marked as ignored, consider adding unit tests that can run without database dependencies to improve test coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
stats/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
stats/Cargo.toml(1 hunks)stats/stats-server/src/blockscout_waiter.rs(21 hunks)stats/stats-server/src/settings.rs(5 hunks)stats/stats/src/charts/chart.rs(3 hunks)stats/stats/src/charts/counters/average_block_time.rs(2 hunks)stats/stats/src/charts/counters/new_contracts_24h.rs(2 hunks)stats/stats/src/charts/counters/new_operational_txns_24h.rs(2 hunks)stats/stats/src/charts/counters/new_verified_contracts_24h.rs(2 hunks)stats/stats/src/charts/counters/pending_txns.rs(2 hunks)stats/stats/src/charts/counters/total_addresses.rs(2 hunks)stats/stats/src/charts/counters/total_blocks.rs(2 hunks)stats/stats/src/charts/counters/total_contracts.rs(2 hunks)stats/stats/src/charts/counters/total_operational_txns.rs(2 hunks)stats/stats/src/charts/counters/total_tokens.rs(2 hunks)stats/stats/src/charts/counters/total_txns.rs(2 hunks)stats/stats/src/charts/counters/total_verified_contracts.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/average_txn_fee_24h.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/new_txns_24h.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/txns_fee_24h.rs(2 hunks)stats/stats/src/charts/counters/yesterday_operational_txns.rs(2 hunks)stats/stats/src/charts/counters/yesterday_txns.rs(2 hunks)stats/stats/src/charts/indexing_status.rs(1 hunks)stats/stats/src/charts/lines/contracts/new_contracts.rs(2 hunks)stats/stats/src/charts/lines/transactions/new_operational_txns_window.rs(2 hunks)stats/stats/src/charts/lines/transactions/new_txns_window.rs(2 hunks)stats/stats/src/charts/mod.rs(1 hunks)stats/stats/src/data_source/source.rs(1 hunks)stats/stats/src/lib.rs(1 hunks)stats/stats/src/update_group.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (61)
stats/stats/src/charts/counters/txns_stats_24h/average_txn_fee_24h.rs (2)
8-8: Imports look consistent with new indexing status structure.
No issues found with adding BlockscoutIndexingStatus, UserOpsIndexingStatus, and IndexingStatusTrait to the import list.
50-53: Double-check “NoneIndexed” for blockscout in average fee calculations.
Currently, this chart’s indexing_status_requirement() returns BlockscoutIndexingStatus::NoneIndexed, which may be too permissive to reliably compute an average transaction fee. Typically, block-level data is needed to calculate transaction fees, so consider switching to BlocksIndexed or a more restrictive status if required by logic.stats/stats/src/charts/indexing_status.rs (4)
21-67: Structured approach for multiple indexing statuses looks good.
The IndexingStatus struct cleanly composes blockscout and user_ops components, and is_requirement_satisfied is clearly implemented.
69-91: BlockscoutIndexingStatus ordering is intuitive.
NoneIndexed < BlocksIndexed < InternalTransactionsIndexed aligns well with typical indexing progress.
93-113: UserOpsIndexingStatus approach is consistent.
IndexingPastOperations < PastOperationsIndexed provides a clear progression for user-ops data.
115-206: Extensive test coverage is appreciated.
Tests demonstrate correct behavior for combining partial statuses. The empty iterator edge case is also tested.stats/stats/src/charts/chart.rs (3)
17-17: Revised import usage aligns with new IndexingStatus design.
No issues with referencing BlockscoutIndexingStatus and UserOpsIndexingStatus here.
151-153: Macro imports appear consistent.
Bringing in MissingDatePolicy, ResolutionKind, and ChartKey is aligned with the refactor.
183-188: Default indexing requirement now sets blockscout = BlocksIndexed.
This makes sense for most charts that need block data. However, note that other charts (e.g., average_txn_fee_24h) have a less restrictive requirement, which may be intentional or an oversight. Verify consistency across the codebase.stats/stats-server/src/blockscout_waiter.rs (9)
7-9: New indexing status imports look good.
Bringing in BlockscoutIndexingStatus, UserOpsIndexingStatus, and IndexingStatusTrait provides the granularity needed for more robust status tracking.
29-55: Clear separation for Blockscout indexing logic.
Renaming the function to “blockscout_internal_status_from_api_status” and returning a specialized BlockscoutIndexingStatus clarifies the domain logic. The threshold checks nicely handle partial vs. fully indexed states.
57-75: UserOps indexing function handles null gracefully.
The additional logging for null finished_past_indexing and the fallback to false is appropriate. The new user_ops states offer better insight into partial vs. full indexing.
77-114: Consecutive error handling is consistent and robust.
Incrementing consecutive_errors and returning an error only after exceeding RETRIES helps avoid failing too quickly. The logic to update and log the new status, or log “unchanged,” is clean.
173-196: Run loop is clear; watchers update is straightforward.
The code properly checks each status if enabled and avoids CPU burn by sleeping. The oversleep multiplier for IndexingStatus::MAX is a neat way to reduce overhead once fully indexed.
259-286: Initialization logic for partial or full coverage is well-structured.
Initiating blockscout and user_ops statuses according to the toggles is intuitive. The watch channel’s default values align with whether checks are enabled.
322-323: New ToggleableCheck import is consistent.
Ensuring ToggleableCheck is accessible throughout the file helps unify config-driven thresholds and enabling logic.
351-559: test_aggregator enhancements: correct param usage.
Allowing an Option for user_ops ensures better coverage in tests. The approach of verifying aggregator behavior via wait_until_status_at_least is well-structured.
566-737: Extensive test coverage validations.
These tests thoroughly check combinations of partial indexing, timeouts, retries, and subordinated checks. The logs and trial outcomes are well covered. This should maintain confidence that real-world indexing states are properly recognized.stats/stats/src/charts/mod.rs (1)
4-10: Introducing indexing_status as a separate module is a good design.
It cleanly segments indexing status functionality from the other chart components. Removing IndexingStatus from the top-level re-exports aligns with its new dedicated placement.stats/stats/src/lib.rs (1)
24-28: Re-exporting the new indexing_status module.
This centralizes the indexing logic for external callers while preserving naming clarity. The consolidated IndexingStatus re-export is also consistent with the new structured approach.stats/stats/src/charts/counters/total_tokens.rs (1)
9-9: LGTM! Clean implementation of the new indexing status structure.The changes correctly implement the new indexing status structure while maintaining the same level of restriction for blockscout operations and appropriately handling user operations.
Also applies to: 57-62
stats/stats/src/charts/counters/total_verified_contracts.rs (1)
10-10: LGTM! Consistent implementation of the new indexing status structure.The changes maintain consistency with the new indexing status pattern while preserving the original indexing requirements.
Also applies to: 56-61
stats/stats/src/charts/counters/txns_stats_24h/txns_fee_24h.rs (2)
8-8: LGTM! Consistent implementation of the new indexing status structure.The changes correctly implement the new indexing status structure while maintaining the original indexing requirements.
Also applies to: 49-54
67-93: Verify test coverage for the new indexing status structure.While the existing tests cover the functionality, consider adding test cases to verify the behavior with different indexing status combinations.
stats/stats/src/charts/counters/new_verified_contracts_24h.rs (3)
11-11: LGTM! Consistent implementation of the new indexing status structure.The changes maintain consistency with the new indexing status pattern while preserving the original indexing requirements.
Also applies to: 58-63
74-83: Verify test coverage for the new indexing status structure.While the existing test covers the basic functionality, consider adding test cases to verify the behavior with different indexing status combinations.
1-85: Verify consistent implementation across the codebase.Let's verify that all implementations of
ChartPropertieshave been updated to use the newIndexingStatusstructure:✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for any remaining old-style IndexingStatus usage echo "Checking for old IndexingStatus usage..." rg "IndexingStatus::(None|Partial|Full)Indexed" -l # Find all ChartProperties implementations and verify they use the new structure echo "Checking ChartProperties implementations..." ast-grep --pattern 'impl ChartProperties for $_ { $$$ fn indexing_status_requirement() -> IndexingStatus { $$$ } $$$ }'Length of output: 29467
Consistent Use of New IndexingStatus Confirmed
The verification confirms that all ChartProperties implementations—including the one in new_verified_contracts_24h.rs—consistently return an IndexingStatus struct built from its component values (e.g., BlockscoutIndexingStatus::NoneIndexed and UserOpsIndexingStatus::LEAST_RESTRICTIVE). Although the underlying enum variant names (e.g., NoneIndexed) are still present, they are now part of the new IndexingStatus structure, and no legacy usage of an entire old IndexingStatus enum was found.
- All files implementing ChartProperties return an IndexingStatus struct.
- The new structure’s fields are consistently populated across the codebase.
- No outdated IndexingStatus enum patterns were detected beyond the constituent enum variants, which are used as intended.
stats/stats/src/charts/counters/yesterday_operational_txns.rs (2)
13-13: LGTM!The import statement correctly includes the new indexing status types.
63-68: LGTM!The implementation correctly returns a structured
IndexingStatuswith appropriate values for both blockscout and user ops components.stats/stats/src/charts/counters/total_contracts.rs (3)
9-9: LGTM!The import statement correctly includes the new indexing status types.
64-69: LGTM!The implementation correctly returns a structured
IndexingStatuswith appropriate values for both blockscout and user ops components.
72-74: Track issue #845 for future improvements.The TODO comment provides good context with a link to the issue for tracking future improvements.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if issue #845 is still open gh issue view 845Length of output: 108
I'll generate a new script that explicitly queries the issue in the remote repository using the --repo flag.
🏁 Script executed:
#!/bin/bash # Verify the state of issue #845 in the blockscout/blockscout-rs repository using GitHub CLI gh issue view --repo blockscout/blockscout-rs 845 --json state,titleLength of output: 133
Issue #845 is confirmed to be open, so the current TODO comment is valid and provides good context.
- The GitHub CLI query confirms that issue stats: optimism
new_contractsbug #845 ("stats: optimismnew_contractsbug") is still open.- The TODO comment properly tracks the issue for future improvements.
stats/stats/src/charts/counters/txns_stats_24h/new_txns_24h.rs (2)
7-7: LGTM!The import statement correctly includes the new indexing status types.
47-52: LGTM!The implementation correctly returns a structured
IndexingStatuswith appropriate values for both blockscout and user ops components.stats/stats/src/charts/counters/new_contracts_24h.rs (3)
11-11: LGTM!The import statement correctly includes the new indexing status types.
60-65: LGTM!The implementation correctly returns a structured
IndexingStatuswith appropriate values for both blockscout and user ops components.
68-71: Performance consideration is well documented.The comment clearly explains the performance trade-off of not including contracts from internal transactions.
stats/stats/src/charts/counters/total_operational_txns.rs (1)
9-9: LGTM! The changes align with the new indexing status structure.The implementation correctly separates blockscout and user operations indexing statuses, maintaining the original logic while providing more granular control.
Also applies to: 38-43
stats/stats/src/charts/counters/new_operational_txns_24h.rs (1)
13-13: LGTM! The changes maintain consistency with the new indexing status structure.The implementation correctly adopts the new structured approach to indexing status, separating blockscout and user operations concerns.
Also applies to: 72-77
stats/stats/src/charts/counters/pending_txns.rs (1)
9-10: LGTM! The changes follow the new indexing status pattern.The implementation correctly adopts the structured approach to indexing status, maintaining consistency with other files.
Also applies to: 87-92
stats/stats/src/charts/counters/total_addresses.rs (1)
11-11: LGTM! The changes maintain consistency with the new indexing status structure.The implementation correctly separates blockscout and user operations indexing statuses, following the pattern established across the codebase.
Also applies to: 61-66
stats/stats/src/charts/counters/yesterday_txns.rs (1)
14-14: LGTM!The import changes and indexing status update align well with the PR objectives.
stats/stats/src/charts/counters/total_txns.rs (1)
13-13: LGTM!The changes to imports and indexing status are well-implemented. The code maintains good error handling and includes a fallback estimation mechanism.
Also applies to: 77-82
stats/stats/src/charts/lines/transactions/new_txns_window.rs (1)
23-23: LGTM!The changes to imports and indexing status are well-implemented. The code includes comprehensive tests and proper window calculations.
Also applies to: 86-91
stats/stats/src/charts/lines/transactions/new_operational_txns_window.rs (1)
22-22: LGTM!The changes to imports and indexing status are well-implemented. The code effectively reuses components and includes comprehensive tests.
Also applies to: 91-96
stats/stats/src/charts/counters/total_blocks.rs (2)
13-13: LGTM!The import statement has been updated to include the new indexing status types, aligning with the broader codebase update.
74-78: LGTM!The
indexing_status_requirementmethod has been updated to return a structuredIndexingStatuswith appropriate values for both blockscout and user operations components.stats/stats/src/charts/counters/average_block_time.rs (2)
12-12: LGTM!The import statement has been updated to include the new indexing status types, aligning with the broader codebase update.
137-140: LGTM!The
indexing_status_requirementmethod has been updated to return a structuredIndexingStatuswith appropriate values for both blockscout and user operations components.stats/stats/src/charts/lines/contracts/new_contracts.rs (2)
22-22: LGTM!The import statement has been updated to include the new indexing status types, aligning with the broader codebase update.
140-143: LGTM!The
indexing_status_requirementmethod has been updated to return a structuredIndexingStatuswith appropriate values for both blockscout and user operations components.stats/stats-server/src/settings.rs (6)
12-12: LGTM!The import statement has been updated to include
BlockscoutIndexingStatus, aligning with the broader codebase update.
199-199: LGTM!Added new field
user_ops_past_indexing_finishedto support user operations indexing status checks.
209-209: LGTM!Default implementation initializes
user_ops_past_indexing_finishedwith default settings.
216-221: LGTM!Added helper methods to check the enabled status of blockscout and user operations checks, improving code readability and maintainability.
263-273: LGTM!Added new
ToggleableCheckstruct with a simple enabled flag and default implementation.
113-115: LGTM!Updated the filter condition to use
BlockscoutIndexingStatusinstead ofIndexingStatus, aligning with the new structured approach.stats/stats/src/data_source/source.rs (1)
10-14: LGTM!The import changes align with the broader refactoring of indexing status management, correctly importing the new types from the
indexing_statusmodule.stats/stats/src/update_group.rs (2)
47-48: LGTM!The import changes correctly align with the new indexing status management structure.
284-300: LGTM!The changes to
dependency_indexing_status_requirementcorrectly integrate the new indexing status types:
- Updated return type to use the fully qualified path
- Added the
IndexingStatusTraitimport for accessing trait methods- Updated the method call to use the new trait functionality
stats/Cargo.toml (1)
13-13: Verify the changes in the new blockscout-client revision.The dependency update to revision
d07ed56should be verified to ensure it aligns with the indexing status changes.✅ Verification successful
🌐 Web query:
What changes were introduced in blockscout-rs commit d07ed56?💡 Result:
The commit d07ed56 introduced several key changes to Blockscout's contract processing and error handling systems based on OpenZeppelin's ERC standards implementation: **Core contract error enhancements** - Added explicit custom errors for ERC1155 token operations: - `ERC1155InsufficientBalance` for transfer attempts exceeding balances - `ERC1155InvalidSender`/`InvalidReceiver` for address validation - `ERC1155MissingApprovalForAll` for operator permissions - `ERC1155InvalidArrayLength` for batch operation mismatches[1] **ERC20 improvements** - Implemented strict validation checks in transfers: ```solidity function _transfer(address from, address to, uint256 value) internal { if (from == address(0)) revert ERC20InvalidSender(address(0)); if (to == address(0)) revert ERC20InvalidReceiver(address(0)); _update(from, to, value); } ``` - Added gas-efficient balance update logic using unchecked blocks[1] - Introduced `_approve` variants with conditional event emission flags[1] These changes align with Blockscout's v6.10.0 backend improvements that emphasize stricter validation and optimized contract interaction patterns[2]. The explicit error definitions particularly enhance debugging capabilities when processing complex token operations through the explorer interface[1][2]. Citations: - 1: https://github.com/blockscout/blockscout-rs/issues/1010 - 2: https://www.blog.blockscout.com/blockscout-v6-10-features-and-improvements/ - 3: https://github.com/blockscout/blockscout-rs - 4: https://github.com/blockscout/blockscout-rs/releases
Indexing Consistency Verified for blockscout-client Rev d07ed56
The commit introduces enhanced contract error handling and stricter validations (such as explicit custom errors for ERC1155 operations and improved ERC20 transfer validations) that support the needed robustness for indexing status changes. These improvements, as part of Blockscout's v6.10.0 backend updates, align with the indexing enhancements we expect.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
stats/stats/src/charts/indexing_status.rs (5)
1-19: Add missing documentation for trait methods and constants.The trait is well-designed, but could benefit from additional documentation:
- Add doc comments for
LEAST_RESTRICTIVEandMOST_RESTRICTIVEconstants- Add doc comments for
is_requirement_satisfiedandmost_restrictive_frommethodspub trait IndexingStatusTrait { // constants for status itself /// Indexing status at the start of blockscout & user ops const MIN: Self; /// Finished indexing everything const MAX: Self; // constants corresponding to status requirement - /// The most relaxed requirement + /// The most relaxed requirement that can be satisfied by any status const LEAST_RESTRICTIVE: Self; - /// The hardest to achieve requirement + /// The most stringent requirement that can only be satisfied by the highest status const MOST_RESTRICTIVE: Self; + /// Checks if the current status satisfies the given requirement + /// Returns true if the current status is greater than or equal to the requirement fn is_requirement_satisfied(&self, requirement: &Self) -> bool; + /// Returns the most restrictive status from the given iterator of requirements + /// If the iterator is empty, returns LEAST_RESTRICTIVE fn most_restrictive_from(requirements: impl Iterator<Item = Self> + Clone) -> Self; }
21-67: Add documentation for the IndexingStatus struct.The struct and its implementation are well-designed, but would benefit from documentation explaining its purpose and the relationship between its fields.
#[derive(Debug, Clone, PartialEq, Eq)] +/// Represents the combined indexing status of both blockscout and user operations +/// Both conditions must be satisfied for the overall status to be considered satisfied pub struct IndexingStatus { + /// The current status of blockscout indexing pub blockscout: BlockscoutIndexingStatus, + /// The current status of user operations indexing pub user_ops: UserOpsIndexingStatus, }
69-94: Add comprehensive documentation for BlockscoutIndexingStatus.The enum is well-structured, but would benefit from more detailed documentation explaining the progression of states.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +/// Represents the different stages of blockscout indexing +/// The variants are ordered from least to most complete: +/// NoneIndexed < BlocksIndexed < InternalTransactionsIndexed pub enum BlockscoutIndexingStatus { + /// Initial state: no data has been indexed NoneIndexed, + /// Basic blocks have been indexed, but internal transactions are not yet processed BlocksIndexed, - /// Implies that blocks are also indexed + /// Both blocks and their internal transactions have been indexed + /// This is the final state of indexing InternalTransactionsIndexed, }
96-119: Add comprehensive documentation for UserOpsIndexingStatus.The enum is well-structured, but would benefit from documentation explaining the progression of states.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +/// Represents the different stages of user operations indexing +/// The variants are ordered from least to most complete: +/// IndexingPastOperations < PastOperationsIndexed pub enum UserOpsIndexingStatus { + /// Currently processing and indexing historical user operations IndexingPastOperations, + /// All historical user operations have been successfully indexed PastOperationsIndexed, }
121-212: Add test coverage for additional functionality.While the current tests are thorough for
most_restrictive_from, consider adding tests for:
is_requirement_satisfiedmethod- Edge cases for both
BlockscoutIndexingStatusandUserOpsIndexingStatusExample test cases to add:
#[test] fn test_requirement_satisfaction() { let status = IndexingStatus { blockscout: BlockscoutIndexingStatus::InternalTransactionsIndexed, user_ops: UserOpsIndexingStatus::PastOperationsIndexed, }; // Test satisfying requirements assert!(status.is_requirement_satisfied(&IndexingStatus { blockscout: BlockscoutIndexingStatus::BlocksIndexed, user_ops: UserOpsIndexingStatus::IndexingPastOperations, })); // Test not satisfying requirements assert!(!IndexingStatus { blockscout: BlockscoutIndexingStatus::BlocksIndexed, user_ops: UserOpsIndexingStatus::IndexingPastOperations, }.is_requirement_satisfied(&status)); } #[test] fn test_blockscout_status_ordering() { assert!(BlockscoutIndexingStatus::InternalTransactionsIndexed > BlockscoutIndexingStatus::BlocksIndexed); assert!(BlockscoutIndexingStatus::BlocksIndexed > BlockscoutIndexingStatus::NoneIndexed); } #[test] fn test_user_ops_status_ordering() { assert!(UserOpsIndexingStatus::PastOperationsIndexed > UserOpsIndexingStatus::IndexingPastOperations); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stats/stats/src/charts/indexing_status.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
|
also add requirement for user ops charts |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (1)
53-66: 💡 Verification agent🧩 Analysis chain
Consider optimizing the SQL query for better performance and reliability.
The SQL query could benefit from the following improvements:
- Add an index on
(sender, timestamp)to optimize the DISTINCT ON operation- Document why
to_timestamp(0)records are excluded- Handle potential NULL values in the consensus column
Consider creating an index:
CREATE INDEX IF NOT EXISTS idx_user_operations_sender_timestamp ON user_operations (sender, blocks.timestamp);Let's verify if the index already exists:
🏁 Script executed:
#!/bin/bash # Check if the suggested index exists rg -A 5 'CREATE.*INDEX.*user_operations.*sender'Length of output: 1426
SQL Query Optimization: Missing Recommended Index and Documentation Improvements
Based on the grep results, we found an existing index—
user_operations_sender_factory_indexon(sender, factory)—but there is no index on(sender, timestamp)(or its equivalent) to optimize the DISTINCT ON operation in the query. Additionally, the query’s exclusion of records withblocks.timestamp = to_timestamp(0)is not documented, and potential NULL values in theblocks.consensuscolumn aren’t explicitly handled.
- Index Issue: The suggested index on
(sender, blocks.timestamp)is not present. Instead, an index on(sender, factory)exists, which may not adequately optimize the DISTINCT ON ordering byblocks.timestamp.- Documentation & NULL Handling: Consider adding documentation for the exclusion of
to_timestamp(0)values and review how NULL consensus values should be treated.
🧹 Nitpick comments (3)
stats/stats-server/src/blockscout_waiter.rs (3)
29-75: Add more detailed logging for status transitions.Consider adding more detailed logging in both status conversion methods to help with debugging and monitoring.
fn blockscout_internal_status_from_api_status( api_status: blockscout_client::models::IndexingStatus, wait_config: &StartConditionSettings, ) -> anyhow::Result<BlockscoutIndexingStatus> { + tracing::debug!( + blocks_ratio = ?api_status.indexed_blocks_ratio, + internal_transactions_ratio = ?api_status.indexed_internal_transactions_ratio, + "Converting blockscout API status" + ); let blocks_passed = is_threshold_passed( &wait_config.blocks_ratio, api_status.indexed_blocks_ratio.clone(), "indexed_blocks_ratio", ) .context("checking indexed block ratio")?; let status = if blocks_passed { let internal_transactions_passed = is_threshold_passed( &wait_config.internal_transactions_ratio, api_status.indexed_internal_transactions_ratio.clone(), "indexed_internal_transactions_ratio", ) .context("checking indexed internal transactions ratio")?; if internal_transactions_passed { BlockscoutIndexingStatus::InternalTransactionsIndexed } else { BlockscoutIndexingStatus::BlocksIndexed } } else { BlockscoutIndexingStatus::NoneIndexed }; + tracing::debug!(status = ?status, "Converted blockscout API status"); Ok(status) }
77-166: Align error handling between status check methods.The error handling approaches differ between
check_blockscout_statusandcheck_user_ops_status. Consider applying the same retry mechanism tocheck_user_ops_statusfor consistency.async fn check_user_ops_status(&self) { + let mut consecutive_errors = 0; match blockscout_client::apis::proxy_api::get_account_abstraction_status(&self.api_config) .await { Ok(status) => { + consecutive_errors = 0; let status = Self::user_ops_internal_status_from_api_status(status, &self.wait_config); let modified = self.sender.send_if_modified(|val| { if val.user_ops != status { val.user_ops = status.clone(); true } else { false } }); if modified { tracing::info!("Observed new indexing status: {:?}", status); } else { tracing::info!("Indexing status is unchanged"); } } Err(e) => { + if consecutive_errors >= RETRIES { + tracing::error!(error = ?e, "Failed to get user ops indexing status after {RETRIES} retries"); + return; + } match &e { blockscout_client::Error::ResponseError(bad_request) if bad_request.status == reqwest::StatusCode::BAD_REQUEST => { tracing::warn!( error =? e, "Got response with HTTP 400. This likely means that blockscout version \ is <7.0.0.", ); } _ => { tracing::error!( error =? e, "Failed to get user ops indexing status", ); } } + consecutive_errors += 1; // don't need to change if disabled, because it's handled // in `init` if self.wait_config.user_ops_past_indexing_finished.enabled { tracing::warn!( "User ops related charts are turned off to avoid \ incorrect data. Set `STATS__CONDITIONAL_START__USER_OPS_PAST_INDEXING_FINISHED__ENABLED=false` \ to ignore this check and update the charts." ); } } } }
168-191: Optimize wait time calculation.The wait time calculation could be simplified and made more efficient.
- let wait_time = if let IndexingStatus::MAX = *self.sender.borrow() { - self.wait_config.check_period_secs.saturating_mul(100) - } else { - self.wait_config.check_period_secs - }; + let wait_time = self.wait_config.check_period_secs + * if *self.sender.borrow() == IndexingStatus::MAX { 100 } else { 1 };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
stats/stats-server/src/blockscout_waiter.rs(21 hunks)stats/stats-server/src/server.rs(2 hunks)stats/stats-server/tests/it/chart_endpoints/mod.rs(1 hunks)stats/stats-server/tests/it/indexing_status.rs(2 hunks)stats/stats/src/charts/counters/total_aa_wallets.rs(2 hunks)stats/stats/src/charts/counters/total_user_ops.rs(2 hunks)stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_bundlers.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_paymasters.rs(2 hunks)stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs(2 hunks)stats/stats/src/charts/lines/user_ops/new_user_ops.rs(2 hunks)stats/stats/src/charts/lines/user_ops/user_ops_growth.rs(2 hunks)stats/stats/src/tests/mock_blockscout.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
stats/stats-server/tests/it/indexing_status.rs (1)
Learnt from: bragov4ik
PR: blockscout/blockscout-rs#1226
File: stats/stats-server/tests/it/mock_blockscout_simple/stats_full.rs:0-0
Timestamp: 2025-02-04T10:08:27.299Z
Learning: In stats-server tests, waiting for server initialization requires not just checking server health but also ensuring all chart values have been calculated and initialized.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (17)
stats/stats-server/tests/it/indexing_status.rs (2)
5-8: LGTM!The import changes correctly include the new
user_ops_status_response_jsonfunction for mocking user ops status.
23-33: LGTM!The mock API initialization is correctly updated to include user ops status response, aligning with the PR objectives to add user ops charts requirement.
stats/stats/src/tests/mock_blockscout.rs (3)
20-33: LGTM!The
default_mock_blockscout_apifunction is correctly updated to include user ops status response with default values.
35-54: LGTM!The
mock_blockscout_apifunction is correctly updated to handle user ops status response:
- Accepts an optional user ops status response.
- Mounts the response at the correct endpoint.
56-74: LGTM!The
user_ops_status_response_jsonfunction correctly generates a JSON string for user ops status:
- Takes a boolean parameter to indicate if past indexing is finished.
- Includes status for both v06 and v07 entry point versions.
stats/stats/src/charts/lines/user_ops/active_paymasters.rs (1)
57-62: LGTM! Well-structured indexing status requirements.The implementation correctly specifies the indexing requirements for both blockscout and user operations components, using appropriate status variants.
stats/stats-server/src/blockscout_waiter.rs (1)
304-694: LGTM! Excellent test coverage.The test suite is comprehensive and covers important scenarios:
- Success cases for both status types
- Error handling and retries
- Disabled checks
- Various response delays and timeouts
stats/stats/src/charts/counters/total_user_ops.rs (1)
6-6: LGTM! The indexing status requirements are well-defined.The implementation correctly requires both
BlocksIndexedandPastOperationsIndexedstatuses, which is appropriate for a total user operations counter that depends on historical data.Also applies to: 8-8, 31-36
stats/stats/src/charts/counters/total_aa_wallets.rs (1)
6-6: LGTM! Consistent implementation of indexing status requirements.The implementation mirrors the approach used in
total_user_ops.rs, requiring bothBlocksIndexedandPastOperationsIndexedstatuses, which is appropriate for tracking account abstraction wallet totals.Also applies to: 8-8, 31-36
stats/stats/src/charts/lines/user_ops/active_bundlers.rs (1)
16-16: LGTM! Appropriate indexing requirements for active bundlers chart.The implementation correctly requires both
BlocksIndexedandPastOperationsIndexedstatuses, which is necessary for accurately tracking active bundlers over time.Also applies to: 57-62
stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs (1)
16-16: LGTM! Consistent implementation for active AA wallets chart.The implementation maintains consistency with other chart types, requiring both
BlocksIndexedandPastOperationsIndexedstatuses, which is necessary for tracking active AA wallets.Also applies to: 62-67
stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (1)
11-11: LGTM!The implementation correctly specifies the indexing status requirements for the AA wallets growth chart, requiring both blocks to be indexed and past operations to be indexed.
Also applies to: 38-43
stats/stats/src/charts/lines/user_ops/user_ops_growth.rs (1)
11-11: LGTM!The implementation correctly specifies the indexing status requirements for the user ops growth chart, maintaining consistency with other chart implementations.
Also applies to: 13-13, 38-43
stats/stats/src/charts/lines/user_ops/new_user_ops.rs (1)
22-22: LGTM!The implementation correctly specifies the indexing status requirements for new user ops chart, maintaining consistency with other chart implementations. The SQL query properly handles consensus blocks and timestamp filtering.
Also applies to: 92-97
stats/stats-server/src/server.rs (1)
63-66: Consider alternatives to infinite sleep.Using
sleep_indefinitelywithu64::MAXduration seems like a workaround. This could lead to issues:
- Unclear why the task needs to be kept alive indefinitely
- Using maximum duration value might not be the best approach
Could you explain why the task needs to be kept alive indefinitely? Consider alternatives like:
- Using a proper shutdown mechanism
- Implementing a cancellation token
- Using a channel-based approach for task coordination
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (2)
25-25: LGTM!The import statement is well-organized and correctly imports the necessary types for the indexing status functionality.
126-131: LGTM!The implementation correctly specifies the indexing requirements for new account abstraction wallets tracking:
- Requires blocks to be indexed for timestamp information
- Requires past user operations to be indexed for wallet tracking
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
stats/stats-server/src/blockscout_waiter.rs (2)
168-191: Extract magic numbers into named constants.The code uses magic numbers that should be extracted into named constants for better maintainability.
Consider extracting the magic number into a named constant:
+const MAX_STATUS_CHECK_PERIOD_MULTIPLIER: u32 = 100; + pub async fn run(&self) -> Result<(), anyhow::Error> { // ... rest of the code ... let wait_time = if let IndexingStatus::MAX = *self.sender.borrow() { - self.wait_config.check_period_secs.saturating_mul(100) + self.wait_config.check_period_secs.saturating_mul(MAX_STATUS_CHECK_PERIOD_MULTIPLIER) } else { self.wait_config.check_period_secs }; // ... rest of the code ... }
652-694: Improve test organization with test data builders.The test setup could be improved by using the builder pattern for test data construction.
Consider creating a test data builder for better test organization:
#[cfg(test)] mod test_helpers { use super::*; pub struct TestConfigBuilder { blocks_ratio: Option<ToggleableThreshold>, internal_transactions_ratio: Option<ToggleableThreshold>, user_ops_enabled: bool, check_period_secs: u32, } impl Default for TestConfigBuilder { fn default() -> Self { Self { blocks_ratio: None, internal_transactions_ratio: None, user_ops_enabled: true, check_period_secs: 1, } } } impl TestConfigBuilder { pub fn with_blocks_ratio(mut self, threshold: ToggleableThreshold) -> Self { self.blocks_ratio = Some(threshold); self } pub fn build(self) -> StartConditionSettings { StartConditionSettings { blocks_ratio: self.blocks_ratio.unwrap_or_else(ToggleableThreshold::default), internal_transactions_ratio: self.internal_transactions_ratio .unwrap_or_else(ToggleableThreshold::default), user_ops_past_indexing_finished: ToggleableCheck { enabled: self.user_ops_enabled, }, check_period_secs: self.check_period_secs, } } } }This would make the test setup more readable:
#[tokio::test] async fn waiter_ignores_errors_when_checks_are_disabled() { let config_b_off = TestConfigBuilder::default() .with_blocks_ratio(ToggleableThreshold::disabled()) .build(); // ... rest of the test ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stats/stats-server/src/blockscout_waiter.rs(21 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
bragov4ik
force-pushed
the
bragov4ik/stats-user-ops-indexing-status
branch
from
04b3137 to
5922a70
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
stats/stats-server/src/blockscout_waiter.rs (1)
116-166: 🛠️ Refactor suggestionImprove error handling in check_user_ops_status.
The error handling could be improved to be consistent with
check_blockscout_status:
- No retry mechanism for failed requests
- No consecutive error tracking
- No error propagation to the caller
- async fn check_user_ops_status(&self) { + async fn check_user_ops_status( + &self, + consecutive_errors: &mut u64, + ) -> Result<(), anyhow::Error> { match blockscout_client::apis::proxy_api::get_account_abstraction_status(&self.api_config) .await { Ok(status) => { + *consecutive_errors = 0; let status = Self::user_ops_internal_status_from_api_status(status, &self.wait_config); // ... rest of the success handling ... + Ok(()) } Err(e) => { + if *consecutive_errors >= RETRIES { + return Err(e).context("Requesting user ops indexing status"); + } match &e { // ... rest of the error handling ... } + *consecutive_errors += 1; + Ok(()) } } - } + }
🧹 Nitpick comments (7)
stats/stats/src/charts/lines/user_ops/user_ops_growth.rs (1)
38-43: Add documentation for the indexing status requirement.Consider adding a doc comment explaining the indexing requirements and their significance for user operations growth tracking.
+ /// Specifies that both blocks and past user operations must be indexed + /// for this chart to function correctly. fn indexing_status_requirement() -> IndexingStatus { IndexingStatus { blockscout: BlockscoutIndexingStatus::BlocksIndexed, user_ops: UserOpsIndexingStatus::PastOperationsIndexed, } }stats/stats-server/tests/it/mock_blockscout_simple/stats_not_indexed.rs (1)
37-47: Enhance test coverage for indexing status.Consider adding test cases for different indexing ratios and user operations status to ensure robust handling of various indexing scenarios.
let blockscout_api = mock_blockscout_api( ResponseTemplate::new(200).set_body_string( r#"{ "finished_indexing": false, "finished_indexing_blocks": false, "indexed_blocks_ratio": "0.10", "indexed_internal_transactions_ratio": null }"#, ), None, )Consider adding these test cases:
#[tokio::test] async fn test_different_indexing_ratios() { // Test with 0% indexing let response_zero = r#"{ "finished_indexing": false, "finished_indexing_blocks": false, "indexed_blocks_ratio": "0.00", "indexed_internal_transactions_ratio": null }"#; // Test with 100% indexing let response_complete = r#"{ "finished_indexing": true, "finished_indexing_blocks": true, "indexed_blocks_ratio": "1.00", "indexed_internal_transactions_ratio": "1.00" }"#; // Add assertions for each case }stats/stats-server/src/server.rs (1)
64-67: Consider using a more maintainable sleep duration.Using
u64::MAXfor sleep duration could lead to overflow issues. Consider using a more reasonable duration or implementing a proper cancellation mechanism.- tokio::time::sleep(Duration::from_secs(u64::MAX)).await; + // Use a day as the interval and loop + loop { + tokio::time::sleep(Duration::from_secs(86400)).await; + }stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (1)
51-71: Consider optimizing the SQL query performance.The current query uses
DISTINCT ONwithORDER BYwhich might be inefficient for large datasets. Consider adding an index to improve performance.Add an index on
(sender, block_hash)to improve the query performance:CREATE INDEX idx_user_operations_sender_block ON user_operations (sender, block_hash);This will help optimize the
DISTINCT ONoperation and the subsequent join with the blocks table.stats/stats-server/src/settings.rs (2)
125-128: Consider enhancing error handling.The error handling in
handle_disable_internal_transactionscould be more robust by:
- Adding error context using
with_context()- Logging the specific charts being disabled
- .filter(|(_k, req)| { - req.blockscout == BlockscoutIndexingStatus::InternalTransactionsIndexed - }) + .filter(|(k, req)| { + let is_dependent = req.blockscout == BlockscoutIndexingStatus::InternalTransactionsIndexed; + if is_dependent { + tracing::info!("Chart {} depends on internal transactions", k); + } + is_dependent + })
159-161: Consider making the warning message more concise.The warning message is quite long and could be made more concise while maintaining clarity.
- "User ops related charts are turned off to avoid \ - incorrect data. Set `STATS__CONDITIONAL_START__USER_OPS_PAST_INDEXING_FINISHED__ENABLED=false` \ - to ignore this check and update the charts." + "User ops charts disabled to prevent incorrect data. \ + Set `STATS__CONDITIONAL_START__USER_OPS_PAST_INDEXING_FINISHED__ENABLED=false` to override."stats/stats/src/tests/mock_blockscout.rs (1)
56-74: Consider parameterizing the entry point status fields.The current implementation hardcodes the status fields for v0.6 and v0.7 entry points. To improve test coverage, consider making these fields configurable:
-pub fn user_ops_status_response_json(past_finished: bool) -> String { +pub fn user_ops_status_response_json( + past_finished: bool, + v06_status: Option<EntryPointStatus>, + v07_status: Option<EntryPointStatus>, +) -> String { + let default_status = EntryPointStatus { + enabled: true, + live: false, + past_db_logs_indexing_finished: false, + past_rpc_logs_indexing_finished: false, + }; + let v06 = v06_status.unwrap_or(default_status.clone()); + let v07 = v07_status.unwrap_or(default_status); format!( r#"{{ "finished_past_indexing": {past_finished}, "v06": {{ - "enabled": true, - "live": false, - "past_db_logs_indexing_finished": false, - "past_rpc_logs_indexing_finished": false + "enabled": {}, + "live": {}, + "past_db_logs_indexing_finished": {}, + "past_rpc_logs_indexing_finished": {} }}, "v07": {{ - "enabled": true, - "live": false, - "past_db_logs_indexing_finished": false, - "past_rpc_logs_indexing_finished": false + "enabled": {}, + "live": {}, + "past_db_logs_indexing_finished": {}, + "past_rpc_logs_indexing_finished": {} }} }}"#, + v06.enabled, v06.live, v06.past_db_logs_indexing_finished, v06.past_rpc_logs_indexing_finished, + v07.enabled, v07.live, v07.past_db_logs_indexing_finished, v07.past_rpc_logs_indexing_finished, ) } +#[derive(Clone)] +pub struct EntryPointStatus { + pub enabled: bool, + pub live: bool, + pub past_db_logs_indexing_finished: bool, + pub past_rpc_logs_indexing_finished: bool, +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
stats/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
stats/Cargo.toml(1 hunks)stats/README.md(1 hunks)stats/stats-server/src/blockscout_waiter.rs(21 hunks)stats/stats-server/src/server.rs(2 hunks)stats/stats-server/src/settings.rs(5 hunks)stats/stats-server/tests/it/mock_blockscout_simple/stats_not_indexed.rs(1 hunks)stats/stats-server/tests/it/mock_blockscout_simple/stats_not_updated.rs(1 hunks)stats/stats/src/charts/chart.rs(3 hunks)stats/stats/src/charts/counters/average_block_time.rs(2 hunks)stats/stats/src/charts/counters/new_contracts_24h.rs(2 hunks)stats/stats/src/charts/counters/new_operational_txns_24h.rs(2 hunks)stats/stats/src/charts/counters/new_verified_contracts_24h.rs(2 hunks)stats/stats/src/charts/counters/pending_txns.rs(2 hunks)stats/stats/src/charts/counters/total_aa_wallets.rs(2 hunks)stats/stats/src/charts/counters/total_addresses.rs(2 hunks)stats/stats/src/charts/counters/total_blocks.rs(2 hunks)stats/stats/src/charts/counters/total_contracts.rs(2 hunks)stats/stats/src/charts/counters/total_operational_txns.rs(2 hunks)stats/stats/src/charts/counters/total_tokens.rs(2 hunks)stats/stats/src/charts/counters/total_txns.rs(2 hunks)stats/stats/src/charts/counters/total_user_ops.rs(2 hunks)stats/stats/src/charts/counters/total_verified_contracts.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/average_txn_fee_24h.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/new_txns_24h.rs(2 hunks)stats/stats/src/charts/counters/txns_stats_24h/txns_fee_24h.rs(2 hunks)stats/stats/src/charts/counters/yesterday_operational_txns.rs(2 hunks)stats/stats/src/charts/counters/yesterday_txns.rs(2 hunks)stats/stats/src/charts/indexing_status.rs(1 hunks)stats/stats/src/charts/lines/contracts/new_contracts.rs(2 hunks)stats/stats/src/charts/lines/transactions/new_operational_txns_window.rs(2 hunks)stats/stats/src/charts/lines/transactions/new_txns_window.rs(2 hunks)stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_bundlers.rs(2 hunks)stats/stats/src/charts/lines/user_ops/active_paymasters.rs(2 hunks)stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs(2 hunks)stats/stats/src/charts/lines/user_ops/new_user_ops.rs(2 hunks)stats/stats/src/charts/lines/user_ops/user_ops_growth.rs(2 hunks)stats/stats/src/charts/mod.rs(1 hunks)stats/stats/src/data_source/source.rs(1 hunks)stats/stats/src/lib.rs(1 hunks)stats/stats/src/tests/mock_blockscout.rs(1 hunks)stats/stats/src/update_group.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (30)
- stats/stats/src/charts/lines/user_ops/new_user_ops.rs
- stats/stats/src/data_source/source.rs
- stats/stats/src/charts/counters/txns_stats_24h/new_txns_24h.rs
- stats/stats/src/charts/lines/contracts/new_contracts.rs
- stats/stats/src/charts/counters/total_txns.rs
- stats/Cargo.toml
- stats/stats/src/charts/counters/yesterday_txns.rs
- stats/stats/src/charts/counters/total_addresses.rs
- stats/stats/src/lib.rs
- stats/stats/src/charts/counters/new_operational_txns_24h.rs
- stats/stats/src/charts/counters/total_operational_txns.rs
- stats/stats/src/charts/lines/user_ops/active_aa_wallets.rs
- stats/stats/src/charts/lines/transactions/new_operational_txns_window.rs
- stats/stats/src/charts/lines/transactions/new_txns_window.rs
- stats/stats/src/charts/mod.rs
- stats/stats/src/charts/indexing_status.rs
- stats/stats/src/charts/counters/total_blocks.rs
- stats/stats/src/charts/lines/user_ops/active_paymasters.rs
- stats/stats/src/charts/counters/total_verified_contracts.rs
- stats/stats/src/charts/counters/txns_stats_24h/txns_fee_24h.rs
- stats/stats/src/charts/counters/pending_txns.rs
- stats/stats/src/charts/counters/txns_stats_24h/average_txn_fee_24h.rs
- stats/README.md
- stats/stats/src/charts/counters/yesterday_operational_txns.rs
- stats/stats/src/charts/counters/total_contracts.rs
- stats/stats/src/charts/counters/new_contracts_24h.rs
- stats/stats/src/charts/lines/user_ops/active_bundlers.rs
- stats/stats/src/charts/counters/total_aa_wallets.rs
- stats/stats/src/charts/counters/total_tokens.rs
- stats/stats/src/update_group.rs
🧰 Additional context used
🧠 Learnings (1)
stats/stats-server/tests/it/mock_blockscout_simple/stats_not_indexed.rs (1)
Learnt from: bragov4ik
PR: blockscout/blockscout-rs#1226
File: stats/stats-server/tests/it/mock_blockscout_simple/stats_full.rs:0-0
Timestamp: 2025-02-04T10:08:27.299Z
Learning: In stats-server tests, waiting for server initialization requires not just checking server health but also ensuring all chart values have been calculated and initialized.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (9)
stats/stats/src/charts/counters/total_user_ops.rs (1)
6-8: LGTM! Well-structured implementation of indexing status requirements.The implementation correctly specifies both blockscout and user ops indexing requirements, which aligns with the PR objectives for user ops charts.
Also applies to: 31-36
stats/stats/src/charts/counters/new_verified_contracts_24h.rs (1)
11-12: LGTM! Appropriate use of LEAST_RESTRICTIVE for user ops status.The implementation correctly uses
LEAST_RESTRICTIVEfor user ops since verified contracts are independent of user operations indexing.Also applies to: 59-64
stats/stats/src/charts/lines/user_ops/aa_wallets_growth.rs (1)
11-11: LGTM! Appropriate indexing requirements for AA wallets growth.The implementation correctly requires both blockscout blocks and past operations to be indexed, which is necessary for accurate AA wallets growth data.
Also applies to: 38-43
stats/stats-server/src/server.rs (1)
127-135: Previous review comment about task management is still relevant.The current implementation still swallows the success case by sleeping indefinitely, which makes it difficult to properly handle task lifecycle and error conditions.
stats/stats/src/charts/lines/user_ops/new_aa_wallets.rs (1)
126-131: LGTM! The indexing status requirement is well-defined.The implementation correctly specifies that both blocks and past user operations must be indexed for this chart to function.
stats/stats/src/charts/counters/average_block_time.rs (1)
137-141: LGTM! The indexing status requirements are correctly specified.The implementation properly indicates that this chart:
- Does not require block indexing (
NoneIndexed)- Does not depend on user operations indexing (
LEAST_RESTRICTIVE)stats/stats/src/charts/chart.rs (1)
193-198: LGTM! The default indexing status requirements are well-chosen.The implementation sets sensible defaults:
- Most charts need indexed blocks (
BlockscoutIndexingStatus::BlocksIndexed)- Most charts don't depend on user ops (
UserOpsIndexingStatus::IndexingPastOperations)The comments clearly explain the reasoning behind these defaults.
stats/stats/src/tests/mock_blockscout.rs (2)
20-33: LGTM! Default mock implementation correctly configured.The default mock server is properly configured to handle both blockscout and user operations indexing status endpoints with appropriate default responses.
35-54: LGTM! Mock API implementation follows best practices.The updated implementation:
- Maintains backward compatibility with optional user ops parameter
- Correctly mounts the user ops status endpoint only when needed
- Uses consistent API path conventions
stats/stats-server/tests/it/mock_blockscout_simple/stats_not_updated.rs
Outdated
Show resolved
Hide resolved
| with: | ||
| args: docker restart postgres | ||
|
|
||
| - name: DB tests temp |
| blockscout-client = { git = "https://github.com/blockscout/blockscout-rs/", rev = "506b821" } | ||
| blockscout-service-launcher = { version = "0.16.1" } | ||
| blockscout-client = { git = "https://github.com/blockscout/blockscout-rs/", rev = "d02877c" } | ||
| blockscout-service-launcher = { git = "https://github.com/blockscout/blockscout-rs/", rev = "3908e8bf" } |
| } | ||
|
|
||
| /// for `init_server` | ||
| pub async fn healthcheck_successful(response: Response) -> bool { |
There was a problem hiding this comment.
remove after fixing launcher
Summary by CodeRabbit
New Features
Documentation
Chores