fix: delegate IpfsGatewayManager disposal to root provider#3047
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a resource management issue by moving the disposal responsibility of IpfsGatewayManager from individual bloc instances to the root level. This ensures proper lifecycle management and prevents potential memory leaks.
- Removes manual disposal of
IpfsGatewayManagerfromNftImageBloc.close() - Adds automatic disposal handling at the root level via
RepositoryProvider
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/bloc/nft_image/nft_image_bloc.dart | Removes manual disposal call and fixes code formatting |
| lib/bloc/app_bloc_root.dart | Adds dispose callback to RepositoryProvider for IpfsGatewayManager |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix impossible nested condition logic * handle uninitialized logger exceptions gratefully * caches `typeName` to avoid unneccesary `getCoinTypeName` calls * adds caching for `isParent` * set parents as native; use cached coin values * special case to cover ARB * chore: roll SDK (#3036) Roll SDK for critical KMD API pricing fix * chore: remove FEEDBACK_TEST_URL references * fix(ui): close dropdown on window resize * Fix null safety issues in UiDropdown widget Co-authored-by: charl <charl@vanstaden.info> * fix(build): correct env vars passing to Docker and Dart via --dart-define (#3037) * refactor: simplify Docker env vars and build command logic - Remove unnecessary environment variables from Docker container - Keep only GITHUB_API_PUBLIC_READONLY_TOKEN as env var - Replace hardcoded build logic with conditional credential handling - Add proper validation for Trello and Cloudflare feedback services - Use dart-define flags instead of environment variables for build configs - Add warning messages for incomplete credential sets * fix executable rights on build.sh * remove the redundant FEEDBACK_TEST_URL, as it will no longer be used in the GUI * swap BUILD_COMMAND and BUILD_CMD (important) BUILD_CMD, which is built dynamically with --dart-define= arguments, should be called last (not during the assets-fetch stage) to ensure the app is actually built with the required defines and that all String.fromEnvironment(...) statements in the Dart code work as expected. * fix(nft): add IPFS gateway resolution, retry, and fallback to improve NFT image loading (#3020) * feat(nft): tolerate activation failures * fix(nft): skip missing parents in firstWhere Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-main-repo): fix parent coin filter and use logging package * fix(nft): don't return an error if there are no NFTs returned * fix(nft): revert map-based error responses in favor of manual activation * fix(skeleton-list-tile): add height constraint to fix renderflex assert * fix(coin): temporarily skip calling deactivation RPC circumvent "NoSuchCoin" error when re-enabling the coin in the same session. * fix(activation): use more complete coin conversion function * feat(nft): add nft image bloc for url resolution, fallbacks and retry * fix(ipfs-gateway-manager): move gateway match above subdomain gateway pattern matches subdomain due to loose alphanumeric requirement * test: add unit tests for ipfs url normaliser and fix case sensitivity * refactor(nft-image): remove dead code & move url testing out of bloc * fix(nft): delegate IpfsGatewayManager disposal to root provider (#3047) * fix: delegate IpfsGatewayManager disposal to root provider * refactor: remove unnecessary line break Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(nft): reset NFT image on url change (#3046) * fix: reset NFT image on url change * refactor(nft-image): use ValueKey instead of Key Using Key(imageUrl!) creates a new Key object on every build. Consider using ValueKey(imageUrl!) instead, which is more semantically appropriate for value-based keys and may have better performance characteristics. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(nft-image): use ValueKey instead of Key Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * style: apply updated dart formatting to modified files * fix(coins): filter excluded and testnet coins out of price updates * fix(nft-image): reduce the number of success events fired --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(build): add --no-web-resources-cdn to the web build in build.sh (#3055) * Remove .dgph build artifacts from iOS project (#3058) * remove dgph binaries * Ignore .dgph build artifacts in iOS * fix arb parent ticker * include qtum parent * chore(sdk): bump submodule to 170aab4 for parent display name suffix Pulls in AssetId.displayName and CoinSubClass.tokenStandardSuffix * feat(ui): disambiguate parent chain names via SDK displayName - Use AssetId.displayName for top-level assets (SDK)\n- Update UI to show e.g. Ethereum (ARB20) for parents\n- Keep protocol pill consistent using coin.typeName (NATIVE for parents)\n- Search compares against displayName\n\nRefs: #2988 (comment) --------- Co-authored-by: Charl (Nitride) <77973576+CharlVS@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: charl <charl@vanstaden.info> Co-authored-by: DeckerSU <deckersu@protonmail.com> Co-authored-by: Francois <takenagain@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Summary
IpfsGatewayManagerinsideNftImageBlocIpfsGatewayManagerat root level viaRepositoryProviderTesting
flutter pub get --enforce-lockfiledart format .flutter analyzehttps://chatgpt.com/codex/tasks/task_e_689083342bac83318e794472240dba9f