Cleanup legacy code from thales era #2340

Alenar · 2025-02-25T15:39:29Z

Content

Important

A backward compatibility workflow should be run after the merge of this PR.

This PR removes legacy code that was remaining to ensure backward compatibility with previous nodes from Thales era. With the transition to the Pythagoras era that code can now safely be removed because all actives nodes in the network have support for the updated code.

Context

Pythagoras era was available since distribution 2445.0, released Nov 7 2024, which include mithril-signer version 0.2.209 and open api version 0.1.35.

Details

Remove pending certificate support

It was needed to synchronise signers when they could not compute the beacon to sign themselves (capacity added in distribution 2442).

Messages changes:

AggregatorFeaturesMessage: remove cardano_transactions_signing_config from capabilities (superseded by epoch settings equivalent fields since distribution 2442).
EpochSettingsMessage: remove deprecated protocol_parameters and next_protocol_parameters (superseded with signer_registration_protocol_parameters since distribution 2445).
RegisterSignatureMessage: signed_message is now mandatory (always send by signers released since distribution 2442).
Remove CardanoDbBeaconMessagePart and SignedEntityMessagePart and use their entities instead: they were added for bacward compatibility in distribution 2450 ⚠️, but the only sensitive case for signers is the fact that nodes released before this PR send an additional network field in the RegisterSignatureMessage (which will now be ignored by serde when de-serializing in said messages in mithril-aggregator).
Remove backward compatibility tests for messages that predate openapi 0.1.35.

Nodes specific changes:

Aggregator stress test (aka load-aggregator): Refactored to send "buffer-able" single signatures messages, so it doesn't need to relies on the /certificate-pending route for synchronisation.
mithril-aggregator: Remove replace_cardano_signing_config_empty_values, it was used as a band-aid to add data for nodes that were running before the introduction of migration 28. All aggregators should have been updated by now.
mithril-client: Add missing SignedEntityType from common re-export.

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 documentation website (if relevant)

Issue(s)

Relates to #2316

github-actions · 2025-02-25T15:47:10Z

Test Results

3 files ± 0 52 suites ±0 10m 24s ⏱️ -13s
1 660 tests - 35 1 660 ✅ - 35 0 💤 ±0 0 ❌ ±0
2 042 runs - 35 2 042 ✅ - 35 0 💤 ±0 0 ❌ ±0

Results for commit dac8109. ± Comparison against base commit 3f77f6b.

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

mithril-aggregator ‑ database::repository::epoch_settings_store::tests::replace_cardano_signing_config_empty_values_updates_only_empty_values
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::get_certificate_pending_with_existing_certificate
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::get_certificate_pending_with_no_existing_certificate
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::remove_certificate_pending
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::save_certificate_pending_once
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::should_migrate_data_from_adapter
mithril-aggregator ‑ database::repository::pending_certificate_repository::test::update_certificate_pending
mithril-aggregator ‑ http_server::routes::certificate_routes::tests::test_certificate_pending_get_ko_500
mithril-aggregator ‑ http_server::routes::certificate_routes::tests::test_certificate_pending_with_content_get_ok_200
mithril-aggregator ‑ http_server::routes::certificate_routes::tests::test_certificate_pending_without_content_get_ok_204
…

mithril-common ‑ messages::aggregator_features::tests::test_current_json_deserialized_into_message_supported_until_open_api_0_1_45
mithril-common ‑ messages::register_signature::tests::test_json_until_open_api_0_1_45_deserialized_into_current_message
mithril-persistence ‑ database::version_checker::tests::test_upgrade_with_migration_with_a_version_gap

♻️ This comment has been updated with latest results.

sfauvel

LGTM

jpraynaud

LGTM 🔥

openapi.yaml

dlachaume

LGTM 🚀

Removing the need to wait for pending certificates.

…ion tests + simplify CardanoTransactions integration tests by prepending blocks to scan in init so first signature don't fail because of an empty merkle tree.

…ureMessage` All signers released since the `Pythagoras` era addition always send this metadata, allowing us to make it mandatory.

There's still some signers running a version without the backward compatibility support (v`0.2.209`, distribution `2445.0`), but they don't need it to be compatibile with aggregators that include this commit as it only means that they will send an extra field when registering signatures (those fields will be discarded by serde).

…age`

…cardano_signing_config_empty_values` As all aggregators have been updated.

…orFeaturesMessage` As signers now retrieve them using the `/epoch-settings` route instead since they can vary over epochs.

