Conversation
…atform/dev chore(release): v1.0.0-pre.1
Co-authored-by: charl <charl@vanstaden.info>
* Refactor ZHTLC activation strategy with stream-based task handling Co-authored-by: charl <charl@vanstaden.info> * Refactor activation steps to use strongly-typed ActivationStep enum Co-authored-by: charl <charl@vanstaden.info> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…dk-flutter into cursor/integrate-zhtlc-types-and-rpcs-4654 # Conflicts: # packages/dragon_logs/.gitignore # packages/dragon_logs/example/.gitignore # packages/dragon_logs/example/android/.gitignore # packages/dragon_logs/example/ios/.gitignore # packages/dragon_logs/example/macos/.gitignore # packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/activation_params.dart # packages/komodo_defi_rpc_methods/lib/src/rpc_methods_library.dart # packages/komodo_defi_sdk/example/pubspec.lock # packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/qtum_activation_strategy.dart # packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_activation_strategy.dart # packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/utxo_activation_strategy.dart # packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart # playground/pubspec.lock # products/dex_dungeon/pubspec.lock # products/komodo_compliance_console/pubspec.lock
…212) Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Clean base/subclass separation, canonical serialization + encoder strategy, user configuration + persistence, BLoC timeout, SDK integration via AssetId extension and resolveActivationParams, and migration steps. No backwards compatibility required.
…ionParams; add ActivationConfigService; unify priv_key_policy serialization; normalize toRpcParams; formatting fixes
Co-authored-by: charl <charl@vanstaden.info>
Co-authored-by: charl <charl@vanstaden.info>
Co-authored-by: charl <charl@vanstaden.info>
- Add Firebase hosting configuration for packages/komodo_defi_sdk/example - Configure to use kdf-sdk hosting site on komodo-defi-sdk project - Set up hosting target mapping - Add Firebase hosting configuration for playground - Configure to use default hosting site on komodo-playground project - Update GitHub Actions workflows to use target parameter for SDK example - Update .gitignore to allow Firebase config files in example and playground
- Update zhtlc_activation_strategy for improved activation handling - Add activation config service enhancements - Update logged_in_view_widget with improved UI for instance management - Export additional SDK functionality in public API
- Move Firebase scripts to .github/scripts/firebase/ directory - Rename scripts for clarity: - setup-firebase-secrets.sh → setup-github-secrets.sh - verify-firebase-secrets.sh → verify-github-secrets.sh - Move documentation to docs/firebase/ directory - Rename documentation file to firebase-deployment-setup.md - Update script paths in documentation - Add README for scripts directory
- Remove global GetIt.I registration to prevent conflicts between multiple SDK instances - Pass ActivationConfigService through constructor chain instead of global resolution - Update ZhtlcActivationStrategy to accept service via constructor - Update ActivationStrategyFactory and ActivationManager to propagate the service - Prevent memory leaks and use-after-dispose issues when SDK instances are disposed This ensures each SDK instance maintains its own isolated ActivationConfigService, fixing issues where only the first SDK's service would be globally registered.
…dk-flutter into cursor/integrate-zhtlc-types-and-rpcs-4654 # Conflicts: # packages/komodo_defi_sdk/lib/src/komodo_defi_sdk.dart
- Ensure service is available before ActivationManager to fix ZHTLC/Z-coin activation flow\n- Remove duplicate registration from KomodoDefiSdk.init\n- Example: use instance sdk instead of context.read for ZHTLC prompt
…te in activation flow; update bootstrap, activation service, and example widget
…7191935]] Resolved conflicts in: - packages/komodo_defi_sdk/lib/src/komodo_defi_sdk.dart - packages/komodo_defi_sdk/lib/src/bootstrap.dart - packages/komodo_defi_sdk/lib/src/activation/activation_manager.dart Key changes: - Integrated KomodoAssetsUpdateManager alongside ActivationConfigService - Added conditional ActivationConfigService registration in SDK initialization - Updated ActivationManager to use both config service and assets update manager - Merged custom token handling improvements
- Fix native callback memory leak by disposing _kdfOperations in KomodoDefiFramework - Remove debug-only conditional logging in bootstrap.dart - Fix potential double subscription in komodo_defi_sdk.dart - Add comprehensive error handling to log stream listeners - Replace empty catch blocks with proper error handling - Add safe logging helpers to prevent callback exceptions from crashing - Ensure log streams continue operating even after callback errors
…dermint flows - add _legacyManager constructor param and field - delegate Tendermint preview/withdraw to injected manager - minor formatting and watch() simplifications
|
Visit the preview URL for this PR (updated for commit aff12a8): https://kdf-sdk--pr227-bugfix-zhltc-activat-z4cxh5nn.web.app (expires Thu, 09 Oct 2025 15:59:41 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9c1b6e6c010cf0b965c455ba7a69c4aedafa8a1d |
|
Visit the preview URL for this PR (updated for commit aff12a8): https://komodo-playground--pr227-bugfix-zhltc-activat-jj7t2kz5.web.app (expires Thu, 09 Oct 2025 15:59:39 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47 |
CharlVS
changed the base branch from
cursor/investigate-and-fix-z-coin-activation-requests-cdce
to
dev
* feat(sdk): add zhltc params file downloader * feat(sdk-example): integrate zcash downloader for desktop * chore: remove groth from default params download list * test(komodo-defi-sdk): fix failing zcash unit tests * refactor: clean up imports, tests, and dialog date conversion * feat(zhtlc): add mobile config params downloader
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements ZHTLC activation fixes and enhancements to the Komodo DeFi SDK, primarily focused on improving the activation process for ZHTLC protocol assets with better configuration management and user interaction.
- Introduces a comprehensive ZHTLC activation configuration service with Hive-based persistence
- Adds ZCash parameter downloader system with platform-specific implementations for cross-platform support
- Refactors activation progress tracking with standardized step enums and UI signals
- Integrates activation configuration service into the SDK bootstrap process
Reviewed Changes
Copilot reviewed 99 out of 106 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
playground/firebase.json |
Firebase hosting configuration for playground deployment |
playground/.firebaserc |
Firebase project configuration for playground |
packages/komodo_defi_types/lib/src/protocols/zhtlc/zhtlc_protocol.dart |
Updated ZHTLC protocol validation to support both electrum and light wallet servers |
packages/komodo_defi_types/lib/src/activation/activation_progress.dart |
Added structured activation steps enum and UI signals for better progress tracking |
| Multiple ZCash params test files | Comprehensive test coverage for ZCash parameter downloading functionality |
packages/komodo_defi_sdk/pubspec.yaml |
Added dependencies for activation config persistence and ZCash params |
| ZCash params downloader implementation files | Platform-specific ZCash parameter downloaders with factory pattern |
| Activation config service files | Hive-based configuration service with type-safe persistence |
packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart |
Enhanced ZHTLC activation with config service integration and improved error handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| 'uiSignal': ?uiSignal?.serializedName, | ||
| 'deadlineAt': ?deadlineAt?.toIso8601String(), | ||
| 'errorCode': ?errorCode, | ||
| 'errorDetails': ?errorDetails, | ||
| 'stackTrace': ?stackTrace, | ||
| 'startedAt': ?startedAt?.toIso8601String(), | ||
| 'completedAt': ?completedAt?.toIso8601String(), |
There was a problem hiding this comment.
Invalid syntax using ? operator before nullable expressions. In Dart, conditional member access should use ?. or null-aware assignments. These lines should use if (condition) key: value syntax instead."
| 'uiSignal': ?uiSignal?.serializedName, | |
| 'deadlineAt': ?deadlineAt?.toIso8601String(), | |
| 'errorCode': ?errorCode, | |
| 'errorDetails': ?errorDetails, | |
| 'stackTrace': ?stackTrace, | |
| 'startedAt': ?startedAt?.toIso8601String(), | |
| 'completedAt': ?completedAt?.toIso8601String(), | |
| if (uiSignal != null) 'uiSignal': uiSignal!.serializedName, | |
| if (deadlineAt != null) 'deadlineAt': deadlineAt!.toIso8601String(), | |
| if (errorCode != null) 'errorCode': errorCode, | |
| if (errorDetails != null) 'errorDetails': errorDetails, | |
| if (stackTrace != null) 'stackTrace': stackTrace, | |
| if (startedAt != null) 'startedAt': startedAt!.toIso8601String(), | |
| if (completedAt != null) 'completedAt': completedAt!.toIso8601String(), |
There was a problem hiding this comment.
@Copilot this is a new syntax in a recent Dart version. Don't gaslight me, please :(
takenagain
force-pushed
the
bugfix/zhltc-activation-fixes
branch
from
15c43ca to
8626160
Compare
|
@coderabbitai review @codex review (Avengers assemble) |
|
@CharlVS I'll assemble the review forces and get this PR analyzed! 🦸♂️ Let me trigger a comprehensive review of your ZHTLC activation fixes. ✅ Actions performedReview triggered.
|
![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
| 'uiSignal': ?uiSignal?.serializedName, | ||
| 'deadlineAt': ?deadlineAt?.toIso8601String(), | ||
| 'errorCode': ?errorCode, |
There was a problem hiding this comment.
[P0] Fix invalid null-aware map entries
The toJson map for ActivationProgressDetails uses entries like 'uiSignal': ?uiSignal?.serializedName. Dart does not allow the ? prefix in map literals, so this file fails to compile. If the intention is to omit keys when the value is null, each field should be guarded with if clauses (e.g. if (uiSignal != null) 'uiSignal': uiSignal.serializedName). As written, the SDK cannot be built.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart (1)
324-327: Dispose the HTTP client when tearing down
dispose()is our one place to release native resources, yet we leave theClientopen. That leaks sockets/handles across repeated start/stop cycles and defeats the purpose of the new lifecycle hook. Close the client before returning.- void dispose() { - _logCallback.close(); // Ensure the NativeCallable is properly closed - } + void dispose() { + _client.close(); + _logCallback.close(); // Ensure the NativeCallable is properly closed + }
🧹 Nitpick comments (35)
packages/komodo_defi_rpc_methods/lib/src/rpc_methods/zhtlc/zhtlc_rpc_namespace.dart (1)
176-180: Reuse the shared RpcVersion constant.Every other request in this namespace is already pulling
RpcVersion.v2_0; hard-coding'2.0'here adds an unnecessary special case and risks a typo if the constant ever changes. Please swap this to the shared constant for consistency.packages/komodo_defi_sdk/test/zcash_params/platforms/unix_zcash_params_downloader_test.dart (3)
12-35: Suggest improved environment variable testing approach.The current approach acknowledges that
Platform.environmentcannot be modified in tests, making the helper function ineffective. Consider using dependency injection or platform abstraction for better testability.-// Helper function to run tests with a custom HOME environment variable -Future<T> withEnvironmentVariable<T>( - String key, - String? value, - Future<T> Function() testFunction, -) async { - final originalValue = Platform.environment[key]; - if (value == null) { - Platform.environment.remove(key); - } else { - // Note: In test environment, we can't actually modify Platform.environment - // So we'll create a custom downloader with the override instead - } - - try { - return await testFunction(); - } finally { - // Restore original value (though this won't work in test environment either) - if (originalValue != null) { - // We can't restore in test environment, so this is a no-op - } - } -} +// Consider removing this unused helper function or replace with a proper abstraction +// for environment variable access that can be mocked in tests
77-83: Clarify platform-specific behavior in test comments.The test comment mentions that macOS logic takes precedence, but this might be confusing since the test path starts with
/home/(typically Linux). Consider making the expected behavior more explicit.
200-206: Add adownloadProgressstream‐closure test afterdispose.
Inunix_zcash_params_downloader_test.dart, calldownloader.dispose()and then assert the stream completes—for example:downloader.dispose(); await expectLater(downloader.downloadProgress, emitsDone);packages/komodo_defi_framework/lib/komodo_defi_framework.dart (1)
94-98: Harden _log against close race.Between isClosed check and add, the stream may close and throw StateError. Wrap add in try/catch.
- void _log(String message) { - if (!_logStream.isClosed) { - _logStream.add(message); - } - } + void _log(String message) { + if (_logStream.isClosed) return; + try { + _logStream.add(message); + } on StateError { + // Stream closed between check and add; ignore. + } + }packages/komodo_defi_sdk/lib/src/activation_config/hive_registrar.g.dart (1)
14-18: IsolatedHive registrar duplication risk.Same concern applies for IsolatedHiveInterface. Ensure only one registrar is invoked at runtime or use the guarded top-level registration function.
packages/komodo_defi_sdk/test/zcash_params/models/zcash_params_config_test.dart (4)
522-527: Strengthen hash validation (case-insensitive, exact length).Allow A–F and assert exactly 64 hex chars with a single regex.
- expect(RegExp(r'^[a-f0-9]+$').hasMatch(file.sha256Hash), isTrue); + expect(RegExp(r'^[A-Fa-f0-9]{64}$').hasMatch(file.sha256Hash), isTrue);
237-245: Reduce brittleness of URL list assertion.Exact list equality will fail if mirrors/ordering change. Prefer containsAllInOrder.
- expect( - config.downloadUrls, - equals([ - 'https://komodoplatform.com/downloads/', - 'https://z.cash/downloads/', - ]), - ); + expect( + config.downloadUrls, + containsAllInOrder([ + 'https://komodoplatform.com/downloads/', + 'https://z.cash/downloads/', + ]), + );
419-445: Track and enable full JSON (de)serialization tests.The TODO indicates known gaps. Suggest opening a tracking issue and enabling these once nested JSON is fixed. I can help wire this up.
300-306: Hard-coded size may be brittle.If upstream params rotate, this exact size assertion will start failing. Consider asserting non-null and > reasonable lower bound instead.
packages/komodo_defi_sdk/test/zcash_params/platform_implementations/windows_zcash_params_downloader_test.dart (3)
82-101: “Already downloading” test can be flaky.Without controlling when the first download sets/clears the flag, racing awaits may pass/fail unpredictably. Stub getParamsPath to await a Completer (or inject a path resolver) so the second call happens while downloading is definitely true.
327-333: Await stream completion for deterministic assertion.Use expectLater and await to avoid timing flakiness after dispose.
- expect(stream, emitsDone); + await expectLater(stream, emitsDone);
286-293: Broadcast check is weak.Optionally assert identical stream instance accepts multiple listeners and events are shared (or at least no StateError). Current check is okay but minimal.
packages/komodo_defi_framework/lib/src/operations/kdf_operations_local_executable.dart (3)
301-318: Dispose should be awaitable to guarantee cleanup ordering.dispose() runs async work (kdfStop timeout, process shutdown) but returns void. Consider Future dispose() and await internal tasks so callers can deterministically wait for teardown.
- @override - void dispose() { + @override + Future<void> dispose() async { // Cancel and clean up subscriptions - stdoutSub?.cancel().ignore(); + await (stdoutSub?.cancel() ?? Future.value()); stdoutSub = null; - stderrSub?.cancel().ignore(); + await (stderrSub?.cancel() ?? Future.value()); stderrSub = null; // Gracefully stop the process if running final capturedProcess = _process; if (capturedProcess != null) { - _kdfRemote.kdfStop().timeout(const Duration(seconds: 3)).ignore(); - unawaited(_gracefulProcessShutdown(capturedProcess)); + await _kdfRemote.kdfStop().timeout(const Duration(seconds: 3)).catchError((_) {}); + await _gracefulProcessShutdown(capturedProcess); } // Clean up remote resources - _kdfRemote.dispose(); + _kdfRemote.dispose(); }Note: If IKdfOperations.dispose is not async, consider adding disposeAsync in a later minor and keep dispose delegating to it.
259-271: Unify shutdown path in kdfStop with _gracefulProcessShutdown.After waiting for exitCode with timeout, call the same graceful shutdown helper to avoid duplicate logic and ensure kill-on-timeout behavior.
- if (_process != null && _process!.pid != 0) { - await _process?.exitCode.timeout( - const Duration(seconds: 10), - onTimeout: () { - _logCallback('KDF Process did not terminate in time.'); - stopStatus = StopStatus.errorStopping; - return -1; // not used - }, - ); - } - - _process = null; + final p = _process; + if (p != null && p.pid != 0) { + try { + await p.exitCode.timeout(const Duration(seconds: 10), onTimeout: () { + _logCallback('KDF Process did not terminate in time.'); + stopStatus = StopStatus.errorStopping; + return -1; + }); + } finally { + await _gracefulProcessShutdown(p); + } + }
117-120: Minor: pass runInShell on Windows if needed.If the found path isn’t directly executable on some Windows setups, consider runInShell: Platform.isWindows. Likely fine since you resolve absolute path.
packages/komodo_defi_framework/lib/src/operations/kdf_operations_native_stub.dart (2)
5-10: Suppress unused-parameter lint or use them explicitly
logCallbackandconfigare required but unused. This will tripavoid_unused_parametersin many lint sets. Add an ignore or wire them into the stub’s error messages.+// ignore: avoid_unused_parameters IKdfOperations createLocalKdfOperations({ required void Function(String)? logCallback, required LocalConfig config, }) { return KdfOperationsNativeLibrary(); }
37-46: Clear error surface for unsupported opsThrowing
UnsupportedErroris good. Consider includingoperationsNamein the message to aid diagnostics when multiple implementations exist.- throw UnsupportedError( - 'Native operations not available on this platform', - ); + throw UnsupportedError( + '${operationsName}: native operations not available on this platform', + );packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/qtum_activation_strategy.dart (2)
47-52: Add a safety stop: timeout/backoff for polling loopThe 500ms tight loop can run indefinitely on backend issues. Add a max duration/attempts and exponential backoff.
- var isComplete = false; + var isComplete = false; + final started = DateTime.now(); + const maxDuration = Duration(minutes: 5); + var delay = const Duration(milliseconds: 500); while (!isComplete) { + if (DateTime.now().difference(started) > maxDuration) { + yield ActivationProgress( + status: 'Activation timed out', + errorMessage: 'No completion within ${maxDuration.inMinutes} minutes', + isComplete: true, + progressDetails: const ActivationProgressDetails( + currentStep: ActivationStep.error, + stepCount: _qtumStepCount, + errorCode: 'QTUM_ACTIVATION_TIMEOUT', + ), + ); + break; + } @@ - await Future<void>.delayed(const Duration(milliseconds: 500)); + await Future<void>.delayed(delay); + if (delay < const Duration(seconds: 5)) { + delay += const Duration(milliseconds: 250); + }Also applies to: 79-92
27-37: Extract and reuse QTUM stepCount constant
Consolidate all five hard-codedstepCount: 4occurrences inqtum_activation_strategy.dart(lines 31, 58, 72, 86, 100) into a singlestatic const int _qtumStepCount = 4;and replace each literal with
_qtumStepCount.packages/komodo_defi_sdk/lib/src/zcash_params/platforms/mobile_zcash_params_downloader.dart (5)
189-211: Guard against use-after-disposeAdd a small helper and call it at the top of public methods to fail fast when disposed.
/// Disposes of resources used by this downloader. @override void dispose() { @@ } + + void _ensureNotDisposed() { + if (_isDisposed) { + throw StateError('MobileZcashParamsDownloader is disposed'); + } + }And invoke
_ensureNotDisposed();at the start ofdownloadParams,getParamsPath,areParamsAvailable,cancelDownload,validateParams,validateFileHash,getFileHash, andclearParams.
125-129: Prefer structured logging over printReplace
package:logging), and include context (class, method) for better observability. Based on learnings.
60-67: Consider signaling “in-progress” instead of failure on reentrancyReturning a failure when a download is already in progress can confuse callers. Optionally return a distinct “inProgress” state or the current
paramsPathand rely ondownloadProgressfor updates.
96-104: Cancellation semantics: propagate a distinct cancellation outcomeIf
downloadMissingFilesreturns false both on errors and cancellations, callers can’t distinguish. If the model allows, return a specific “cancelled” result when_isCancelledis true.
138-145: Micro-optimization: avoid duplicate missing-files scan
areParamsAvailableduplicates the scan logic found indownloadParams. If used frequently, consider caching the last known good state or consolidating the check in the service for re-use.packages/komodo_defi_sdk/example/lib/widgets/instance_manager/zhtlc_config_dialog.dart (2)
355-391: Validate numeric inputs (positive height, >0 blocks, non-negative interval)Add basic validation to prevent bad configs.
@@ - ZhtlcSyncParams? syncParams; + // Validate numeric fields + final blocks = + int.tryParse(blocksPerIterController.text.trim()) ?? + 1000; + if (blocks <= 0) { + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar( + content: + Text('Blocks per iteration must be greater than 0'), + ), + ); + return; + } + final intervalMs = + int.tryParse(intervalMsController.text.trim()) ?? 0; + if (intervalMs < 0) { + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar( + content: Text('Scan interval must not be negative'), + ), + ); + return; + } + + ZhtlcSyncParams? syncParams; if (syncType == 'earliest') { syncParams = ZhtlcSyncParams.earliest(); } else if (syncType == 'height') { - final v = int.tryParse(syncValueController.text.trim()); - if (v == null) { + final v = int.tryParse(syncValueController.text.trim()); + if (v == null || v <= 0) { ScaffoldMessenger.of(context).showSnackBar( const SnackBar( - content: Text('Enter a valid block height'), + content: Text('Enter a valid positive block height'), ), ); return; } syncParams = ZhtlcSyncParams.height(v); @@ - result = ZhtlcUserConfig( + result = ZhtlcUserConfig( zcashParamsPath: path, - scanBlocksPerIteration: - int.tryParse(blocksPerIterController.text) ?? 1000, - scanIntervalMs: - int.tryParse(intervalMsController.text) ?? 0, + scanBlocksPerIteration: blocks, + scanIntervalMs: intervalMs, syncParams: syncParams, );
257-265: Consider allowing override of prefilled pathLocking the field when prefilled can block users who installed params to a custom location. Consider keeping it editable with a helper text indicating auto-detection.
packages/komodo_defi_sdk/lib/src/bootstrap.dart (1)
54-57: Preserve logging when no externalLogger is passedPassing
externalLoggerdirectly drops prior fallback logging. If intentional, ignore. Otherwise, add a default to keep diagnostics.return KomodoDefiFramework.create( hostConfig: resolvedHostConfig, - externalLogger: externalLogger, + externalLogger: externalLogger ?? (msg) => log(msg, name: 'KDF'), );packages/komodo_defi_sdk/lib/src/zcash_params/platforms/windows_zcash_params_downloader.dart (2)
108-115: Prefer LOCALAPPDATA fallback for Windows params pathSome setups lack
APPDATAor preferLOCALAPPDATAfor large files. Safely fall back.@override Future<String?> getParamsPath() async { - final appData = Platform.environment['APPDATA']; - if (appData == null) { + final base = + Platform.environment['APPDATA'] ?? + Platform.environment['LOCALAPPDATA']; + if (base == null) { return null; } - return path.join(appData, 'ZcashParams'); + return path.join(base, 'ZcashParams'); }
45-47: Progress stream lifecycleBroadcast controller is good. Ensure consumers always cancel subscriptions; consider emitting a terminal “complete” event after success/failure for simpler UI completion handling.
packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart (1)
176-186: Minor: clarify display name includes macOS.- case ZcashParamsPlatform.unix: - return 'Unix/Linux'; + case ZcashParamsPlatform.unix: + return 'Unix/Linux/macOS';packages/komodo_defi_sdk/test/zcash_params/test_helpers/mock_classes.dart (2)
6-6: Avoid importingpackage:http/src/...; use the public API.
ByteStreamis publicly exported bypackage:http/http.dart. Usingsrc/ties you to internals.-import 'package:http/src/byte_stream.dart';- ).thenAnswer((_) => ByteStream(controller.stream)); + ).thenAnswer((_) => http.ByteStream(controller.stream));- (_) => ByteStream( + (_) => http.ByteStream( Stream.error(HttpException('Download failed with status $statusCode')), ),Also applies to: 83-86, 95-98
180-184: Optional: 10 MB test payloads slow CI.Consider reducing
largeSampleData(e.g., 2–4 MB) or making size configurable per test to keep suites fast.packages/komodo_defi_sdk/lib/src/zcash_params/platforms/unix_zcash_params_downloader.dart (2)
143-146: PreferdebugPrintoverKeeps logs concise and avoids dropping long messages in Flutter.
- if (kDebugMode) { - print('Error getting application documents directory: $e'); - } + if (kDebugMode) { + debugPrint('Error getting application documents directory: $e'); + }
126-148: VerifyPlatform.environment['HOME']behavior on all intended Unix targets.On some runtimes this may be unset; your fallback covers it. Ensure tests set
homeDirectoryOverrideto avoid plugin calls in headless environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/dragon_logs/web/favicon.pngis excluded by!**/*.pngpackages/dragon_logs/web/icons/Icon-192.pngis excluded by!**/*.pngpackages/dragon_logs/web/icons/Icon-512.pngis excluded by!**/*.pngpackages/dragon_logs/web/icons/Icon-maskable-192.pngis excluded by!**/*.pngpackages/dragon_logs/web/icons/Icon-maskable-512.pngis excluded by!**/*.pngpackages/komodo_defi_sdk/example/macos/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (101)
.github/scripts/firebase/README.md(1 hunks).github/scripts/firebase/setup-github-secrets.sh(1 hunks).github/scripts/firebase/verify-github-secrets.sh(1 hunks).github/workflows/firebase-hosting-merge.yml(1 hunks).github/workflows/firebase-hosting-pull-request.yml(1 hunks).gitignore(1 hunks)ACTIVATION_PARAMS_REFACTORING_PLAN.md(1 hunks)docs/firebase/firebase-deployment-setup.md(1 hunks)docs/tech_debt/Activation_and_ZHTLC_Tech_Debt.md(1 hunks)packages/dragon_charts_flutter/.gitignore(1 hunks)packages/dragon_logs/.gitignore(1 hunks)packages/dragon_logs/example/.gitignore(1 hunks)packages/dragon_logs/web/index.html(0 hunks)packages/dragon_logs/web/manifest.json(0 hunks)packages/komodo_coin_updates/lib/src/coins_config/config_transform.dart(3 hunks)packages/komodo_coin_updates/pubspec.yaml(1 hunks)packages/komodo_coins/pubspec.yaml(1 hunks)packages/komodo_defi_framework/lib/komodo_defi_framework.dart(6 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_factory.dart(1 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_interface.dart(3 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_local_executable.dart(6 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_native.dart(7 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_native_stub.dart(1 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_remote.dart(5 hunks)packages/komodo_defi_framework/lib/src/operations/kdf_operations_wasm.dart(6 hunks)packages/komodo_defi_framework/pubspec.yaml(1 hunks)packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/activation_params.dart(7 hunks)packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/activation_params_index.dart(1 hunks)packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/erc20_activation_params.dart(1 hunks)packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/eth_activation_params.dart(1 hunks)packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/zhtlc_activation_params.dart(1 hunks)packages/komodo_defi_rpc_methods/lib/src/rpc_methods/zhtlc/zhtlc_rpc_namespace.dart(2 hunks)packages/komodo_defi_sdk/analysis_options.yaml(1 hunks)packages/komodo_defi_sdk/build.yaml(1 hunks)packages/komodo_defi_sdk/example/.firebaserc(1 hunks)packages/komodo_defi_sdk/example/android/app/build.gradle(1 hunks)packages/komodo_defi_sdk/example/firebase.json(1 hunks)packages/komodo_defi_sdk/example/lib/widgets/instance_manager/logged_in_view_widget.dart(6 hunks)packages/komodo_defi_sdk/example/lib/widgets/instance_manager/zhtlc_config_dialog.dart(1 hunks)packages/komodo_defi_sdk/example/macos/Podfile(1 hunks)packages/komodo_defi_sdk/example/macos/Runner.xcodeproj/project.pbxproj(3 hunks)packages/komodo_defi_sdk/lib/komodo_defi_sdk.dart(3 hunks)packages/komodo_defi_sdk/lib/src/_internal_exports.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation/activation_manager.dart(6 hunks)packages/komodo_defi_sdk/lib/src/activation/base_strategies/activation_strategy_base.dart(3 hunks)packages/komodo_defi_sdk/lib/src/activation/base_strategies/activation_strategy_factory.dart(3 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/bch_activation_strategy.dart(8 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/custom_erc20_activation_strategy.dart(4 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/erc20_activation_strategy.dart(4 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_task_activation_strategy.dart(6 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_with_tokens_activation_strategy.dart(5 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/protocol_error_handler.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/qtum_activation_strategy.dart(5 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/slp_activation_strategy.dart(5 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_activation_strategy.dart(9 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_task_activation_strategy.dart(7 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_token_activation_strategy.dart(4 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/utxo_activation_strategy.dart(7 hunks)packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart(10 hunks)packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation_config/hive_activation_config_repository.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation_config/hive_adapters.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation_config/hive_adapters.g.dart(1 hunks)packages/komodo_defi_sdk/lib/src/activation_config/hive_adapters.g.yaml(1 hunks)packages/komodo_defi_sdk/lib/src/activation_config/hive_registrar.g.dart(1 hunks)packages/komodo_defi_sdk/lib/src/bootstrap.dart(10 hunks)packages/komodo_defi_sdk/lib/src/komodo_defi_sdk.dart(5 hunks)packages/komodo_defi_sdk/lib/src/withdrawals/withdrawal_manager.dart(5 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/_zcash_params_index.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_progress.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_progress.freezed.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_progress.g.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_result.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_result.freezed.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/download_result.g.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.freezed.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.g.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/platforms/mobile_zcash_params_downloader.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/platforms/unix_zcash_params_downloader.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/platforms/web_zcash_params_downloader.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/platforms/windows_zcash_params_downloader.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/services/zcash_params_download_service.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader.dart(1 hunks)packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart(1 hunks)packages/komodo_defi_sdk/pubspec.yaml(2 hunks)packages/komodo_defi_sdk/test/zcash_params/integration/mobile_integration_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/models/download_progress_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/models/download_result_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/models/zcash_params_config_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/platform_implementations/mobile_zcash_params_downloader_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/platform_implementations/web_zcash_params_downloader_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/platform_implementations/windows_zcash_params_downloader_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/platforms/unix_zcash_params_downloader_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/services/zcash_params_download_service_test.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/test_helpers/mock_classes.dart(1 hunks)packages/komodo_defi_sdk/test/zcash_params/zcash_params_downloader_factory_test.dart(1 hunks)packages/komodo_defi_types/lib/src/activation/activation_progress.dart(11 hunks)packages/komodo_defi_types/lib/src/protocols/zhtlc/zhtlc_protocol.dart(2 hunks)playground/.firebaserc(1 hunks)playground/firebase.json(1 hunks)
💤 Files with no reviewable changes (2)
- packages/dragon_logs/web/index.html
- packages/dragon_logs/web/manifest.json
🧰 Additional context used
🪛 Shellcheck (0.11.0)
.github/scripts/firebase/verify-github-secrets.sh
[warning] 55-55: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 98-98: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 150-150: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (49)
packages/komodo_defi_sdk/example/macos/Podfile (1)
1-1: Deployment target bump looks goodRaising the macOS deployment target to 10.15 matches the minimum supported by current Firebase/Catalina-only pods, so this should unblock CocoaPods resolution without impacting newer macOS builds.
packages/komodo_defi_framework/pubspec.yaml (1)
33-34: Dependency bump matches our Flutter floor.
path_provider2.1.5 raises the minimum to Flutter 3.19/Dart 3.3, and our pubspec already enforces Flutter ≥3.35, so this patch upgrade stays within supported bounds without new action. Based on learningspackages/komodo_coins/pubspec.yaml (1)
23-23: Dependency bump aligns with platform requirements.path_provider 2.1.5’s stronger Flutter (>=3.19) and new-embedding requirements are already covered by this package’s
environmentconstraint (>=3.35.0), so the upgrade is safe from my side.packages/komodo_defi_sdk/example/macos/Runner.xcodeproj/project.pbxproj (1)
559-559: Coordinate CocoaPods + templates with the new 10.15 floorGood call bumping the app configs to 10.15—Flutter itself raised the macOS minimum earlier this year, so we need this to stay compatible. Please double-check that the Podfile (and any other build scripts/templates) now declare
platform :osx, '10.15'as well; otherwise a futurepod installcan reintroduce 10.14 into the Pods targets and trigger linker warnings or mismatched ABI settings. (github.com)Also applies to: 644-644, 694-694
packages/dragon_logs/.gitignore (1)
3-8: Scoped ignore rules look goodFocusing the package-level ignore list on pub artifacts and the generated web build output keeps the repo tidy without silencing unrelated files.
packages/komodo_defi_sdk/lib/src/zcash_params/models/download_result.freezed.dart (1)
210-303: Generated union covers both result paths cleanlyThe Freezed output provides the success/failure variants with full JSON + pattern helpers, which matches how we surface the download outcome.
.github/workflows/firebase-hosting-pull-request.yml (1)
98-98: Explicit hosting target prevents misdeploys.Pointing the preview deploy at
kdf-sdkkeeps the action aligned with the Firebase target configuration so previews don’t accidentally hit the default site..github/workflows/firebase-hosting-merge.yml (1)
92-92: Live deploy also aligned with kdf-sdk site.Making the merge workflow target explicit avoids drifting back to the default site when pushing to
dev.packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/protocol_error_handler.dart (1)
24-30: Enum-backed error step keeps progress typing consistentMoving
currentSteptoActivationStep.errorlines up with the rest of the enum migration and ditches the fragile string literal. Nicely done.packages/komodo_defi_sdk/build.yaml (1)
4-10: Scoped Hive CE builder wiring looks sanePointing the Hive CE generator at
lib/src/**.dartkeeps codegen focused on the actual source libraries while avoiding the example tree—exactly what we want for the new adapters.packages/komodo_defi_sdk/example/firebase.json (1)
1-17: Firebase hosting target matches the web build layout
public: build/webwith the blanket rewrite to/index.htmlis the right setup for the example’s SPA hosting, and the ignore list mirrors Firebase’s defaults. All good here.packages/komodo_defi_sdk/lib/src/_internal_exports.dart (1)
9-9: Re-export keeps the Zcash params module reachableAdding the zcash params barrel to
_internal_exportskeeps the new subsystem discoverable without forcing callers to dig through folders. Looks correct.packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/custom_erc20_activation_strategy.dart (1)
44-94: Progress updates now use the typed ActivationStep enumReplacing the string literals with
ActivationStep.initialization/complete/erroraligns this strategy with the new typed progress pipeline and prevents mismatches during serialization. Nice cleanup.packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_token_activation_strategy.dart (1)
46-46: LGTM - Consistent ActivationStep enum adoption.The conversion from string literals to
ActivationStepenum values is well-executed and maintains clear progression through the activation flow: initialization → processing → complete/error.Also applies to: 61-61, 81-81, 88-88, 104-104
packages/komodo_defi_sdk/lib/src/zcash_params/models/download_result.g.dart (1)
1-33: LGTM - Standard generated serialization code.The generated JSON serialization logic for
DownloadResultunion type follows expected patterns with proper type discrimination viaruntimeTypefield.packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/utxo_activation_strategy.dart (2)
36-36: LGTM - Consistent ActivationStep enum migration.The conversion from string-based to enum-based activation steps is properly implemented throughout the UTXO activation flow.
Also applies to: 56-56, 70-70, 89-89, 105-105, 133-133
143-175: LGTM - Enhanced status parsing with typed steps.The
_parseUtxoStatusfunction correctly maps status strings to appropriateActivationStepenum values, providing better type safety and consistency across the activation system.packages/komodo_defi_sdk/lib/src/zcash_params/models/download_progress.g.dart (1)
1-22: LGTM - Standard generated serialization.The JSON serialization for
DownloadProgressfollows standard patterns with proper type casting for numeric fields (downloaded,total).packages/komodo_defi_sdk/pubspec.yaml (1)
17-17: Verify dependency versions locally. Ensure thatcrypto^3.0.6,freezed_annotation^3.0.0,hive_ce^2.11.3,hive_ce_flutter,json_annotation,logging,path,path_provider^2.1.5 (requires Flutter 3.19/Dart 3.3+) and dev deps (build_runner,freezed,hive_ce_generator,json_serializable) are available and conflict-free by runningflutter pub deps --jsonandflutter pub getin your local environment.packages/komodo_defi_types/lib/src/protocols/zhtlc/zhtlc_protocol.dart (3)
5-5: LGTM: Constructor formatting improved.The private constructor signature is now more readable with consistent formatting.
24-38: LGTM: Improved validation logic with backward compatibility.The validation correctly implements backward compatibility by treating
electrumas equivalent toelectrum_servers. The relaxed requirement to accept either electrum or light wallet servers is appropriate for ZHTLC's flexibility.
40-42: LGTM: Cleaner config reading with null safety.The updated getter properly uses
valueOrNulland provides a sensible default empty string, improving null safety and removing hardcoded placeholder behavior.packages/komodo_defi_framework/lib/src/operations/kdf_operations_wasm.dart (6)
83-86: LGTM: Cleaner JS config construction.The inline map construction with direct
.jsify()call is more readable than the previous approach.
200-204: LGTM: Better parameter formatting for JS interop.The multi-line parameter formatting for
parseJsInteropMaybePromiseimproves readability without changing functionality.
245-248: LGTM: Consistent multi-line formatting.The formatting change makes the JS call more readable while maintaining the same functionality.
256-271: LGTM: Improved error handling formatting.The multi-line error handling is more readable and the error messages provide better context for debugging.
361-366: LGTM: Better formatted module import.The multi-line formatting improves readability of the complex JS interop chain.
400-406: LGTM: Proper resource disposal implementation.The
dispose()method correctly implements cleanup by nullifying the KDF module reference and resetting the library loaded flag. This aligns with the disposal pattern implemented across other KDF operation implementations.packages/komodo_defi_sdk/test/zcash_params/platform_implementations/web_zcash_params_downloader_test.dart (1)
1-324: LGTM: Comprehensive test suite for web platform downloader.The test suite thoroughly covers all aspects of the WebZcashParamsDownloader including:
- Core functionality: All public methods are tested with expected no-op/virtual behavior
- State consistency: Multiple calls return consistent results
- Stream semantics: Properly validates empty broadcast stream behavior
- Lifecycle management: Tests setup, teardown, and dispose operations
- Integration scenarios: Complete workflows and rapid sequential calls
- Error conditions: Graceful handling of dispose during operations
- Resource management: Multiple instance independence
The tests properly validate the web platform's virtual implementation where actual file operations aren't needed, and ensure the interface contract is maintained. The use of
setUp/tearDownwith proper disposal in tearDown follows testing best practices.packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/slp_activation_strategy.dart (1)
29-29: LGTM: Consistent adoption of ActivationStep enum.All
currentStepassignments now use the standardizedActivationStepenum values (initialization,platformSetup,tokenActivation,complete,error) instead of string literals. This improves type safety and consistency across activation strategies.Also applies to: 46-46, 70-70, 82-82, 96-96
packages/komodo_defi_sdk/test/zcash_params/platforms/unix_zcash_params_downloader_test.dart (2)
144-180: LGTM: Proper error condition testing.The test correctly validates that the downloader handles null params paths gracefully and returns appropriate failure results. The mock setup properly simulates download failure conditions.
184-203: LGTM: Consistent null path handling validation.The test properly validates that
areParamsAvailablereturns false when the params path cannot be determined, maintaining consistent behavior with the download functionality.packages/komodo_defi_framework/lib/komodo_defi_framework.dart (2)
71-82: Resilient external logger wiring — LGTM.Subscription reset, onError guard, and cancelOnError: false are correct.
214-226: No changes needed for operations.dispose() call
Thedispose()method onIKdfOperationsis declared synchronous (void dispose()), and every implementation defines a non-asyncdispose()(noFuturereturn), so there’s no async contract to await.packages/komodo_defi_sdk/lib/src/activation/base_strategies/activation_strategy_factory.dart (1)
34-35: ZHTLC wiring — LGTM.Passing ActivationConfigService into ZhtlcActivationStrategy is the right injection point.
packages/komodo_defi_sdk/lib/src/komodo_defi_sdk.dart (3)
198-201: Public accessor for ActivationConfigService — LGTM.Good to surface via DI with initialized guard.
290-292: logStream passthrough — LGTM.Avoids duplicate subscriptions; clean API.
332-351: Initialization logging — LGTM.Stopwatch + dart:developer log provides useful timing without side effects.
packages/komodo_defi_sdk/lib/src/activation_config/hive_adapters.dart (1)
17-21: Guard adapter typeId and register for isolates
- Define
const int kActivationConfigWrapperTypeId = 20to replace hardcoded literals.- Use this constant in both
Hive.isAdapterRegistered/registerAdapterandIsolatedHive.isAdapterRegistered/registerAdapter.- Verify that
IsolatedHiveis available in scope (add the proper import fromhive_ceif needed).+const int kActivationConfigWrapperTypeId = 20; // Ensure unique repo-wide + void registerActivationConfigAdapters() { - if (!Hive.isAdapterRegistered(20)) { - Hive.registerAdapter(HiveActivationConfigWrapperAdapter()); - } + if (!Hive.isAdapterRegistered(kActivationConfigWrapperTypeId)) { + Hive.registerAdapter(HiveActivationConfigWrapperAdapter()); + } + if (!IsolatedHive.isAdapterRegistered(kActivationConfigWrapperTypeId)) { + IsolatedHive.registerAdapter(HiveActivationConfigWrapperAdapter()); + } }packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_with_tokens_activation_strategy.dart (1)
55-140: Enum migration keeps step semantics intactNice job moving the ETH-with-tokens progress updates off string literals and onto
ActivationStep; the step transitions still line up with the three-phase flow while giving us compile-time safety and consistency with the shared activation UI.packages/komodo_defi_sdk/test/zcash_params/services/zcash_params_download_service_test.dart (1)
1-723: Thorough test coverage for the downloaderThis suite exercises every public path (happy, error, cancellation, timeout, disposal), so the new download service should be well-protected against regressions—great work blending mocks with real hash verification data.
packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/bch_activation_strategy.dart (1)
45-160: ActivationStep adoption matches BCH flowThe enum-backed steps mirror the old BCH/SLP sequence while aligning with the shared activation progress model, so downstream consumers get consistent semantics without losing detail.
packages/komodo_defi_framework/lib/src/operations/kdf_operations_local_executable.dart (1)
196-198: Good: sensitive params masked in logs.Omitting coins and applying .censored() is the right call.
packages/komodo_defi_sdk/lib/src/zcash_params/platforms/mobile_zcash_params_downloader.dart (1)
34-47: Verify enableHashValidation in all download paths (including streams)
Confirm that DefaultZcashParamsDownloadService honours the enableHashValidation flag in every code path—particularly any streamed or progress-based download methods—and add coverage tests if it’s not already exercised.packages/komodo_defi_sdk/example/lib/widgets/instance_manager/zhtlc_config_dialog.dart (1)
151-156: Confirm platform condition for auto-downloadVerify that
ZcashParamsDownloaderFactory.requiresDownloadcorrectly reflects platforms needing downloads (Windows/macOS/Linux) and excludes mobile/web as intended.Would you like me to run a repo scan to find all call sites and platform guards for
requiresDownload?packages/komodo_defi_sdk/lib/src/bootstrap.dart (2)
27-32: Hive.initFlutter in non-Flutter contextsIf
bootstrap()is ever used in tests/CLI,Hive.initFlutter()can throw. Confirm this file is only consumed in Flutter apps; otherwise gate with a platform check or provide an alternate initializer.I can scan the repo for non-Flutter entry points calling
bootstrap()to validate usage.
88-92: Null walletId handling
walletIdResolvercan returnnullif no user is signed in. EnsureActivationConfigServicegracefully handles a null wallet id (separate storage namespace, no crashes).I can grep usages of
ActivationConfigServiceto confirm null-safety around wallet IDs.packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart (1)
45-47:kIsWasmis undefined; usekIsWebonly.
kIsWasmis not provided bypackage:flutter/foundation.dart. It causes a compile error. For Flutter web (including wasm-in-browser),kIsWebis true.- if (kIsWeb || kIsWasm) { + if (kIsWeb) { return WebZcashParamsDownloader(config: config); }- if (kIsWeb || kIsWasm) { + if (kIsWeb) { return ZcashParamsPlatform.web; }- return !kIsWeb && !kIsWasm; + return !kIsWeb;Also applies to: 117-119, 137-138
⛔ Skipped due to learnings
Learnt from: takenagain PR: KomodoPlatform/komodo-defi-sdk-flutter#228 File: packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart:42-45 Timestamp: 2025-09-25T14:41:07.414Z Learning: `kIsWasm` is a valid Flutter constant available in `flutter/foundation.dart` for detecting WebAssembly compilation targets. It's used alongside `kIsWeb` in Flutter applications targeting version 3.35.0+ for comprehensive web platform detection.Learnt from: takenagain PR: KomodoPlatform/komodo-defi-sdk-flutter#228 File: packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart:42-45 Timestamp: 2025-09-25T14:41:07.414Z Learning: `kIsWasm` is a valid Flutter constant available in `flutter/foundation.dart` for detecting WebAssembly compilation targets, used alongside `kIsWeb` for comprehensive web platform detection in Flutter applications.packages/komodo_defi_types/lib/src/activation/activation_progress.dart (1)
260-271: Fix invalid map entry syntax for optionals.Line 263 and the surrounding entries use
?expr, which is not valid Dart syntax and will fail to compile. Use collectionifelements to conditionally add the keys instead.- 'uiSignal': ?uiSignal?.serializedName, - 'deadlineAt': ?deadlineAt?.toIso8601String(), - 'errorCode': ?errorCode, - 'errorDetails': ?errorDetails, - 'stackTrace': ?stackTrace, - 'startedAt': ?startedAt?.toIso8601String(), - 'completedAt': ?completedAt?.toIso8601String(), + if (uiSignal != null) 'uiSignal': uiSignal!.serializedName, + if (deadlineAt != null) 'deadlineAt': deadlineAt!.toIso8601String(), + if (errorCode != null) 'errorCode': errorCode, + if (errorDetails != null) 'errorDetails': errorDetails, + if (stackTrace != null) 'stackTrace': stackTrace, + if (startedAt != null) 'startedAt': startedAt!.toIso8601String(), + if (completedAt != null) 'completedAt': completedAt!.toIso8601String(),
| for role in "${roles[@]}"; do | ||
| print_status "Granting ${role}..." | ||
| gcloud projects add-iam-policy-binding "${project_id}" \ | ||
| --member="serviceAccount:${service_account_email}" \ | ||
| --role="${role}" \ | ||
| --quiet &> /dev/null || true | ||
| done | ||
|
|
||
| print_success "Permissions granted" |
There was a problem hiding this comment.
Don't swallow IAM role assignment failures
By appending || true we ignore any gcloud projects add-iam-policy-binding failure (missing permissions, typos, quota issues, etc.), yet the script still prints “Permissions granted”. That can leave the service account without hosting/rules/token roles and the subsequent deployments will fail in CI with hard‑to‑trace errors. Let set -e surface the failure (or handle it explicitly) so operators know something went wrong.
- gcloud projects add-iam-policy-binding "${project_id}" \
- --member="serviceAccount:${service_account_email}" \
- --role="${role}" \
- --quiet &> /dev/null || true
+ if ! gcloud projects add-iam-policy-binding "${project_id}" \
+ --member="serviceAccount:${service_account_email}" \
+ --role="${role}" \
+ --quiet; then
+ print_error "Failed to grant ${role} on ${project_id}"
+ return 1
+ fi📝 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.
| for role in "${roles[@]}"; do | |
| print_status "Granting ${role}..." | |
| gcloud projects add-iam-policy-binding "${project_id}" \ | |
| --member="serviceAccount:${service_account_email}" \ | |
| --role="${role}" \ | |
| --quiet &> /dev/null || true | |
| done | |
| print_success "Permissions granted" | |
| for role in "${roles[@]}"; do | |
| print_status "Granting ${role}..." | |
| if ! gcloud projects add-iam-policy-binding "${project_id}" \ | |
| --member="serviceAccount:${service_account_email}" \ | |
| --role="${role}" \ | |
| --quiet; then | |
| print_error "Failed to grant ${role} on ${project_id}" | |
| return 1 | |
| fi | |
| done | |
| print_success "Permissions granted" |
🤖 Prompt for AI Agents
.github/scripts/firebase/setup-github-secrets.sh around lines 101 to 109
currently swallows any gcloud IAM binding errors by appending "|| true", then
prints "Permissions granted"; remove the "|| true" so failures propagate (or
explicitly capture the command exit status and fail the script with a clear
error message), and ensure the script runs with set -e (or exits non‑zero) so
any failed gcloud projects add-iam-policy-binding stops execution and prevents
the misleading "Permissions granted" message.
| SDK_SERVICE_ACCOUNT_NAME="github-action-839744467" | ||
| PLAYGROUND_SERVICE_ACCOUNT_NAME="github-action-839744467" | ||
|
|
There was a problem hiding this comment.
Align service-account naming with the setup flow
The verification script now hard-codes github-action-839744467 for both projects, but the automated and manual setup steps create the service account as github-actions-deploy. Anyone following the documented flow (or recreating the key) will end up with github-actions-deploy@…, and this script will report “service account does not exist.” Please revert to the documented name or make it configurable so the checker stays in sync with the setup script.
Apply this diff:
-SDK_SERVICE_ACCOUNT_NAME="github-action-839744467"
-PLAYGROUND_SERVICE_ACCOUNT_NAME="github-action-839744467"
+SDK_SERVICE_ACCOUNT_NAME="github-actions-deploy"
+PLAYGROUND_SERVICE_ACCOUNT_NAME="github-actions-deploy"📝 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.
| SDK_SERVICE_ACCOUNT_NAME="github-action-839744467" | |
| PLAYGROUND_SERVICE_ACCOUNT_NAME="github-action-839744467" | |
| SDK_SERVICE_ACCOUNT_NAME="github-actions-deploy" | |
| PLAYGROUND_SERVICE_ACCOUNT_NAME="github-actions-deploy" |
🤖 Prompt for AI Agents
.github/scripts/firebase/verify-github-secrets.sh around lines 21 to 23
currently hard-codes SDK_SERVICE_ACCOUNT_NAME and
PLAYGROUND_SERVICE_ACCOUNT_NAME to "github-action-839744467", which doesn't
match the documented service account name "github-actions-deploy"; change these
variables to the documented name (github-actions-deploy) or make them
configurable by reading from an environment variable or a shared config (e.g.,
use ${SERVICE_ACCOUNT_NAME:-github-actions-deploy}) and apply the change to both
SDK and PLAYGROUND variables so the verifier stays in sync with the setup flow.
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | ||
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | ||
| --role="roles/firebase.hosting.admin" | ||
|
|
||
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | ||
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | ||
| --role="roles/firebase.rules.admin" | ||
|
|
||
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | ||
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | ||
| --role="roles/iam.serviceAccountTokenCreator" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Correct the IAM role identifiers
The commands here use roles/firebase.hosting.admin and roles/firebase.rules.admin, but those role names don’t exist. The gcloud CLI only recognizes roles/firebasehosting.admin and roles/firebaserules.admin, so anyone following the doc will get IAM binding failures and be blocked from completing setup. Please fix the role identifiers throughout this section.
Apply this diff:
- --role="roles/firebase.hosting.admin"
+ --role="roles/firebasehosting.admin"
- --role="roles/firebase.rules.admin"
+ --role="roles/firebaserules.admin"📝 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.
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/firebase.hosting.admin" | |
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/firebase.rules.admin" | |
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/iam.serviceAccountTokenCreator" | |
| ``` | |
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/firebasehosting.admin" | |
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/firebaserules.admin" | |
| gcloud projects add-iam-policy-binding ${PROJECT_ID} \ | |
| --member="serviceAccount:${SERVICE_ACCOUNT_EMAIL}" \ | |
| --role="roles/iam.serviceAccountTokenCreator" |
🤖 Prompt for AI Agents
In docs/firebase/firebase-deployment-setup.md around lines 107 to 119, the IAM
role identifiers are incorrect; replace roles/firebase.hosting.admin with
roles/firebasehosting.admin and replace roles/firebase.rules.admin with
roles/firebaserules.admin for each gcloud projects add-iam-policy-binding
command in this section so gcloud recognizes the roles and bindings succeed.
| bool needsTransform(JsonMap config) { | ||
| // Only run on web or wasm platforms | ||
| if (!kIsWeb && !kIsWasm) return false; | ||
|
|
||
| // Only run for ZHTLC coin type | ||
| final coinType = config.valueOrNull<String>('type'); | ||
| if (coinType != 'ZHTLC') return false; | ||
|
|
||
| final lightWalletServersWss = config.valueOrNull<List>( |
There was a problem hiding this comment.
ZHTLC transform never activates
needsTransform checks the top-level type, but the coin configs store the protocol identifier under protocol.type (see CoinFilter just above on Lines 142-144). That means this transform never runs, so the Web/WASM remap for ZHTLC lightwallet servers is a no-op. Please switch the check to read protocol.type.
- final coinType = config.valueOrNull<String>('type');
- if (coinType != 'ZHTLC') return false;
+ final protocolType = config.valueOrNull<String>('protocol', 'type');
+ if (protocolType != 'ZHTLC') return false;📝 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.
| bool needsTransform(JsonMap config) { | |
| // Only run on web or wasm platforms | |
| if (!kIsWeb && !kIsWasm) return false; | |
| // Only run for ZHTLC coin type | |
| final coinType = config.valueOrNull<String>('type'); | |
| if (coinType != 'ZHTLC') return false; | |
| final lightWalletServersWss = config.valueOrNull<List>( | |
| bool needsTransform(JsonMap config) { | |
| // Only run on web or wasm platforms | |
| if (!kIsWeb && !kIsWasm) return false; | |
| // Only run for ZHTLC coin type | |
| final protocolType = config.valueOrNull<String>('protocol', 'type'); | |
| if (protocolType != 'ZHTLC') return false; | |
| final lightWalletServersWss = config.valueOrNull<List>( |
🤖 Prompt for AI Agents
In packages/komodo_coin_updates/lib/src/coins_config/config_transform.dart
around lines 257-265, the needsTransform function is checking the top-level
"type" field but the protocol identifier lives under "protocol.type", so the
ZHTLC transform never runs; change the coinType lookup to read
config.valueOrNull<String>('protocol.type') (or navigate into protocol map
first) and use that value for the comparison to 'ZHTLC' so the Web/WASM remap
activates as intended.
| @@ -1,5 +1,6 @@ | |||
| import 'dart:async'; | |||
|
|
|||
| import 'package:flutter/foundation.dart'; | |||
There was a problem hiding this comment.
Avoid Flutter-only dependency in framework package.
Importing package:flutter/foundation.dart ties the framework to Flutter and breaks pure Dart/CLI/WASM use-cases. Replace kDebugMode with a Dart-native check (bool.fromEnvironment) to keep the package Flutter-agnostic.
Apply:
-import 'package:flutter/foundation.dart';
+// Keep this package Flutter-agnostic: avoid importing Flutter.
+const bool _kProduct = bool.fromEnvironment('dart.vm.product');And below (Lines 75-79):
- if (kDebugMode) {
+ if (!_kProduct) {📝 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.
| import 'package:flutter/foundation.dart'; | |
| // At the top of the file, remove Flutter dependency and add Dart‐native product check | |
| // Keep this package Flutter‐agnostic: avoid importing Flutter. | |
| const bool _kProduct = bool.fromEnvironment('dart.vm.product'); | |
| // ... | |
| // Later, around line 75, replace kDebugMode with the inverse of your Dart product flag | |
| if (!_kProduct) { |
🤖 Prompt for AI Agents
In packages/komodo_defi_framework/lib/komodo_defi_framework.dart around line 3
and lines ~75-79, the file imports package:flutter/foundation.dart and uses
kDebugMode which ties the package to Flutter; remove the Flutter import and
replace uses of kDebugMode with a Dart-native compile-time check such as
defining a constant like const bool isDebug =
!bool.fromEnvironment('dart.vm.product') (or use const bool.fromEnvironment
directly where needed), then update the code at lines 75-79 to use that constant
so the package remains Flutter-agnostic and works in pure Dart/CLI/WASM
environments.
| if (Platform.isWindows) { | ||
| return WindowsZcashParamsDownloader( | ||
| downloadService: downloadService, | ||
| config: config, | ||
| enableHashValidation: enableHashValidation, | ||
| ); | ||
| } | ||
|
|
||
| if (Platform.isIOS || Platform.isAndroid) { | ||
| return MobileZcashParamsDownloader( | ||
| downloadService: downloadService, | ||
| config: config, | ||
| enableHashValidation: enableHashValidation, | ||
| ); | ||
| } | ||
|
|
||
| // macOS, Linux, and other Unix-like platforms | ||
| return UnixZcashParamsDownloader( | ||
| downloadService: downloadService, | ||
| config: config, | ||
| enableHashValidation: enableHashValidation, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Replace Platform.* branching with defaultTargetPlatform to stay web-safe.
Avoid dart:io here; defaultTargetPlatform covers Windows, Android, iOS, macOS, Linux.
- if (Platform.isWindows) {
+ final platform = defaultTargetPlatform;
+ if (platform == TargetPlatform.windows) {
return WindowsZcashParamsDownloader(
downloadService: downloadService,
config: config,
enableHashValidation: enableHashValidation,
);
}
- if (Platform.isIOS || Platform.isAndroid) {
+ if (platform == TargetPlatform.iOS || platform == TargetPlatform.android) {
return MobileZcashParamsDownloader(
downloadService: downloadService,
config: config,
enableHashValidation: enableHashValidation,
);
}
- // macOS, Linux, and other Unix-like platforms
+ // macOS and Linux (Unix-like)
return UnixZcashParamsDownloader(
downloadService: downloadService,
config: config,
enableHashValidation: enableHashValidation,
);📝 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.
| if (Platform.isWindows) { | |
| return WindowsZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); | |
| } | |
| if (Platform.isIOS || Platform.isAndroid) { | |
| return MobileZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); | |
| } | |
| // macOS, Linux, and other Unix-like platforms | |
| return UnixZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); | |
| } | |
| // Use Flutter's defaultTargetPlatform instead of dart:io Platform to stay web-safe. | |
| final platform = defaultTargetPlatform; | |
| if (platform == TargetPlatform.windows) { | |
| return WindowsZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); | |
| } | |
| if (platform == TargetPlatform.iOS || platform == TargetPlatform.android) { | |
| return MobileZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); | |
| } | |
| // macOS and Linux (Unix-like) | |
| return UnixZcashParamsDownloader( | |
| downloadService: downloadService, | |
| config: config, | |
| enableHashValidation: enableHashValidation, | |
| ); |
| if (Platform.isWindows) { | ||
| return ZcashParamsPlatform.windows; | ||
| } | ||
|
|
||
| if (Platform.isIOS || Platform.isAndroid) { | ||
| return ZcashParamsPlatform.mobile; | ||
| } | ||
|
|
||
| return ZcashParamsPlatform.unix; | ||
| } |
There was a problem hiding this comment.
Make detectPlatform() web-safe (no Platform.*).
- if (Platform.isWindows) {
- return ZcashParamsPlatform.windows;
- }
-
- if (Platform.isIOS || Platform.isAndroid) {
- return ZcashParamsPlatform.mobile;
- }
-
- return ZcashParamsPlatform.unix;
+ final p = defaultTargetPlatform;
+ switch (p) {
+ case TargetPlatform.windows:
+ return ZcashParamsPlatform.windows;
+ case TargetPlatform.android:
+ case TargetPlatform.iOS:
+ return ZcashParamsPlatform.mobile;
+ case TargetPlatform.macOS:
+ case TargetPlatform.linux:
+ case TargetPlatform.fuchsia:
+ return ZcashParamsPlatform.unix;
+ }📝 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.
| if (Platform.isWindows) { | |
| return ZcashParamsPlatform.windows; | |
| } | |
| if (Platform.isIOS || Platform.isAndroid) { | |
| return ZcashParamsPlatform.mobile; | |
| } | |
| return ZcashParamsPlatform.unix; | |
| } | |
| final p = defaultTargetPlatform; | |
| switch (p) { | |
| case TargetPlatform.windows: | |
| return ZcashParamsPlatform.windows; | |
| case TargetPlatform.android: | |
| case TargetPlatform.iOS: | |
| return ZcashParamsPlatform.mobile; | |
| case TargetPlatform.macOS: | |
| case TargetPlatform.linux: | |
| case TargetPlatform.fuchsia: | |
| return ZcashParamsPlatform.unix; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/komodo_defi_sdk/lib/src/zcash_params/zcash_params_downloader_factory.dart
around lines 121-130, detectPlatform() currently calls Platform.* which breaks
web builds; make it web-safe by importing package:flutter/foundation.dart (for
kIsWeb), add an initial if (kIsWeb) branch that returns the appropriate
ZcashParamsPlatform for web (e.g., unix or a new web enum if needed), and only
use Platform.isWindows / Platform.isIOS / Platform.isAndroid inside the non-web
branch (i.e., if (!kIsWeb) { ... }) so no Platform.* is referenced when
compiling for web.
| test('handles very long file names', () { | ||
| final longFileName = 'very-long-file-name' * 10 + '.params'; | ||
| expect(config.isValidFileName(longFileName), isFalse); | ||
| expect(config.getExpectedFileSize(longFileName), isNull); | ||
| }); |
There was a problem hiding this comment.
Fix: invalid string multiplication (won’t compile in Dart).
Use join to build the long file name.
- final longFileName = 'very-long-file-name' * 10 + '.params';
+ final longFileName =
+ List.filled(10, 'very-long-file-name').join() + '.params';📝 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.
| test('handles very long file names', () { | |
| final longFileName = 'very-long-file-name' * 10 + '.params'; | |
| expect(config.isValidFileName(longFileName), isFalse); | |
| expect(config.getExpectedFileSize(longFileName), isNull); | |
| }); | |
| test('handles very long file names', () { | |
| final longFileName = | |
| List.filled(10, 'very-long-file-name').join() + '.params'; | |
| expect(config.isValidFileName(longFileName), isFalse); | |
| expect(config.getExpectedFileSize(longFileName), isNull); | |
| }); |
🤖 Prompt for AI Agents
In
packages/komodo_defi_sdk/test/zcash_params/models/zcash_params_config_test.dart
around lines 491 to 495, the test uses invalid Python-style string
multiplication ('very-long-file-name' * 10) which doesn’t compile in Dart;
replace it by constructing the repeated segment with Dart APIs (for example
List.filled(10, 'very-long-file-name').join() + '.params' or using
StringBuffer/Iterable.generate to repeat and then append '.params') so the test
builds a longFileName correctly.
| test('handles special characters in URLs', () { | ||
| const baseUrl = 'https://example.com/path with spaces/'; | ||
| const fileName = 'test.params'; | ||
|
|
||
| final url = config.getFileUrl(baseUrl, fileName); | ||
| expect(url, equals('https://example.com/path with spaces/test.params')); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Test encodes spaces in URLs (percent-encode).
The expected URL should be encoded; current assertion locks in an invalid URL.
- expect(url, equals('https://example.com/path with spaces/test.params'));
+ expect(
+ url,
+ equals('https://example.com/path%20with%20spaces/test.params'),
+ );Optionally, update getFileUrl to use Uri.parse(baseUrl).resolve(fileName).toString() to ensure correct encoding.
🏁 Script executed:
#!/bin/bash
# Search for getFileUrl definition and usage in Dart code
rg -nP --type=dart "getFileUrl" -C3Length of output: 7917
🏁 Script executed:
#!/bin/bash
# Print lines around getFileUrl implementation
sed -n '140,200p' packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.dartLength of output: 557
Percent-encode special characters in file URLs
Update getFileUrl (in packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.dart lines 152–159) to use
Uri.parse(baseUrl).resolve(fileName).toString()so spaces and other reserved characters are properly escaped; then adjust the test (packages/komodo_defi_sdk/test/zcash_params/models/zcash_params_config_test.dart lines 497–503) to expect
'https://example.com/path%20with%20spaces/test.params'instead of the unencoded URL.
🤖 Prompt for AI Agents
In packages/komodo_defi_sdk/lib/src/zcash_params/models/zcash_params_config.dart
around lines 152–159 and the test in
packages/komodo_defi_sdk/test/zcash_params/models/zcash_params_config_test.dart
lines 497–503, getFileUrl currently constructs URLs that leave spaces and
reserved characters unencoded; update getFileUrl to build the URL via
Uri.parse(baseUrl).resolve(fileName).toString() so special characters are
percent-encoded, and update the test expectation to
'https://example.com/path%20with%20spaces/test.params' to match the encoded
result.
| test('returns null when APPDATA environment variable missing', () async { | ||
| // On non-Windows platforms, APPDATA won't exist | ||
| final path = await downloader.getParamsPath(); | ||
| expect(path, isNull); | ||
| }); | ||
|
|
||
| test('returns normally but fails due to missing APPDATA', () { | ||
| // This test would need to mock Platform.environment in a real scenario | ||
| // For now, we just verify the method doesn't throw | ||
| expect(downloader.getParamsPath(), completes); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Tests depend on host APPDATA; risk of OS/env flakiness.
These tests assume APPDATA is missing; on Windows CI they’ll be nondeterministic. Inject an environment provider into WindowsZcashParamsDownloader (e.g., environmentProvider: () => {'APPDATA': ...}) and stub it in tests.
Also applies to: 60-80
…n support (#229) * feat(zhtlc-tx): add from_id pagination support from RPC docs * fix(orderbook-v2): use v2 fields and types * fix(source-address-field): use consistent text theme to fix visibility * fix(order-address): change address data to optional for shielded ops * fix(json-utils): convert int to string in traverse as it's lossless * test(sdk): fix missing type in zcash mock classes
* feat(zhtlc): estimate activation percentage based on block height * refactor(review): remove unnecessary casts, states, and configurable polling * fix(config-transform): only replace lightwalletservers if wss is not empty
|
@cursor, please compile all review comments into a markdown file for tech debt (think very hard) |
|
I created a detailed tech debt document,
Learn more about Cursor Agents |
Co-authored-by: charl <charl@vanstaden.info>
Thank you Kanye, very cool! |
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
Tests