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.

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

Retrieve custom signing configuration with epoch settings in Signer #1939

Merged
merged 9 commits into from
Sep 20, 2024
Merged

Retrieve custom signing configuration with epoch settings in Signer #1939

merged 9 commits into from
Sep 20, 2024

Conversation

dlachaume
Copy link
Collaborator

@dlachaume dlachaume commented Sep 18, 2024 •
edited
Loading

Content

This PR includes a way for the signer to access the Cardano transaction signing configuration from the aggregator via the /epoch-settings route.
The configuration is retrieved by the signer at each epoch transition and stored in its 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 #1923

@dlachaume dlachaume self-assigned this Sep 18, 2024
@dlachaume dlachaume force-pushed the ensemble/1922/refactor-signer-state-machine branch from ea61195 to a8a3b5e Compare September 18, 2024 16:20
@dlachaume dlachaume force-pushed the ensemble/1923/retrieve-custom-signing-config-with-epoch-settings branch from 1d63ff8 to 74e0494 Compare September 18, 2024 16:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 18, 2024 •
edited
Loading

Test Results

    4 files  ±0     54 suites  ±0   9m 32s ⏱️ -8s
1 293 tests +3  1 293 ✅ +3  0 💤 ±0  0 ❌ ±0 
1 504 runs  +3  1 504 ✅ +3  0 💤 ±0  0 ❌ ±0 

Results for commit 6e9fb44. ± Comparison against base commit 27fdfa0.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both. 
mithril-common ‑ messages::epoch_settings::tests::test_actual_json_deserialized_into_previous_message

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-common ‑ messages::epoch_settings::tests::test_actual_json_deserialized_into_message_supported_until_open_api_0_1_28
mithril-common ‑ messages::epoch_settings::tests::test_actual_json_deserialized_into_message_supported_until_open_api_0_1_29

♻️ This comment has been updated with latest results.

@dlachaume dlachaume marked this pull request as ready for review September 19, 2024 08:03
Base automatically changed from ensemble/1922/refactor-signer-state-machine to main September 19, 2024 08:05
sfauvel
sfauvel approved these changes Sep 19, 2024
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

mithril-aggregator/src/http_server/routes/epoch_routes.rs Outdated Show resolved Hide resolved
Alenar
Alenar approved these changes Sep 19, 2024
View reviewed changes
openapi.yaml Outdated Show resolved Hide resolved
dlachaume and others added 7 commits September 19, 2024 18:30
@dlachaume
feat: add current_cardano_transactions_signing_config to `EpochSett…
5b3bcbe 
…ingsMessage`

- Add optional `current_cardano_transactions_signing_config` field, used only when 'CardanoTransactions' is activated by the aggregator.

- Add retro compatibility test with the original 'EpochSettingsMessage' structure, identified as 'Legacy', and the previous 'EpochSettingsMessage', identified as 'Previous'.
@dlachaume
feat: update OpenAPI specification
694734a 
Add new `current_cardano_transactions_signing_config` field which is returned by the '/epoch-settings' route.
@dlachaume
feat: update /epoch-routes with `current_cardano_transactions_signi…
ee63b2b 
…ng_config`
@dlachaume
feat: store current_cardano_transactions_signing_config in signer e…
44f93b3 
…poch service
@dlachaume
feat: add next_cardano_transactions_signing_config to `EpochSetting…
48c5115 
…sMessage` and rename `current_cardano_transactions_signing_config` to `cardano_transactions_signing_config`

- refactor backward compatibility tests with the latest OpenAPI version supported by each structure
- update OpenAPI specifications with `next_cardano_transactions_signing_config` and refactor by extracting to a dedicated 'CardanoTransactionsSigningConfig' component
- make the '/epoch-settings' HTTP route return the correct data
@dlachaume
feat: adapt signer with the new message structure
d4330bb
@dlachaume @sfauvel
refactor: remove unnecessary dependencies, refactor 'epoch_settings' …
72766c9 
…function by separating responsibilities

Co-authored-by: Sébastien Fauvel <sfauvel@users.noreply.github.com>
@dlachaume dlachaume force-pushed the ensemble/1923/retrieve-custom-signing-config-with-epoch-settings branch from 74e0494 to 72766c9 Compare September 19, 2024 16:37
@dlachaume
chore: reference the feature in the CHANGELOG
45bed99
@dlachaume
chore: bump crates versions and openapi.yaml version
6e9fb44 
* mithril-aggregator from `0.5.64` to `0.5.65`
* mithril-common from `0.4.52` to `0.4.53`
* mithril-signer from `0.2.184` to `0.2.185`
jpraynaud
jpraynaud approved these changes Sep 20, 2024
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 👍

@dlachaume dlachaume merged commit 64daa84 into main Sep 20, 2024
39 of 40 checks passed
@dlachaume dlachaume deleted the ensemble/1923/retrieve-custom-signing-config-with-epoch-settings branch September 20, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpraynaud jpraynaud jpraynaud approved these changes

@sfauvel sfauvel sfauvel approved these changes

@Alenar Alenar Alenar approved these changes

Assignees

@dlachaume dlachaume

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Retrieve custom signing configurations with epoch settings in signer
4 participants
@dlachaume @Alenar @sfauvel @jpraynaud