-
Notifications
You must be signed in to change notification settings - Fork 4
Feat: End to end wallet Integration with minting pages #5
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
WalkthroughThis update introduces a comprehensive NFT minting workflow, including new pages for entering coordinates, details, uploading images, and submitting NFTs, as well as a settings page and enhanced navigation. It adds new providers, utility services for location and IPFS, blockchain contract interaction logic, and a redesigned universal navbar with externalized chain switching. Android permissions and dependencies are expanded to support new features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant MintNftProvider
participant LocationService
participant IPFS
participant WalletProvider
participant Blockchain
User->>App: Navigates to "Mint NFT"
App->>MintNftProvider: Initialize minting state
App->>LocationService: Request current location
LocationService-->>App: Returns coordinates
App->>User: Displays map and coordinate form
User->>App: Edits coordinates/details
App->>MintNftProvider: Update state
User->>App: Uploads images
App->>IPFS: Upload image(s)
IPFS-->>App: Returns IPFS hash
App->>MintNftProvider: Update images
User->>App: Submits NFT
App->>WalletProvider: Prepare contract call
WalletProvider->>Blockchain: Write contract (mint NFT)
Blockchain-->>WalletProvider: Returns transaction hash
WalletProvider-->>App: Notify minting result
App->>User: Show success/error dialog
Estimated code review effort🎯 5 (Critical) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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: 31
🧹 Nitpick comments (23)
lib/utils/logger.dart (1)
3-8
: Prefer explicit type annotation for clarityAnnotating the variable clarifies the intended type and avoids implicit-dynamic warnings in stricter analysis settings.
-final logger = Logger( +final Logger logger = Logger(lib/utils/constants/tree_images.dart (1)
1-15
: Make the whole declarationconst
Because the list is immutable, declare the variable itself
const
instead offinal … = const […]
; this avoids redundancy and enables compile-time inlining.-final List<String> treeImages = const [ +const List<String> treeImages = [Also ensure every referenced asset path is included under
flutter/assets:
inpubspec.yaml
; otherwise the images won’t load at runtime..env.stencil (1)
1-5
: Minor .env hygieneAlphabetise keys and end the file with a trailing newline to silence dotenv-linter warnings.
- WALLETCONNECT_PROJECT_ID=<WALLETCONNECT_PROJECT_ID> - API_KEY=<PINATA_API_KEY> - API_SECRET=<PINATA_API_SECRET> - ALCHEMY_API_KEY=<ALCHEMY_API_KEY> - CONTRACT_ADDRESS=<CONTRACT_ADDRESS> + ALCHEMY_API_KEY=<ALCHEMY_API_KEY> + API_KEY=<PINATA_API_KEY> + API_SECRET=<PINATA_API_SECRET> + CONTRACT_ADDRESS=<CONTRACT_ADDRESS> + WALLETCONNECT_PROJECT_ID=<WALLETCONNECT_PROJECT_ID> +lib/utils/constants/contractDetails.dart (1)
1-6
: Turn comment-only addresses into typed constantsStoring addresses as code constants (e.g.
const String treeNftAddress = '0x757d…';
) allows static checking, centralised imports, and eliminates “magic strings” scattered across the codebase.pubspec.yaml (1)
54-55
: Fix trailing spaces.Static analysis detected trailing spaces that should be removed for clean formatting.
permission_handler: ^12.0.1 - # fluttertoast: ^8.2.8 - + # fluttertoast: ^8.2.8lib/pages/trees_page.dart (1)
27-27
: Consider using route constants for maintainability.The hardcoded route
'/mint-nft'
should ideally use a route constant for consistency and easier maintenance.- context.push('/mint-nft'); + context.push(RouteConstants.mintNftPath);lib/utils/services/ipfs_services.dart (1)
24-29
: Add logging for better debugging.Consider adding proper logging instead of just returning null on failures to help with debugging upload issues.
Since the app already includes the
logger
dependency, consider using it:+import 'package:logger/logger.dart'; + +final _logger = Logger(); if (response.statusCode == 200) { var jsonResponse = json.decode(await response.stream.bytesToString()); + _logger.i('IPFS upload successful: ${jsonResponse['IpfsHash']}'); return "https://gateway.pinata.cloud/ipfs/${jsonResponse['IpfsHash']}"; } else { + final errorBody = await response.stream.bytesToString(); + _logger.e('IPFS upload failed: ${response.statusCode} - $errorBody'); return null; }lib/utils/constants/route_constants.dart (1)
10-12
: Consider reducing duplication between route and path constants.The path constants have identical values to their corresponding route constants. Consider refactoring to eliminate duplication:
- static const String mintNftPath = '/mint-nft'; - static const String mintNftDetailsPath = '/mint-nft/details'; - static const String mintNftImagesPath = '/mint-nft/images'; + static const String mintNftPath = mintNft; + static const String mintNftDetailsPath = mintNftDetails; + static const String mintNftImagesPath = mintNftImages;lib/pages/settings_page.dart (1)
17-42
: Good use of Consumer pattern for reactive UI.The Consumer widget correctly rebuilds when WalletProvider state changes, and the UI displays relevant wallet information clearly.
Consider adding null safety checks for wallet provider data:
Text( - 'User Address: ${walletProvider.userAddress}', + 'User Address: ${walletProvider.userAddress ?? 'Not connected'}', ),lib/widgets/tree_NFT_view_widget.dart (2)
1-5
: Remove unused import.The import
package:tree_planting_protocol/utils/services/wallet_provider_utils.dart
appears to be unused in this file.-import 'package:tree_planting_protocol/utils/services/wallet_provider_utils.dart';
64-68
: Make the helper function static for better performance.Since the
_formatDescription
function doesn't access instance state, it should be static to avoid unnecessary object allocations.-String _formatDescription(String description) { +static String _formatDescription(String description) {android/app/src/main/AndroidManifest.xml (1)
10-10
: Remove duplicate permission declaration.
ACCESS_BACKGROUND_LOCATION
is declared twice on lines 10 and 16. Remove the duplicate to clean up the manifest.- <uses-permission android:name="android.permission.ACCESS_BACKGROUND_LOCATION" />
Also applies to: 16-16
lib/utils/services/switch_chain_utils.dart (1)
3-3
: Remove unused import.The
mint_nft_provider.dart
import appears to be unused in this file.-import 'package:tree_planting_protocol/providers/mint_nft_provider.dart';
lib/providers/mint_nft_provider.dart (1)
3-67
: Consider using a single coordinate setter.For better consistency and fewer listener notifications, consider adding a method to set both latitude and longitude together since they're often updated as a pair.
+ void setCoordinates(double latitude, double longitude) { + _latitude = latitude; + _longitude = longitude; + notifyListeners(); + }lib/pages/mint_nft/mint_nft_images.dart (3)
15-16
: Remove unnecessary ignore directiveThe
library_private_types_in_public_api
ignore is not needed for standard Flutter State class naming conventions.@override - // ignore: library_private_types_in_public_api _MultipleImageUploadPageState createState() =>
51-93
: Consider concurrent uploads for better performanceThe current implementation uploads images sequentially, which could be slow for multiple images. Consider implementing concurrent uploads with a concurrency limit.
Would you like me to implement a concurrent upload solution with a configurable concurrency limit (e.g., 3 simultaneous uploads)?
323-323
: Add const constructor if applicableIf
NewNFTMapWidget
has a const constructor, consider using it for better performance.- NewNFTMapWidget(), + const NewNFTMapWidget(),lib/widgets/tree_nft_view_details_with_map.dart (1)
174-187
: Move utility function inside the widget classThe
_formatDescription
function should be a static method of the widget class to avoid namespace pollution.Move this function inside the
_NewNFTMapWidgetState
class as a static method:-String _formatDescription(String description, double screenWidth) { + static String _formatDescription(String description, double screenWidth) {lib/components/universal_navbar.dart (3)
26-29
: Extract theme colors to constants for better maintainability.Consider moving these hardcoded color values to a theme configuration or constants file.
Create a theme constants file and use it:
// In a separate theme_constants.dart file: class NavbarTheme { static const Color darkModeBackground = Color.fromARGB(255, 1, 135, 12); static const Color lightModeBackground = Color.fromARGB(255, 28, 211, 129); } // Then use: color: themeProvider.isDarkMode ? NavbarTheme.darkModeBackground : NavbarTheme.lightModeBackground,
173-175
: Remove unnecessary spacing calculation when spacing is zero.The
plantSpacing
is hardcoded to 0.0, making the addition unnecessary.- final plantSpacing = 0.0; - final totalPlantWidth = plantWidth + plantSpacing; + final totalPlantWidth = plantWidth;
189-192
: Inconsistent image dimensions between rendering modes.Images are rendered at 28x28 in the spaced layout but 35x35 in the scrollable layout. This inconsistency could cause visual differences.
Use consistent dimensions:
// In spaced layout: - width: 28, - height: 28, + width: 32, + height: 32, // In scrollable layout: - width: 35, - height: 35, + width: 32, + height: 32,Also applies to: 216-219
lib/pages/mint_nft/mint_nft_coordinates.dart (1)
143-144
: Consider preserving field values for better UX.Clearing the text fields after submission might confuse users. Consider keeping the values visible or showing a confirmation that coordinates were saved.
Option 1: Keep the values:
- latitudeController.clear(); - longitudeController.clear();Option 2: Show saved values differently:
- latitudeController.clear(); - longitudeController.clear(); + // Show a visual indicator that values are saved + setState(() { + _locationStatus = "Coordinates saved: ${lat.toStringAsFixed(6)}, ${lng.toStringAsFixed(6)}"; + });lib/providers/wallet_provider.dart (1)
479-479
: Consider dynamic gas limit fallback based on function complexity.The hardcoded fallback of 100000 gas might be insufficient for complex contract interactions.
Consider using different defaults based on function type or allowing callers to specify a fallback:
- BigInt estimatedGas = gasLimit ?? BigInt.from(100000); + BigInt estimatedGas = gasLimit ?? BigInt.from(300000); // Higher default for safety
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
assets/tree-navbar-images/logo.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-1.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-10.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-11.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-12.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-13.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-2.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-3.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-4.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-5.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-6.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-7.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-8.png
is excluded by!**/*.png
assets/tree-navbar-images/tree-9.png
is excluded by!**/*.png
pubspec.lock
is excluded by!**/*.lock
📒 Files selected for processing (30)
.env.stencil
(1 hunks)android/app/src/main/AndroidManifest.xml
(2 hunks)lib/components/universal_navbar.dart
(2 hunks)lib/main.dart
(4 hunks)lib/models/wallet_chain_option.dart
(1 hunks)lib/models/wallet_option.dart
(0 hunks)lib/pages/home_page.dart
(1 hunks)lib/pages/mint_nft/mint_nft_coordinates.dart
(1 hunks)lib/pages/mint_nft/mint_nft_details.dart
(1 hunks)lib/pages/mint_nft/mint_nft_images.dart
(1 hunks)lib/pages/mint_nft/submit_nft_page.dart
(1 hunks)lib/pages/settings_page.dart
(1 hunks)lib/pages/trees_page.dart
(2 hunks)lib/providers/counter_provider.dart
(1 hunks)lib/providers/mint_nft_provider.dart
(1 hunks)lib/providers/wallet_provider.dart
(11 hunks)lib/utils/constants/bottom_nav_constants.dart
(1 hunks)lib/utils/constants/contractDetails.dart
(1 hunks)lib/utils/constants/contract_abis/tree_nft_contract_abi.dart
(1 hunks)lib/utils/constants/route_constants.dart
(1 hunks)lib/utils/constants/tree_images.dart
(1 hunks)lib/utils/logger.dart
(1 hunks)lib/utils/services/get_current_location.dart
(1 hunks)lib/utils/services/ipfs_services.dart
(1 hunks)lib/utils/services/switch_chain_utils.dart
(1 hunks)lib/utils/services/wallet_provider_utils.dart
(0 hunks)lib/widgets/flutter_map_widget.dart
(1 hunks)lib/widgets/tree_NFT_view_widget.dart
(1 hunks)lib/widgets/tree_nft_view_details_with_map.dart
(1 hunks)pubspec.yaml
(2 hunks)
💤 Files with no reviewable changes (2)
- lib/utils/services/wallet_provider_utils.dart
- lib/models/wallet_option.dart
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.stencil
[warning] 2-2: [UnorderedKey] The API_KEY key should go before the WALLETCONNECT_PROJECT_ID key
[warning] 3-3: [UnorderedKey] The API_SECRET key should go before the WALLETCONNECT_PROJECT_ID key
[warning] 4-4: [UnorderedKey] The ALCHEMY_API_KEY key should go before the API_KEY key
[warning] 5-5: [EndingBlankLine] No blank line at the end of the file
[warning] 5-5: [UnorderedKey] The CONTRACT_ADDRESS key should go before the WALLETCONNECT_PROJECT_ID key
🪛 YAMLlint (1.37.1)
pubspec.yaml
[error] 55-55: trailing spaces
(trailing-spaces)
🔇 Additional comments (34)
lib/pages/home_page.dart (1)
26-26
: UI spacing looks goodThe additional 16 px spacer improves visual balance without side-effects.
pubspec.yaml (2)
46-53
: LGTM - Well-chosen dependencies for new features.The new dependencies align well with the NFT minting and location features:
image_picker
for image selectionlogger
for improved debuggingdart_geohash
,flutter_map
,latlong2
for geolocation featuresgeolocator
,location
for location servicespermission_handler
for runtime permissions
81-82
: LGTM - Asset directories properly configured.The new asset directories are correctly added to support the navbar images and other app assets.
lib/providers/counter_provider.dart (1)
7-10
: LGTM - Clean increment implementation.The increment method correctly updates the count and notifies listeners.
lib/pages/trees_page.dart (1)
14-32
: LGTM - Well-structured UI update.The conversion to a column layout with centered content and proper spacing looks good. The button styling and text theme usage follow Flutter best practices.
lib/utils/constants/route_constants.dart (1)
4-6
: LGTM! Route constants follow existing patterns.The new NFT minting route constants are correctly structured and follow the established naming conventions in the file.
lib/pages/settings_page.dart (2)
1-6
: LGTM! Clean imports and good separation of concerns.The imports are well-organized and only include necessary dependencies.
7-12
: LGTM! Proper StatefulWidget structure.The widget declaration follows Flutter best practices with correct constructor and state class setup.
lib/widgets/tree_NFT_view_widget.dart (2)
6-12
: LGTM! Proper widget structure.The StatefulWidget declaration and constructor follow Flutter best practices.
15-61
: Excellent UI implementation with responsive design.The widget provides a clean, well-structured display of NFT data with appropriate styling and constraints. The use of Consumer ensures reactive updates.
lib/utils/constants/contract_abis/tree_nft_contract_abi.dart (2)
3-4
: LGTM! Contract address follows standard format.The contract address constant is properly formatted as a hexadecimal Ethereum address.
1-1
: Manual ABI Verification RequiredThe automated ABI fetch failed due to a missing
ETHERSCAN_API_KEY
. Please verify that the ABI inlib/utils/constants/contract_abis/tree_nft_contract_abi.dart
exactly matches the deployed contract at 0x6d19e13a9a0A43267467E35B5393439562fD066f to prevent runtime mismatches.• Set the
ETHERSCAN_API_KEY
environment variable and re-run the comparison script.
• Or manually copy the on-chain ABI from Etherscan (Contract → Code → ABI) and compare it against the local constant.
• Ensure there are no missing or extra function/event definitions between the local and deployed ABIs.lib/pages/mint_nft/submit_nft_page.dart (5)
1-10
: LGTM! Well-organized imports.The imports are comprehensive and properly organized for the functionality needed.
27-49
: LGTM! Excellent success dialog implementation.The success dialog provides clear visual feedback with appropriate icons and messaging.
69-84
: Good wallet validation and state management.The wallet connection check and loading state management are implemented correctly.
86-99
: Coordinate validation is comprehensive.The latitude and longitude range validation correctly checks for valid geographic coordinates.
113-119
: Contract interaction looks correct.The
writeContract
call uses appropriate parameters and follows the expected pattern for blockchain transactions.android/app/src/main/AndroidManifest.xml (1)
34-37
: LGTM: Proper foreground service configuration.The geolocator service is correctly configured as non-exported with the appropriate location service type.
lib/utils/services/switch_chain_utils.dart (2)
6-62
: Well-structured chain selector UI.The modal bottom sheet implementation is clean with proper styling and good UX patterns. The chain selection logic correctly handles the current chain highlighting and prevents unnecessary switches.
64-85
: Robust error handling in chain switching.The function properly checks
context.mounted
before showing snackbars and handles both success and error cases appropriately. The error message includes the actual error details for debugging.lib/providers/mint_nft_provider.dart (1)
13-20
: LGTM: Clean getter methods.The getter methods follow proper naming conventions and provide clean access to the private fields.
lib/models/wallet_chain_option.dart (2)
6-30
: LGTM: Well-structured WalletOption class.The WalletOption class is properly designed with required fields and optional fallback URL. The single MetaMask entry is configured correctly.
38-60
: Comprehensive chain configuration.The chain info structure is well-organized with all necessary fields for both mainnet and testnet. The block explorer URLs and native currency details are correct.
lib/pages/mint_nft/mint_nft_details.dart (4)
22-41
: Robust form validation and submission.The validation logic properly checks for empty fields and provides clear error messages. The provider integration is clean and the navigation flow is appropriate.
43-75
: Excellent custom snackbar implementation.The styled snackbar with icons and conditional coloring provides great user feedback. The floating behavior and rounded corners enhance the UX.
100-255
: Well-designed form section with consistent styling.The gradient backgrounds, shadows, and icon integration create a cohesive design. The form fields are properly styled and accessible.
376-381
: Proper resource disposal.The text controllers are correctly disposed in the dispose method to prevent memory leaks.
lib/main.dart (2)
30-30
: Good practice: Using structured loggingReplacing print statements with logger calls improves log management and debugging capabilities.
113-113
: LGTM: Provider registrationThe
MintNftProvider
is correctly registered in the provider tree, following the established pattern.lib/pages/mint_nft/mint_nft_images.dart (1)
314-322
: Verify if images are required before submissionThe "Submit NFT" button is always enabled, allowing navigation without uploaded images. Please verify if this is intentional.
If images are required, consider:
ElevatedButton( - onPressed: () { + onPressed: _uploadedHashes.isEmpty ? null : () { context.push('/mint-nft/submit-nft'); },lib/utils/services/get_current_location.dart (3)
5-20
: Well-structured error handlingExcellent implementation of custom exceptions with typed errors for different failure scenarios.
68-71
: Correct singleton implementationThe singleton pattern is properly implemented with lazy initialization.
116-128
: Elegant timeout handlingGood use of
Future.any
for implementing timeout logic with clear error messaging.lib/widgets/flutter_map_widget.dart (1)
294-298
: Proper resource cleanupGood practice: Both map widgets correctly dispose of the
MapController
to prevent memory leaks.Also applies to: 603-607
<uses-permission android:name="android.permission.INTERNET"/> | ||
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/> | ||
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/> | ||
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/> | ||
<uses-permission android:name="android.permission.VIBRATE"/> | ||
<uses-permission android:name="android.permission.WAKE_LOCK"/> | ||
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION"/> | ||
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/> | ||
<uses-permission android:name="android.permission.ACCESS_BACKGROUND_LOCATION"/> | ||
<uses-permission android:name="android.permission.CAMERA"/> | ||
<uses-permission android:name="android.permission.RECORD_AUDIO"/> | ||
<uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES"/> | ||
<uses-permission android:name="android.permission.BLUETOOTH"/> | ||
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" /> | ||
<uses-permission android:name="android.permission.ACCESS_BACKGROUND_LOCATION" /> | ||
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/> | ||
<uses-permission android:name="android.permission.POST_NOTIFICATIONS"/> |
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.
💡 Verification agent
🧩 Analysis chain
Review and justify the extensive permission set.
The app requests many high-privilege permissions including location, camera, audio recording, and external storage. Ensure each permission is actually required by the NFT minting workflow and consider requesting them at runtime when needed rather than at install time for better user experience.
Please verify that all these permissions are necessary by running this script to check their usage:
🏁 Script executed:
#!/bin/bash
# Check if permissions are actually used in the codebase
echo "Checking permission usage in Flutter code..."
# Location permissions
echo "=== Location permissions usage ==="
rg -i "location|gps|coordinates" --type dart -A 3
# Camera permission
echo "=== Camera permission usage ==="
rg -i "camera|image_picker|photo" --type dart -A 3
# Audio permission
echo "=== Audio permission usage ==="
rg -i "audio|microphone|record" --type dart -A 3
# Storage permissions
echo "=== Storage permissions usage ==="
rg -i "storage|file|external" --type dart -A 3
Length of output: 97548
Audit AndroidManifest permissions
Several permissions in android/app/src/main/AndroidManifest.xml
aren’t actually exercised by the current NFT-minting flow. Please trim the install-time permission set to only what’s used and move high-privilege requests to runtime:
• Keep
• <uses-permission android:name="android.permission.INTERNET"/>
• <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
• <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
/ <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
(required by image_picker and IPFS upload)
• <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION"/>
/ <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
(used by LocationService)
• Remove or defer to runtime
• <uses-permission android:name="android.permission.CAMERA"/>
– no camera capture calls (pickImage(source: camera)
) in code
• <uses-permission android:name="android.permission.RECORD_AUDIO"/>
– no audio recording
• <uses-permission android:name="android.permission.ACCESS_BACKGROUND_LOCATION"/>
, <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
, <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/>
– app only fetches location in-app, no background service
• <uses-permission android:name="android.permission.VIBRATE"/>
, <uses-permission android:name="android.permission.WAKE_LOCK"/>
– no vibration or wake-lock usage
• <uses-permission android:name="android.permission.BLUETOOTH"/>
, <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES"/>
, <uses-permission android:name="android.permission.POST_NOTIFICATIONS"/>
– not referenced by any Dart or native code
Locations to update:
• android/app/src/main/AndroidManifest.xml (lines 2–18) – remove unused <uses-permission>
entries
• lib/utils/services/get_current_location.dart – continue prompting for fine/coarse location at runtime when the user lands on the map screen
By stripping out unused install-time permissions and requesting only what you need when you need it, you’ll improve the user’s trust and comply with Play Store best practices.
🤖 Prompt for AI Agents
In android/app/src/main/AndroidManifest.xml lines 2 to 18, remove all
<uses-permission> entries that are not used by the current NFT-minting flow,
specifically delete CAMERA, RECORD_AUDIO, ACCESS_BACKGROUND_LOCATION,
FOREGROUND_SERVICE, FOREGROUND_SERVICE_LOCATION, VIBRATE, WAKE_LOCK, BLUETOOTH,
REQUEST_INSTALL_PACKAGES, and POST_NOTIFICATIONS permissions. Retain only
INTERNET, ACCESS_NETWORK_STATE, READ_EXTERNAL_STORAGE, WRITE_EXTERNAL_STORAGE,
ACCESS_FINE_LOCATION, and ACCESS_COARSE_LOCATION permissions. Ensure that
high-privilege permissions are requested at runtime in the code, such as in
lib/utils/services/get_current_location.dart for location permissions.
<application | ||
android:label="tree_planting_protocol" | ||
android:name="${applicationName}" | ||
android:usesCleartextTraffic="true" |
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.
Security concern: Cleartext network traffic enabled.
Enabling android:usesCleartextTraffic="true"
allows the app to use unencrypted HTTP connections, which poses security risks. This should only be enabled if absolutely necessary for development or specific API requirements.
If this is needed for development, consider using build variants:
- android:usesCleartextTraffic="true"
+ android:usesCleartextTraffic="false"
Or use network security config for specific domains if HTTP is required for certain services.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
android:usesCleartextTraffic="true" | |
android:usesCleartextTraffic="false" |
🤖 Prompt for AI Agents
In android/app/src/main/AndroidManifest.xml at line 32, the attribute
android:usesCleartextTraffic="true" enables unencrypted HTTP traffic, which is a
security risk. To fix this, remove or set this attribute to false unless
absolutely necessary. If needed for development, configure it only in debug
build variants or use a network security config XML to allow cleartext traffic
selectively for specific domains instead of globally.
@override | ||
Size get preferredSize => const Size.fromHeight(kToolbarHeight); | ||
|
||
Size get preferredSize => const Size.fromHeight(120.0); |
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.
Fix size mismatch between preferredSize and container height.
The preferredSize
returns 120.0 but the container height is set to 140. This inconsistency could cause layout issues.
Apply this diff to fix the inconsistency:
- Size get preferredSize => const Size.fromHeight(120.0);
+ Size get preferredSize => const Size.fromHeight(140.0);
Also applies to: 24-24
🤖 Prompt for AI Agents
In lib/components/universal_navbar.dart at lines 18 and 24, the preferredSize
getter returns a height of 120.0 while the container height is set to 140,
causing a size mismatch. Update the preferredSize to return
Size.fromHeight(140.0) to match the container height and ensure consistent
layout sizing.
Container( | ||
width: 10, | ||
child: Flexible( | ||
child: Text( | ||
formatAddress(walletProvider.currentAddress!), | ||
style: TextStyle( | ||
color: Colors.green[700], | ||
fontWeight: FontWeight.w600, | ||
fontSize: 10, | ||
), | ||
overflow: TextOverflow.ellipsis, | ||
), | ||
), | ||
), |
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.
Fix incorrect usage of Flexible widget inside fixed-width Container.
The Flexible widget is wrapped in a Container with width: 10, which defeats its purpose and will cause text overflow issues.
Remove the Container wrapper:
- Container(
- width: 10,
- child: Flexible(
- child: Text(
- formatAddress(walletProvider.currentAddress!),
- style: TextStyle(
- color: Colors.green[700],
- fontWeight: FontWeight.w600,
- fontSize: 10,
- ),
- overflow: TextOverflow.ellipsis,
- ),
- ),
- ),
+ Flexible(
+ child: Text(
+ formatAddress(walletProvider.currentAddress!),
+ style: TextStyle(
+ color: Colors.green[700],
+ fontWeight: FontWeight.w600,
+ fontSize: 10,
+ ),
+ overflow: TextOverflow.ellipsis,
+ ),
+ ),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Container( | |
width: 10, | |
child: Flexible( | |
child: Text( | |
formatAddress(walletProvider.currentAddress!), | |
style: TextStyle( | |
color: Colors.green[700], | |
fontWeight: FontWeight.w600, | |
fontSize: 10, | |
), | |
overflow: TextOverflow.ellipsis, | |
), | |
), | |
), | |
Flexible( | |
child: Text( | |
formatAddress(walletProvider.currentAddress!), | |
style: TextStyle( | |
color: Colors.green[700], | |
fontWeight: FontWeight.w600, | |
fontSize: 10, | |
), | |
overflow: TextOverflow.ellipsis, | |
), | |
), |
🤖 Prompt for AI Agents
In lib/components/universal_navbar.dart around lines 269 to 282, the Flexible
widget is incorrectly wrapped inside a Container with a fixed width of 10, which
restricts its flexibility and causes text overflow issues. To fix this, remove
the Container wrapper entirely and let the Flexible widget manage the available
space for the Text widget properly.
lib/main.dart
Outdated
static GlobalKey<NavigatorState> navigatorKey = GlobalKey<NavigatorState>(); | ||
} | ||
|
||
final contractAddress = "0xa122109493B90e322824c3444ed8D6236CAbAB7C"; |
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.
Move contract address to environment configuration
Hardcoding the contract address is problematic for multi-environment deployments and security. Contract addresses typically differ between development, staging, and production environments.
-final contractAddress = "0xa122109493B90e322824c3444ed8D6236CAbAB7C";
+final contractAddress = dotenv.env['CONTRACT_ADDRESS'] ?? '';
Also update .env.stencil
to include:
CONTRACT_ADDRESS=0xa122109493B90e322824c3444ed8D6236CAbAB7C
🤖 Prompt for AI Agents
In lib/main.dart at line 25, the contract address is hardcoded, which is not
suitable for multi-environment setups. Remove the hardcoded address and instead
load the contract address from an environment configuration variable. Also,
update the .env.stencil file to include CONTRACT_ADDRESS with the current
address value to support environment-based configuration.
double _sanitizeCoordinate(double value, double defaultValue) { | ||
if (value.isNaN || value.isInfinite || value == double.infinity || value == double.negativeInfinity) { | ||
print('Invalid coordinate detected: $value, using default: $defaultValue'); | ||
return defaultValue; |
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.
🛠️ Refactor suggestion
Replace print statements with logger
Debug print statements should use the logger utility for consistency with the rest of the codebase.
- print('Invalid coordinate detected: $value, using default: $defaultValue');
+ logger.w('Invalid coordinate detected: $value, using default: $defaultValue');
- // Debug print to see what values we're getting
- print('StaticDisplayMap - lat: $latitude, lng: $longitude');
- print('Widget.lat: ${widget.lat}, Widget.lng: ${widget.lng}');
+ logger.d('StaticDisplayMap - lat: $latitude, lng: $longitude');
+ logger.d('Widget.lat: ${widget.lat}, Widget.lng: ${widget.lng}');
Also add the import:
import 'package:tree_planting_protocol/utils/logger.dart';
Also applies to: 342-343
🤖 Prompt for AI Agents
In lib/widgets/flutter_map_widget.dart at lines 329 and 342-343, replace all
print statements with calls to the logger utility to maintain consistency with
the codebase's logging approach. Also, add the import statement "import
'package:tree_planting_protocol/utils/logger.dart';" at the top of the file to
enable the logger usage.
lat: Provider.of<MintNftProvider>(context).getLatitude().toDouble(), | ||
lng: Provider.of<MintNftProvider>(context).getLongitude().toDouble(), | ||
), |
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.
Handle potential null coordinates safely
The .toDouble()
call will throw if coordinates are null. Add null safety checks.
child: StaticDisplayMap(
- lat: Provider.of<MintNftProvider>(context).getLatitude().toDouble(),
- lng: Provider.of<MintNftProvider>(context).getLongitude().toDouble(),
+ lat: Provider.of<MintNftProvider>(context).getLatitude()?.toDouble() ?? 0.0,
+ lng: Provider.of<MintNftProvider>(context).getLongitude()?.toDouble() ?? 0.0,
),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
lat: Provider.of<MintNftProvider>(context).getLatitude().toDouble(), | |
lng: Provider.of<MintNftProvider>(context).getLongitude().toDouble(), | |
), | |
child: StaticDisplayMap( | |
lat: Provider.of<MintNftProvider>(context).getLatitude()?.toDouble() ?? 0.0, | |
lng: Provider.of<MintNftProvider>(context).getLongitude()?.toDouble() ?? 0.0, | |
), |
🤖 Prompt for AI Agents
In lib/widgets/tree_nft_view_details_with_map.dart around lines 48 to 50, the
calls to .toDouble() on latitude and longitude can throw exceptions if the
coordinates are null. Modify the code to first check if the coordinates are null
and provide a safe fallback value (e.g., 0.0) before calling .toDouble(),
ensuring null safety and preventing runtime errors.
child: Image.network( | ||
provider.getInitialPhotos()[index], | ||
height: 100, | ||
width: double.infinity, | ||
fit: BoxFit.cover, | ||
), |
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.
🛠️ Refactor suggestion
Add error handling for network images
Network images should handle loading and error states gracefully.
child: Image.network(
provider.getInitialPhotos()[index],
height: 100,
width: double.infinity,
fit: BoxFit.cover,
+ errorBuilder: (context, error, stackTrace) {
+ return Container(
+ height: 100,
+ color: Colors.grey[300],
+ child: const Icon(Icons.broken_image, color: Colors.grey),
+ );
+ },
+ loadingBuilder: (context, child, loadingProgress) {
+ if (loadingProgress == null) return child;
+ return Container(
+ height: 100,
+ child: Center(
+ child: CircularProgressIndicator(
+ value: loadingProgress.expectedTotalBytes != null
+ ? loadingProgress.cumulativeBytesLoaded /
+ loadingProgress.expectedTotalBytes!
+ : null,
+ ),
+ ),
+ );
+ },
),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
child: Image.network( | |
provider.getInitialPhotos()[index], | |
height: 100, | |
width: double.infinity, | |
fit: BoxFit.cover, | |
), | |
child: Image.network( | |
provider.getInitialPhotos()[index], | |
height: 100, | |
width: double.infinity, | |
fit: BoxFit.cover, | |
errorBuilder: (context, error, stackTrace) { | |
return Container( | |
height: 100, | |
width: double.infinity, | |
color: Colors.grey[300], | |
child: const Icon(Icons.broken_image, color: Colors.grey), | |
); | |
}, | |
loadingBuilder: (context, child, loadingProgress) { | |
if (loadingProgress == null) return child; | |
return Container( | |
height: 100, | |
width: double.infinity, | |
child: Center( | |
child: CircularProgressIndicator( | |
value: loadingProgress.expectedTotalBytes != null | |
? loadingProgress.cumulativeBytesLoaded / | |
loadingProgress.expectedTotalBytes! | |
: null, | |
), | |
), | |
); | |
}, | |
), |
🤖 Prompt for AI Agents
In lib/widgets/tree_nft_view_details_with_map.dart around lines 117 to 122, the
Image.network widget lacks error handling and loading state management. Update
the Image.network call to include the loadingBuilder parameter to show a
placeholder or progress indicator while the image loads, and the errorBuilder
parameter to display an error widget or fallback image if the network image
fails to load. This will ensure graceful handling of loading and error states
for network images.
provider: ^6.1.5 | ||
shared_preferences: ^2.2.2 | ||
go_router: ^15.2.4 | ||
go_router: ^16.0.0 |
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.
💡 Verification agent
🧩 Analysis chain
Verify go_router major version upgrade compatibility.
The upgrade from ^15.2.4
to ^16.0.0
is a major version change that may introduce breaking changes. Ensure all existing routing code is compatible with the new version.
🏁 Script executed:
#!/bin/bash
# Check for potential go_router breaking changes in the codebase
echo "Searching for go_router usage patterns that might be affected by v16 changes..."
rg -A 3 -B 1 "context\.(go|push|pop|replace)" --type dart
rg -A 3 -B 1 "GoRouter\(" --type dart
Length of output: 2693
🌐 Web query:
GoRouter 16.0.0 release notes breaking changes context.go context.push migration guide
💡 Result:
GoRouter 16.0.0 introduces breaking changes centered on the migration to a new, more type-safe route API using GoRouteData
and its navigation helpers like .go(context)
and .push(context)
—requiring adjustments for users of the older context.go
and context.push
patterns[1][2][4].
Key Breaking Changes in GoRouter 16.0.0
- Type-Safe Routes:
GoRouteData
now defines navigation helpers:.location
,.go(context)
,.push(context)
,.pushReplacement(context)
, and.replace(context)
, intended to be used for all type-safe routing. - Deprecated: Direct usage of
context.go
andcontext.push
is deprecated or discouraged for type-safe routes; you should migrate to the new.go(context)
and.push(context)
APIs defined on your route data classes[1][2]. - Dependency Upgrade: This upgrade requires
go_router_builder >= 3.0.0
[1][2].
Implications for Migration
-
If you previously used navigation like:
context.go('/path'); context.push('/path');
With type-safe routing and GoRouter 16, you now do:
MyRouteData().go(context); MyRouteData().push(context);
Here,
MyRouteData
is a generated class representing your route, and these methods are generated for you bygo_router_builder
[1][2]. -
Older Usage:
- You could navigate imperatively without any generated helpers.
-
Migrated Usage:
- Navigation is through the methods provided by your strongly typed route data classes.
Before 16.0.0 | After 16.0.0 (Migrated Type-safe) |
---|---|
context.go('/details/1') |
DetailsRoute(id: 1).go(context) |
context.push('/home') |
HomeRoute().push(context) |
Additional Breaking Change Notes
- The 16.0.0 update enforces that URLs with different cases (e.g.,
/Home
vs/home
) are treated as distinct routes[1][2]. - You need to ensure all code aligns with this higher level of type-safety, and you must use the
go_router_builder
generator.
Migration Guide Location
Direct migration guides for major releases, including 16.0.0, are provided in the go_router documentation[4] (but the specific migration steps above summarize the core updates you’ll need).
Contextual Details
- Why migrate?: The new API allows for compile-time route validation, auto-completion, and reduces possible runtime navigation errors.
- Partial backward compatibility: For non-type-safe (string based) navigation, the old imperative methods may still exist, but they are no longer the recommended approach.
To migrate:
- Update your route definitions and navigation calls to use generated
GoRouteData
classes and their methods. - Run code generation with
go_router_builder >= 3.0.0
after updating your route annotations.
If you have a large codebase using context.go/string
, this may require a significant but largely mechanical refactor to the new API.
For further reading or specific migration steps, consult the Migration Guide linked in the GoRouter documentation on pub.dev[4].
Summary:
Update to GoRouter 16.0.0 by migrating away from context.go
/context.push
for type-safe routes, adopting the new .go(context)
/.push(context)
methods generated on your route data classes, and ensure you are using at least go_router_builder
3.0.0[1][2][4].
Citations:
- 1: https://pub.dev/packages/go_router/changelog
- 2: https://github.com/flutter/packages/blob/main/packages/go_router/CHANGELOG.md?plain=1
- 3: https://codewithandrea.com/articles/flutter-navigation-gorouter-go-vs-push/
- 4: https://pub.dev/packages/go_router
- 5: [go_router] GoRouterState incorrect for ShellRoute on .push and .pop flutter/flutter#125752
- 6: https://files.flutter-io.cn/flutter-design-docs/%5BDraft%5DGo_Router%205.0%20migration%20guide%20(PUBLICLY%20SHARED).docx
- 7: https://go.libhunt.com/gorouter-changelog
- 8: https://docs.flutter.dev/release/breaking-changes
- 9: [go_router_builder][go_router]
.location
,.go(context)
and.push(context)
are only defined in the generated extension flutter/flutter#106790 - 10: https://www.youtube.com/watch?v=-RBckaDcAq4
Migrate routing calls to GoRouter 16’s type-safe API
GoRouter 16.0.0 deprecates the old context.go('/…')
/context.push('/…')
pattern in favor of generated GoRouteData
classes with .go(context)
/.push(context)
helpers. It also requires go_router_builder >= 3.0.0
and a build step to generate your route data classes. Please update your code or explicitly confirm you’re OK continuing with string-based navigation (which is still supported but no longer recommended).
Affected files and locations:
- lib/main.dart
• Ensure you’ve addedgo_router_builder: ^3.0.0
underdev_dependencies
and are runningbuild_runner
. - lib/pages/trees_page.dart
•context.push('/mint-nft');
- lib/components/bottom_navigation_widget.dart
•context.go(route);
- lib/pages/home_page.dart
•context.push(RouteConstants.allTreesPath);
- lib/pages/mint_nft/mint_nft_details.dart
•context.push(RouteConstants.mintNftImagesPath);
- lib/pages/mint_nft/mint_nft_images.dart
•context.push('/mint-nft/submit-nft');
- lib/pages/mint_nft/mint_nft_coordinates.dart
•context.push(RouteConstants.mintNftDetailsPath);
Example migration:
- context.go('/details/1');
+ DetailsRoute(id: 1).go(context);
Pubspec.yaml additions:
dev_dependencies:
go_router_builder: ^3.0.0
Then run:
flutter pub get
flutter pub run build_runner build --delete-conflicting-outputs
🤖 Prompt for AI Agents
In pubspec.yaml at line 43, add go_router_builder version 3.0.0 or higher under
dev_dependencies to support GoRouter 16's type-safe API. Then, update all
routing calls in the listed Dart files to use the generated GoRouteData classes
with their .go(context) or .push(context) methods instead of string-based
navigation. Finally, run flutter pub get and flutter pub run build_runner build
--delete-conflicting-outputs to generate the necessary route data classes.
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: 1
♻️ Duplicate comments (2)
lib/widgets/flutter_map_widget.dart (1)
11-11
: Remove unused constructor parametersThe
lat
andlng
parameters are required in the constructor but never used. The widget always uses values from the provider instead.lib/pages/mint_nft/mint_nft_coordinates.dart (1)
539-541
: Optimize Provider usage to prevent unnecessary rebuildsUsing
Provider.of
in the build method without specifyinglisten
parameter causes rebuilds on any provider change.
🧹 Nitpick comments (2)
lib/widgets/flutter_map_widget.dart (1)
105-105
: Consider increasing the initial zoom levelAn initial zoom of 1.0 shows the entire world, requiring users to zoom in significantly to select a location. Consider starting with a zoom level of 10-15 for better UX.
- initialZoom: 1.0, + initialZoom: 13.0,lib/pages/mint_nft/mint_nft_coordinates.dart (1)
143-144
: Consider keeping coordinates visible after submissionClearing the text fields after submission removes visual feedback. Users might want to verify or edit the coordinates later.
- latitudeController.clear(); - longitudeController.clear(); + // Keep the coordinates visible for user reference + // latitudeController.clear(); + // longitudeController.clear();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/main.dart
(4 hunks)lib/pages/mint_nft/mint_nft_coordinates.dart
(1 hunks)lib/widgets/flutter_map_widget.dart
(1 hunks)lib/widgets/tree_NFT_view_widget.dart
(1 hunks)lib/widgets/tree_nft_view_details_with_map.dart
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/widgets/tree_nft_view_details_with_map.dart
- lib/widgets/tree_NFT_view_widget.dart
- lib/main.dart
void _startLocationUpdates() { | ||
_locationTimer = Timer.periodic(const Duration(seconds: 5), (timer) { | ||
if (!_userHasManuallySetCoordinates) { | ||
_getCurrentLocation(); | ||
} | ||
}); | ||
} |
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.
🛠️ Refactor suggestion
Consider optimizing location polling frequency
Polling location every 5 seconds can drain battery. Consider using location change listeners or stopping updates after the initial location is obtained.
void _startLocationUpdates() {
- _locationTimer = Timer.periodic(const Duration(seconds: 5), (timer) {
+ _locationTimer = Timer.periodic(const Duration(seconds: 30), (timer) {
if (!_userHasManuallySetCoordinates) {
_getCurrentLocation();
}
});
}
Alternatively, consider stopping the timer after initial location:
if (_isInitialLocationSet) {
_locationTimer?.cancel();
}
🤖 Prompt for AI Agents
In lib/pages/mint_nft/mint_nft_coordinates.dart around lines 58 to 64, the
location polling every 5 seconds can cause unnecessary battery drain. Modify the
code to stop the timer after the initial location is obtained by checking a flag
like _isInitialLocationSet and canceling _locationTimer accordingly.
Alternatively, replace the periodic timer with a location change listener to
update coordinates only when the location actually changes.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
This pull request introduces several key features and improvements to the application:
Demo video showcasing the updates:
https://drive.google.com/file/d/1Di5rIMc_TM-OrNRUAOhgY179WMcAa6hR/view?usp=sharing
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores