Skip to content

sync main #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ca333 merged 4 commits into from
Oct 28, 2025
Merged

sync main #265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee23e0c
only use sync_params when asked
smk762 Oct 27, 2025
12de137
allow optional sync params
smk762 Oct 27, 2025
1fa903d
sign out clean-up for one-shot sync params
ca333 Oct 27, 2025
8561d33
Merge pull request #264 from KomodoPlatform/zhtlc-sync-params
ca333 Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions packages/komodo_defi_sdk/example/lib/widgets/instance_manager/logged_in_view_widget.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,22 @@ class _LoggedInViewWidgetState extends State<LoggedInViewWidget> {
final existing = await sdk.activationConfigService
.getSavedZhtlc(asset.id);
if (existing == null && mounted) {
final config =
final dialogResult =
await ZhtlcConfigDialogHandler.handleZhtlcConfigDialog(
context,
asset,
);
context,
asset,
);
if (!mounted) return;
if (config != null) {
if (dialogResult != null) {
if (dialogResult.oneShotSync != null) {
await sdk.activationConfigService.setOneShotSyncParams(
asset.id,
dialogResult.oneShotSync,
);
}
await sdk.activationConfigService.saveZhtlcConfig(
asset.id,
config,
dialogResult.config,
);
} else {
return; // User cancelled
Expand Down
28 changes: 17 additions & 11 deletions packages/komodo_defi_sdk/example/lib/widgets/instance_manager/zhtlc_config_dialog.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import 'dart:async';

import 'package:flutter/material.dart';
import 'package:komodo_defi_rpc_methods/komodo_defi_rpc_methods.dart'
show ZhtlcSyncParams;
import 'package:komodo_defi_sdk/komodo_defi_sdk.dart'
show
DownloadProgress,
DownloadResultPatterns,
ZcashParamsDownloader,
ZcashParamsDownloaderFactory,
ZhtlcUserConfig;
ZhtlcUserConfig,
ZhtlcSyncParams;
import 'package:komodo_defi_types/komodo_defi_types.dart';

/// Handles ZHTLC configuration dialog with optional automatic Zcash parameters download.
Expand All @@ -19,6 +18,12 @@ import 'package:komodo_defi_types/komodo_defi_types.dart';
/// - Shows progress dialog during download
/// - Displays configuration dialog for user input
/// - Handles download failures gracefully with fallback to manual configuration
class ZhtlcConfigDialogResult {
ZhtlcConfigDialogResult({required this.config, this.oneShotSync});
final ZhtlcUserConfig config;
final ZhtlcSyncParams? oneShotSync;
}

class ZhtlcConfigDialogHandler {
/// Shows a download progress dialog for Zcash parameters.
///
Expand Down Expand Up @@ -144,7 +149,7 @@ class ZhtlcConfigDialogHandler {
/// On desktop platforms, this method will attempt to download Zcash parameters
/// automatically. If successful, it prefills the parameters path in the dialog.
/// Returns null if the user cancels the download or configuration.
static Future<ZhtlcUserConfig?> handleZhtlcConfigDialog(
static Future<ZhtlcConfigDialogResult?> handleZhtlcConfigDialog(
BuildContext context,
Asset asset,
) async {
Expand Down Expand Up @@ -199,7 +204,7 @@ class ZhtlcConfigDialogHandler {
///
/// If [prefilledZcashPath] is provided, the Zcash parameters path field
/// will be prefilled and made read-only.
static Future<ZhtlcUserConfig?> _showZhtlcConfigDialog(
static Future<ZhtlcConfigDialogResult?> _showZhtlcConfigDialog(
BuildContext context,
Asset asset, {
String? prefilledZcashPath,
Expand All @@ -208,6 +213,7 @@ class ZhtlcConfigDialogHandler {
final blocksPerIterController = TextEditingController(text: '1000');
final intervalMsController = TextEditingController(text: '0');

// Sync params controls (used for initial sync and intentional rewinds)
var syncType = 'date'; // earliest | height | date
final syncValueController = TextEditingController();
DateTime? selectedDateTime;
Expand All @@ -225,21 +231,19 @@ class ZhtlcConfigDialogHandler {
);

if (picked != null) {
// Default to midnight (00:00) of the selected day
selectedDateTime = DateTime(picked.year, picked.month, picked.day);
syncValueController.text = formatDate(selectedDateTime!);
}
}

// Initialize with default date (2 days ago)
void initializeDate() {
selectedDateTime = DateTime.now().subtract(const Duration(days: 2));
syncValueController.text = formatDate(selectedDateTime!);
}

initializeDate();

ZhtlcUserConfig? result;
ZhtlcConfigDialogResult? result;

await showDialog<void>(
context: context,
Expand Down Expand Up @@ -375,19 +379,21 @@ class ZhtlcConfigDialogHandler {
);
return;
}
// Convert to Unix timestamp (seconds since epoch)
final unixTimestamp =
selectedDateTime!.millisecondsSinceEpoch ~/ 1000;
syncParams = ZhtlcSyncParams.date(unixTimestamp);
}

result = ZhtlcUserConfig(
final config = ZhtlcUserConfig(
zcashParamsPath: path,
scanBlocksPerIteration:
int.tryParse(blocksPerIterController.text) ?? 1000,
scanIntervalMs:
int.tryParse(intervalMsController.text) ?? 0,
syncParams: syncParams,
);
result = ZhtlcConfigDialogResult(
config: config,
oneShotSync: syncParams,
);
Comment on lines +393 to 397
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The syncParams variable may be null if syncType is 'earliest', but ZhtlcConfigDialogResult accepts nullable oneShotSync. Consider adding a comment explaining that syncParams will be null when 'earliest' is selected, as this is intentional behavior for that sync type.

Copilot uses AI. Check for mistakes.
Navigator.of(context).pop();
},
Expand Down
32 changes: 19 additions & 13 deletions ...ges/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// TODO(komodo-team): Allow passing the start sync mode; currently hard-coded
// to sync from the time of activation.
// Start sync mode can be passed via one-shot sync params through
// ActivationConfigService.setOneShotSyncParams() before activation.
// See zhtlc_config_dialog.dart for UI implementation.

import 'dart:convert';
import 'dart:developer' show log;
Expand Down Expand Up @@ -81,17 +82,22 @@ class ZhtlcActivationStrategy extends ProtocolActivationStrategy {
privKeyPolicy: privKeyPolicy,
);

// Apply sync params if provided by the user configuration via rpc_data
if (params.mode?.rpcData != null && userConfig.syncParams != null) {
final rpcData = params.mode!.rpcData!;
final updatedRpcData = ActivationRpcData(
lightWalletDServers: rpcData.lightWalletDServers,
electrum: rpcData.electrum,
syncParams: userConfig.syncParams,
);
params = params.copyWith(
mode: ActivationMode(rpc: params.mode!.rpc, rpcData: updatedRpcData),
);
// Apply one-shot sync_params only when explicitly provided via config form
// right before activation. This avoids caching and unintended rewinds.
if (params.mode?.rpcData != null) {
final oneShotSync = await configService.takeOneShotSyncParams(asset.id);
if (oneShotSync != null) {
final rpcData = params.mode!.rpcData!;
final updatedRpcData = ActivationRpcData(
lightWalletDServers: rpcData.lightWalletDServers,
electrum: rpcData.electrum,
syncParams: oneShotSync,
);
Comment on lines +87 to +95
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve rpc connection limits when overriding sync params

When applying one‑shot sync_params we rebuild ActivationRpcData with only lightWalletDServers and electrum, dropping other properties such as minConnected and maxConnected. For ZHTLC assets that define non‑default connection limits, supplying a one‑shot sync parameter will silently revert those settings to the defaults (maxConnected becomes 1), potentially reducing reliability. Consider cloning the existing rpcData or copying all fields so connection limits are preserved when oneShotSync is applied.

Useful? React with 👍 / 👎.

params = params.copyWith(
mode:
ActivationMode(rpc: params.mode!.rpc, rpcData: updatedRpcData),
);
}
}

yield ZhtlcActivationProgress.validation(protocol);
Expand Down
70 changes: 65 additions & 5 deletions packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:hive_ce/hive.dart';
import 'package:komodo_defi_rpc_methods/komodo_defi_rpc_methods.dart';
import 'package:komodo_defi_types/komodo_defi_type_utils.dart';
import 'package:komodo_defi_types/komodo_defi_types.dart';
import 'package:komodo_defi_local_auth/komodo_defi_local_auth.dart';

typedef JsonMap = Map<String, dynamic>;

Expand Down Expand Up @@ -51,15 +52,20 @@ class ZhtlcUserConfig {
final int scanBlocksPerIteration;
final int scanIntervalMs;
final int? taskStatusPollingIntervalMs;
/// Optional, accepted for backward compatibility. Not persisted.
/// If provided to saveZhtlcConfig, it will be applied as a one-shot
/// sync override for the next activation and then discarded.
final ZhtlcSyncParams? syncParams;
// Sync params are no longer persisted here; they are supplied one-shot
// via ActivationConfigService at activation time when the user requests
// an intentional resync.

JsonMap toJson() => {
'zcashParamsPath': zcashParamsPath,
'scanBlocksPerIteration': scanBlocksPerIteration,
'scanIntervalMs': scanIntervalMs,
if (taskStatusPollingIntervalMs != null)
'taskStatusPollingIntervalMs': taskStatusPollingIntervalMs,
if (syncParams != null) 'syncParams': syncParams!.toJsonRequest(),
};

static ZhtlcUserConfig fromJson(JsonMap json) => ZhtlcUserConfig(
Expand All @@ -70,9 +76,6 @@ class ZhtlcUserConfig {
taskStatusPollingIntervalMs: json.valueOrNull<int>(
'taskStatusPollingIntervalMs',
),
syncParams: ZhtlcSyncParams.tryParse(
json.valueOrNull<dynamic>('syncParams'),
),
);
}

Expand Down Expand Up @@ -189,10 +192,31 @@ class ActivationConfigService {
ActivationConfigService(
this.repo, {
required WalletIdResolver walletIdResolver,
}) : _walletIdResolver = walletIdResolver;
Stream<KdfUser?>? authStateChanges,
}) : _walletIdResolver = walletIdResolver {
// Listen to auth state changes to clear one-shot params on sign-out
_authStateSubscription = authStateChanges?.listen((user) {
if (user == null) {
// User signed out, clear all one-shot params
_oneShotSyncParams.clear();
} else {
// User signed in or changed, clear one-shot params for previous wallet
// if it was different from the current one
if (_lastWalletId != null && _lastWalletId != user.walletId) {
clearOneShotSyncParamsForWallet(_lastWalletId!);
}
_lastWalletId = user.walletId;
}
});
}
Comment on lines 192 to +211
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify auth state stream behavior and consider initialization.

The auth state listener correctly handles sign-out and wallet switching, but there are two potential edge cases:

  1. If authStateChanges doesn't emit the current authenticated user immediately upon subscription (e.g., not a BehaviorSubject), _lastWalletId will remain null even when a user is already authenticated. This means the first wallet switch won't trigger cleanup of the previous wallet's one-shot params (line 205 check fails).

  2. The authStateChanges parameter is optional. If null, one-shot params won't be automatically cleared on sign-out, potentially causing them to persist across sessions.

Consider either:

  • Initializing _lastWalletId by querying current wallet state via _walletIdResolver() in the constructor
  • Documenting that authStateChanges should emit the current user upon subscription
  • Requiring authStateChanges to be non-null for production use

Run the following script to check how authStateChanges is wired in the bootstrap:

🏁 Script executed:

#!/bin/bash
# Check how authStateChanges stream is provided to ActivationConfigService
rg -A 5 -B 5 'ActivationConfigService\(' --type=dart

Length of output: 2378


---

🏁 Script executed:

```shell
#!/bin/bash
# Find KomodoDefiLocalAuth class definition and authStateChanges property
rg -n 'class KomodoDefiLocalAuth' --type=dart -A 20

Length of output: 2265

🏁 Script executed:

#!/bin/bash
# Search for authStateChanges stream definition/initialization
rg -n 'authStateChanges' --type=dart -B 2 -A 2

Length of output: 17571


---

🏁 Script executed:

```shell
#!/bin/bash
# Check for any initialization or handling of _lastWalletId in ActivationConfigService
rg -n '_lastWalletId' --type=dart -B 2 -A 2 packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart

Length of output: 582

🏁 Script executed:

#!/bin/bash
# Check full constructor and initialization logic
sed -n '192,220p' packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart

Length of output: 1091


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for tests of ActivationConfigService auth state handling
rg -n 'ActivationConfigService' --type=dart packages/komodo_defi_sdk/test/ -B 2 -A 5 | head -100

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for dispose method in ActivationConfigService
sed -n '301,310p' packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart

Length of output: 214

Initialize _lastWalletId with current wallet to fix wallet-switch edge case.

The auth state stream (authStateChanges) is an async* generator that doesn't emit the current user on subscription—only subsequent state changes. When ActivationConfigService is created mid-session with an already-authenticated user, _lastWalletId remains null. This causes the first wallet switch to skip cleanup of the previous wallet's one-shot params (line 205 check fails).

Fix: Initialize _lastWalletId in the constructor using the _walletIdResolver:

ActivationConfigService(
  this.repo, {
  required WalletIdResolver walletIdResolver,
  Stream<KdfUser?>? authStateChanges,
}) : _walletIdResolver = walletIdResolver {
  // Initialize current wallet ID
  walletIdResolver().then((walletId) {
    _lastWalletId = walletId;
  });
  
  // Listen to auth state changes...
  _authStateSubscription = authStateChanges?.listen((user) { ... });
}

Additionally, dispose() should clear _oneShotSyncParams map to ensure params don't persist after disposal.

🤖 Prompt for AI Agents 
In
packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart
around lines 192 to 211, initialize _lastWalletId at construction so a service
created mid-session knows the current wallet and will correctly clear
previous-wallet one-shot params on the first switch: call _walletIdResolver()
(or walletIdResolver()) and set _lastWalletId from its result (use then/await as
appropriate) before subscribing to authStateChanges; also update dispose() to
clear the _oneShotSyncParams map so one-shot params do not persist after
disposal.


final ActivationConfigRepository repo;
final WalletIdResolver _walletIdResolver;
StreamSubscription<KdfUser?>? _authStateSubscription;
WalletId? _lastWalletId;

// One-shot sync params coordinator. Not persisted; cleared after use.
final Map<_WalletAssetKey, ZhtlcSyncParams?> _oneShotSyncParams = {};

Future<WalletId> _requireActiveWallet() async {
final walletId = await _walletIdResolver();
Expand Down Expand Up @@ -234,6 +258,11 @@ class ActivationConfigService {

Future<void> saveZhtlcConfig(AssetId id, ZhtlcUserConfig config) async {
final walletId = await _requireActiveWallet();
// If legacy callers provide syncParams in the config, convert it to
// a one-shot sync override and do not persist it.
if (config.syncParams != null) {
_oneShotSyncParams[_WalletAssetKey(walletId, id)] = config.syncParams;
}
await repo.saveConfig(walletId, id, config);
}

Expand All @@ -243,6 +272,37 @@ class ActivationConfigService {
_awaitingControllers[_WalletAssetKey(walletId, id)]?.complete(config);
}

/// Sets a one-shot sync params value for the next activation of [id].
/// This is not persisted and will be consumed and cleared on activation.
Future<void> setOneShotSyncParams(
AssetId id,
ZhtlcSyncParams? syncParams,
) async {
final walletId = await _requireActiveWallet();
_oneShotSyncParams[_WalletAssetKey(walletId, id)] = syncParams;
}

/// Returns and clears any pending one-shot sync params for [id].
Future<ZhtlcSyncParams?> takeOneShotSyncParams(AssetId id) async {
final walletId = await _requireActiveWallet();
final key = _WalletAssetKey(walletId, id);
final value = _oneShotSyncParams.remove(key);
return value;
}

/// Clears all one-shot sync params for the specified wallet.
/// This should be called when a user signs out to prevent stale one-shot
/// params from being applied on the next activation after re-login.
void clearOneShotSyncParamsForWallet(WalletId walletId) {
_oneShotSyncParams.removeWhere((key, _) => key.walletId == walletId);
}

/// Disposes of the service and cleans up resources.
void dispose() {
_authStateSubscription?.cancel();
_authStateSubscription = null;
}
Comment on lines +300 to +304
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Complete the cleanup by clearing one-shot params.

The dispose method cancels the auth state subscription but doesn't clear the _oneShotSyncParams map. For complete resource cleanup and to prevent potential issues if the service is accessed after disposal, the map should also be cleared.

Apply this diff:

 /// Disposes of the service and cleans up resources.
 void dispose() {
   _authStateSubscription?.cancel();
   _authStateSubscription = null;
+  _oneShotSyncParams.clear();
 }
📝 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.

Suggested change
/// Disposes of the service and cleans up resources.
void dispose() {
_authStateSubscription?.cancel();
_authStateSubscription = null;
}
/// Disposes of the service and cleans up resources.
void dispose() {
_authStateSubscription?.cancel();
_authStateSubscription = null;
_oneShotSyncParams.clear();
}
🤖 Prompt for AI Agents 
In
packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart
around lines 300 to 304, the dispose method cancels the auth subscription but
doesn’t clear the _oneShotSyncParams map; add a call to
_oneShotSyncParams.clear() (after cancelling and nulling the subscription) to
fully release one-shot parameters and avoid leftover state if the service is
accessed post-disposal.


final Map<_WalletAssetKey, Completer<ZhtlcUserConfig?>> _awaitingControllers =
{};
}
Expand Down
1 change: 1 addition & 0 deletions packages/komodo_defi_sdk/lib/src/bootstrap.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ Future<void> bootstrap({
return ActivationConfigService(
repo,
walletIdResolver: () async => (await auth.currentUser)?.walletId,
authStateChanges: auth.authStateChanges,
);
}, dependsOn: [KomodoDefiLocalAuth]);

Expand Down
3 changes: 3 additions & 0 deletions packages/komodo_defi_sdk/lib/src/komodo_defi_sdk.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ class KomodoDefiSdk with SecureRpcPasswordMixin {
_disposeIfRegistered<KomodoDefiLocalAuth>((m) => m.dispose()),
_disposeIfRegistered<AssetManager>((m) => m.dispose()),
_disposeIfRegistered<ActivationManager>((m) => m.dispose()),
_disposeIfRegistered<ActivationConfigService>(
(m) async => m.dispose(),
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dispose callback uses async => but the other dispose calls use synchronous callbacks. Since dispose() is a synchronous method (returns void not Future<void>), this should use a synchronous callback (m) => m.dispose() to maintain consistency with the other dispose calls.

Suggested change
(m) async => m.dispose(),
(m) => m.dispose(),

Copilot uses AI. Check for mistakes.
),
_disposeIfRegistered<BalanceManager>((m) => m.dispose()),
_disposeIfRegistered<PubkeyManager>((m) => m.dispose()),
_disposeIfRegistered<TransactionHistoryManager>((m) => m.dispose()),
Expand Down
Loading