Skip to content

refactor+test: prover preparatory work#3072

Merged
Alenar merged 8 commits into
mainfrom
djo/2987/prover_preparatory_work
Mar 10, 2026
Merged

refactor+test: prover preparatory work#3072
Alenar merged 8 commits into
mainfrom
djo/2987/prover_preparatory_work

Conversation

@Alenar

@Alenar Alenar commented Mar 6, 2026

Copy link
Copy Markdown
Collaborator

Content

This PR includes refactors and new tests utilities that will be used for the upcoming blocks and transactions prover.

  • refactor: extract CardanoBlockTransactionMkTreeNode to a dedicated file module
  • multiple improvements for InMemoryChainDataStore (mithril-cardano-node-chain) in order to be able to use it in the upcoming prover tests:
    • impl mithril_common::signable_builder::BlockRangeRootRetriever
    • new retrieval methods: get_blocks_by_hashes, get_transactions_by_hashes, get_blocks_with_transactions_in_block_ranges, get_blocks_with_transactions_in_ranges
    • add compute_block_ranges to its builder to generate realistic block ranges root, avoiding the need to provide them manually (which is still an option)
  • refactor CardanoTransactionsBuilder (mithril-common):
    • rename build_block_ranges to build_transactions_for_block_ranges: to avoid confusion with a new method
    • new build_blocks and build_blocks_for_block_ranges that produces CardanoBlockWithTransactions instead of CardanoTransaction
    • refactor internal mechanisms to works on blocks, allowing a shared logic between old an new functions
    • group tests for each of the builders output with a module

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
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • No new TODOs introduced

Issue(s)

Relates to #2987

@Alenar
Alenar requested review from jpraynaud and turmelclem March 6, 2026 14:34
@Alenar Alenar self-assigned this Mar 6, 2026
@Alenar Alenar added refactoring 🛠️ Code refactoring and enhancements testing 🔁 Something related to tests labels Mar 6, 2026
@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown

Test Results

    5 files  ± 0    190 suites  ±0   57m 53s ⏱️ + 1m 41s
2 671 tests +18  2 671 ✅ +18  0 💤 ±0  0 ❌ ±0 
9 586 runs  +81  9 586 ✅ +81  0 💤 ±0  0 ❌ ±0 

Results for commit f41e270. ± Comparison against base commit 925875b.

This pull request removes 25 and adds 43 tests. Note that renamed tests count towards both.
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_blocks_and_transactions_in_range_filters_correctly
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_blocks_and_transactions_in_range_returns_empty_when_store_empty
mithril-common ‑ entities::cardano_block::tests::block_node_leaf_identifier
mithril-common ‑ entities::cardano_block::tests::convert_block_node_into_mktree_nodes
mithril-common ‑ entities::cardano_block::tests::convert_tx_node_into_mktree_nodes
mithril-common ‑ entities::cardano_block::tests::mk_tree_node_ordering::order_block_nodes_first_then_transaction_nodes
mithril-common ‑ entities::cardano_block::tests::mk_tree_node_ordering::order_blocks_by_block_hash_third
mithril-common ‑ entities::cardano_block::tests::mk_tree_node_ordering::order_blocks_by_block_number_first
mithril-common ‑ entities::cardano_block::tests::mk_tree_node_ordering::order_blocks_by_slot_number_second
mithril-common ‑ entities::cardano_block::tests::mk_tree_node_ordering::order_transactions_by_block_hash_third
…
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::builder_can_compute_block_range_roots
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_blocks_by_hashes_filters_correctly
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_blocks_with_transactions_in_ranges_filters_correctly
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_blocks_with_transactions_in_ranges_returns_empty_when_store_empty
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::get_transactions_by_hashes_filters_correctly
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::retrieve_blocks_range_root::returns_empty_when_store_empty
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::retrieve_blocks_range_root::up_to_above_all_stored_ranges_returns_all
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::retrieve_blocks_range_root::up_to_below_start_of_the_first_range_returns_nothing
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::retrieve_blocks_range_root::up_to_right_after_start_of_the_third_range_returns_the_first_three_ranges
mithril-cardano-node-chain ‑ test::double::chain_data_store::tests::retrieve_blocks_range_root::up_to_right_at_start_of_the_third_range_returns_the_first_two_ranges
…

