Bugfix: Block range computation ignored last block range & last block range if it finished with empty blocks

[Alenar]

Content

This PR fixes two problems:

1. The last advertised block range isn't imported

Following #1795 merge we still could not compute proof for transaction on the last certified block number, even more the whole last block range was not provable (reverting to the state before #1762).

Contrary to the last two fix attempts this time the problem was not in the prover nor the merkle root computation, instead it came from the block range root computation.

This is because in #1795 we change the way the higher bound to certify is computed by subtracting one to the previous computed number so it would fit to the end of a block range (instead of trying to certify 15, 30 or 45 it now certify 14, 29 or 44).

This -1 echo to the block range sequence computation (see BlockRange::all_block_ranges_in) which was excluding in its upper bound.
To illustrate, with a start of 30 and an end of 75 (now 74):

• before: BlockRange::all_block_ranges_in(30..75) = [(30..45), (45..60), (60..75)]
• after: BlockRange::all_block_ranges_in(30..74) = [(30..45), (45..60)] a block range disappeared

The fix is simply to be inclusive for the upper bound:

• now: BlockRange::all_block_ranges_in(30..=74) = [(30..45), (45..60), (60..75)]

2. The last block range isn't imported if it finish with one or more block without transaction

A block without transactions is rare on the mainnet, but not impossible, and is frequent in testing networks.

This is not correctly handled by the importer since it use the highest stored block number in the cardano_tx table as the upper bound in the computation of the block range sequence to compute (same as explained in the previous problem).

In order to fix that this PR change the upper bound used: it use the one passed to the transaction importer instead.

We can do that since this block number passer to the importer is computed by the state machine based on the chain current state and the cardano transaction signing configuration, meaning that this block exist.

Note: Another way of doing that would be by also storing the blocks in database so we could use the highest stored block number based on 'real' blocks instead of blocks associated to transactions .

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 README file (if relevant)
  • Update documentation website (if relevant)
  • Add dev blog post (if relevant)

Comments

Since get_block_interval_without_block_range_root is supersed by a new method that only retrieve the highest stored block range interval, its code and the query associated have been removed.

Issue(s)

Relates to #1785

[github-actions]

Test Results

    4 files  ±0     52 suites  ±0   8m 51s ⏱️ +6s
1 146 tests +1  1 146 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 312 runs  +1  1 312 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 73f7be8. ± Comparison against base commit 702e519.

This pull request removes 7 and adds 8 tests. Note that renamed tests count towards both.

mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_with_both_block_range_and_transactions_yield_a_range
mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_with_last_transaction_block_number_below_the_end_of_the_last_block_range_yield_an_error
mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_with_last_transaction_block_number_right_at_the_end_of_the_last_block_range_yield_none
mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_with_only_block_range_and_no_transactions_yield_none
mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_with_only_transactions_yield_a_range
mithril-persistence ‑ database::record::interval_without_block_range_root::tests::db_without_block_range_nor_transactions_yield_none
mithril-persistence ‑ database::repository::cardano_transaction_repository::tests::repository_get_block_interval_without_block_range_root

mithril-aggregator ‑ services::cardano_transactions_importer::tests::can_compute_block_ranges_even_if_last_blocks_in_range_dont_have_transactions
mithril-aggregator ‑ services::cardano_transactions_importer::tests::can_compute_block_ranges_up_to_the_strict_end_of_a_block_range
mithril-common ‑ entities::block_range::tests::test_building_sequence_with_start_greater_than_end_yield_empty_iterator
mithril-persistence ‑ database::query::block_range_root::get_block_range_root::tests::test_get_highest
mithril-persistence ‑ database::query::block_range_root::get_block_range_root::tests::test_get_highest_with_empty_db
mithril-persistence ‑ database::repository::cardano_transaction_repository::tests::repository_retrieve_highest_block_range_roots
mithril-signer ‑ cardano_transactions_importer::tests::can_compute_block_ranges_even_if_last_blocks_in_range_dont_have_transactions
mithril-signer ‑ cardano_transactions_importer::tests::can_compute_block_ranges_up_to_the_strict_end_of_a_block_range

♻️ This comment has been updated with latest results.