…ince Pythagoras era The transition to the Pythagoras era guarentee us that the nodes communication are using at least open api v`0.1.35`.

Resulting in a simple file. Migrations have been squashed up to `v29` as `v30` was the first included after distribution `2445.0`.

* mithril-persistence from `0.2.46` to `0.2.47` * mithril-aggregator from `0.7.6` to `0.7.7` * mithril-client-cli from `0.11.4` to `0.11.5` * mithril-client from `0.11.6` to `0.11.7` * mithril-common from `0.5.6` to `0.5.7` * mithril-relay from `0.1.35` to `0.1.36` * mithril-signer from `0.2.230` to `0.2.231` * mithril-aggregator-fake from `0.4.1` to `0.4.2` * mithril-end-to-end from `0.4.71` to `0.4.72` * openapi.yaml from `0.1.45` to `0.1.46`

Alenar added the refactoring 🛠️ Code refactoring and enhancements label Feb 25, 2025

Alenar requested review from sfauvel, jpraynaud and dlachaume February 25, 2025 15:39

Alenar self-assigned this Feb 25, 2025

sfauvel approved these changes Feb 25, 2025

View reviewed changes

Alenar force-pushed the djo/2316/cleanup-legacy-code-from-thales-era branch from be7c8bb to f838b52 Compare February 25, 2025 16:22

Alenar temporarily deployed to testing-preview February 25, 2025 16:31 — with GitHub Actions Inactive

Alenar had a problem deploying to testing-preview February 25, 2025 16:55 — with GitHub Actions Failure

jpraynaud approved these changes Feb 25, 2025

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

dlachaume approved these changes Feb 25, 2025

View reviewed changes

Alenar added 14 commits February 26, 2025 10:51

feat: make load-aggregator test compatible with buffered signatures

07916fb

Removing the need to wait for pending certificates.

refactor: remove pending certificate

2abc85f

refactor(signer): remove reference to pending certificate in integrat…

201bec0

…ion tests + simplify CardanoTransactions integration tests by prepending blocks to scan in init so first signature don't fail because of an empty merkle tree.

refactor(message): make signed_message mandatory in `RegisterSignat…

c2f0e4c

…ureMessage` All signers released since the `Pythagoras` era addition always send this metadata, allowing us to make it mandatory.

feat(client): add missing SignedEntityType in reexported common types

383b772

refactor: remove protocol & next_protocol from `EpochSettingsMess…

675d212

…age`

refactor(aggregator): remove deprecated `EpochSettingsStore::replace_…

ef4c286

…cardano_signing_config_empty_values` As all aggregators have been updated.

refactor: remove cardano_transactions_signing_config from `Aggregat…

f4692ae

…orFeaturesMessage` As signers now retrieve them using the `/epoch-settings` route instead since they can vary over epochs.

test(common): remove backward compatibility messages tests obsolete s…

9cac3d3

…ince Pythagoras era The transition to the Pythagoras era guarentee us that the nodes communication are using at least open api v`0.1.35`.

chore(aggregator-fake): update json test data

d0b4912

chore: remove remaining reference to pending certificate

1681670

chore(aggregator): squash main database migrations

b384a30

Resulting in a simple file. Migrations have been squashed up to `v29` as `v30` was the first included after distribution `2445.0`.

Alenar force-pushed the djo/2316/cleanup-legacy-code-from-thales-era branch from 1e6868d to dac8109 Compare February 26, 2025 09:51

Alenar temporarily deployed to testing-preview February 26, 2025 10:00 — with GitHub Actions Inactive

Alenar merged commit daedb00 into main Feb 26, 2025
37 of 41 checks passed

Alenar deleted the djo/2316/cleanup-legacy-code-from-thales-era branch February 26, 2025 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cleanup legacy code from thales era #2340

Cleanup legacy code from thales era #2340

Uh oh!

Alenar commented Feb 25, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 25, 2025 •

edited

Loading

Uh oh!

sfauvel left a comment

Uh oh!

jpraynaud left a comment

Uh oh!

Uh oh!

dlachaume left a comment

Uh oh!

Uh oh!

Uh oh!

Cleanup legacy code from thales era #2340

Cleanup legacy code from thales era #2340

Uh oh!

Conversation

Alenar commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Context

Details

Remove pending certificate support

Messages changes:

Nodes specific changes:

Pre-submit checklist

Issue(s)

Uh oh!

github-actions bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

sfauvel left a comment

Choose a reason for hiding this comment

Uh oh!

jpraynaud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dlachaume left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Alenar commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading