Conversation
chore: sync hot-fixes to `dev`
…tivation checks - Refactor activation logic in ActivationManager and SmartAssetActivator to utilize ActivatedAssetsCache, reducing direct RPC calls and improving performance. - Update activation strategy factory to include ActivatedAssetsCache as a parameter. - Increase cache TTL from 2 seconds to 10 seconds for better efficiency in asset activation status checks.
…rums fix(config): add ssl-only transform for native platforms
…tivation checks (#282) - Refactor activation logic in ActivationManager and SmartAssetActivator to utilize ActivatedAssetsCache, reducing direct RPC calls and improving performance. - Update activation strategy factory to include ActivatedAssetsCache as a parameter. - Increase cache TTL from 2 seconds to 10 seconds for better efficiency in asset activation status checks.
…r-2683 update build config to use kdf_1874d43
use kdf dev branch ci build
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughThis PR adds SSL/HTTPS security filtering for electrum and node servers, integrates an ActivatedAssetsCache dependency throughout the activation pipeline, refines host normalization logic, updates build configurations with new checksums and API commit hashes, and increases the activated assets cache TTL from 2 to 10 seconds. Changes
Sequence Diagram(s)sequenceDiagram
participant AM as ActivationManager
participant Cache as ActivatedAssetsCache
participant Factory as ActivationStrategyFactory
participant Activator as SmartAssetActivator
Note over AM,Cache: New Cache-Based Activation Flow
AM->>Cache: getActivatedAssetIds()
Cache-->>AM: activated asset IDs (cached)
AM->>Factory: createStrategy(..., cache)
Factory->>Activator: new SmartAssetActivator(..., cache)
Activator-->>Factory: activator instance
AM->>Activator: activate asset
Activator->>Cache: check isActive (containment check)
Cache-->>Activator: activation status
Activator-->>AM: activation complete
AM->>Cache: invalidate()
Note over Cache: Cache cleared for next check
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
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. Comment |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR syncs main with version 0.9.3, introducing performance optimizations to reduce excessive RPC calls through caching, security improvements for non-web platforms, and Android-specific DNS resolution fixes.
- Increased activated assets cache TTL from 2 to 10 seconds to reduce unnecessary RPC requests
- Added new
SslElectrumTransformto filter insecure connections on non-web platforms - Fixed localhost DNS resolution issues on Android by normalizing to 127.0.0.1
- Updated API binaries and coin configurations to latest versions
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/komodo_defi_sdk/lib/src/sdk/komodo_defi_sdk_config.dart | Increased cache TTL from 2 to 10 seconds for activated assets |
| packages/komodo_defi_sdk/lib/src/activation/base_strategies/activation_strategy_factory.dart | Added activatedAssetsCache parameter to strategy factory |
| packages/komodo_defi_sdk/lib/src/activation/base_strategies/activation_strategy_base.dart | Updated SmartAssetActivator to use cache instead of direct RPC calls for activation checks |
| packages/komodo_defi_sdk/lib/src/activation/activation_manager.dart | Refactored to use cached asset activation status instead of direct RPC calls |
| packages/komodo_defi_framework/lib/src/operations/kdf_operations_remote.dart | Fixed localhost normalization to use exact matching instead of substring contains |
| packages/komodo_defi_framework/app_build/build_config.json | Updated API commit hash, platform checksums, coins repo commit, and build configuration |
| packages/komodo_coin_updates/lib/src/coins_config/config_transform.dart | Added SslElectrumTransform to filter non-SSL/HTTPS connections on non-web platforms |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final electrum = config.valueOrNull<JsonList>('electrum'); | ||
| if (electrum != null) { | ||
| final originalCount = electrum.length; | ||
| final filteredElectrums = electrum.where((JsonMap e) { |
There was a problem hiding this comment.
The where clause explicitly casts list elements to JsonMap without validation. If the electrum list contains non-map elements, this will cause a runtime type error. Consider using safer type checking:
final filteredElectrums = electrum.where((e) {
if (e is! Map<String, dynamic>) return false;
final protocol = (e as JsonMap).valueOrNull<String>('protocol');
return protocol == 'SSL';
}).cast<JsonMap>().toList();The same issue exists on line 265 for rpcNodes.
Comment on lines
+265
to
+268
| final filteredRpcNodes = rpcNodes.where((JsonMap node) { | ||
| final url = node.valueOrNull<String>('url'); | ||
| return url != null && url.startsWith('https://'); | ||
| }).toList(); |
There was a problem hiding this comment.
Similar to the electrum filtering issue above, the where clause explicitly casts list elements to JsonMap without validation. If the nodes list contains non-map elements, this will cause a runtime type error. Consider using safer type checking:
final filteredRpcNodes = rpcNodes.where((node) {
if (node is! Map<String, dynamic>) return false;
final url = (node as JsonMap).valueOrNull<String>('url');
return url != null && url.startsWith('https://');
}).cast<JsonMap>().toList();
Suggested change
| final filteredRpcNodes = rpcNodes.where((JsonMap node) { | |
| final url = node.valueOrNull<String>('url'); | |
| return url != null && url.startsWith('https://'); | |
| }).toList(); | |
| final filteredRpcNodes = rpcNodes.where((node) { | |
| if (node is! Map<String, dynamic>) return false; | |
| final url = (node as JsonMap).valueOrNull<String>('url'); | |
| return url != null && url.startsWith('https://'); | |
| }).cast<JsonMap>().toList(); |
Contributor
|
Visit the preview URL for this PR (updated for commit 5a7050f): https://kdf-sdk--pr292-dev-c5h9v2g2.web.app (expires Thu, 20 Nov 2025 10:45:11 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9c1b6e6c010cf0b965c455ba7a69c4aedafa8a1d |
Contributor
|
Visit the preview URL for this PR (updated for commit 5a7050f): https://komodo-playground--pr292-dev-ybun53a5.web.app (expires Thu, 20 Nov 2025 10:45:16 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sync main
Summary by CodeRabbit
New Features
Bug Fixes
Chores