Skip to content

sync main#261

Merged
ca333 merged 4 commits intomainfrom
dev
Oct 26, 2025
Merged

sync main#261
ca333 merged 4 commits intomainfrom
dev

Conversation

@ca333
Copy link
Copy Markdown
Contributor

@ca333 ca333 commented Oct 26, 2025

logging
max conns

Summary by CodeRabbit

  • New Features

    • Added configurable minimum and maximum connection parameters for protocol activation settings.
  • Tests

    • Enhanced test mocking infrastructure for activation health monitoring.

Copilot AI review requested due to automatic review settings October 26, 2025 20:15
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR adds comprehensive logging infrastructure for activation operations across the Komodo DeFi SDK. It introduces activation parameter logging in RPC clients, extends ActivationRpcData with min/max connected fields, and embeds debug logging throughout protocol-specific activation strategies for observability.

Changes

Cohort / File(s) Summary
RPC Client Activation Logging
packages/komodo_defi_framework/lib/komodo_defi_framework.dart, packages/komodo_defi_sdk/lib/src/client/kdf_api_client.dart
Introduces _isActivationMethod() and _logActivationParameters() helpers to detect and log activation-related RPC methods before execution, capturing ticker, node counts, servers, RPC URLs, tokens, and contract details with safe-fail error handling.
Test Infrastructure
packages/komodo_defi_local_auth/test/src/trezor/trezor_auth_service_test.dart
Adds ensureKdfHealthy() override and tracking fields (ensureHealthyReturn, ensureHealthyCalls) to _FakeAuthService for controlled test responses and invocation counting.
Activation Parameters
packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/activation_params.dart
Extends ActivationRpcData constructor with minConnected and maxConnected fields (default maxConnected=1), adds validation asserts, and updates fromJson and toJsonRequest serialization.
Protocol Activation Strategy Logging
packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/{bch,custom_erc20,erc20,eth_task,eth_with_tokens,qtum,tendermint,utxo,zhtlc}_activation_strategy.dart
Adds debug-level logging around activation procedures, including pre/post-call logs with JSON-encoded parameters (ticker, protocol, token counts, config details) and success confirmations; extracts activation parameters into local variables for clarity.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review the activation parameter logging logic in RPC clients (_isActivationMethod, _logActivationParameters) for correctness and safe-fail semantics.
  • Verify ActivationRpcData field additions, validation asserts, and JSON serialization round-trip consistency.
  • Spot-check logging statements across multiple strategy files for consistency in parameter extraction and debug message formatting.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • smk762
  • CharlVS

Poem

🐰 Activation logs now flow with care,
Each parameter tracked with rabbit flair,
Debug messages glow in strategic hue,
Tendermint, ETH, and UTXO too,
Observability blooms when the SDK's true!

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

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7197de6 and 17311fc.

📒 Files selected for processing (13)
  • packages/komodo_defi_framework/lib/komodo_defi_framework.dart (2 hunks)
  • packages/komodo_defi_framework/lib/src/client/kdf_api_client.dart (2 hunks)
  • packages/komodo_defi_local_auth/test/src/trezor/trezor_auth_service_test.dart (2 hunks)
  • packages/komodo_defi_rpc_methods/lib/src/common_structures/activation/activation_params/activation_params.dart (4 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/bch_activation_strategy.dart (3 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/custom_erc20_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/erc20_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_task_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_with_tokens_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/qtum_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/tendermint_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/utxo_activation_strategy.dart (2 hunks)
  • packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/zhtlc_activation_strategy.dart (2 hunks)

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.

@ca333 ca333 merged commit 49fac52 into main Oct 26, 2025
6 of 9 checks passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds debug logging for coin activation operations and introduces configuration for controlling electrum server connections. The changes enhance observability of activation processes and provide more control over server connectivity.

  • Adds comprehensive debug logging across all activation strategy implementations
  • Introduces min_connected and max_connected parameters for electrum server configuration
  • Implements activation parameter logging in API client layers
  • Adds test infrastructure for health check functionality

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
zhtlc_activation_strategy.dart Adds debug logging for ZHTLC coin activation parameters
utxo_activation_strategy.dart Adds debug logging for UTXO/Electrum activation with task tracking
tendermint_activation_strategy.dart Adds debug logging for Tendermint platform activation
qtum_activation_strategy.dart Adds debug logging for QTUM coin activation
eth_with_tokens_activation_strategy.dart Adds debug logging for ETH platform activation with token support
eth_task_activation_strategy.dart Adds debug logging for task-based ETH activation
erc20_activation_strategy.dart Adds debug logging for ERC20 token activation
custom_erc20_activation_strategy.dart Adds debug logging for custom ERC20 token activation
bch_activation_strategy.dart Adds debug logging for BCH and SLP token activation
activation_params.dart Adds min/max connected server configuration with validation
trezor_auth_service_test.dart Adds test stub for ensureKdfHealthy method
kdf_api_client.dart Implements activation parameter logging at API client level
komodo_defi_framework.dart Implements activation parameter logging in framework layer

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

(forLightWallet ? 'electrum_servers' : 'servers'): electrum!
.map((e) => e.toJsonRequest())
.toList(),
if (electrum != null) 'max_connected': (maxConnected ?? 1),
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The max_connected parameter is being included whenever electrum is not null, but it should only be included when explicitly set. This could send default values even when not intended. Change condition to if (maxConnected != null) to only include when explicitly provided.

Suggested change
if (electrum != null) 'max_connected': (maxConnected ?? 1),
if (maxConnected != null) 'max_connected': maxConnected,

Copilot uses AI. Check for mistakes.
}
} catch (e) {
// Silently ignore logging errors
_logger.info('[ACTIVATION] Error logging parameters: $e');
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

Error logging uses info level instead of warning or error level. Change to _logger.warning('[ACTIVATION] Error logging parameters: $e'); to properly indicate this is an error condition.

Suggested change
_logger.info('[ACTIVATION] Error logging parameters: $e');
_logger.warning('[ACTIVATION] Error logging parameters: $e');

Copilot uses AI. Check for mistakes.
}
} catch (e) {
// Silently ignore logging errors
_logger.fine('[ACTIVATION] Error logging parameters: $e');
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

Error logging uses fine level instead of warning or error level. Change to _logger.warning('[ACTIVATION] Error logging parameters: $e'); to properly indicate this is an error condition.

Suggested change
_logger.fine('[ACTIVATION] Error logging parameters: $e');
_logger.warning('[ACTIVATION] Error logging parameters: $e');

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit 17311fc):

https://kdf-sdk--pr261-dev-7ni5xs9c.web.app

(expires Sun, 02 Nov 2025 20:20:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 9c1b6e6c010cf0b965c455ba7a69c4aedafa8a1d

@github-actions
Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit 17311fc):

https://komodo-playground--pr261-dev-528l63z8.web.app

(expires Sun, 02 Nov 2025 20:21:15 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47

This was referenced Oct 27, 2025
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.

2 participants