Skip to content

Conversation

@turmelclem
Copy link
Collaborator

@turmelclem turmelclem commented Jun 26, 2025

Content

This PR includes...

mithril aggragtor :

  • Remove CardanoImmutableFilesFull to keep only MithrilStakeDistribution in the DEFAULT_ALLOWED_DISCRIMINANTS
  • replace CardanoImmutableFilesFull with CardanoDatabase in allowed_discriminants (signed entity config) of aggregator's tests
  • replace CardanoImmutableFilesFull with CardanoDatabase in signed_entity_types of aggregator's integration tests

mithril signer :

  • add CardanoImmutableFilesFull in signed entities of signer's integration tests

test-lab :

  • add CardanoImmutableFilesFull in signed_entity_types of stress test

backward compatibility github action :

  • add CardanoImmutableFilesFull in signed_entity_types of stress test

mithril client CLI & test-client.yml :

  • rename cardano database directory from db_v2 to db when using cardano-db download --backend v2

and some documentation improvements

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
    • All check jobs of the CI have succeeded
    • 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)
    • Add ADR blog post or Dev ADR entry (if relevant)
    • No new TODOs introduced

Issue(s)

Closes #2577

@turmelclem turmelclem self-assigned this Jun 26, 2025
@github-actions
Copy link

github-actions bot commented Jun 26, 2025

Test Results

    3 files  ±0    105 suites  ±0   17m 29s ⏱️ +41s
2 071 tests ±0  2 071 ✅ ±0  0 💤 ±0  0 ❌ ±0 
4 285 runs  ±0  4 285 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 77385e4. ± Comparison against base commit 028683b.

♻️ This comment has been updated with latest results.

@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch 2 times, most recently from c4ea96d to 683b515 Compare June 26, 2025 10:22
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2025 10:31 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from b993cdb to 66a378d Compare June 26, 2025 15:55
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2025 16:18 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 66a378d to f10f9fc Compare June 27, 2025 07:46
@turmelclem turmelclem temporarily deployed to testing-preview June 27, 2025 08:03 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from f10f9fc to 538b882 Compare June 27, 2025 10:10
@turmelclem turmelclem temporarily deployed to testing-preview June 27, 2025 10:27 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 538b882 to c825406 Compare June 27, 2025 12:57
@turmelclem turmelclem temporarily deployed to testing-preview June 27, 2025 13:14 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch 3 times, most recently from 191fa50 to e4324e0 Compare June 27, 2025 14:32
@turmelclem turmelclem temporarily deployed to testing-preview June 27, 2025 14:51 — with GitHub Actions Inactive

This comment was marked as outdated.

@turmelclem turmelclem temporarily deployed to testing-preview June 27, 2025 15:30 — with GitHub Actions Inactive
@turmelclem turmelclem marked this pull request as ready for review June 27, 2025 15:30
@turmelclem turmelclem requested a review from Copilot June 27, 2025 15:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR stabilizes the Cardano DB v2 aggregator and signer tests by adjusting allowed discriminants and updating download directory parameters. Key changes include removing CardanoImmutableFilesFull from the default allowed discriminants, adding it explicitly in tests and configurations, and standardizing download directory naming with the new --download-dir option.

  • Updated allowed discriminants in various configuration and test files.
  • Revised CLI download commands to use a consistent directory naming scheme.
  • Improved documentation to reflect these changes.

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.

Show a summary per file
File Description
mithril-test-lab/mithril-end-to-end/src/stress_test/aggregator_helpers.rs Adds CardanoImmutableFilesFull to test configuration.
mithril-test-lab/mithril-end-to-end/src/mithril/client.rs Introduces the --download-dir parameter for both v1 and v2 downloads.
mithril-test-lab/mithril-end-to-end/src/main.rs Updates the default signed_entity_types to include CardanoImmutableFilesFull.
mithril-signer/tests/create_cardano_transaction_single_signature.rs Updates tests to allow CardanoImmutableFilesFull as a signed entity.
mithril-common/src/entities/signed_entity_config.rs Removes CardanoImmutableFilesFull from DEFAULT_ALLOWED_DISCRIMINANTS.
mithril-client-cli/src/commands/cardano_db/download/* Refactors download commands to use a constant directory name (DB_DIRECTORY_NAME).
mithril-aggregator/tests/* & src/runtime/runner.rs Adapts test helpers to handle the explicit allowed discriminants configuration.
docs/website/* Adjusts documentation to reflect the new download directory logic and configuration changes.
.github/workflows/* Updates CI workflow commands to include --download-dir with the appropriate directory names.
Comments suppressed due to low confidence (2)

mithril-aggregator/src/configuration.rs:491

  • The documentation comment is unclear; consider rephrasing it to 'The value MithrilStakeDistribution is automatically prepended to the list.'
    /// The value `MithrilStakeDistribution` is prepended is automatically to the list.

docs/website/root/manual/develop/nodes/mithril-client.md:581

  • Please verify that updating the default backend to 'v2' in the documentation aligns with the actual default behavior in the client implementation.
| `backend`                  | `--backend`                  |         `-b`         | -                          | -                                                                               | `v2`          | -       |         -          |

@turmelclem turmelclem changed the title Stabilisation of cardano db v2 (aggregator) Stabilization of cardano db v2 (aggregator) Jun 30, 2025
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 🚀

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 🚀

@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 3438ccb to a31cdad Compare June 30, 2025 14:07
@turmelclem turmelclem temporarily deployed to testing-preview June 30, 2025 14:16 — with GitHub Actions Inactive
Copy link
Collaborator

@Alenar Alenar 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 temporarily deployed to testing-preview July 1, 2025 07:25 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch 3 times, most recently from 4742a28 to 03001b4 Compare July 2, 2025 12:07
@turmelclem turmelclem temporarily deployed to testing-preview July 2, 2025 12:16 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch 5 times, most recently from 017ca63 to 2cf0638 Compare July 2, 2025 12:45
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 2cf0638 to 47ab327 Compare July 2, 2025 12:53
@turmelclem turmelclem temporarily deployed to testing-preview July 2, 2025 13:02 — with GitHub Actions Inactive
@turmelclem turmelclem temporarily deployed to testing-preview July 2, 2025 13:22 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 15db34a to 61096f3 Compare July 2, 2025 13:24
* mithril-aggregator from `0.7.70` to `0.7.71`
* mithril-client-cli from `0.12.16` to `0.12.17`
* mithril-common from `0.6.4` to `0.6.5`
* mithril-signer from `0.2.257` to `0.2.258`
* mithril-end-to-end from `0.4.93` to `0.4.94`
@turmelclem turmelclem force-pushed the ctl/2577-stabilize-cardano-db-v2-aggregator branch from 61096f3 to 77385e4 Compare July 2, 2025 13:26
@turmelclem turmelclem temporarily deployed to testing-preview July 2, 2025 13:40 — with GitHub Actions Inactive
@turmelclem turmelclem merged commit d3d0037 into main Jul 2, 2025
39 checks passed
@turmelclem turmelclem deleted the ctl/2577-stabilize-cardano-db-v2-aggregator branch July 2, 2025 13:41
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.

Stabilize Cardano DB v2 client CLI/library - Phase 2

5 participants