Skip to content

Feat: implement aggregator slave signer registration mode #2351

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.

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
merged 23 commits into from
Mar 10, 2025

Conversation

jpraynaud
Copy link
Member

@jpraynaud jpraynaud commented Mar 5, 2025

Content

This PR includes the implementation of a slave signer registration mode in the aggregator.

This pull request introduces several changes to the mithril-aggregator project, focusing on implementing a slave signer registration mode in the aggregator, updating dependencies, and refining the dependency injection system. The most important changes are summarized below:

New Features and Enhancements:

  • Implemented a slave signer registration mode in the aggregator, allowing the aggregator to fetch the latest epoch settings and store signer registrations from a master aggregator endpoint. (CHANGELOG.md - [1] mithril-aggregator/src/commands/serve_command.rs - [2] [3] [4] mithril-aggregator/src/configuration.rs - [5] [6] [7] [8]

Dependency Updates:

  • Added new dependencies http and httpmock. (mithril-aggregator/Cargo.toml - [1] [2]

Dependency Injection Improvements:

  • Added new services and clients to the dependency injection builder to support the slave signer registration mode, including AggregatorClient, MithrilSignerRegistrationMaster, MithrilSignerRegistrationSlave, and others. (mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs - [1] [2] [3] mithril-aggregator/src/dependency_injection/builder/mod.rs - [4] [5] [6] [7] [8] [9] [10] [11]

Configuration Changes:

  • Added a new configuration parameter master_aggregator_endpoint to indicate whether the aggregator is running in master or slave mode. (mithril-aggregator/src/configuration.rs - [1] [2] [3]

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Closes #2334
Closes #2335

@jpraynaud jpraynaud self-assigned this Mar 5, 2025
Copy link

github-actions bot commented Mar 5, 2025

Test Results

    3 files  ± 0     56 suites  +1   10m 32s ⏱️ +3s
1 713 tests +30  1 713 ✅ +30  0 💤 ±0  0 ❌ ±0 
2 105 runs  +30  2 105 ✅ +30  0 💤 ±0  0 ❌ ±0 

Results for commit 4c90983. ± Comparison against base commit 468de2d.

This pull request removes 23 and adds 53 tests. Note that renamed tests count towards both.
mithril-aggregator ‑ http_server::routes::epoch_routes::tests::get_epoch_settings_message_retrieves_protocol_parameters_from_epoch_service
mithril-aggregator ‑ http_server::routes::epoch_routes::tests::get_epoch_settings_message_retrieves_signing_configuration_from_epoch_service
mithril-aggregator ‑ http_server::routes::epoch_routes::tests::get_epoch_settings_message_with_cardano_transactions_enabled
mithril-aggregator ‑ http_server::routes::epoch_routes::tests::get_epoch_settings_message_with_cardano_transactions_not_enabled
mithril-aggregator ‑ message_adapters::from_register_signature::tests::test_simple_message
mithril-aggregator ‑ message_adapters::from_register_signer::tests::test_simple_message
mithril-aggregator ‑ message_adapters::to_cardano_transactions_proof_message::tests::test_simple_message
mithril-aggregator ‑ runtime::state_machine::tests::critical_error
mithril-aggregator ‑ runtime::state_machine::tests::idle_check_certificate_chain_is_not_valid
mithril-aggregator ‑ runtime::state_machine::tests::idle_check_certificate_chain_is_valid
…
mithril-aggregator ‑ configuration::test::is_slave_aggregator_returns_false_when_in_master_mode
mithril-aggregator ‑ configuration::test::is_slave_aggregator_returns_true_when_in_slave_mode
mithril-aggregator ‑ message_adapters::from_epoch_settings::tests::try_adapt_epoch_settings_message_to_entity
mithril-aggregator ‑ message_adapters::from_register_signature::tests::try_adapt_single_signatures_message_to_entity
mithril-aggregator ‑ message_adapters::from_register_signer::tests::try_adapt_signer_registration_message_to_entity
mithril-aggregator ‑ message_adapters::to_cardano_transactions_proof_message::tests::try_adapt_cardano_transaction_proof_to_message
mithril-aggregator ‑ runtime::state_machine::tests::master::critical_error
mithril-aggregator ‑ runtime::state_machine::tests::master::idle_check_certificate_chain_is_not_valid
mithril-aggregator ‑ runtime::state_machine::tests::master::idle_check_certificate_chain_is_valid
mithril-aggregator ‑ runtime::state_machine::tests::master::ready_certificate_does_not_exist_for_time_point
…

♻️ This comment has been updated with latest results.

@jpraynaud jpraynaud temporarily deployed to testing-preview March 5, 2025 17:26 — with GitHub Actions Inactive
@jpraynaud jpraynaud force-pushed the jpraynaud/2334-aggregator-slave-registration-mode branch from 335da84 to b967279 Compare March 5, 2025 17:49
@jpraynaud jpraynaud temporarily deployed to testing-preview March 5, 2025 18:01 — with GitHub Actions Inactive
@jpraynaud jpraynaud force-pushed the jpraynaud/2334-aggregator-slave-registration-mode branch from b967279 to 11f1df6 Compare March 6, 2025 14:14
@jpraynaud jpraynaud force-pushed the jpraynaud/2334-aggregator-slave-registration-mode branch from 11f1df6 to 866de28 Compare March 6, 2025 14:23
@jpraynaud jpraynaud marked this pull request as ready for review March 6, 2025 14:26
@jpraynaud jpraynaud temporarily deployed to testing-preview March 6, 2025 14:32 — with GitHub Actions Inactive
Copy link
Collaborator

@Alenar Alenar left a comment

Choose a reason for hiding this comment

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

LGTM

@jpraynaud jpraynaud temporarily deployed to testing-preview March 6, 2025 18:07 — with GitHub Actions Inactive
Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM
Some remarks to improve readability

@jpraynaud jpraynaud force-pushed the jpraynaud/2334-aggregator-slave-registration-mode branch from 802e171 to 4c90983 Compare March 7, 2025 17:05
@jpraynaud jpraynaud temporarily deployed to testing-preview March 7, 2025 17:13 — with GitHub Actions Inactive
@jpraynaud jpraynaud merged commit 971b382 into main Mar 10, 2025
37 checks passed
@jpraynaud jpraynaud deleted the jpraynaud/2334-aggregator-slave-registration-mode branch March 10, 2025 08:36
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.

Add integration test for mode for the signer registration in the aggregator Implement a follower mode for the signer registration in the aggregator
4 participants