Block Number beacon in cardano transaction

[Alenar]

Content

This PR changes the beacon used for the CardanoTransactions signed entity type:

• before: a CardanoDbBeacon (aka: network, epoch, immutable file number)
• now: Epoch and block number tuple.

This changes decouples the production of cardano transactions signatures and proofs from the immutable file number and the cardano node database structure. Allowing us to go closer to the tip of the chain since, with a adapted block streamer, we won't have to wait for a immutable to be complete update our stored transactions.

In depth

• Add a new parameter to the aggregator to configure the pacing at which the chain should be parsed: cardano_transactions_signing_config.
• Update related type:
  • CardanoTransactionsSnapshot artifact: replace beacon with block_number (note: the epoch can still be fetched if it's wrapped by a SignedEntity<T> and is added to the related messages).
  • In ProtocolMessage and CardanoTransactionsProofsMessage: rename latest_immutable_file_number to latest_block_number.
• Update services:
  • BlockScanner trait: use a Option<ChainPoint> for its lower bound and a BlockNumber for the upper bound. Previously it used a ImmutableFileNumber for both.
  • CardanoBlockScanner: ignore the bounds given as parameter and compute them itself (lower bound from db and upper bound take all completed immutables) since it still works using immutables files and there's no way go get the immutable file that contains a given block from the chain.
  • Updated the signable builder for cardano transactions, and the TransactionsImporter and BlockRangeRootRetriever traits, to use a BlockNumber instead of a ImmutableFileNumber.
• Cleanup all certificate and signed entities based on CardanoTransactions from Aggregator database since their beacons changed.
• Explorer: Updated to support the new beacon + use dynamic display for certificate signed_entity instead of relying on the deprecated beacon property.

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

This would have been considered a network breaking change on a stable signed entity type.

Issue(s)

Closes #1697

[github-actions]

Test Results

    3 files  ±0     43 suites  ±0   8m 34s ⏱️ ±0s
1 012 tests +9  1 012 ✅ +9  0 💤 ±0  0 ❌ ±0 
1 110 runs  +9  1 110 ✅ +9  0 💤 ±0  0 ❌ ±0 

Results for commit 70a8a61. ± Comparison against base commit 3b542fd.

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

mithril-aggregator ‑ http_server::routes::proof_routes::tests::build_response_message_return_immutable_file_number_from_artifact_beacon
mithril-aggregator ‑ services::cardano_transactions_importer::tests::change_parsed_lower_bound_when_rescan_limit_is_set
mithril-common ‑ cardano_block_scanner::block_scanner::tests::test_parse_from_lower_bound_until_upper_bound
mithril-common ‑ cardano_block_scanner::block_scanner::tests::test_parse_up_to_given_beacon
mithril-persistence ‑ database::repository::cardano_transaction_repository::tests::get_transactions_up_to_return_empty_list_when_no_record_found_with_provided_immutable_file_number
mithril-persistence ‑ database::repository::cardano_transaction_repository::tests::repository_get_up_to_beacon_transactions
mithril-signer ‑ cardano_transactions_importer::tests::change_parsed_lower_bound_when_rescan_limit_is_set

mithril-aggregator ‑ configuration::test::can_build_config_with_ctx_signing_config_from_default_configuration
mithril-aggregator ‑ http_server::routes::proof_routes::tests::build_response_message_return_latest_block_number_from_artifact_beacon
mithril-common ‑ cardano_block_scanner::block_scanner::tests::change_parsed_lower_bound_when_rescan_limit_is_set
mithril-common ‑ cardano_block_scanner::block_scanner::tests::test_scan_ignore_given_lower_bound_instead_find_it_using_an_external_service
mithril-common ‑ cardano_block_scanner::block_scanner::tests::test_scan_with_lower_bound_ignore_upper_bound
mithril-common ‑ cardano_block_scanner::block_scanner::tests::test_scan_without_lower_bound_ignore_upper_bound
mithril-common ‑ entities::signed_entity_type::tests::computing_block_number_to_be_signed
mithril-common ‑ entities::signed_entity_type::tests::computing_block_number_to_be_signed_round_step_to_a_block_range_start
mithril-common ‑ entities::signed_entity_type::tests::serialize_beacon_to_json
mithril-common ‑ entities::signed_entity_type::tests::verify_signed_entity_type_properties_are_included_in_computed_hash
…

♻️ This comment has been updated with latest results.

[jpraynaud]

Good job! LGTM 🚀

I left few comments and typo fixes below.

[jpraynaud]

Do we really need the full ChainPoint here?

Suggested change

	from: Option<ChainPoint>,
	from: Option<BlockNumber>,

[Alenar]

We need the full chain point for the BlockStreamer called by this method.

[Alenar]

We may use a BlockNumber but doing so we need to transfer the responsibility to obtain a chain point from a block number to the streamer.

This would be cumbersome since:

• The lower bound is acquired by the importer (that part of its current role), to do so it do a database round-trip.
• We would need to define a trait that take a block number as parameter and return a chain point ... implemented by doing another database round-trip (so two needed instead of one as now).

Another possibility would be to remove altogether the lower bound computation from the importer and transfer it to the scanner.

[jpraynaud]

Indeed this shoudl probably be the responsibility of the scanner.
This can be done in a separate PR 👍

[sfauvel]

Looks good