Skip to content

refactor: move SignaturePublisher to a dedicated module #2437

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

Jump to bottom
Merged
merged 3 commits into from
Apr 29, 2025
Merged

refactor: move SignaturePublisher to a dedicated module #2437

merged 3 commits into from
Apr 29, 2025

Conversation

@turmelclem
Copy link
Collaborator

@turmelclem turmelclem commented Apr 24, 2025 •
edited by Alenar
Loading

Content

This PR refactor the SignaturePublisher Trait by moving it to a dedicated module, making it more visible and facilitating future addition of new implementations.

Pre-submit checklist

  • Branch
    • 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
  • Documentation
    • No new TODOs introduced

Comments

The work asked in ticket #2427 (creation and implementation of SignaturePublisher Trait) was already done in PR #1979

Issue(s)

Closes #2427

@turmelclem turmelclem self-assigned this Apr 24, 2025
@github-actions
Copy link

github-actions bot commented Apr 24, 2025 •
edited
Loading

Test Results

    3 files  ±0     57 suites  ±0   11m 34s ⏱️ +4s
1 875 tests ±0  1 875 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 337 runs  ±0  2 337 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b96b34e. ± Comparison against base commit f8f036c.

This pull request removes 12 and adds 12 tests. Note that renamed tests count towards both. 
mithril-aggregator ‑ database::record::buffered_single_signature_record::tests::test_convert_single_signatures
mithril-aggregator ‑ database::record::single_signature::tests::test_convert_single_signatures
mithril-aggregator ‑ message_adapters::from_register_signature::tests::try_adapt_single_signatures_message_to_entity
mithril-common ‑ entities::single_signatures::tests::single_signatures_should_convert_to_protocol_signatures
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ko_400
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ko_409
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ko_412
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ko_500
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ok_201
mithril-signer ‑ services::aggregator_client::tests::test_register_signatures_ok_202
…

mithril-aggregator ‑ database::record::buffered_single_signature_record::tests::test_convert_single_signature
mithril-aggregator ‑ database::record::single_signature::tests::test_convert_single_signature
mithril-aggregator ‑ message_adapters::from_register_signature::tests::try_adapt_single_signature_message_to_entity
mithril-common ‑ entities::single_signature::tests::single_signatures_should_convert_to_protocol_signatures
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ko_400
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ko_409
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ko_412
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ko_500
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ok_201
mithril-signer ‑ services::aggregator_client::tests::test_register_signature_ok_202
…

♻️ This comment has been updated with latest results.

dlachaume
dlachaume approved these changes Apr 24, 2025
View reviewed changes
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 🚀

jpraynaud
jpraynaud reviewed Apr 24, 2025
View reviewed changes
@turmelclem turmelclem force-pushed the ensemble/2427/Implement_signer_publisher_abstraction branch from 623bf5c to 079f847 Compare April 24, 2025 14:48
@turmelclem turmelclem force-pushed the ensemble/2427/Implement_signer_publisher_abstraction branch from 0b65707 to ce75827 Compare April 25, 2025 12:32
@turmelclem turmelclem force-pushed the ensemble/2427/Implement_signer_publisher_abstraction branch from ce75827 to 93e8797 Compare April 25, 2025 12:52
jpraynaud
jpraynaud approved these changes Apr 25, 2025
View reviewed changes
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

sfauvel
sfauvel approved these changes Apr 25, 2025
View reviewed changes
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

@turmelclem turmelclem force-pushed the ensemble/2427/Implement_signer_publisher_abstraction branch from 93e8797 to 1ed6934 Compare April 29, 2025 07:49
Clément TURMEL added 3 commits April 29, 2025 10:42
refactor: move SignaturePublisher to a dedicated module
056dac3
refactor: rename SingleSignatures to SingleSignature
3a35a8d
chore: upgrade crate versions
b96b34e 
* mithril-aggregator from `0.7.41` to `0.7.42`
* mithril-common from `0.5.25` to `0.5.26`
* mithril-signer from `0.2.240` to `0.2.241`
@turmelclem turmelclem force-pushed the ensemble/2427/Implement_signer_publisher_abstraction branch from 1ed6934 to b96b34e Compare April 29, 2025 08:51
@turmelclem turmelclem merged commit b9230e9 into main Apr 29, 2025
38 of 39 checks passed
@turmelclem turmelclem deleted the ensemble/2427/Implement_signer_publisher_abstraction branch April 29, 2025 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alenar Alenar Alenar approved these changes

@jpraynaud jpraynaud jpraynaud approved these changes

+2 more reviewers

@sfauvel sfauvel sfauvel approved these changes

@dlachaume dlachaume dlachaume approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@turmelclem turmelclem

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement a publisher abstraction for publication of signatures in signer

6 participants

@turmelclem @Alenar @sfauvel @jpraynaud @dlachaume