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

Provide latest immutable file number with certified ctx #1571

Conversation

sfauvel
Copy link
Collaborator

@sfauvel sfauvel commented Mar 15, 2024

Content

This PR add the latest immutable file number in the sign certificate protocol message.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are 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 #1536

Copy link

github-actions bot commented Mar 15, 2024

Test Results

    3 files  ±0     42 suites  ±0   8m 36s ⏱️ -11s
  920 tests +7    920 ✅ +7  0 💤 ±0  0 ❌ ±0 
1 009 runs  +7  1 009 ✅ +7  0 💤 ±0  0 ❌ ±0 

Results for commit 98e7ee5. ± Comparison against base commit 9622126.

This pull request removes 1 and adds 8 tests. Note that renamed tests count towards both.
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash
mithril-aggregator ‑ http_server::routes::proof_routes::tests::build_response_message_return_immutable_file_number_from_artifact_beacon
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash_include_cardano_transactions_merkle_root
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash_include_lastest_immutable_file_number
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash_include_next_aggregate_verification_key
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash_include_snapshot_digest
mithril-common ‑ entities::protocol_message::tests::test_protocol_message_compute_hash_the_same_hash_with_same_protocol_message
mithril-common ‑ messages::cardano_transactions_proof::tests::verify_hashes_from_verified_cardano_transaction_and_from_signable_builder_are_equals
mithril-common ‑ signable_builder::cardano_transactions::tests::test_compute_signable_with_no_transaction_return_error

♻️ This comment has been updated with latest results.

@dlachaume dlachaume force-pushed the ensemble/1536/provide-latest-immutable-file-number-with-certified-ctx branch from c0ba84d to 58f2850 Compare March 15, 2024 15:23
@dlachaume dlachaume temporarily deployed to testing-preview March 15, 2024 15:52 — with GitHub Actions Inactive
@dlachaume dlachaume temporarily deployed to testing-sanchonet March 15, 2024 15:52 — with GitHub Actions Inactive
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.

Looks good overall 👍

The PR is missing the client CLI and WASM parts.
I have also left few comments below.

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 👍

@sfauvel sfauvel force-pushed the ensemble/1536/provide-latest-immutable-file-number-with-certified-ctx branch from 98364df to 98e7ee5 Compare March 19, 2024 09:40
@sfauvel sfauvel temporarily deployed to testing-preview March 19, 2024 09:46 — with GitHub Actions Inactive
@sfauvel sfauvel temporarily deployed to testing-sanchonet March 19, 2024 09:46 — with GitHub Actions Inactive
@sfauvel sfauvel merged commit a52e282 into main Mar 19, 2024
41 checks passed
@sfauvel sfauvel deleted the ensemble/1536/provide-latest-immutable-file-number-with-certified-ctx branch March 19, 2024 09:49
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.

Provide latest immutable file number with certified Cardano transactions in client
3 participants