Signer certifier service sign & send signatures

[Alenar]

Content

This PR refactor the mithril-signer in order to move the sign & send signatures responsibility from the state machine runner to the signer Certifier service.

One of the main concern for this refactor was to minimize the number of new dependencies that needed to be add to the certifier.
Simply moving the code from the runner to the certifier would have result in adding the following dependencies:

• epoch_service and protocol_initializer_store and single_signer: to issue the single signature.
• certificate_handler: to send the signatures but we would only need one method of it's whole api.

Avoiding protocol_initializer_store dependency

The protocol initializer of the current epoch is now stored on the epoch service.
It's stored as an option since it's only available if a signer is registered in the Mithril stake distribution of the current epoch.
This is used in the next refactor.

Refocus single signer service

The single_signer service should be able to issue single signatures without needing parameters that must be computed with other dependencies.
To do so the compute_single_signatures now only take a protocol message as parameter, the protocol_initializer and the list of signers_with_stake are fetched from the epoch service.

The compute_aggregate_verification_key was only used by the SignerSignableSeedBuilder and need not the current protocol_initializer but the next one, a data not available in the epoch service.
I choose to remove this method from the single signer api and instead allow the SignerSignableSeedBuilderto do the computation by using the lower level tools since it already had the signers & protocol initializer needed.

Dependency inversion for signature publishing

By adding a SignaturePublisher trait defined for the certifier needs. It's automatically implement it for every AggregatorClient trait implemetors.

Additional refactor

Since the epoch service now hold all data needed for the can_signer_sign_current_epoch function, it's code is moved from the signer state machine runner directly to the epoch service.

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
• Documentation
  • Update README file (if relevant)
  • Update documentation website (if relevant)
  • Add dev blog post (if relevant)

Issue(s)

Closes #1945

[github-actions]

Test Results

    4 files  ±0     54 suites  ±0   9m 17s ⏱️ -14s
1 330 tests ±0  1 330 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 541 runs  ±0  1 541 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 8c8b8d2. ± Comparison against base commit d0ecaad.

This pull request removes 9 and adds 9 tests. Note that renamed tests count towards both.

mithril-signer ‑ runtime::runner::tests::can_sign_current_epoch_returns_false_when_cannot_get_protocol_initializer_for_current_epoch
mithril-signer ‑ runtime::runner::tests::can_sign_current_epoch_returns_false_when_epoch_service_returns_false
mithril-signer ‑ runtime::runner::tests::can_sign_current_epoch_returns_true_when_epoch_service_returns_true
mithril-signer ‑ runtime::runner::tests::can_sign_current_epoch_when_epoch_service_returns_error
mithril-signer ‑ runtime::runner::tests::test_compute_message
mithril-signer ‑ runtime::runner::tests::test_compute_single_signature
mithril-signer ‑ runtime::runner::tests::test_send_single_signature
mithril-signer ‑ services::certifier::tests::mark_beacon_as_signed_update_the_store
mithril-signer ‑ services::single_signer::tests::compute_aggregate_verification_key_success

mithril-signer ‑ services::certifier::tests::beacon_isnt_mark_as_signed_if_computing_signature_fails
mithril-signer ‑ services::certifier::tests::beacon_isnt_mark_as_signed_if_publishing_signature_fails
mithril-signer ‑ services::certifier::tests::compute_publish_single_signature_success_but_dont_publish_if_no_signature_were_issued
mithril-signer ‑ services::certifier::tests::compute_publish_single_signature_success_if_a_signature_was_issued
mithril-signer ‑ services::epoch_service::tests::can_signer_sign_current_epoch::can_sign_if_in_current_signers_and_use_the_stored_protocol_initializer
mithril-signer ‑ services::epoch_service::tests::can_signer_sign_current_epoch::cant_sign_if_no_protocol_initializer_are_stored
mithril-signer ‑ services::epoch_service::tests::can_signer_sign_current_epoch::cant_sign_if_not_in_current_signers
mithril-signer ‑ services::epoch_service::tests::can_signer_sign_current_epoch::cant_sign_if_stored_protocol_initializer_belong_to_another_party
mithril-signer ‑ services::epoch_service::tests::test_protocol_initializer_is_available_after_register_epoch_settings_call_if_in_store

♻️ This comment has been updated with latest results.

[jpraynaud]

LGTM 👍

[sfauvel]

Good job
Just few remarks on tests organization

[sfauvel]

We probably can create a function to initialize the epoch_service without losing important information for the test.
This would highlight data that varies between tests.

Below is an example of what this could look like

    fn build_epoch_service() {
        let epoch = Epoch(12);
        let stake_store = Arc::new(StakeStore::new(Box::new(DumbStoreAdapter::new()), None));
        let protocol_initializer_store = Arc::new(ProtocolInitializerStore::new(
            Box::new(DumbStoreAdapter::new()),
            None,
        ));

        let epoch_service = MithrilEpochService::new(stake_store, protocol_initializer_store)
            .set_data_to_default_or_fake(epoch);
        return epoch_service;
    }

    #[test]
    fn signer_can_sign_if_in_current_signers_and_use_the_stored_protocol_initializer() {

        let fixtures = MithrilFixtureBuilder::default().with_signers(10).build();
 
        let epoch_service = build_epoch_service()
            .alter_data(|data| {
                data.protocol_initializer =
                    Some(fixtures.signers_fixture()[0].protocol_initializer.clone());
                data.current_signers = fixtures.signers();
            });

        let can_sign_current_epoch = epoch_service
            .can_signer_sign_current_epoch(fixtures.signers()[0].party_id.clone())
            .unwrap();
        assert!(can_sign_current_epoch);
    }

[Alenar]

I used MithrilEpochService::new_with_dumb_dependencies test method which does exactly that.

[sfauvel]

Those 4 tests about protocol initializer may be into a sub-module.
We also may have test_protocol_initializer_is_available_after_register_epoch_settings_call_if_in_store() in this module

[Alenar]

I grouped the four tests together under a 'can_signer_sign_current_epoch' umbrella, but since this name doesn't fit the last test that you suggested to move I kept it where it was.

[dlachaume]

LGTM 👍