Adjustements from PR reviews & fix broken doc tests

- Aggregator `cardano_transactions_signing_config.step` from 90 to 120. - Adjusts/fix multiples comments - Add missing types in `mithril-client` re-exported common types. Co-authored-by: Sébastien Fauvel <[email protected]> Co-authored-by: Damien Lachaume <[email protected]>
input-output-hk · Jun 5, 2024 · c80f17e · c80f17e
1 parent 8840169
commit c80f17e
Show file tree

Hide file tree

Showing 11 changed files with 27 additions and 22 deletions.
diff --git a/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs b/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs
@@ -106,7 +106,7 @@ impl CardanoTransactionRepository {
             .fetch_collect(InsertBlockRangeRootQuery::insert_many(records)?)
     }
 
-    /// Get the highest [BlockNumber] of the cardano transactions stored in the database.
+    /// Get the highest [ChainPoint] of the cardano transactions stored in the database.
     pub async fn get_transaction_highest_chain_point(&self) -> StdResult<Option<ChainPoint>> {
         let first_transaction_with_highest_block_number = self
             .connection

diff --git a/mithril-aggregator/src/artifact_builder/cardano_transactions.rs b/mithril-aggregator/src/artifact_builder/cardano_transactions.rs
@@ -43,7 +43,7 @@ impl ArtifactBuilder<BlockNumber, CardanoTransactionsSnapshot>
             ))
             .with_context(|| {
                 format!(
-                    "Can not compute CardanoTransactionsCommitment artifact for signed_entity: {:?}",
+                    "Can not compute CardanoTransactionsSnapshot artifact for signed_entity: {:?}",
                     SignedEntityType::CardanoTransactions(certificate.epoch, beacon)
                 )
             })?;

diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs
@@ -158,7 +158,7 @@ pub struct Configuration {
     pub cardano_transactions_prover_cache_pool_size: usize,
 
     /// Cardano transactions signing configuration
-    #[example = "`{ security_parameter: 3000, step: 90 }`"]
+    #[example = "`{ security_parameter: 3000, step: 120 }`"]
     pub cardano_transactions_signing_config: CardanoTransactionsSigningConfig,
 }
 
@@ -405,7 +405,7 @@ impl Default for DefaultConfiguration {
             cardano_transactions_prover_cache_pool_size: 10,
             cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
                 security_parameter: 3000,
-                step: 90,
+                step: 120,
             },
         }
     }

diff --git a/mithril-aggregator/src/services/signed_entity.rs b/mithril-aggregator/src/services/signed_entity.rs
@@ -135,7 +135,7 @@ impl MithrilSignedEntityService {
                     .await
                     .with_context(|| {
                         format!(
-                            "Signed Entity Service can not compute artifact for entity type: '{block_number}'"
+                            "Signed Entity Service can not compute artifact for entity type: '{signed_entity_type}'"
                         )
                     })?,
             )),

diff --git a/mithril-client/src/cardano_transaction_client.rs b/mithril-client/src/cardano_transaction_client.rs
@@ -51,7 +51,7 @@
 //! let client = ClientBuilder::aggregator("YOUR_AGGREGATOR_ENDPOINT", "YOUR_GENESIS_VERIFICATION_KEY").build()?;
 //! let cardano_transaction_snapshot = client.cardano_transaction().get_snapshot("CARDANO_TRANSACTION_SNAPSHOT_HASH").await?.unwrap();
 //!
-//! println!("Cardano transaction snapshot hash={}, epoch={}", cardano_transaction_snapshot.hash, cardano_transaction_snapshot.beacon.epoch);
+//! println!("Cardano transaction snapshot hash={}, epoch={}", cardano_transaction_snapshot.hash, cardano_transaction_snapshot.epoch);
 //! #    Ok(())
 //! # }
 //! ```
@@ -68,7 +68,7 @@
 //! let cardano_transaction_snapshots = client.cardano_transaction().list_snapshots().await?;
 //!
 //! for cardano_transaction_snapshot in cardano_transaction_snapshots {
-//!     println!("Cardano transaction snapshot hash={}, epoch={}", cardano_transaction_snapshot.hash, cardano_transaction_snapshot.beacon.epoch);
+//!     println!("Cardano transaction snapshot hash={}, epoch={}", cardano_transaction_snapshot.hash, cardano_transaction_snapshot.epoch);
 //! }
 //! #    Ok(())
 //! # }

