Skip to content

Commit

Permalink
Merge pull request #2198 from input-output-hk/jpraynaud/fix-cardano-d…
Browse files Browse the repository at this point in the history
…atabase-id-collision

Fix: `CardanoDatabase` artifact identifier collisions
  • Loading branch information
jpraynaud authored Jan 3, 2025
2 parents 04f1108 + d7e3507 commit 27cee3f
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 31 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mithril-aggregator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-aggregator"
version = "0.6.7"
version = "0.6.8"
description = "A Mithril Aggregator server"
authors = { workspace = true }
edition = { workspace = true }
Expand Down
2 changes: 2 additions & 0 deletions mithril-aggregator/src/database/record/signed_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ impl TryFrom<SignedEntityRecord> for CardanoDatabaseSnapshotMessage {
fn try_from(value: SignedEntityRecord) -> Result<Self, Self::Error> {
let artifact = serde_json::from_str::<CardanoDatabaseSnapshot>(&value.artifact)?;
let cardano_database_snapshot_message = CardanoDatabaseSnapshotMessage {
hash: artifact.hash,
merkle_root: artifact.merkle_root,
beacon: artifact.beacon,
certificate_hash: value.certificate_id,
Expand All @@ -197,6 +198,7 @@ impl TryFrom<SignedEntityRecord> for CardanoDatabaseSnapshotListItemMessage {
fn try_from(value: SignedEntityRecord) -> Result<Self, Self::Error> {
let artifact = serde_json::from_str::<CardanoDatabaseSnapshot>(&value.artifact)?;
let cardano_database_snapshot_list_item_message = CardanoDatabaseSnapshotListItemMessage {
hash: artifact.hash,
merkle_root: artifact.merkle_root,
beacon: artifact.beacon,
certificate_hash: value.certificate_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ mod tests {
async fn test_cardano_database_detail_increments_artifact_detail_total_served_since_startup_metric(
) {
let method = Method::GET.as_str();
let path = "/artifact/cardano-database/{merkle_root}";
let path = "/artifact/cardano-database/{hash}";
let dependency_manager = Arc::new(initialize_dependencies().await);
let initial_counter_value = dependency_manager
.metrics_service
Expand Down Expand Up @@ -270,7 +270,7 @@ mod tests {
dependency_manager.message_service = Arc::new(mock_http_message_service);

let method = Method::GET.as_str();
let path = "/artifact/cardano-database/{merkle_root}";
let path = "/artifact/cardano-database/{hash}";

let response = request()
.method(method)
Expand Down Expand Up @@ -304,7 +304,7 @@ mod tests {
dependency_manager.message_service = Arc::new(mock_http_message_service);

let method = Method::GET.as_str();
let path = "/artifact/cardano-database/{merkle_root}";
let path = "/artifact/cardano-database/{hash}";

let response = request()
.method(method)
Expand Down Expand Up @@ -337,7 +337,7 @@ mod tests {
dependency_manager.message_service = Arc::new(mock_http_message_service);

let method = Method::GET.as_str();
let path = "/artifact/cardano-database/{merkle_root}";
let path = "/artifact/cardano-database/{hash}";

let response = request()
.method(method)
Expand Down
2 changes: 1 addition & 1 deletion mithril-common/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-common"
version = "0.4.99"
version = "0.4.100"
description = "Common types, interfaces, and utilities for Mithril nodes."
authors = { workspace = true }
edition = { workspace = true }
Expand Down
130 changes: 126 additions & 4 deletions mithril-common/src/entities/cardano_database.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use semver::Version;
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use strum::EnumDiscriminants;

use crate::{
Expand All @@ -10,6 +11,9 @@ use crate::{
/// Cardano database snapshot.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct CardanoDatabaseSnapshot {
/// Unique hash of the Cardano database snapshot.
pub hash: String,

/// Merkle root of the Cardano database snapshot.
pub merkle_root: String,

Expand Down Expand Up @@ -40,15 +44,27 @@ impl CardanoDatabaseSnapshot {
cardano_node_version: &Version,
) -> Self {
let cardano_node_version = format!("{cardano_node_version}");

Self {
let mut cardano_database_snapshot = Self {
hash: "".to_string(),
merkle_root,
beacon,
locations,
total_db_size_uncompressed,
compression_algorithm,
cardano_node_version,
}
};
cardano_database_snapshot.hash = cardano_database_snapshot.compute_hash();

cardano_database_snapshot
}

/// Cardano database snapshot hash computation
fn compute_hash(&self) -> String {
let mut hasher = Sha256::new();
hasher.update(self.beacon.epoch.to_be_bytes());
hasher.update(self.merkle_root.as_bytes());

hex::encode(hasher.finalize())
}
}

Expand Down Expand Up @@ -106,6 +122,112 @@ pub struct ArtifactsLocations {
#[typetag::serde]
impl Artifact for CardanoDatabaseSnapshot {
fn get_id(&self) -> String {
self.merkle_root.clone()
self.hash.clone()
}
}

#[cfg(test)]
mod tests {
use super::*;

fn dummy() -> CardanoDatabaseSnapshot {
CardanoDatabaseSnapshot::new(
"mk-root-1111111111".to_string(),
CardanoDbBeacon::new(2222, 55555),
0,
ArtifactsLocations {
digests: vec![],
immutables: vec![],
ancillary: vec![],
},
CompressionAlgorithm::Gzip,
&Version::new(1, 0, 0),
)
}

#[test]
fn test_cardano_database_snapshot_compute_hash() {
let cardano_database_snapshot = CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
};

assert_eq!(
"b1cc5e0deaa7856e8e811e349d6e639fa667aa70288602955f438c5893ce29c8",
cardano_database_snapshot.compute_hash()
);
}

#[test]
fn compute_hash_returns_same_hash_with_same_cardano_database_snapshot() {
assert_eq!(
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash(),
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash()
);
}

#[test]
fn compute_hash_returns_different_hash_with_different_merkle_root() {
assert_ne!(
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash(),
CardanoDatabaseSnapshot {
merkle_root: "mk-root-456".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash()
);
}

#[test]
fn compute_hash_returns_different_hash_with_same_epoch_in_beacon() {
assert_eq!(
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash(),
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 12),
..dummy()
}
.compute_hash()
);
}

#[test]
fn compute_hash_returns_different_hash_with_different_beacon() {
assert_ne!(
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(123, 98),
..dummy()
}
.compute_hash(),
CardanoDatabaseSnapshot {
merkle_root: "mk-root-123".to_string(),
beacon: CardanoDbBeacon::new(456, 98),
..dummy()
}
.compute_hash()
);
}
}
6 changes: 6 additions & 0 deletions mithril-common/src/messages/cardano_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ impl From<ArtifactsLocations> for ArtifactsLocationsMessagePart {
/// Cardano database snapshot.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct CardanoDatabaseSnapshotMessage {
/// Hash of the Cardano database snapshot.
pub hash: String,

/// Merkle root of the Cardano database snapshot.
pub merkle_root: String,

Expand Down Expand Up @@ -59,6 +62,7 @@ impl CardanoDatabaseSnapshotMessage {
/// Return a dummy test entity (test-only).
pub fn dummy() -> Self {
Self {
hash: "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb".to_string(),
merkle_root: "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6"
.to_string(),
beacon: CardanoDbBeacon {
Expand Down Expand Up @@ -99,6 +103,7 @@ mod tests {

const ACTUAL_JSON: &str = r#"
{
"hash": "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb",
"merkle_root": "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6",
"beacon": {
"epoch": 123,
Expand Down Expand Up @@ -137,6 +142,7 @@ mod tests {

fn golden_actual_message() -> CardanoDatabaseSnapshotMessage {
CardanoDatabaseSnapshotMessage {
hash: "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb".to_string(),
merkle_root: "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6"
.to_string(),
beacon: CardanoDbBeacon {
Expand Down
6 changes: 6 additions & 0 deletions mithril-common/src/messages/cardano_database_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ pub type CardanoDatabaseSnapshotListMessage = Vec<CardanoDatabaseSnapshotListIte
/// Message structure of a Cardano database snapshot list item
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct CardanoDatabaseSnapshotListItemMessage {
/// Hash of the Cardano database snapshot.
pub hash: String,

/// Merkle root of the Cardano database snapshot
pub merkle_root: String,

Expand All @@ -35,6 +38,7 @@ impl CardanoDatabaseSnapshotListItemMessage {
/// Return a dummy test entity (test-only).
pub fn dummy() -> Self {
Self {
hash: "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb".to_string(),
merkle_root: "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6"
.to_string(),
beacon: CardanoDbBeacon {
Expand All @@ -60,6 +64,7 @@ mod tests {
const ACTUAL_JSON: &str = r#"
[
{
"hash": "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb",
"merkle_root": "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6",
"beacon": {
"epoch": 123,
Expand All @@ -75,6 +80,7 @@ mod tests {

fn golden_actual_message() -> CardanoDatabaseSnapshotListMessage {
vec![CardanoDatabaseSnapshotListItemMessage {
hash: "d4071d518a3ace0f6c04a9c0745b9e9560e3e2af1b373bafc4e0398423e9abfb".to_string(),
merkle_root: "c8224920b9f5ad7377594eb8a15f34f08eb3103cc5241d57cafc5638403ec7c6"
.to_string(),
beacon: CardanoDbBeacon {
Expand Down
2 changes: 1 addition & 1 deletion mithril-test-lab/mithril-end-to-end/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-end-to-end"
version = "0.4.58"
version = "0.4.59"
authors = { workspace = true }
edition = { workspace = true }
documentation = { workspace = true }
Expand Down
20 changes: 9 additions & 11 deletions mithril-test-lab/mithril-end-to-end/src/assertions/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ pub async fn assert_node_producing_cardano_database_snapshot(
let url = format!("{aggregator_endpoint}/artifact/cardano-database");
info!("Waiting for the aggregator to produce a Cardano database snapshot");

async fn fetch_last_cardano_database_snapshot_merkle_root(
url: String,
) -> StdResult<Option<String>> {
async fn fetch_last_cardano_database_snapshot_hash(url: String) -> StdResult<Option<String>> {
match reqwest::get(url.clone()).await {
Ok(response) => match response.status() {
StatusCode::OK => match response
Expand All @@ -205,7 +203,7 @@ pub async fn assert_node_producing_cardano_database_snapshot(
.as_deref()
{
Ok([cardano_database_snapshot, ..]) => {
Ok(Some(cardano_database_snapshot.merkle_root.clone()))
Ok(Some(cardano_database_snapshot.hash.clone()))
}
Ok(&[]) => Ok(None),
Err(err) => Err(anyhow!("Invalid Cardano database snapshot body : {err}",)),
Expand All @@ -218,11 +216,11 @@ pub async fn assert_node_producing_cardano_database_snapshot(

// todo: reduce the number of attempts if we can reduce the delay between two immutables
match attempt!(45, Duration::from_millis(2000), {
fetch_last_cardano_database_snapshot_merkle_root(url.clone()).await
fetch_last_cardano_database_snapshot_hash(url.clone()).await
}) {
AttemptResult::Ok(merkle_root) => {
info!("Aggregator produced a Cardano database snapshot"; "merkle_root" => &merkle_root);
Ok(merkle_root)
AttemptResult::Ok(hash) => {
info!("Aggregator produced a Cardano database snapshot"; "hash" => &hash);
Ok(hash)
}
AttemptResult::Err(error) => Err(error),
AttemptResult::Timeout() => Err(anyhow!(
Expand All @@ -233,13 +231,13 @@ pub async fn assert_node_producing_cardano_database_snapshot(

pub async fn assert_signer_is_signing_cardano_database_snapshot(
aggregator_endpoint: &str,
merkle_root: &str,
hash: &str,
expected_epoch_min: Epoch,
) -> StdResult<String> {
let url = format!("{aggregator_endpoint}/artifact/cardano-database/{merkle_root}");
let url = format!("{aggregator_endpoint}/artifact/cardano-database/{hash}");
info!(
"Asserting the aggregator is signing the Cardano database snapshot message `{}` with an expected min epoch of `{}`",
merkle_root,
hash,
expected_epoch_min
);

Expand Down
Loading

0 comments on commit 27cee3f

Please sign in to comment.