Skip to content

Zhtlc sync params#264

Merged
ca333 merged 3 commits intodevfrom
zhtlc-sync-params
Oct 27, 2025
Merged

Zhtlc sync params#264
ca333 merged 3 commits intodevfrom
zhtlc-sync-params

Conversation

@smk762
Copy link
Copy Markdown
Contributor

@smk762 smk762 commented Oct 27, 2025

Closes #263

To test:

  • login with a fresh wallet and select ARRR for activation
  • A config for sync params should appear
  • logout, then log in to the same wallet
  • Arr should activate automatically without needing to select, without config for sync params shown.
  • Enter the ARRR wallet page
  • Click on the config button to change config params
  • See ARRR automatically disable and begin re-activation with new sync params.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 27, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch zhtlc-sync-params

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 27, 2025

Visit the preview URL for this PR (updated for commit 1fa903d):

https://komodo-playground--pr264-zhtlc-sync-params-6heo42v1.web.app

(expires Mon, 03 Nov 2025 21:04:20 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 27, 2025

Visit the preview URL for this PR (updated for commit 1fa903d):

https://kdf-sdk--pr264-zhtlc-sync-params-fkmsz92c.web.app

(expires Mon, 03 Nov 2025 21:04:18 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 9c1b6e6c010cf0b965c455ba7a69c4aedafa8a1d

@CharlVS
Copy link
Copy Markdown
Collaborator

CharlVS commented Oct 27, 2025

@cursoragent thoroughly consider and review the changes.

@cursor
Copy link
Copy Markdown

cursor bot commented Oct 27, 2025

Summary

  • This PR makes ZHTLC sync parameters optional and ephemeral: they’re set as a one-shot override at activation time instead of being persisted in user config.
  • The example app prompts for ZHTLC config on first activation (including optional sync params), persists only stable settings, and applies sync params just for the next activation when requested.

Scope of changes

  • packages/komodo_defi_sdk/example/lib/widgets/instance_manager/logged_in_view_widget.dart
  • packages/komodo_defi_sdk/example/lib/widgets/instance_manager/zhtlc_config_dialog.dart
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart
  • packages/komodo_defi_sdk/lib/src/activation_config/activation_config_service.dart

Behavior vs acceptance criteria

  • Fresh wallet selecting ARRR shows config including sync params — yes (example dialog includes start-from controls).
  • Subsequent login auto-activates without prompt — yes, config is persisted while sync params are not, so no re-prompt.
  • Config button change triggers re-activation with new sync params — code provides one-shot sync handoff and consumption at activation; ensure the UI flow for “disable and re-activate” invokes reactivation (outside this diff).

Key implementation details (code references)

  • Apply one-shot sync_params only at activation, then clear:
      // 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,
          );
          params = params.copyWith(
            mode:
                ActivationMode(rpc: params.mode!.rpc, rpcData: updatedRpcData),
          );
        }
      }
  • Stop persisting syncParams in ZhtlcUserConfig:
  JsonMap toJson() => {
    'zcashParamsPath': zcashParamsPath,
    'scanBlocksPerIteration': scanBlocksPerIteration,
    'scanIntervalMs': scanIntervalMs,
    if (taskStatusPollingIntervalMs != null)
      'taskStatusPollingIntervalMs': taskStatusPollingIntervalMs,
  };

  static ZhtlcUserConfig fromJson(JsonMap json) => ZhtlcUserConfig(
    zcashParamsPath: json.value<String>('zcashParamsPath'),
    scanBlocksPerIteration:
        json.valueOrNull<int>('scanBlocksPerIteration') ?? 1000,
    scanIntervalMs: json.valueOrNull<int>('scanIntervalMs') ?? 0,
    taskStatusPollingIntervalMs: json.valueOrNull<int>(
      'taskStatusPollingIntervalMs',
    ),
  );
  • Provide one-shot sync params API:
  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);
  }
  /// 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;
  }
  • Dialog returns config + optional one-shot sync:
                    final config = ZhtlcUserConfig(
                      zcashParamsPath: path,
                      scanBlocksPerIteration:
                          int.tryParse(blocksPerIterController.text) ?? 1000,
                      scanIntervalMs:
                          int.tryParse(intervalMsController.text) ?? 0,
                    );
                    result = ZhtlcConfigDialogResult(
                      config: config,
                      oneShotSync: syncParams,
                    );
                    Navigator.of(context).pop();
  • Example wires one-shot before saving config:
                  final dialogResult =
                      await ZhtlcConfigDialogHandler.handleZhtlcConfigDialog(
                    context,
                    asset,
                  );
                  if (!mounted) return;
                  if (dialogResult != null) {
                    if (dialogResult.oneShotSync != null) {
                      await sdk.activationConfigService.setOneShotSyncParams(
                        asset.id,
                        dialogResult.oneShotSync,
                      );
                    }
                    await sdk.activationConfigService.saveZhtlcConfig(
                      asset.id,
                      dialogResult.config,
                    );

Review notes

  • Design is sound: decouples persisted config from sync rewind state, preventing accidental rewinds on re-login. The one-shot store is wallet+asset key scoped and consumed atomically at activation.
  • Backward compatibility: ZhtlcUserConfig.syncParams accepted but not persisted; saveZhtlcConfig migrates it into the one-shot map. Previously saved configs with syncParams will simply ignore that field now, which is safe.
  • Example UI: clear UX with defaults; date sync uses midnight of selected date (label says “Date & Time”, but time is not selectable—minor wording nit).

Risks and edge cases

  • Stale one-shot: If a one-shot is set but activation doesn’t run immediately (e.g., user cancels), it will apply to the next activation (even after app relaunch) since it’s in-memory only for the running process. Consider clearing on wallet sign-out to avoid unintended later rewinds.
  • Multiple edits quickly before activation: last writer wins in the one-shot map; acceptable but worth noting.
  • Config button behavior: reactivation trigger depends on the UI calling disable/enable; ensure the product UI performs that flow after saving config and setting the one-shot.

Recommendations

  • Clear any pending one-shot entries on wallet sign-out/auth-change to prevent surprise rewinds on a later session.
  • Update the TODO at the top of zhtlc_activation_strategy.dart (now that start-sync is supported via one-shot).
  • Minor UX nit: rename “Date & Time” label to “Date” or add time picker if time is important.

Tests to consider

  • ActivationConfigService: saving config with syncParams does not persist it; takeOneShotSyncParams returns and clears; subsequent call returns null.
  • ZhtlcActivationStrategy: with and without one-shot present, toRpcParams() includes/excludes sync_params.
  • Dialog mapping: earliest/height/date produce correct ZhtlcSyncParams values; invalid height shows error.

Verdict

  • The implementation cleanly meets the stated goals and acceptance criteria. With the minor follow-ups above (stale one-shot clearing, labeling), this is good to merge.

Open in Cursor Open in Web

Learn more about Cursor Agents

@cursor
Copy link
Copy Markdown

cursor bot commented Oct 27, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

Copy link
Copy Markdown
Collaborator

@CharlVS CharlVS left a comment

Choose a reason for hiding this comment

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

Please change it to clear the stored values after sign-out

@ca333
Copy link
Copy Markdown
Contributor

ca333 commented Oct 27, 2025

thanks @smk762 - I added some bits to clear / clean out one-shot sync params once the user signs out

@ca333 ca333 merged commit 8561d33 into dev Oct 27, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants