Skip to content

Conversation

@jpraynaud
Copy link
Member

@jpraynaud jpraynaud commented Aug 25, 2025

Content

This PR includes a fix for the circular dependency of the Operational Certificate in the context of KES signature:

  • Removed the usage of Operational certificate from ChainObserver trait KES period computation function.
  • Adapted the implementations of the ChainObserver trait.
  • Adapted the KES signature in the DMQ message builder.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Closes #2668

@jpraynaud jpraynaud self-assigned this Aug 25, 2025
@jpraynaud jpraynaud requested a review from Copilot August 25, 2025 14:39

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Aug 25, 2025

Test Results

    4 files  ±0    158 suites  ±0   22m 54s ⏱️ +5s
2 152 tests ±0  2 152 ✅ ±0  0 💤 ±0  0 ❌ ±0 
6 602 runs  ±0  6 602 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 3d8b215. ± Comparison against base commit 7ec438a.

♻️ This comment has been updated with latest results.

@jpraynaud jpraynaud force-pushed the jpraynaud/2668-fix-opcert-circular-dependency branch from 5e3fae0 to 9f688ed Compare August 25, 2025 14:53
@jpraynaud jpraynaud requested a review from rezabaram August 25, 2025 14:54
@jpraynaud jpraynaud force-pushed the jpraynaud/2668-fix-opcert-circular-dependency branch from 9f688ed to ad3862d Compare August 26, 2025 09:47
@jpraynaud jpraynaud requested a review from Copilot August 26, 2025 09:57
Copy link

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 fixes a circular dependency in the KES signature process by removing the requirement for an operational certificate parameter from the ChainObserver trait's get_current_kes_period method. Instead of relying on the operational certificate for KES period computation, the implementation now calculates the current KES period directly from the current slot and a hardcoded slots-per-KES-period value.

  • Removed OpCert parameter from get_current_kes_period method signature across all ChainObserver implementations
  • Updated CLI observer to calculate KES period from current slot instead of querying operational certificate info
  • Modified callers to use the parameterless method and compute relative KES periods locally

Reviewed Changes

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

Show a summary per file
File Description
mithril-signer/src/runtime/runner.rs Updated to call parameterless get_current_kes_period() method
mithril-signer/src/dependency_injection/builder.rs Added operational certificate path to chain observer configuration
mithril-aggregator/src/services/signer_registration/verifier.rs Updated to call parameterless get_current_kes_period() method
internal/mithril-dmq/src/message.rs Updated to call parameterless get_current_kes_period() method
internal/cardano-node/mithril-cardano-node-chain/src/chain_observer/interface.rs Updated trait definition to remove OpCert parameter
internal/cardano-node/mithril-cardano-node-chain/src/chain_observer/cli_observer.rs Refactored to calculate KES period from slot number instead of operational certificate
internal/cardano-node/mithril-cardano-node-chain/src/chain_observer/pallas_observer.rs Updated implementation to match new interface
Various Cargo.toml files Version bumps for affected crates
Test files Updated mocks and test implementations to match new interface

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jpraynaud jpraynaud force-pushed the jpraynaud/2668-fix-opcert-circular-dependency branch from ad3862d to 4eda1f9 Compare August 26, 2025 10:42
* mithril-cardano-node-chain from `0.1.7` to `0.1.8`
* mithril-dmq from `0.1.8` to `0.1.9`
* mithril-aggregator from `0.7.81` to `0.7.82`
* mithril-signer from `0.2.265` to `0.2.266`
@jpraynaud jpraynaud force-pushed the jpraynaud/2668-fix-opcert-circular-dependency branch from 4eda1f9 to 3d8b215 Compare August 26, 2025 11:57
@jpraynaud jpraynaud merged commit 8098cb9 into main Aug 26, 2025
72 of 73 checks passed
@jpraynaud jpraynaud deleted the jpraynaud/2668-fix-opcert-circular-dependency branch August 26, 2025 12:29
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.

Remove Operational Certificate circular dependency in KES signature

3 participants