Skip to content
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

Refactor signer state machine #1931

Merged
merged 25 commits into from
Sep 19, 2024
Merged

Conversation

dlachaume
Copy link
Collaborator

@dlachaume dlachaume commented Sep 11, 2024

Content

This PR includes an update of the state machine of the signer in order to prepare the phase of the decentralization of the signature orchestration.

States modifications:

  • Signed state is removed
  • ReadyToSign and RegisterNotAbleToSign states are added

Transitions:

  • Unregistered -> RegisteredNotAbleToSign when registration succeeds but the signer can not sign the current epoch
  • Unregistered -> ReadyToSign when registration succeeds and the signer can sign the current epoch
  • ReadyToSign -> ReadyToSign when trying to create a new message if any and sign it

Mithril - Signer Runtime Loop v3

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 #1922

@dlachaume dlachaume self-assigned this Sep 11, 2024
Copy link

github-actions bot commented Sep 11, 2024

Test Results

    4 files  ±0     53 suites  ±0   9m 40s ⏱️ -1s
1 261 tests +9  1 261 ✅ +9  0 💤 ±0  0 ❌ ±0 
1 472 runs  +9  1 472 ✅ +9  0 💤 ±0  0 ❌ ±0 

Results for commit a8a3b5e. ± Comparison against base commit 9fd9ae8.

This pull request removes 9 and adds 18 tests. Note that renamed tests count towards both.
mithril-signer ‑ runtime::runner::tests::test_can_i_sign
mithril-signer ‑ runtime::state_machine::tests::registered_to_registered
mithril-signer ‑ runtime::state_machine::tests::registered_to_signed
mithril-signer ‑ runtime::state_machine::tests::registered_to_unregistered
mithril-signer ‑ runtime::state_machine::tests::signed_to_registered
mithril-signer ‑ runtime::state_machine::tests::signed_to_signed_no_pending_certificate
mithril-signer ‑ runtime::state_machine::tests::signed_to_signed_unsigned_pending_certificate
mithril-signer ‑ runtime::state_machine::tests::signed_to_unregistered
mithril-signer ‑ runtime::state_machine::tests::unregistered_to_registered
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::can_sign_signed_entity_type_when_signed_entity_type_is_locked
mithril-signer ‑ runtime::runner::tests::can_sign_signed_entity_type_when_signed_entity_type_is_not_locked
mithril-signer ‑ runtime::state_machine::tests::ready_to_sign_to_ready_to_sign_when_can_sign_the_first_signed_entity_type
mithril-signer ‑ runtime::state_machine::tests::ready_to_sign_to_ready_to_sign_when_different_state_than_previous_one
mithril-signer ‑ runtime::state_machine::tests::ready_to_sign_to_ready_to_sign_when_different_state_than_previous_return_state_with_same_epoch
mithril-signer ‑ runtime::state_machine::tests::ready_to_sign_to_ready_to_sign_when_same_signed_entity_type
…

♻️ This comment has been updated with latest results.

@dlachaume dlachaume force-pushed the ensemble/1922/refactor-signer-state-machine branch from c8f619c to 2064e90 Compare September 11, 2024 15:55
mithril-signer/src/runtime/state_machine.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Outdated Show resolved Hide resolved
mithril-signer/src/services/epoch_service.rs Outdated Show resolved Hide resolved
@dlachaume dlachaume changed the title Ensemble/1922/refactor signer state machine Refactor signer state machine Sep 12, 2024
@dlachaume dlachaume marked this pull request as ready for review September 12, 2024 15:12
@sfauvel sfauvel force-pushed the ensemble/1922/refactor-signer-state-machine branch from fbfe294 to 1aac955 Compare September 13, 2024 09:13
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.

Overall looks good 👍

I left few comments and suggestions below.

docs/website/root/mithril/mithril-network/signer.md Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/state_machine.rs Outdated Show resolved Hide resolved
mithril-signer/src/runtime/runner.rs Outdated Show resolved Hide resolved
@sfauvel sfauvel force-pushed the ensemble/1922/refactor-signer-state-machine branch from 54e66db to 89ffca3 Compare September 16, 2024 16:24
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 👍

dlachaume and others added 7 commits September 18, 2024 16:00
Co-authored-by: Jean-Philippe Raynaud <jpraynaud@users.noreply.github.com>
Co-authored-by: Sébastien Fauvel <sfauvel@users.noreply.github.com>
Co-authored-by: Sébastien Fauvel <sfauvel@users.noreply.github.com>
…oSign' when the last signed entity type is the same as the one in the pending certificate.
Co-authored-by: Sébastien Fauvel <sfauvel@users.noreply.github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
@dlachaume dlachaume force-pushed the ensemble/1922/refactor-signer-state-machine branch from ea61195 to a8a3b5e Compare September 18, 2024 16:20
@sfauvel sfauvel merged commit 62dbe88 into main Sep 19, 2024
39 of 49 checks passed
@sfauvel sfauvel deleted the ensemble/1922/refactor-signer-state-machine branch September 19, 2024 08:05
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.

Refactor state machine of the signer
4 participants