Activation task deduplication and SharedActivationCoordinator enhancements#274
Activation task deduplication and SharedActivationCoordinator enhancements#274
Conversation
|
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 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. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Stream<ActivationProgress> activateAssets(List<Asset> assets) { | ||
| final coordinator = _activationCoordinator?.call(); | ||
| if (coordinator == null) { | ||
| return _activationManager().activateAssets(assets); | ||
| } | ||
|
|
||
| final controller = StreamController<ActivationProgress>.broadcast(); | ||
|
|
||
| scheduleMicrotask(() { | ||
| for (final asset in assets) { | ||
| coordinator | ||
| .activateAssetStream(asset) | ||
| .listen(controller.add, onError: controller.addError); | ||
| } | ||
| }); | ||
|
|
||
| return controller.stream; |
There was a problem hiding this comment.
The new coordinator-backed activateAssets path builds a broadcast controller but never closes it when the underlying per‑asset streams finish. Consumers waiting on the returned stream to complete (e.g. await manager.activateAssets([...]).last or using await for expecting the loop to end) will now hang indefinitely and keep subscriptions alive, whereas the previous ActivationManager.activateAssets stream finished once all activations completed. Consider closing the controller once all child streams emit a terminal event.
Useful? React with 👍 / 👎.
|
Visit the preview URL for this PR (updated for commit 631cf0e): https://komodo-playground--pr274-deduplicate-activati-7rj38wzj.web.app (expires Fri, 07 Nov 2025 11:33:18 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47 |
2 participants
closes #273
Summary
AssetIdacross the SDK by routing all public activation calls throughSharedActivationCoordinatorand makingActivationManageridempotent at init.Key changes
activateIfNeeded=falsebut activation is pending, await coordinator activation to avoid racing wallet-account creation and pubkeys.closure invoked recursively or after being droppederrors.SharedActivationCoordinatorintoAssetManagerto make it the single entry path.Rationale (maps to #273 goals)
AssetIdand shared result across call sites.Files touched
packages/komodo_defi_sdk/lib/src/assets/asset_manager.dartpackages/komodo_defi_sdk/lib/src/activation/activation_manager.dartpackages/komodo_defi_sdk/lib/src/balances/balance_manager.dartpackages/komodo_defi_framework/lib/src/streaming/event_streaming_service.dartpackages/komodo_defi_sdk/lib/src/bootstrap.dartBehavior notes
Futurepath waits for coin availability before success; stream path exposes raw progress and closes on completion (unchanged semantics).ActivationManagerusage remains internal; public APIs now converge via coordinator.Testing plan
Follow-ups (tracked separately)
SharedActivationCoordinatorenhancements #273.Acceptance criteria