♻️ This comment has been updated with latest results.

@Alenar
Alenar temporarily deployed to testing-preview March 6, 2026 14:48 — with GitHub Actions Inactive
@Alenar
Alenar force-pushed the djo/2987/prover_preparatory_work branch from 32d69a0 to 570b408 Compare March 6, 2026 14:55
@Alenar
Alenar temporarily deployed to testing-preview March 6, 2026 15:09 — with GitHub Actions Inactive
@jpraynaud
jpraynaud requested a review from Copilot March 6, 2026 15:11

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 is preparatory work for an upcoming blocks and transactions prover (tracking issue #2987). It cleans up the existing code and adds new test utilities so they can be used in future prover tests.

Changes:

  • Extract CardanoBlockTransactionMkTreeNode from cardano_block.rs into its own dedicated module, and add a From<CardanoBlockWithTransactions> for CardanoBlock conversion.
  • Refactor CardanoTransactionsBuilder in mithril-common: rename build_block_ranges to build_transactions_for_block_ranges, change slot number generation from per-transaction to per-block, and add new build_blocks / build_blocks_for_block_ranges methods that produce CardanoBlockWithTransactions.
  • Enhance InMemoryChainDataStore in mithril-cardano-node-chain: add get_blocks_by_hashes, get_transactions_by_hashes, get_blocks_with_transactions_in_block_ranges, get_blocks_with_transactions_in_ranges, implement BlockRangeRootRetriever, and add a compute_block_ranges builder method.

Reviewed changes

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

Show a summary per file
File Description
mithril-common/src/entities/cardano_block_transaction_mktree_node.rs New file: extracted CardanoBlockTransactionMkTreeNode type and all its tests from cardano_block.rs.
mithril-common/src/entities/cardano_block.rs Removed CardanoBlockTransactionMkTreeNode (now extracted); added From<CardanoBlockWithTransactions> for CardanoBlock.
mithril-common/src/entities/mod.rs Re-exports the new cardano_block_transaction_mktree_node module.
mithril-common/src/test/builder/cardano_transactions_builder.rs Major refactor: new block-centric builder logic; renamed method; new build_blocks* methods; reorganized tests into sub-modules.
internal/cardano-node/mithril-cardano-node-chain/src/test/double/chain_data_store.rs New retrieval methods, BlockRangeRootRetriever impl, and compute_block_ranges builder method; new and updated tests.
internal/cardano-node/mithril-cardano-node-chain/src/chain_importer/mod.rs Exposes BlockRangeImporter as pub(crate) for use in the test double.
mithril-aggregator/src/services/prover_legacy.rs Updates test calls from build_block_ranges to the renamed build_transactions_for_block_ranges.

Comment thread mithril-common/src/test/builder/cardano_transactions_builder.rs Outdated

@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 👍

@turmelclem turmelclem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

Alenar added 7 commits March 9, 2026 17:51
…ChainDataStore`

So it can be used in the new prover tests
…new prover to `InMemoryChainDataStore`

To allow its usage in the new prover test
…ranges`

By leveraging tested `BlockRangeImporter`
…ges` to `build_transactions_for_block_ranges` and restructure tests

thoses changes prepare the extension of capabilities of the builder to
also build `CardanoBlockWithTransaction`.
…y on blocks instead of transactions

this will allow to extend it's capabilities to the builds of
`CardanoBlockWithTransactions`.
@Alenar
Alenar force-pushed the djo/2987/prover_preparatory_work branch from 570b408 to 0c8750a Compare March 9, 2026 16:52
* mithril-cardano-node-chain from `0.1.23` to `0.1.24`
* mithril-aggregator from `0.8.32` to `0.8.33`
* mithril-common from `0.6.54` to `0.6.55`
@Alenar
Alenar force-pushed the djo/2987/prover_preparatory_work branch from 0c8750a to f41e270 Compare March 9, 2026 16:53
@Alenar
Alenar temporarily deployed to testing-preview March 9, 2026 17:08 — with GitHub Actions Inactive
@Alenar
Alenar merged commit b114b4e into main Mar 10, 2026
58 checks passed
@Alenar
Alenar deleted the djo/2987/prover_preparatory_work branch March 10, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring 🛠️ Code refactoring and enhancements testing 🔁 Something related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants