-
Notifications
You must be signed in to change notification settings - Fork 1
Problem: Cannot query smart contracts #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@BigtoC has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis update introduces a comprehensive integration of a blockchain proposal manager feature into a Flutter application. It adds new infrastructure for querying smart contracts, a BLoC for state management, and multiple UI widgets for displaying contract and proposal data. Example scripts, LLDB debugging helpers, and extensive tests are included. Minor dependency and configuration updates are also present. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ProposalManagerCard/ProposalManagerPage
participant Bloc as ProposalManagerBloc
participant Repo as ContractRepository
participant Chain as Blockchain Node
UI->>Bloc: loadContractData()
Bloc->>Repo: queryContract({"status": {}})
Bloc->>Repo: queryContract({"proposals": {}})
Repo->>Chain: HTTP/RPC Query (status, proposals)
Chain-->>Repo: Contract Data (status, proposals)
Repo-->>Bloc: Parsed Data
Bloc-->>UI: Stream Updates (status, proposals)
UI-->>User: Display Proposal Data
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
🧹 Nitpick comments (12)
ios/Flutter/ephemeral/flutter_lldb_helper.py (1)
24-32: LGTM! Correct LLDB module initialization.The initialization properly:
- Uses regex-based breakpoint creation (required for dummy target as documented)
- Configures auto-continue to avoid interrupting debugging flow
- Sets up the callback function correctly
Consider adding documentation about when this debugging helper should be enabled, as memory modification during debugging could affect application behavior.
test/proposal_manager_page_test.dart (2)
1-32: Consider mocking dependencies for more reliable tests.The test correctly validates widget rendering and loading states, but it appears to rely on real blockchain network calls through the ProposalManagerBloc. This could make tests flaky and slow.
Consider mocking the
proposalManagerBlocto provide predictable test data and faster execution.// Example approach: // 1. Create a mock ProposalManagerBloc // 2. Inject it into the widget or use dependency injection // 3. Control the stream data for predictable test outcomes
26-27: Long timeout may slow down test suite.The 3-second timeout could significantly slow down the test suite, especially if this pattern is used across multiple tests.
Consider using a shorter timeout or mocking the data loading to avoid network dependencies.
test/proposal_manager_data_test.dart (1)
19-19: Remove debug print statements.Print statements should be removed from production test code or replaced with proper test logging if debugging information is needed.
- print("Status result: $status"); - print("Proposals result: $proposalsResult"); - print("Parsed ${proposals.length} proposals"); - print("Error loading proposals: $e");Also applies to: 34-34, 44-44, 47-47
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_card.dart (2)
19-19: Consider dependency injection for better testability.Using the global
proposalManagerBlocsingleton directly makes the widget harder to test and less flexible. Consider accepting the bloc as a parameter or using dependency injection.class ProposalManagerCard extends StatefulWidget { final ProposalManagerBloc? bloc; const ProposalManagerCard({super.key, this.bloc}); // Then in build method: final bloc = widget.bloc ?? proposalManagerBloc;
45-49: Consider using a more maintainable string formatting approach.The multi-line string with interpolation could be refactored for better readability and maintainability.
final cardContent = [ "Proposal Manager", "", "Total: $totalProposals", "Pending: $pendingProposals", "Approved: $approvedProposals" ].join("\n");lib/application/proposal_manager/proposal_manager_bloc.dart (2)
6-6: Consider making contract address configurable.The hardcoded contract address reduces flexibility for different environments (testing, staging, production) or different contracts.
class ProposalManagerBloc { final String contractAddress; ProposalManagerBloc({ this.contractAddress = "mantra17p9u09rgfd2nwr52ayy0aezdc42r2xd2g5d70u00k5qyhzjqf89q08tazu" });
88-96: Inconsistent error handling approach.Some methods rethrow exceptions while others handle them differently. Consider standardizing the error handling approach across all methods.
Either:
- Handle all errors consistently within each method, or
- Let all methods rethrow and handle errors at the UI level
Also applies to: 99-105, 117-123
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart (2)
225-225: Replace deprecatedwithAlphamethod.The
withAlphamethod is deprecated in newer Flutter versions. UsewithOpacityinstead.- color: statusColor.withAlpha(10), + color: statusColor.withOpacity(0.04),
260-265: Add input validation to address shortening method.The method should handle null or empty addresses more gracefully.
String _shortenAddress(String address) { + if (address.isEmpty) return "N/A"; if (address.length <= 20) return address; final firstPart = address.substring(0, 10); final lastPart = address.substring(address.length - 6); return "$firstPart...$lastPart"; }lib/examples/mantra_proposal_manager_example.dart (2)
12-15: Consider making ContractRepository configuration more explicit.The constructor only accepts
rpcEndpointbut the repository requires both RPC and REST endpoints. This could lead to incomplete configuration or reliance on defaults.- MantraProposalManagerExample({String? rpcEndpoint}) + MantraProposalManagerExample({ + String? rpcEndpoint, + String? restEndpoint, + }) : _contractRepo = ContractRepository( rpcEndpoint: rpcEndpoint ?? chainRpcUrl, + restEndpoint: restEndpoint ?? chainRestUrl, );
96-116: Add pagination support for proposals query.The comment on line 96 mentions pagination but doesn't implement it. Large contracts could return many proposals, potentially causing performance issues.
Future<void> _queryProposals() async { print("📝 === Getting Proposals List ==="); try { - // Query all proposals (may need pagination parameters) - final proposals = await _contractRepo.queryContract({"proposals": {}}); + // Query proposals with pagination support + final proposals = await _contractRepo.queryContract({ + "proposals": { + "start_after": null, + "limit": 10, + } + }); print("✅ Proposals: $proposals");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignore(1 hunks)ios/Flutter/ephemeral/flutter_lldb_helper.py(1 hunks)ios/Flutter/ephemeral/flutter_lldbinit(1 hunks)ios/Podfile(1 hunks)lib/application/proposal_manager/proposal_manager_bloc.dart(1 hunks)lib/examples/mantra_proposal_manager_example.dart(1 hunks)lib/infrastructure/blockchain/contract_repository.dart(1 hunks)lib/presentation/home_page/dapps/proposal_manager/proposal_manager_card.dart(1 hunks)lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart(1 hunks)lib/presentation/home_page/dapps/wallet_balance/wallet_balance_card.dart(1 hunks)lib/presentation/home_page/home_ui.dart(2 hunks)pubspec.yaml(1 hunks)test/infrastructure/blockchain/wallet_repository_test.dart(2 hunks)test/proposal_manager_bloc_test.dart(1 hunks)test/proposal_manager_data_test.dart(1 hunks)test/proposal_manager_page_test.dart(1 hunks)test/widget_test.dart(0 hunks)
💤 Files with no reviewable changes (1)
- test/widget_test.dart
🧰 Additional context used
🪛 Pylint (3.3.7)
ios/Flutter/ephemeral/flutter_lldb_helper.py
[refactor] 7-7: Useless return at end of function or method
(R1711)
🔇 Additional comments (11)
.gitignore (1)
50-51: LGTM! Appropriate Android build artifacts exclusion.The added patterns correctly exclude Android build directories that should not be committed to version control.
ios/Flutter/ephemeral/flutter_lldbinit (1)
1-6: LGTM! Standard LLDB initialization script.The script correctly loads the Python helper using relative path resolution, which is the proper approach for LLDB script imports.
ios/Podfile (1)
1-44: LGTM! Standard Flutter iOS Podfile configuration.The Podfile follows Flutter's recommended structure with proper:
- Build configuration mapping
- Flutter SDK path resolution with error handling
- Standard pod installation and post-install hooks
- Test target configuration
The implementation aligns with Flutter iOS best practices.
ios/Flutter/ephemeral/flutter_lldb_helper.py (1)
7-23: LGTM! Proper LLDB debugging callback implementation.The callback correctly:
- Accesses ARM64 registers for memory parameters
- Writes verification marker with proper error handling
- Follows LLDB Python API conventions
The static analysis hint about a "useless return" appears to be a false positive as there's no return statement at the flagged line.
pubspec.yaml (1)
45-45: Verify compatibility with cosmos_sdk version bump.The cosmos_sdk dependency has been updated from ^2.6.0 to ^2.7.0. While this should be backwards compatible, please ensure all existing functionality still works correctly.
Check the changelog for any breaking changes or deprecations:
cosmos_sdk dart package version 2.7.0 changelog breaking changeslib/presentation/home_page/dapps/wallet_balance/wallet_balance_card.dart (1)
6-6: LGTM!Import path correctly updated to reflect the proper directory structure.
lib/presentation/home_page/home_ui.dart (1)
6-7: Changes look good!Import paths correctly updated and
ProposalManagerCardappropriately added to the widget tree.Also applies to: 71-71
test/infrastructure/blockchain/wallet_repository_test.dart (2)
80-84: LGTM! Test correction aligns with truncation behavior.The updated comment and assertion correctly reflect that fee amounts should be truncated (floored) rather than rounded. The change from
toStringAsFixed(0)tofloor().toString()ensures the test matches the actual implementation behavior.
126-128: LGTM! Consistent truncation behavior applied.The comment and calculation method are now consistent with the truncation approach used elsewhere in the test suite. Using
floor().toString()ensures predictable behavior for fee calculations.lib/application/proposal_manager/proposal_manager_bloc.dart (1)
133-134: I’ll locate the bloc file, inspect its class definition (including anydisposeorclosemethods), and search for calls to both.dispose()and.close()on the global instance.#!/bin/bash # 1. Locate the bloc file file=$(find . -type f -name 'proposal_manager_bloc.dart' | head -n1) echo "Found: $file" if [ -z "$file" ]; then echo "Bloc file not found"; exit 0; fi # 2. Show class signature echo; echo "---- Class Definition ----" rg -n 'class ProposalManagerBloc' -C3 "$file" # 3. Show inheritance (e.g., BlocBase, Cubit) echo; echo "---- Inheritance ----" rg -n 'extends' -C1 "$file" | head -n5 # 4. Check for dispose() definition echo; echo "---- dispose() Method ----" rg -n 'dispose\s*\(' -C2 "$file" || echo "No dispose() in class" # 5. Check for close() definition (flutter_bloc uses close()) echo; echo "---- close() Method ----" rg -n 'close\s*\(' -C2 "$file" || echo "No close() in class" # 6. Search for disposal calls on the global instance echo; echo "---- proposalManagerBloc.dispose() Calls ----" rg -n 'proposalManagerBloc\.dispose' -C2 . echo; echo "---- proposalManagerBloc.close() Calls ----" rg -n 'proposalManagerBloc\.close' -C2 .lib/examples/mantra_proposal_manager_example.dart (1)
169-182: LGTM! Well-structured main function.The main function provides clear output formatting and proper example execution flow.
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart
Outdated
Show resolved
Hide resolved
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart
Show resolved
Hide resolved
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart
Show resolved
Hide resolved
…tract address and improve error handling
…ck contract repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (1)
lib/examples/mantra_proposal_manager_example.dart (1)
7-8: Hard-coded contract address reduces flexibility and testability.The contract address is hard-coded, making it difficult to test with different contracts or use in different environments. This aligns with the previous feedback about making the address configurable.
🧹 Nitpick comments (4)
lib/infrastructure/blockchain/contract_repository.dart (1)
262-277: Consider adding request timeout and rate limiting.The
executeMultipleQueriesmethod runs queries in parallel without any rate limiting or timeout controls, which could overwhelm the RPC endpoint or cause performance issues.Consider adding configurable limits:
Future<List<Map<String, dynamic>?>> executeMultipleQueries( List<Map<String, dynamic>> queries, + {int? maxConcurrent, Duration? timeout} ) async { try { _logger.i("Executing ${queries.length} queries in parallel"); + // Limit concurrent requests if specified + if (maxConcurrent != null && queries.length > maxConcurrent) { + final results = <Map<String, dynamic>?>[]; + for (int i = 0; i < queries.length; i += maxConcurrent) { + final batch = queries.skip(i).take(maxConcurrent); + final batchFutures = batch.map((query) => queryContract(query)).toList(); + final batchResults = await Future.wait(batchFutures); + results.addAll(batchResults); + } + return results; + } + final futures = queries.map((query) => queryContract(query)).toList(); - final results = await Future.wait(futures); + final results = await Future.wait(futures, timeout: timeout);lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart (2)
5-6: Consider extracting hardcoded contract address to configuration.The contract address is hardcoded as a constant, which reduces flexibility for different environments or contract updates.
Consider moving this to a configuration file or environment-specific constants:
-const String proposalManagerContractAddress = - "mantra17p9u09rgfd2nwr52ayy0aezdc42r2xd2g5d70u00k5qyhzjqf89q08tazu";And define it in your constants file with environment-specific values.
280-285: Consider making address truncation configurable.The address shortening logic uses hardcoded values (10 characters prefix, 6 characters suffix) which may not be optimal for all screen sizes.
Consider making the truncation lengths configurable:
-String _shortenAddress(String address) { +String _shortenAddress(String address, {int prefixLength = 10, int suffixLength = 6}) { - if (address.length <= 20) return address; + if (address.length <= prefixLength + suffixLength) return address; - final firstPart = address.substring(0, 10); + final firstPart = address.substring(0, prefixLength); - final lastPart = address.substring(address.length - 6); + final lastPart = address.substring(address.length - suffixLength); return "$firstPart...$lastPart"; }lib/examples/mantra_proposal_manager_example.dart (1)
217-217: Make dummy address configurable or use a more realistic test pattern.The hard-coded dummy address for balance testing could be made configurable or use a more realistic testing approach.
- const dummyAddress = "mantra1dfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf"; + // Use a well-formed test address or make it configurable + const dummyAddress = "mantra1test1234567890abcdefghijklmnopqrstuvwxyz";Alternatively, consider making this address a parameter or using the contract's own address for testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/test.yaml(1 hunks)lib/application/proposal_manager/proposal_manager_bloc.dart(1 hunks)lib/examples/mantra_proposal_manager_example.dart(1 hunks)lib/infrastructure/blockchain/contract_repository.dart(1 hunks)lib/infrastructure/blockchain/wallet_repository.dart(1 hunks)lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart(1 hunks)pubspec.yaml(3 hunks)test/mocks.dart(1 hunks)test/mocks.mocks.dart(1 hunks)test/proposal_manager_data_test.dart(1 hunks)test/proposal_manager_page_test.dart(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- test/mocks.dart
- test/mocks.mocks.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- pubspec.yaml
- test/proposal_manager_data_test.dart
- lib/application/proposal_manager/proposal_manager_bloc.dart
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test.yaml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (11)
lib/infrastructure/blockchain/contract_repository.dart (3)
8-36: Well-implemented HTTP provider with proper resource management.The
_TendermintHTTPProviderclass correctly implements the service provider interface with proper HTTP client management and disposal.
39-59: Good constructor design with proper dependency injection.The
ContractRepositoryconstructor properly accepts required parameters and initializes the HTTP provider with disposal support, addressing previous review feedback about hardcoded values and resource management.
88-92: Improved error handling with generic messages.The error handling correctly logs detailed information internally while throwing generic exceptions to avoid exposing implementation details, addressing previous security concerns.
lib/infrastructure/blockchain/wallet_repository.dart (1)
251-251: Improved precision in fee calculation.The change from
toStringAsFixed(0)tofloor()is more mathematically precise and appropriate for fee calculations, ensuring consistent truncation behavior rather than rounding.test/proposal_manager_page_test.dart (3)
16-75: Comprehensive test coverage for successful data loading.The test properly mocks async contract calls with realistic delays and verifies the complete loading cycle from indicator to displayed data.
77-115: Good error handling test with retry functionality.The test correctly simulates network failures and verifies the error UI components including the retry button.
117-154: Excellent layout conflict prevention test.This test ensures the widget works properly within scrollable containers, which is crucial for preventing layout issues in the actual app.
lib/presentation/home_page/dapps/proposal_manager/proposal_manager_page.dart (2)
22-31: Well-implemented initialization with proper Future caching.The widget correctly initializes the contract repository and caches the Future to prevent redundant network calls, addressing previous performance concerns.
100-107: Proper retry mechanism implementation.The retry functionality correctly uses
setState()to refresh the Future, replacing the deprecatedmarkNeedsBuild()approach mentioned in previous reviews.lib/examples/mantra_proposal_manager_example.dart (2)
22-56: Excellent error handling and method organization.The main
runExamplemethod demonstrates good practices with comprehensive error handling, clear logging, and logical flow. The connection testing before proceeding with queries is particularly well-implemented.
175-224: Well-designed generic query demonstration.This method effectively demonstrates the generic helper functions while clearly explaining their limitations. The approach of showing both successful and expected failure cases is educational and helps users understand the contract interface.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
cosmos_sdkto version ^2.7.0..gitignoreto exclude additional Android build artifacts.Documentation