Skip to content

Make kes_bytes file creation/write atomic for concurrent access#3353

Open
turmelclem wants to merge 2 commits into
mainfrom
ctl/3229-fix-kes-bytes-file-concurency
Open

Make kes_bytes file creation/write atomic for concurrent access#3353
turmelclem wants to merge 2 commits into
mainfrom
ctl/3229-fix-kes-bytes-file-concurency

Conversation

@turmelclem

@turmelclem turmelclem commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Content

Make kes_bytes file creation/write atomic for concurrent access, especially to fix CI test flakiness

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)

this PR closes #3329

@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch from c5f9f45 to c027a80 Compare June 25, 2026 15:06
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Test Results

     5 files  ± 0     206 suites  ±0   56m 22s ⏱️ - 2h 11m 49s
 3 238 tests  - 72   3 238 ✅  - 72  0 💤 ±0  0 ❌ ±0 
10 899 runs   - 77  10 899 ✅  - 77  0 💤 ±0  0 ❌ ±0 

Results for commit f3cd375. ± Comparison against base commit 5d25797.

This pull request removes 73 and adds 1 tests. Note that renamed tests count towards both.
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_out_of_bounds
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_too_large_for_circuit_range
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::indices_not_increasing
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_merkle_path_mismatch
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_swap_keep_merkle_path
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_wrong_verification_key
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_corrupt_sibling
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_flip_position
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_length_long
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_length_short
…
mithril-common ‑ crypto_helper::cardano::codec::test::to_file_must_create_a_file

♻️ This comment has been updated with latest results.

@turmelclem turmelclem temporarily deployed to testing-preview June 25, 2026 15:21 — with GitHub Actions Inactive
@turmelclem turmelclem temporarily deployed to testing-preview June 25, 2026 15:44 — with GitHub Actions Inactive
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 08:28 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch 3 times, most recently from c6318c7 to 08b4c2d Compare June 26, 2026 09:10
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 09:25 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch from 08b4c2d to 4262b67 Compare June 26, 2026 09:30
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 09:46 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch from 4262b67 to 0190334 Compare June 26, 2026 10:19
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 10:30 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch 2 times, most recently from 55ed003 to 5d72cad Compare June 26, 2026 13:15
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 13:31 — with GitHub Actions Inactive
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch 2 times, most recently from 4e8a8ce to a422c8b Compare June 26, 2026 13:41
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 13:56 — with GitHub Actions Inactive
@turmelclem turmelclem marked this pull request as ready for review June 26, 2026 13:59
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch 2 times, most recently from 9f53972 to c2f7289 Compare June 26, 2026 14:01
@turmelclem turmelclem temporarily deployed to testing-preview June 26, 2026 14:17 — with GitHub Actions Inactive
@jpraynaud jpraynaud requested a review from Copilot June 26, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown

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 aims to eliminate CI flakiness caused by concurrent tests reading partially-written kes_bytes/Shelley-format files by switching test file exports to an atomic write pattern (write to a uniquely-named temp file, then rename into place).

Changes:

  • Introduces a test-only SerDeShelleyFileFormatTestExtension::to_file to separate file-export helpers from the core SerDeShelleyFileFormat trait.
  • Updates test/fixture code to use the new to_file extension trait.
  • Implements an atomic-write strategy for to_file (temp file + rename) and adds a regression test ensuring a single final file is produced.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
mithril-common/src/test/crypto_helper/cardano/kes/setup.rs Switches test KES material generation to use the new to_file test extension trait.
mithril-common/src/test/crypto_helper/cardano/extensions.rs Adds a test-only extension trait to expose to_file for Shelley-format exports.
mithril-common/src/test/builder/fixture_builder.rs Updates fixture builder imports/usage to rely on the new to_file extension trait.
mithril-common/src/crypto_helper/mod.rs Re-exports CodecParseError to support the new extension trait signature.
mithril-common/src/crypto_helper/cardano/opcert.rs Updates tests to import the to_file extension trait.
mithril-common/src/crypto_helper/cardano/codec.rs Moves to_file behind a test extension trait and changes it to an atomic temp-write + rename approach; adds a regression test.

Comment thread mithril-common/src/crypto_helper/cardano/codec.rs Outdated
Comment thread mithril-common/src/crypto_helper/cardano/codec.rs Outdated
Comment thread mithril-common/src/crypto_helper/cardano/codec.rs Outdated

@Alenar Alenar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment thread mithril-common/src/crypto_helper/cardano/codec.rs Outdated
Comment thread mithril-common/src/crypto_helper/cardano/codec.rs Outdated
@turmelclem turmelclem force-pushed the ctl/3229-fix-kes-bytes-file-concurency branch from c2f7289 to 7179ff7 Compare June 26, 2026 15:05

@jpraynaud jpraynaud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍

* mithril-common from `0.7.8` to `0.7.9`
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.

Flaky CI on hydra run of ci/hydra-build:x86_64-linux.mithril-common

4 participants