diff --git a/mithril-client/src/type_alias.rs b/mithril-client/src/type_alias.rs
@@ -57,10 +57,10 @@ cfg_unstable! {
 /// `mithril-common` re-exports
 pub mod common {
     pub use mithril_common::entities::{
-        CardanoDbBeacon, CompressionAlgorithm, Epoch, ProtocolMessage, ProtocolMessagePartKey,
-        ProtocolParameters,
+        CardanoDbBeacon, CompressionAlgorithm, Epoch, ImmutableFileNumber, ProtocolMessage,
+        ProtocolMessagePartKey, ProtocolParameters,
     };
     cfg_unstable! {
-        pub use mithril_common::entities::{ChainPoint, TransactionHash};
+        pub use mithril_common::entities::{ChainPoint, TransactionHash, SlotNumber, BlockHash, BlockNumber};
     }
 }
diff --git a/mithril-common/src/cardano_block_scanner/block_scanner.rs b/mithril-common/src/cardano_block_scanner/block_scanner.rs
@@ -9,7 +9,7 @@ use crate::digesters::ImmutableFile;
 use crate::entities::{BlockNumber, ChainPoint, ImmutableFileNumber};
 use crate::StdResult;
 
-/// Trait to find the lower bound that should be used the [block scanner][CardanoBlockScanner] when
+/// Trait to find the lower bound that should be used by the [block scanner][CardanoBlockScanner] when
 /// scanning.
 #[cfg_attr(test, mockall::automock)]
 #[async_trait]

diff --git a/mithril-common/src/entities/signed_entity_type.rs b/mithril-common/src/entities/signed_entity_type.rs
@@ -50,12 +50,13 @@ pub enum SignedEntityType {
 ///
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct CardanoTransactionsSigningConfig {
-    /// Number of blocks to wait before taking in account a transaction.
+    /// Number of blocks to discard from the tip of the chain when importing transactions.
     pub security_parameter: BlockNumber,
 
-    /// The frequency at which the transactions are signed.
+    /// The number of blocks between signature of the transactions.
     ///
-    /// *Note: The step is adjusted to be a multiple of the block range length in order.*
+    /// *Note: The step is adjusted to be a multiple of the block range length in order
+    /// to guarantee that the block number signed in a certificate is effectively signed.*
     pub step: BlockNumber,
 }
 
@@ -70,12 +71,16 @@ impl CardanoTransactionsSigningConfig {
         }
     }
 
-    /// Compute the block number to be signed based on che chain tip block number.
+    /// Compute the block number to be signed based on the chain tip block number.
+    ///
+    /// The latest block number to be signed is the highest multiple of the step less or equal than the
+    /// block number minus the security parameter.
+    ///
+    /// The formula is as follows:
     ///
-    /// Given k' = `security_parameter` and n = `step`,
-    /// the latest block number to be signed is computed as *(this use a integer division)*:
+    /// `block_number = ⌊(tip.block_number - security_parameter) / step⌋ × step`
     ///
-    /// **block_number = ((tip.block_number - k') / n) × n**
+    /// where `⌊x⌋` is the floor function which rounds to the greatest integer less than or equal to `x`.
     ///
     /// *Note: The step is adjusted to be a multiple of the block range length in order
     /// to guarantee that the block number signed in a certificate is effectively signed.*

diff --git a/mithril-common/src/messages/cardano_transaction_snapshot.rs b/mithril-common/src/messages/cardano_transaction_snapshot.rs
@@ -13,7 +13,7 @@ pub struct CardanoTransactionSnapshotMessage {
     /// Epoch of the Cardano transactions snapshot
     pub epoch: Epoch,
 
-    /// ChainPoint of the Cardano transactions snapshot
+    /// Block number of the Cardano transactions snapshot
     pub block_number: BlockNumber,
 
     /// Hash of the Cardano Transactions snapshot
@@ -63,7 +63,7 @@ mod tests {
 
     // Test the retro compatibility with possible future upgrades.
     #[test]
-    fn test_v2() {
+    fn test_v1() {
         let json = r#"{
             "merkle_root": "mkroot-123",
             "epoch": 8,

diff --git a/mithril-common/src/messages/cardano_transaction_snapshot_list.rs b/mithril-common/src/messages/cardano_transaction_snapshot_list.rs
@@ -65,7 +65,7 @@ mod tests {
 
     // Test the retro compatibility with possible future upgrades.
     #[test]
-    fn test_v2() {
+    fn test_v1() {
         let json = r#"[{
         "merkle_root": "mkroot-123",
         "epoch": 7,

diff --git a/openapi.yaml b/openapi.yaml
@@ -980,7 +980,7 @@ components:
           type: string
           format: bytes
         latest_block_number:
-          description: The latest block number of the Cardano chain
+          description: The latest signed block number
           type: string
       example:
         {