From 486f18999eb86d7de3e3c209e4147223d33d505b Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 20 Jun 2024 12:12:18 +0200 Subject: [PATCH 01/17] Implement test tools `equivalent_to` and `assert_equivalent` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-common/src/test_utils/mod.rs | 56 ++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/mithril-common/src/test_utils/mod.rs b/mithril-common/src/test_utils/mod.rs index d013bc11e57..2a4aeede2ac 100644 --- a/mithril-common/src/test_utils/mod.rs +++ b/mithril-common/src/test_utils/mod.rs @@ -41,10 +41,41 @@ macro_rules! assert_same_json { }; } +/// Compare two iterators ignoring the order +pub fn equivalent_to(a: I1, b: I2) -> bool +where + T: PartialEq + Ord, + I1: IntoIterator + Clone, + I2: IntoIterator + Clone, +{ + let a = as_sorted_vec(a); + let b = as_sorted_vec(b); + a == b +} + +/// Assert that two iterators are equivalent +pub fn assert_equivalent(a: I1, b: I2) +where + T: PartialEq + Ord + std::fmt::Debug, + I1: IntoIterator + Clone, + I2: IntoIterator + Clone, +{ + let a = as_sorted_vec(a); + let b = as_sorted_vec(b); + assert_eq!(a, b); +} + +fn as_sorted_vec + Clone>(iter: I) -> Vec { + let mut list: Vec = iter.clone().into_iter().collect(); + list.sort(); + list +} + pub use assert_same_json; #[cfg(test)] mod utils { + use std::collections::HashSet; use std::fs::File; use std::io; use std::sync::Arc; @@ -53,6 +84,8 @@ mod utils { use slog_async::Async; use slog_term::{CompactFormat, PlainDecorator}; + use super::*; + pub struct TestLogger; #[cfg(test)] @@ -72,4 +105,27 @@ mod utils { Self::from_writer(File::create(filepath).unwrap()) } } + + #[test] + fn test_equivalent_to() { + assert!(equivalent_to(vec![1, 2, 3], vec![3, 2, 1])); + assert!(equivalent_to(vec![1, 2, 3], vec![2, 1, 3])); + assert!(!equivalent_to(vec![1, 2, 3], vec![3, 2, 1, 4])); + assert!(!equivalent_to(vec![1, 2, 3], vec![3, 2])); + + assert!(equivalent_to([1, 2, 3], vec![3, 2, 1])); + assert!(equivalent_to(&[1, 2, 3], &vec![3, 2, 1])); + assert!(equivalent_to([1, 2, 3], HashSet::from([3, 2, 1]))); + assert!(equivalent_to(vec![1, 2, 3], HashSet::from([3, 2, 1]))); + assert!(equivalent_to(&vec![1, 2, 3], &HashSet::from([3, 2, 1]))); + + assert_equivalent(vec![1, 2, 3], vec![3, 2, 1]); + assert_equivalent(vec![1, 2, 3], vec![2, 1, 3]); + + assert_equivalent([1, 2, 3], vec![3, 2, 1]); + assert_equivalent(&[1, 2, 3], &vec![3, 2, 1]); + assert_equivalent([1, 2, 3], HashSet::from([3, 2, 1])); + assert_equivalent(vec![1, 2, 3], HashSet::from([3, 2, 1])); + assert_equivalent(&vec![1, 2, 3], &HashSet::from([3, 2, 1])); + } } From 05c197b349d1f2ab81381270a30f4f9547ab71e1 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 19 Jun 2024 18:33:07 +0200 Subject: [PATCH 02/17] Handle deduplicate list of transactions hashes in `compute_transactions_proofs` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- mithril-aggregator/src/services/prover.rs | 100 +++++++++++++++++++--- 1 file changed, 88 insertions(+), 12 deletions(-) diff --git a/mithril-aggregator/src/services/prover.rs b/mithril-aggregator/src/services/prover.rs index 77b76a73f75..d5c630362ed 100644 --- a/mithril-aggregator/src/services/prover.rs +++ b/mithril-aggregator/src/services/prover.rs @@ -110,6 +110,13 @@ impl MithrilProverService { Ok(block_ranges_map) } + + fn filter_valid_hashes(&self, transaction_hashes: &[TransactionHash]) -> Vec { + let mut transaction_hashes = transaction_hashes.to_vec(); + transaction_hashes.sort(); + transaction_hashes.dedup(); + transaction_hashes + } } #[async_trait] @@ -119,8 +126,10 @@ impl ProverService for MithrilProverService { up_to: BlockNumber, transaction_hashes: &[TransactionHash], ) -> StdResult> { + let transaction_hashes = self.filter_valid_hashes(transaction_hashes); + // 1 - Compute the set of block ranges with transactions to prove - let block_ranges_transactions = self.get_block_ranges(transaction_hashes, up_to).await?; + let block_ranges_transactions = self.get_block_ranges(&transaction_hashes, up_to).await?; let block_range_transactions = self .get_all_transactions_for_block_ranges(&block_ranges_transactions) .await?; @@ -145,7 +154,7 @@ impl ProverService for MithrilProverService { } // 5 - Compute the proof for all transactions - if let Ok(mk_proof) = mk_map.compute_proof(transaction_hashes) { + if let Ok(mk_proof) = mk_map.compute_proof(&transaction_hashes) { self.mk_map_pool.give_back_resource_pool_item(mk_map)?; let mk_proof_leaves = mk_proof.leaves(); let transaction_hashes_certified: Vec = transaction_hashes @@ -201,7 +210,9 @@ mod tests { use anyhow::anyhow; use mithril_common::crypto_helper::{MKMap, MKMapNode, MKTreeNode}; use mithril_common::entities::CardanoTransaction; - use mithril_common::test_utils::CardanoTransactionsBuilder; + use mithril_common::test_utils::{ + assert_equivalent, equivalent_to, CardanoTransactionsBuilder, + }; use mockall::mock; use mockall::predicate::eq; @@ -359,6 +370,62 @@ mod tests { ) } + #[tokio::test] + async fn dedup_cardano_transactions_hashes() { + let transactions = vec![ + CardanoTransaction::new("tx-hash-123", 10, 50, "block-hash-10", 50), + CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), + CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), + CardanoTransaction::new("tx-hash-789", 25, 51, "block-hash-25", 100), + CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), + ]; + let transactions_hashes: Vec = transactions + .iter() + .map(|t| t.transaction_hash.clone()) + .collect(); + + let transactions_to_prove = vec![ + CardanoTransaction::new("tx-hash-123", 10, 50, "block-hash-10", 50), + CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), + CardanoTransaction::new("tx-hash-789", 25, 51, "block-hash-25", 100), + ]; + + let test_data = test_data::build_test_data(&transactions_to_prove, &transactions); + let prover = build_prover( + |transaction_retriever_mock| { + let transactions_hashes_to_prove: Vec = transactions_to_prove + .iter() + .map(|t| t.transaction_hash.clone()) + .collect(); + transaction_retriever_mock + .expect_get_by_hashes() + .withf(move |hashes, _| equivalent_to(hashes, &transactions_hashes_to_prove)) + .return_once(move |_, _| Ok(vec![])); + + transaction_retriever_mock + .expect_get_by_block_ranges() + .return_once(move |_| Ok(vec![])); + }, + |block_range_root_retriever_mock| { + let block_ranges_map = test_data.block_ranges_map.clone(); + block_range_root_retriever_mock + .expect_compute_merkle_map_from_block_range_roots() + .return_once(|_| { + Ok(test_data::compute_mk_map_from_block_ranges_map( + block_ranges_map, + )) + }); + }, + ); + + prover.compute_cache(test_data.beacon).await.unwrap(); + + prover + .compute_transactions_proofs(99999, &transactions_hashes) + .await + .unwrap(); + } + #[tokio::test] async fn compute_proof_for_one_set_of_three_certified_transactions() { let transactions = CardanoTransactionsBuilder::new() @@ -374,7 +441,10 @@ mod tests { let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) + .withf(move |transactions_hashes, beacon| { + equivalent_to(transactions_hashes, &transaction_hashes_to_prove) + && *beacon == test_data.beacon + }) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -404,9 +474,9 @@ mod tests { .unwrap(); assert_eq!(transactions_set_proof.len(), 1); - assert_eq!( + assert_equivalent( transactions_set_proof[0].transactions_hashes(), - test_data.transaction_hashes_to_prove + &test_data.transaction_hashes_to_prove, ); transactions_set_proof[0].verify().unwrap(); } @@ -425,7 +495,10 @@ mod tests { let transaction_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) + .withf(move |transactions_hashes, beacon| { + equivalent_to(transactions_hashes, &transaction_hashes_to_prove) + && *beacon == test_data.beacon + }) .return_once(move |_, _| Ok(vec![])); transaction_retriever_mock .expect_get_by_block_ranges() @@ -468,7 +541,10 @@ mod tests { let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) + .withf(move |transactions_hashes, beacon| { + equivalent_to(transactions_hashes, &transaction_hashes_to_prove) + && *beacon == test_data.beacon + }) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -519,11 +595,11 @@ mod tests { .concat(); let prover = build_prover( |transaction_retriever_mock| { - let transaction_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); + let transactions_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) + .withf(move |hashes, _| equivalent_to(hashes, &transactions_hashes_to_prove)) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -553,9 +629,9 @@ mod tests { .unwrap(); assert_eq!(transactions_set_proof.len(), 1); - assert_eq!( + assert_equivalent( transactions_set_proof[0].transactions_hashes(), - transaction_hashes_known + &transaction_hashes_known, ); transactions_set_proof[0].verify().unwrap(); } From e618751c1ffb7223902c43e725e5ee1e6d730e69 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 20 Jun 2024 12:38:08 +0200 Subject: [PATCH 03/17] Handle empty cardano transactions hash in prover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-aggregator/src/services/prover.rs | 24 ++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/mithril-aggregator/src/services/prover.rs b/mithril-aggregator/src/services/prover.rs index d5c630362ed..58b738a60c8 100644 --- a/mithril-aggregator/src/services/prover.rs +++ b/mithril-aggregator/src/services/prover.rs @@ -112,7 +112,11 @@ impl MithrilProverService { } fn filter_valid_hashes(&self, transaction_hashes: &[TransactionHash]) -> Vec { - let mut transaction_hashes = transaction_hashes.to_vec(); + let mut transaction_hashes: Vec = transaction_hashes + .iter() + .filter(|hash| !hash.is_empty()) + .cloned() + .collect(); transaction_hashes.sort(); transaction_hashes.dedup(); transaction_hashes @@ -370,6 +374,24 @@ mod tests { ) } + #[test] + fn filter_valid_hashes_remove_empty_hashes() { + let transactions_hashes = vec![ + "tx-hash-123".to_string(), + "".to_string(), + "tx-hash-789".to_string(), + ]; + + let mithril_service_prover = build_prover(|_| {}, |_| {}); + + let valid_hashes = mithril_service_prover.filter_valid_hashes(&transactions_hashes); + + assert_equivalent( + vec!["tx-hash-123".to_string(), "tx-hash-789".to_string()], + valid_hashes, + ); + } + #[tokio::test] async fn dedup_cardano_transactions_hashes() { let transactions = vec![ From 7cc4a95adeae248791a34f120b286192df41ecf2 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 20 Jun 2024 15:42:27 +0200 Subject: [PATCH 04/17] Create a dedicated structure that filter a list of Cardano transactions hashes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-aggregator/src/services/prover.rs | 245 +++++++++++++++------- 1 file changed, 168 insertions(+), 77 deletions(-) diff --git a/mithril-aggregator/src/services/prover.rs b/mithril-aggregator/src/services/prover.rs index 58b738a60c8..7475ec606e9 100644 --- a/mithril-aggregator/src/services/prover.rs +++ b/mithril-aggregator/src/services/prover.rs @@ -50,12 +50,33 @@ pub trait TransactionsRetriever: Sync + Send { ) -> StdResult>; } +#[cfg_attr(test, mockall::automock)] +trait TransactionsHashFilter: Sync + Send { + fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec; +} + +struct TransactionsHashFilterToProve {} + +impl TransactionsHashFilter for TransactionsHashFilterToProve { + fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec { + let mut transaction_hashes: Vec = transaction_hashes + .iter() + .filter(|hash| hash.len() == 64 && hash.chars().all(|c| c.is_ascii_hexdigit())) + .cloned() + .collect(); + transaction_hashes.sort(); + transaction_hashes.dedup(); + transaction_hashes + } +} + /// Mithril prover pub struct MithrilProverService { transaction_retriever: Arc, block_range_root_retriever: Arc, mk_map_pool: ResourcePool>>, logger: Logger, + filter: Box, } impl MithrilProverService { @@ -65,12 +86,29 @@ impl MithrilProverService { block_range_root_retriever: Arc, mk_map_pool_size: usize, logger: Logger, + ) -> Self { + Self::new_with_filter( + transaction_retriever, + block_range_root_retriever, + mk_map_pool_size, + logger, + Box::new(TransactionsHashFilterToProve {}), + ) + } + + fn new_with_filter( + transaction_retriever: Arc, + block_range_root_retriever: Arc, + mk_map_pool_size: usize, + logger: Logger, + filter: Box, ) -> Self { Self { transaction_retriever, block_range_root_retriever, mk_map_pool: ResourcePool::new(mk_map_pool_size, vec![]), logger, + filter, } } @@ -110,17 +148,6 @@ impl MithrilProverService { Ok(block_ranges_map) } - - fn filter_valid_hashes(&self, transaction_hashes: &[TransactionHash]) -> Vec { - let mut transaction_hashes: Vec = transaction_hashes - .iter() - .filter(|hash| !hash.is_empty()) - .cloned() - .collect(); - transaction_hashes.sort(); - transaction_hashes.dedup(); - transaction_hashes - } } #[async_trait] @@ -130,7 +157,7 @@ impl ProverService for MithrilProverService { up_to: BlockNumber, transaction_hashes: &[TransactionHash], ) -> StdResult> { - let transaction_hashes = self.filter_valid_hashes(transaction_hashes); + let transaction_hashes = self.filter.apply(transaction_hashes); // 1 - Compute the set of block ranges with transactions to prove let block_ranges_transactions = self.get_block_ranges(&transaction_hashes, up_to).await?; @@ -219,6 +246,7 @@ mod tests { }; use mockall::mock; use mockall::predicate::eq; + use test_data::transactions_group_by_block_range; use super::*; @@ -239,6 +267,14 @@ mod tests { } } + struct AcceptAllTransactionsFilter {} + + impl TransactionsHashFilter for AcceptAllTransactionsFilter { + fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec { + transaction_hashes.to_vec() + } + } + mod test_data { use super::*; @@ -366,81 +402,67 @@ mod tests { let mk_map_pool_size = 1; let logger = slog_scope::logger(); - MithrilProverService::new( + MithrilProverService::new_with_filter( Arc::new(transaction_retriever), Arc::new(block_range_root_retriever), mk_map_pool_size, logger, + Box::new(AcceptAllTransactionsFilter {}), ) } - #[test] - fn filter_valid_hashes_remove_empty_hashes() { - let transactions_hashes = vec![ - "tx-hash-123".to_string(), - "".to_string(), - "tx-hash-789".to_string(), - ]; - - let mithril_service_prover = build_prover(|_| {}, |_| {}); - - let valid_hashes = mithril_service_prover.filter_valid_hashes(&transactions_hashes); - - assert_equivalent( - vec!["tx-hash-123".to_string(), "tx-hash-789".to_string()], - valid_hashes, - ); - } - #[tokio::test] - async fn dedup_cardano_transactions_hashes() { - let transactions = vec![ - CardanoTransaction::new("tx-hash-123", 10, 50, "block-hash-10", 50), - CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), - CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), - CardanoTransaction::new("tx-hash-789", 25, 51, "block-hash-25", 100), - CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), - ]; - let transactions_hashes: Vec = transactions - .iter() - .map(|t| t.transaction_hash.clone()) - .collect(); - - let transactions_to_prove = vec![ - CardanoTransaction::new("tx-hash-123", 10, 50, "block-hash-10", 50), - CardanoTransaction::new("tx-hash-duplicated-456", 25, 51, "block-hash-25", 100), - CardanoTransaction::new("tx-hash-789", 25, 51, "block-hash-25", 100), - ]; - - let test_data = test_data::build_test_data(&transactions_to_prove, &transactions); - let prover = build_prover( - |transaction_retriever_mock| { - let transactions_hashes_to_prove: Vec = transactions_to_prove - .iter() - .map(|t| t.transaction_hash.clone()) - .collect(); - transaction_retriever_mock - .expect_get_by_hashes() - .withf(move |hashes, _| equivalent_to(hashes, &transactions_hashes_to_prove)) - .return_once(move |_, _| Ok(vec![])); - - transaction_retriever_mock - .expect_get_by_block_ranges() - .return_once(move |_| Ok(vec![])); - }, - |block_range_root_retriever_mock| { - let block_ranges_map = test_data.block_ranges_map.clone(); - block_range_root_retriever_mock - .expect_compute_merkle_map_from_block_range_roots() - .return_once(|_| { - Ok(test_data::compute_mk_map_from_block_ranges_map( - block_ranges_map, - )) - }); - }, + async fn filter_cardano_transactions_hashes() { + let transactions_hashes = vec!["tx-hash-123".to_string(), "tx-hash-456".to_string()]; + + let transactions_hash_filter = { + let mut transactions_hash_filter = MockTransactionsHashFilter::new(); + transactions_hash_filter + .expect_apply() + .with(eq(transactions_hashes.clone())) + .once() + .return_once(|_t| vec!["tx-hash-123-returned".to_string()]); + transactions_hash_filter + }; + + let transaction_retriever = { + let mut transaction_retriever = MockTransactionsRetriever::new(); + transaction_retriever + .expect_get_by_hashes() + .withf(|transaction_hashes, _| { + transaction_hashes == &vec!["tx-hash-123-returned".to_string()] + }) + .return_once(move |_, _| Ok(vec![])); + + transaction_retriever + .expect_get_by_block_ranges() + .return_once(move |_| Ok(vec![])); + + transaction_retriever + }; + + let block_range_root_retriever = { + let mut block_range_root_retriever = MockBlockRangeRootRetrieverImpl::new(); + + block_range_root_retriever + .expect_compute_merkle_map_from_block_range_roots() + .return_once(move |_| { + Ok(test_data::compute_mk_map_from_block_ranges_map( + transactions_group_by_block_range(&[]), + )) + }); + block_range_root_retriever + }; + + let prover = MithrilProverService::new_with_filter( + Arc::new(transaction_retriever), + Arc::new(block_range_root_retriever), + 1, + slog_scope::logger(), + Box::new(transactions_hash_filter), ); - prover.compute_cache(test_data.beacon).await.unwrap(); + prover.compute_cache(123456).await.unwrap(); prover .compute_transactions_proofs(99999, &transactions_hashes) @@ -721,4 +743,73 @@ mod tests { .await .expect_err("Should have failed because of block range root retriever failure"); } + + #[test] + fn transactions_hash_filter_to_prove_apply_remove_empty_hashes() { + let transactions_hashes = vec!["a".repeat(64), "".to_string(), "b".repeat(64)]; + + let filter = TransactionsHashFilterToProve {}; + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); + } + + #[test] + fn transactions_hash_filter_to_prove_apply_deduplicate_hashes() { + let transactions_hashes = vec![ + "a".repeat(64), + "b".repeat(64), + "b".repeat(64), + "c".repeat(64), + "b".repeat(64), + ]; + + let filter = TransactionsHashFilterToProve {}; + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent( + vec!["a".repeat(64), "b".repeat(64), "c".repeat(64)], + valid_hashes, + ); + } + + #[test] + fn transactions_hash_filter_to_prove_apply_keep_only_hash_that_have_a_size_of_64() { + let transactions_hashes = vec!["a".repeat(64), "b".repeat(65), "c".repeat(64)]; + + let filter = TransactionsHashFilterToProve {}; + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent(vec!["a".repeat(64), "c".repeat(64)], valid_hashes); + } + + #[test] + fn transactions_hash_filter_to_prove_apply_keep_only_hexadecimal_characters() { + let transactions_hashes = vec![ + "a".repeat(63) + "a", + "a".repeat(63) + "g", + "a".repeat(63) + "f", + "a".repeat(63) + "x", + "a".repeat(63) + ";", + "a".repeat(63) + " ", + "a".repeat(63) + "à", + "a".repeat(63) + "9", + ]; + + let filter = TransactionsHashFilterToProve {}; + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent( + vec![ + "a".repeat(63) + "a", + "a".repeat(63) + "f", + "a".repeat(63) + "9", + ], + valid_hashes, + ); + } } From 46dbc1066037cee43b16f0fa7477abc711dd36ef Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 21 Jun 2024 12:28:17 +0200 Subject: [PATCH 05/17] Add a limit for the number of transactions hashes to be certified by the prover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/builder.rs | 2 + mithril-aggregator/src/services/prover.rs | 64 ++++++++++++++++--- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 50916c33120..a7b67800b5b 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -75,6 +75,7 @@ use super::{DependenciesBuilderError, EpochServiceWrapper, Result}; const SQLITE_FILE: &str = "aggregator.sqlite3"; const SQLITE_FILE_CARDANO_TRANSACTION: &str = "cardano-transaction.sqlite3"; +const PROVER_MAX_COMPUTABLE_TRANSACTIONS_HASHES: usize = 100; /// ## Dependencies container builder /// @@ -1431,6 +1432,7 @@ impl DependenciesBuilder { block_range_root_retriever, mk_map_pool_size, logger, + PROVER_MAX_COMPUTABLE_TRANSACTIONS_HASHES, ); Ok(Arc::new(prover_service)) diff --git a/mithril-aggregator/src/services/prover.rs b/mithril-aggregator/src/services/prover.rs index 7475ec606e9..b661a7731c3 100644 --- a/mithril-aggregator/src/services/prover.rs +++ b/mithril-aggregator/src/services/prover.rs @@ -55,13 +55,28 @@ trait TransactionsHashFilter: Sync + Send { fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec; } -struct TransactionsHashFilterToProve {} +struct TransactionsHashFilterToProve { + max_hashes: usize, +} + +impl TransactionsHashFilterToProve { + pub fn new(max_hashes: usize) -> Self { + Self { max_hashes } + } +} + +impl Default for TransactionsHashFilterToProve { + fn default() -> Self { + Self::new(usize::MAX) + } +} impl TransactionsHashFilter for TransactionsHashFilterToProve { fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec { let mut transaction_hashes: Vec = transaction_hashes .iter() .filter(|hash| hash.len() == 64 && hash.chars().all(|c| c.is_ascii_hexdigit())) + .take(self.max_hashes) .cloned() .collect(); transaction_hashes.sort(); @@ -81,18 +96,24 @@ pub struct MithrilProverService { impl MithrilProverService { /// Create a new Mithril prover + /// + /// The parameter `max_computable_transactions_hashes` corresponds to the maximum number + /// of transactions hashes that can be computed by [Self::compute_transactions_proofs]. pub fn new( transaction_retriever: Arc, block_range_root_retriever: Arc, mk_map_pool_size: usize, logger: Logger, + max_computable_transactions_hashes: usize, ) -> Self { Self::new_with_filter( transaction_retriever, block_range_root_retriever, mk_map_pool_size, logger, - Box::new(TransactionsHashFilterToProve {}), + Box::new(TransactionsHashFilterToProve::new( + max_computable_transactions_hashes, + )), ) } @@ -747,8 +768,7 @@ mod tests { #[test] fn transactions_hash_filter_to_prove_apply_remove_empty_hashes() { let transactions_hashes = vec!["a".repeat(64), "".to_string(), "b".repeat(64)]; - - let filter = TransactionsHashFilterToProve {}; + let filter = TransactionsHashFilterToProve::default(); let valid_hashes = filter.apply(&transactions_hashes); @@ -764,8 +784,7 @@ mod tests { "c".repeat(64), "b".repeat(64), ]; - - let filter = TransactionsHashFilterToProve {}; + let filter = TransactionsHashFilterToProve::default(); let valid_hashes = filter.apply(&transactions_hashes); @@ -778,8 +797,7 @@ mod tests { #[test] fn transactions_hash_filter_to_prove_apply_keep_only_hash_that_have_a_size_of_64() { let transactions_hashes = vec!["a".repeat(64), "b".repeat(65), "c".repeat(64)]; - - let filter = TransactionsHashFilterToProve {}; + let filter = TransactionsHashFilterToProve::default(); let valid_hashes = filter.apply(&transactions_hashes); @@ -798,8 +816,7 @@ mod tests { "a".repeat(63) + "à", "a".repeat(63) + "9", ]; - - let filter = TransactionsHashFilterToProve {}; + let filter = TransactionsHashFilterToProve::default(); let valid_hashes = filter.apply(&transactions_hashes); @@ -812,4 +829,31 @@ mod tests { valid_hashes, ); } + + #[test] + fn transactions_hash_filter_to_prove_apply_with_limit_on_transactions_hashes_number() { + let transactions_hashes = vec!["a".repeat(64), "b".repeat(64), "c".repeat(64)]; + let filter = TransactionsHashFilterToProve::new(2); + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); + } + + #[test] + fn transactions_hash_filter_to_prove_apply_with_limit_on_transactions_hashes_number_only_on_valid_hashes( + ) { + let transactions_hashes = vec![ + "zz".to_string(), + "a".repeat(64), + "xx".to_string(), + "b".repeat(64), + "c".repeat(64), + ]; + let filter = TransactionsHashFilterToProve::new(2); + + let valid_hashes = filter.apply(&transactions_hashes); + + assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); + } } From d3ec53ce2c16822a53da2b6fb9076291579833d4 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 21 Jun 2024 17:25:08 +0200 Subject: [PATCH 06/17] Add a validator for the transactions hash list that will be called on the HTTP prover route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-aggregator/src/http_server/mod.rs | 1 + .../src/http_server/validators/mod.rs | 3 + .../prover_transactions_hash_validator.rs | 138 ++++++++++++++++++ .../src/entities/http_server_error.rs | 7 +- 4 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 mithril-aggregator/src/http_server/validators/mod.rs create mode 100644 mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs diff --git a/mithril-aggregator/src/http_server/mod.rs b/mithril-aggregator/src/http_server/mod.rs index c58fe07b4b5..7d0d8dd4ede 100644 --- a/mithril-aggregator/src/http_server/mod.rs +++ b/mithril-aggregator/src/http_server/mod.rs @@ -1,3 +1,4 @@ pub mod routes; +pub mod validators; pub const SERVER_BASE_PATH: &str = "aggregator"; diff --git a/mithril-aggregator/src/http_server/validators/mod.rs b/mithril-aggregator/src/http_server/validators/mod.rs new file mode 100644 index 00000000000..ca5d2d37710 --- /dev/null +++ b/mithril-aggregator/src/http_server/validators/mod.rs @@ -0,0 +1,3 @@ +mod prover_transactions_hash_validator; + +pub use prover_transactions_hash_validator::*; diff --git a/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs new file mode 100644 index 00000000000..8d0ef98f336 --- /dev/null +++ b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs @@ -0,0 +1,138 @@ +use mithril_common::entities::ClientError; + +pub struct ProverTransactionsHashValidator { + max_hashes: usize, +} + +impl ProverTransactionsHashValidator { + const LABEL: &str = "invalid_transaction_hashes"; + + pub fn new(max_hashes: usize) -> Self { + Self { max_hashes } + } + + pub fn validate(&self, hashes: Vec) -> Result<(), ClientError> { + if hashes.len() > self.max_hashes { + return Err(ClientError::new( + Self::LABEL, + format!( + "Transaction hashes list contains more than maximum allowed hashes: '{}'", + self.max_hashes + ), + )); + } + + for hash in hashes { + if hash.is_empty() { + return Err(ClientError::new( + Self::LABEL, + "Transaction hashes cannot be empty", + )); + } + + if hash.chars().count() != 64 { + return Err(ClientError::new( + Self::LABEL, + "Transaction hashes must have 64 characters", + )); + } + + if !hash.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(ClientError::new( + Self::LABEL, + "Transaction hashes must contain only hexadecimal characters", + )); + } + } + + Ok(()) + } +} + +impl Default for ProverTransactionsHashValidator { + fn default() -> Self { + Self::new(usize::MAX) + } +} + +mod tests { + use super::*; + + #[test] + fn prover_transactions_hash_validator_return_error_when_empty_hash() { + let error = ProverTransactionsHashValidator::default() + .validate(vec!["".to_string()]) + .expect_err("Should return an error"); + + assert_eq!( + error, + ClientError::new( + "invalid_transaction_hashes", + "Transaction hashes cannot be empty" + ) + ); + } + + #[test] + fn prover_transactions_hash_validator_return_error_when_hash_size_different_than_64() { + let error = ProverTransactionsHashValidator::default() + .validate(vec!["abc".to_string()]) + .expect_err("Should return an error"); + + assert_eq!( + error, + ClientError::new( + "invalid_transaction_hashes", + "Transaction hashes must have 64 characters" + ) + ); + } + + #[test] + fn prover_transactions_hash_validator_return_error_when_hash_contains_non_hexadecimal_characters( + ) { + for invalid_char in ["g", "x", ";", " ", "à"].iter() { + let hash = format!("{}{}", "a".repeat(63), invalid_char); + let error = ProverTransactionsHashValidator::default() + .validate(vec![hash.clone()]) + .expect_err("Should return an error"); + assert_eq!( + error, + ClientError::new( + "invalid_transaction_hashes", + "Transaction hashes must contain only hexadecimal characters" + ), + "Invalid hash: {}", + hash + ); + } + } + + #[test] + fn prover_transactions_hash_validator_when_hash_contains_only_hexadecimal_characters() { + ProverTransactionsHashValidator::default() + .validate(vec![format!("bcd9{}", "a".repeat(60))]) + .expect("Should succeed"); + } + + #[test] + fn prover_transactions_hash_validator_return_error_when_more_hashes_than_max_allowed() { + let transactions_hashes = vec!["a".repeat(64), "b".repeat(64), "c".repeat(64)]; + let validator = ProverTransactionsHashValidator::new(2); + + let error = validator + .validate(transactions_hashes) + .expect_err("Should return an error"); + + assert_eq!( + error, + ClientError::new( + "invalid_transaction_hashes", + format!( + "Transaction hashes list contains more than maximum allowed hashes: '{}'", + validator.max_hashes + ) + ) + ); + } +} diff --git a/mithril-common/src/entities/http_server_error.rs b/mithril-common/src/entities/http_server_error.rs index 936b22cfe48..4fff1b31eb6 100644 --- a/mithril-common/src/entities/http_server_error.rs +++ b/mithril-common/src/entities/http_server_error.rs @@ -45,7 +45,10 @@ pub struct ClientError { impl ClientError { /// ClientError factory - pub fn new(label: String, message: String) -> ClientError { - ClientError { label, message } + pub fn new, T2: Into>(label: T1, message: T2) -> ClientError { + ClientError { + label: label.into(), + message: message.into(), + } } } From 48f4af1ae1725697d847187cbd098011af57e1be Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 21 Jun 2024 18:10:23 +0200 Subject: [PATCH 07/17] Implement the validator in the proof route Co-authored-by: DJO --- .../src/http_server/routes/middlewares.rs | 16 ++++++ .../src/http_server/routes/proof_routes.rs | 56 +++++++++++++++++-- .../prover_transactions_hash_validator.rs | 16 +++--- mithril-common/src/test_utils/fake_data.rs | 11 ++++ 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/middlewares.rs b/mithril-aggregator/src/http_server/routes/middlewares.rs index 869b493004b..a602b0178e6 100644 --- a/mithril-aggregator/src/http_server/routes/middlewares.rs +++ b/mithril-aggregator/src/http_server/routes/middlewares.rs @@ -112,3 +112,19 @@ pub fn with_prover_service( ) -> impl Filter,), Error = Infallible> + Clone { warp::any().map(move || dependency_manager.prover_service.clone()) } + +pub mod validators { + use crate::http_server::validators::ProverTransactionsHashValidator; + + use super::*; + + /// With Prover Transactions Hash Validator + pub fn with_prover_transations_hash_validator( + _dependency_manager: Arc, + ) -> impl Filter + Clone { + // Todo: get this value from the configuration + let max_hashes = 100; + + warp::any().map(move || ProverTransactionsHashValidator::new(max_hashes)) + } +} diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index 0963d87ea07..ae4c4a8c470 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -32,6 +32,11 @@ fn proof_cardano_transaction( .and(middlewares::with_signed_entity_service( dependency_manager.clone(), )) + .and( + middlewares::validators::with_prover_transations_hash_validator( + dependency_manager.clone(), + ), + ) .and(middlewares::with_prover_service(dependency_manager)) .and_then(handlers::proof_cardano_transaction) } @@ -47,7 +52,7 @@ mod handlers { use warp::http::StatusCode; use crate::{ - http_server::routes::reply, + http_server::{routes::reply, validators::ProverTransactionsHashValidator}, message_adapters::ToCardanoTransactionsProofsMessageAdapter, services::{ProverService, SignedEntityService}, unwrap_to_internal_server_error, @@ -58,6 +63,7 @@ mod handlers { pub async fn proof_cardano_transaction( transaction_parameters: CardanoTransactionProofQueryParams, signed_entity_service: Arc, + validator: ProverTransactionsHashValidator, prover_service: Arc, ) -> Result { let transaction_hashes = transaction_parameters @@ -70,6 +76,11 @@ mod handlers { transaction_parameters.transaction_hashes ); + if let Err(error) = validator.validate(&transaction_hashes) { + warn!("proof_cardano_transaction::bad_request"); + return Ok(reply::bad_request(error.label, error.message)); + } + match unwrap_to_internal_server_error!( signed_entity_service .get_last_cardano_transaction_snapshot() @@ -123,7 +134,7 @@ mod tests { use mithril_common::{ entities::{CardanoTransactionsSetProof, CardanoTransactionsSnapshot, SignedEntity}, - test_utils::apispec::APISpec, + test_utils::{apispec::APISpec, fake_data}, }; use crate::services::MockSignedEntityService; @@ -199,7 +210,9 @@ mod tests { let response = request() .method(method) .path(&format!( - "/{SERVER_BASE_PATH}{path}?transaction_hashes=tx-123,tx-456" + "/{SERVER_BASE_PATH}{path}?transaction_hashes={},{}", + fake_data::transaction_hashes()[0], + fake_data::transaction_hashes()[1] )) .reply(&setup_router(Arc::new(dependency_manager))) .await; @@ -228,7 +241,9 @@ mod tests { let response = request() .method(method) .path(&format!( - "/{SERVER_BASE_PATH}{path}?transaction_hashes=tx-123,tx-456" + "/{SERVER_BASE_PATH}{path}?transaction_hashes={},{}", + fake_data::transaction_hashes()[0], + fake_data::transaction_hashes()[1] )) .reply(&setup_router(Arc::new(dependency_manager))) .await; @@ -262,7 +277,9 @@ mod tests { let response = request() .method(method) .path(&format!( - "/{SERVER_BASE_PATH}{path}?transaction_hashes=tx-123,tx-456" + "/{SERVER_BASE_PATH}{path}?transaction_hashes={},{}", + fake_data::transaction_hashes()[0], + fake_data::transaction_hashes()[1] )) .reply(&setup_router(Arc::new(dependency_manager))) .await; @@ -278,4 +295,33 @@ mod tests { ) .unwrap(); } + + #[tokio::test] + async fn proof_cardano_transaction_return_bad_request_with_invalid_hashes() { + let config = Configuration::new_sample(); + let mut builder = DependenciesBuilder::new(config); + let dependency_manager = builder.build_dependency_container().await.unwrap(); + + let method = Method::GET.as_str(); + let path = "/proof/cardano-transaction"; + + let response = request() + .method(method) + .path(&format!( + "/{SERVER_BASE_PATH}{path}?transaction_hashes=invalid%3A%2F%2Fid,,tx-456" + )) + .reply(&setup_router(Arc::new(dependency_manager))) + .await; + + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + method, + path, + "application/json", + &Null, + &response, + &StatusCode::BAD_REQUEST, + ) + .unwrap(); + } } diff --git a/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs index 8d0ef98f336..c8c03b2c204 100644 --- a/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs +++ b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs @@ -5,13 +5,13 @@ pub struct ProverTransactionsHashValidator { } impl ProverTransactionsHashValidator { - const LABEL: &str = "invalid_transaction_hashes"; + const LABEL: &'static str = "invalid_transaction_hashes"; pub fn new(max_hashes: usize) -> Self { Self { max_hashes } } - pub fn validate(&self, hashes: Vec) -> Result<(), ClientError> { + pub fn validate(&self, hashes: &[String]) -> Result<(), ClientError> { if hashes.len() > self.max_hashes { return Err(ClientError::new( Self::LABEL, @@ -49,19 +49,21 @@ impl ProverTransactionsHashValidator { } } +#[cfg(test)] impl Default for ProverTransactionsHashValidator { fn default() -> Self { Self::new(usize::MAX) } } +#[cfg(test)] mod tests { use super::*; #[test] fn prover_transactions_hash_validator_return_error_when_empty_hash() { let error = ProverTransactionsHashValidator::default() - .validate(vec!["".to_string()]) + .validate(&["".to_string()]) .expect_err("Should return an error"); assert_eq!( @@ -76,7 +78,7 @@ mod tests { #[test] fn prover_transactions_hash_validator_return_error_when_hash_size_different_than_64() { let error = ProverTransactionsHashValidator::default() - .validate(vec!["abc".to_string()]) + .validate(&["abc".to_string()]) .expect_err("Should return an error"); assert_eq!( @@ -94,7 +96,7 @@ mod tests { for invalid_char in ["g", "x", ";", " ", "à"].iter() { let hash = format!("{}{}", "a".repeat(63), invalid_char); let error = ProverTransactionsHashValidator::default() - .validate(vec![hash.clone()]) + .validate(&[hash.clone()]) .expect_err("Should return an error"); assert_eq!( error, @@ -111,7 +113,7 @@ mod tests { #[test] fn prover_transactions_hash_validator_when_hash_contains_only_hexadecimal_characters() { ProverTransactionsHashValidator::default() - .validate(vec![format!("bcd9{}", "a".repeat(60))]) + .validate(&[format!("bcd9{}", "a".repeat(60))]) .expect("Should succeed"); } @@ -121,7 +123,7 @@ mod tests { let validator = ProverTransactionsHashValidator::new(2); let error = validator - .validate(transactions_hashes) + .validate(&transactions_hashes) .expect_err("Should return an error"); assert_eq!( diff --git a/mithril-common/src/test_utils/fake_data.rs b/mithril-common/src/test_utils/fake_data.rs index b1007e48f7a..8f1c2b7801f 100644 --- a/mithril-common/src/test_utils/fake_data.rs +++ b/mithril-common/src/test_utils/fake_data.rs @@ -245,3 +245,14 @@ pub fn cardano_transactions_snapshot(total: u64) -> Vec() -> [&'a str; 5] { + [ + "c96809e2cecd9e27499a4379094c4e1f7b59d918c96327bd8daf1bf909dae332", + "5b8788784af9c414f18fc1e6161005b13b839fd91130b7c109aeba1792feb843", + "8b6ae44edf877ff2ac80cf067809d575ab2bad234b668f91e90decde837b154a", + "3f6f3c981c89097f62c9b43632875db7a52183ad3061c822d98259d18cd63dcf", + "f4fd91dccc25fd63f2caebab3d3452bc4b2944fcc11652214a3e8f1d32b09713", + ] +} From 809d243b96f3738dc903656f27910e81f7acb341 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 24 Jun 2024 10:27:40 +0200 Subject: [PATCH 08/17] Handle deduplicate transactions hashes in the proof route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- .../src/http_server/routes/proof_routes.rs | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index ae4c4a8c470..4921883c4fe 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -66,7 +66,7 @@ mod handlers { validator: ProverTransactionsHashValidator, prover_service: Arc, ) -> Result { - let transaction_hashes = transaction_parameters + let mut transaction_hashes = transaction_parameters .split_transactions_hashes() .iter() .map(|s| s.to_string()) @@ -81,6 +81,9 @@ mod handlers { return Ok(reply::bad_request(error.label, error.message)); } + transaction_hashes.sort(); + transaction_hashes.dedup(); + match unwrap_to_internal_server_error!( signed_entity_service .get_last_cardano_transaction_snapshot() @@ -324,4 +327,55 @@ mod tests { ) .unwrap(); } + + #[tokio::test] + async fn proof_cardano_transaction_route_deduplicate_hashes() { + let config = Configuration::new_sample(); + let mut builder = DependenciesBuilder::new(config); + let mut dependency_manager = builder.build_dependency_container().await.unwrap(); + let mut mock_signed_entity_service = MockSignedEntityService::new(); + mock_signed_entity_service + .expect_get_last_cardano_transaction_snapshot() + .returning(|| Ok(Some(SignedEntity::::dummy()))); + dependency_manager.signed_entity_service = Arc::new(mock_signed_entity_service); + + let mut mock_prover_service = MockProverService::new(); + mock_prover_service + .expect_compute_transactions_proofs() + .withf(|_, transaction_hashes| { + transaction_hashes.len() == 2 + && transaction_hashes.contains(&fake_data::transaction_hashes()[0].to_string()) + && transaction_hashes.contains(&fake_data::transaction_hashes()[1].to_string()) + }) + .returning(|_, _| Ok(vec![CardanoTransactionsSetProof::dummy()])); + dependency_manager.prover_service = Arc::new(mock_prover_service); + + let method = Method::GET.as_str(); + let path = "/proof/cardano-transaction"; + + // We are testing on an unordered list of transaction hashes + let response = request() + .method(method) + .path(&format!( + "/{SERVER_BASE_PATH}{path}?transaction_hashes={},{},{},{},{}", + fake_data::transaction_hashes()[0], + fake_data::transaction_hashes()[1], + fake_data::transaction_hashes()[1], + fake_data::transaction_hashes()[0], + fake_data::transaction_hashes()[1] + )) + .reply(&setup_router(Arc::new(dependency_manager))) + .await; + + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + method, + path, + "application/json", + &Null, + &response, + &StatusCode::OK, + ) + .unwrap(); + } } From 9ec5039f746d043a14ae267e65c7140eca6bcc0e Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 24 Jun 2024 10:55:18 +0200 Subject: [PATCH 09/17] Extend aggregator configuration by adding the maximum number of transactions hashes allowed by request to the prover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- mithril-aggregator/src/configuration.rs | 12 ++++++++++++ .../src/http_server/routes/middlewares.rs | 7 ++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index f1d7a8ea311..7446fbfdd55 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -167,6 +167,9 @@ pub struct Configuration { /// Cardano transactions signing configuration #[example = "`{ security_parameter: 3000, step: 120 }`"] pub cardano_transactions_signing_config: CardanoTransactionsSigningConfig, + + /// Maximum number of transactions hashes allowed by request to the prover + pub cardano_transactions_prover_max_hashes_allowed_by_request: usize, } /// Uploader needed to copy the snapshot once computed. @@ -245,6 +248,7 @@ impl Configuration { security_parameter: 100, step: 15, }, + cardano_transactions_prover_max_hashes_allowed_by_request: 100, } } @@ -358,6 +362,9 @@ pub struct DefaultConfiguration { /// Cardano transactions signing configuration pub cardano_transactions_signing_config: CardanoTransactionsSigningConfig, + + /// Maximum number of transactions hashes allowed by request to the prover + pub cardano_transactions_prover_max_hashes_allowed_by_request: u32, } impl Default for DefaultConfiguration { @@ -384,6 +391,7 @@ impl Default for DefaultConfiguration { security_parameter: 3000, step: 120, }, + cardano_transactions_prover_max_hashes_allowed_by_request: 100, } } } @@ -483,6 +491,10 @@ impl Source for DefaultConfiguration { ), ])), ); + result.insert( + "cardano_transactions_prover_max_hashes_allowed_by_request".to_string(), + into_value(myself.cardano_transactions_prover_max_hashes_allowed_by_request), + ); Ok(result) } diff --git a/mithril-aggregator/src/http_server/routes/middlewares.rs b/mithril-aggregator/src/http_server/routes/middlewares.rs index a602b0178e6..15b5f1c8585 100644 --- a/mithril-aggregator/src/http_server/routes/middlewares.rs +++ b/mithril-aggregator/src/http_server/routes/middlewares.rs @@ -120,10 +120,11 @@ pub mod validators { /// With Prover Transactions Hash Validator pub fn with_prover_transations_hash_validator( - _dependency_manager: Arc, + dependency_manager: Arc, ) -> impl Filter + Clone { - // Todo: get this value from the configuration - let max_hashes = 100; + let max_hashes = dependency_manager + .config + .cardano_transactions_prover_max_hashes_allowed_by_request; warp::any().map(move || ProverTransactionsHashValidator::new(max_hashes)) } From 1d107d43db00e37f6d0629b0c9a5843b5c617fa5 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 24 Jun 2024 11:54:17 +0200 Subject: [PATCH 10/17] Add aggregator parameter `cardano_transactions_prover_max_hashes_allowed_by_request` in `root_routes` HTTP response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- .../src/http_server/routes/root_routes.rs | 86 ++++++++++++++++--- openapi.yaml | 20 ++++- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index efe1ca1b3b6..3312c2365b7 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -16,6 +16,13 @@ pub struct RootRouteMessage { #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct AggregatorCapabilities { pub signed_entity_types: BTreeSet, + #[serde(skip_serializing_if = "Option::is_none")] + pub cardano_transactions_prover: Option, +} + +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub struct CardanoTransactionsProverCapabilities { + max_hashes_allowed_by_request: usize, } pub fn routes( @@ -32,7 +39,10 @@ fn root( .and(middlewares::with_api_version_provider( dependency_manager.clone(), )) - .and(middlewares::with_signed_entity_config(dependency_manager)) + .and(middlewares::with_signed_entity_config( + dependency_manager.clone(), + )) + .and(middlewares::with_config(dependency_manager)) .and_then(handlers::root) } @@ -43,16 +53,19 @@ mod handlers { use warp::http::StatusCode; use mithril_common::api_version::APIVersionProvider; - use mithril_common::entities::SignedEntityConfig; + use mithril_common::entities::{SignedEntityConfig, SignedEntityTypeDiscriminants}; use crate::http_server::routes::reply::json; - use crate::http_server::routes::root_routes::{AggregatorCapabilities, RootRouteMessage}; - use crate::unwrap_to_internal_server_error; + use crate::http_server::routes::root_routes::{ + AggregatorCapabilities, CardanoTransactionsProverCapabilities, RootRouteMessage, + }; + use crate::{unwrap_to_internal_server_error, Configuration}; /// Root pub async fn root( api_version_provider: Arc, signed_entity_config: SignedEntityConfig, + configuration: Configuration, ) -> Result { debug!("⇄ HTTP SERVER: root"); @@ -61,13 +74,23 @@ mod handlers { "root::error" ); + let signed_entity_types = + signed_entity_config.list_allowed_signed_entity_types_discriminants(); + + let cardano_transactions_prover_capabilities = signed_entity_types + .contains(&SignedEntityTypeDiscriminants::CardanoTransactions) + .then_some(CardanoTransactionsProverCapabilities { + max_hashes_allowed_by_request: configuration + .cardano_transactions_prover_max_hashes_allowed_by_request, + }); + Ok(json( &RootRouteMessage { open_api_version: open_api_version.to_string(), documentation_url: env!("CARGO_PKG_HOMEPAGE").to_string(), capabilities: AggregatorCapabilities { - signed_entity_types: signed_entity_config - .list_allowed_signed_entity_types_discriminants(), + signed_entity_types, + cardano_transactions_prover: cardano_transactions_prover_capabilities, }, }, StatusCode::OK, @@ -113,7 +136,7 @@ mod tests { .allowed_discriminants = BTreeSet::from([ SignedEntityTypeDiscriminants::MithrilStakeDistribution, SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, - SignedEntityTypeDiscriminants::CardanoTransactions, + SignedEntityTypeDiscriminants::CardanoStakeDistribution, ]); let expected_open_api_version = dependency_manager .api_version_provider @@ -139,11 +162,12 @@ mod tests { documentation_url: env!("CARGO_PKG_HOMEPAGE").to_string(), capabilities: AggregatorCapabilities { signed_entity_types: BTreeSet::from_iter([ - SignedEntityTypeDiscriminants::CardanoTransactions, + SignedEntityTypeDiscriminants::CardanoStakeDistribution, SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, SignedEntityTypeDiscriminants::MithrilStakeDistribution, - ]) - } + ]), + cardano_transactions_prover: None + }, } ); @@ -158,4 +182,46 @@ mod tests { ) .unwrap(); } + + #[tokio::test] + async fn test_root_route_ok_with_cardano_transactions_prover_capabilities() { + let method = Method::GET.as_str(); + let path = "/"; + let mut dependency_manager = initialize_dependencies().await; + dependency_manager + .signed_entity_config + .allowed_discriminants = + BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]); + dependency_manager + .config + .cardano_transactions_prover_max_hashes_allowed_by_request = 99; + + let response = request() + .method(method) + .path(&format!("/{SERVER_BASE_PATH}{path}")) + .reply(&setup_router(Arc::new(dependency_manager))) + .await; + + let response_body: RootRouteMessage = serde_json::from_slice(response.body()).unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + assert_eq!( + response_body.capabilities.cardano_transactions_prover, + Some(CardanoTransactionsProverCapabilities { + max_hashes_allowed_by_request: 99 + }) + ); + + APISpec::verify_conformity( + APISpec::get_all_spec_files(), + method, + path, + "application/json", + &Null, + &response, + &StatusCode::OK, + ) + .unwrap(); + } } diff --git a/openapi.yaml b/openapi.yaml index 7d704d339c9..0a6e2c1a38e 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -31,6 +31,7 @@ paths: * Open API version * URL of Mithril documentation * Capabilities of the aggregator + * Cardano transactions prover capabilities responses: "200": description: root found @@ -566,14 +567,29 @@ components: - CardanoStakeDistribution - CardanoImmutableFilesFull - CardanoTransactions + cardano_transactions_prover: + description: Cardano transactions prover capabilities + type: object + additionalProperties: false + required: + - max_hashes_allowed_by_request + properties: + max_hashes_allowed_by_request: + description: Maximum number of hashes allowed for a single request + type: integer + format: int64 example: { "open_api_version": "0.1.17", "documentation_url": "https://mithril.network", "capabilities": { - "signed_entity_types": [ "MithrilStakeDistribution", "CardanoImmutableFilesFull", "CardanoTransactions" ] - } + "signed_entity_types": [ "MithrilStakeDistribution", "CardanoImmutableFilesFull", "CardanoTransactions" ], + "cardano_transactions_prover": + { + "max_hashes_allowed_by_request": 100 + } + }, } Epoch: From fc107c9a048c3d098e19d4bcbc2beb951d72cdbd Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 24 Jun 2024 12:01:16 +0200 Subject: [PATCH 11/17] Revert `prover` modifications after moving control and limit logic in the `proof_route` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- .../src/dependency_injection/builder.rs | 2 - mithril-aggregator/src/services/prover.rs | 259 +----------------- 2 files changed, 13 insertions(+), 248 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index a7b67800b5b..50916c33120 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -75,7 +75,6 @@ use super::{DependenciesBuilderError, EpochServiceWrapper, Result}; const SQLITE_FILE: &str = "aggregator.sqlite3"; const SQLITE_FILE_CARDANO_TRANSACTION: &str = "cardano-transaction.sqlite3"; -const PROVER_MAX_COMPUTABLE_TRANSACTIONS_HASHES: usize = 100; /// ## Dependencies container builder /// @@ -1432,7 +1431,6 @@ impl DependenciesBuilder { block_range_root_retriever, mk_map_pool_size, logger, - PROVER_MAX_COMPUTABLE_TRANSACTIONS_HASHES, ); Ok(Arc::new(prover_service)) diff --git a/mithril-aggregator/src/services/prover.rs b/mithril-aggregator/src/services/prover.rs index b661a7731c3..77b76a73f75 100644 --- a/mithril-aggregator/src/services/prover.rs +++ b/mithril-aggregator/src/services/prover.rs @@ -50,86 +50,27 @@ pub trait TransactionsRetriever: Sync + Send { ) -> StdResult>; } -#[cfg_attr(test, mockall::automock)] -trait TransactionsHashFilter: Sync + Send { - fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec; -} - -struct TransactionsHashFilterToProve { - max_hashes: usize, -} - -impl TransactionsHashFilterToProve { - pub fn new(max_hashes: usize) -> Self { - Self { max_hashes } - } -} - -impl Default for TransactionsHashFilterToProve { - fn default() -> Self { - Self::new(usize::MAX) - } -} - -impl TransactionsHashFilter for TransactionsHashFilterToProve { - fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec { - let mut transaction_hashes: Vec = transaction_hashes - .iter() - .filter(|hash| hash.len() == 64 && hash.chars().all(|c| c.is_ascii_hexdigit())) - .take(self.max_hashes) - .cloned() - .collect(); - transaction_hashes.sort(); - transaction_hashes.dedup(); - transaction_hashes - } -} - /// Mithril prover pub struct MithrilProverService { transaction_retriever: Arc, block_range_root_retriever: Arc, mk_map_pool: ResourcePool>>, logger: Logger, - filter: Box, } impl MithrilProverService { /// Create a new Mithril prover - /// - /// The parameter `max_computable_transactions_hashes` corresponds to the maximum number - /// of transactions hashes that can be computed by [Self::compute_transactions_proofs]. pub fn new( transaction_retriever: Arc, block_range_root_retriever: Arc, mk_map_pool_size: usize, logger: Logger, - max_computable_transactions_hashes: usize, - ) -> Self { - Self::new_with_filter( - transaction_retriever, - block_range_root_retriever, - mk_map_pool_size, - logger, - Box::new(TransactionsHashFilterToProve::new( - max_computable_transactions_hashes, - )), - ) - } - - fn new_with_filter( - transaction_retriever: Arc, - block_range_root_retriever: Arc, - mk_map_pool_size: usize, - logger: Logger, - filter: Box, ) -> Self { Self { transaction_retriever, block_range_root_retriever, mk_map_pool: ResourcePool::new(mk_map_pool_size, vec![]), logger, - filter, } } @@ -178,10 +119,8 @@ impl ProverService for MithrilProverService { up_to: BlockNumber, transaction_hashes: &[TransactionHash], ) -> StdResult> { - let transaction_hashes = self.filter.apply(transaction_hashes); - // 1 - Compute the set of block ranges with transactions to prove - let block_ranges_transactions = self.get_block_ranges(&transaction_hashes, up_to).await?; + let block_ranges_transactions = self.get_block_ranges(transaction_hashes, up_to).await?; let block_range_transactions = self .get_all_transactions_for_block_ranges(&block_ranges_transactions) .await?; @@ -206,7 +145,7 @@ impl ProverService for MithrilProverService { } // 5 - Compute the proof for all transactions - if let Ok(mk_proof) = mk_map.compute_proof(&transaction_hashes) { + if let Ok(mk_proof) = mk_map.compute_proof(transaction_hashes) { self.mk_map_pool.give_back_resource_pool_item(mk_map)?; let mk_proof_leaves = mk_proof.leaves(); let transaction_hashes_certified: Vec = transaction_hashes @@ -262,12 +201,9 @@ mod tests { use anyhow::anyhow; use mithril_common::crypto_helper::{MKMap, MKMapNode, MKTreeNode}; use mithril_common::entities::CardanoTransaction; - use mithril_common::test_utils::{ - assert_equivalent, equivalent_to, CardanoTransactionsBuilder, - }; + use mithril_common::test_utils::CardanoTransactionsBuilder; use mockall::mock; use mockall::predicate::eq; - use test_data::transactions_group_by_block_range; use super::*; @@ -288,14 +224,6 @@ mod tests { } } - struct AcceptAllTransactionsFilter {} - - impl TransactionsHashFilter for AcceptAllTransactionsFilter { - fn apply(&self, transaction_hashes: &[TransactionHash]) -> Vec { - transaction_hashes.to_vec() - } - } - mod test_data { use super::*; @@ -423,74 +351,14 @@ mod tests { let mk_map_pool_size = 1; let logger = slog_scope::logger(); - MithrilProverService::new_with_filter( + MithrilProverService::new( Arc::new(transaction_retriever), Arc::new(block_range_root_retriever), mk_map_pool_size, logger, - Box::new(AcceptAllTransactionsFilter {}), ) } - #[tokio::test] - async fn filter_cardano_transactions_hashes() { - let transactions_hashes = vec!["tx-hash-123".to_string(), "tx-hash-456".to_string()]; - - let transactions_hash_filter = { - let mut transactions_hash_filter = MockTransactionsHashFilter::new(); - transactions_hash_filter - .expect_apply() - .with(eq(transactions_hashes.clone())) - .once() - .return_once(|_t| vec!["tx-hash-123-returned".to_string()]); - transactions_hash_filter - }; - - let transaction_retriever = { - let mut transaction_retriever = MockTransactionsRetriever::new(); - transaction_retriever - .expect_get_by_hashes() - .withf(|transaction_hashes, _| { - transaction_hashes == &vec!["tx-hash-123-returned".to_string()] - }) - .return_once(move |_, _| Ok(vec![])); - - transaction_retriever - .expect_get_by_block_ranges() - .return_once(move |_| Ok(vec![])); - - transaction_retriever - }; - - let block_range_root_retriever = { - let mut block_range_root_retriever = MockBlockRangeRootRetrieverImpl::new(); - - block_range_root_retriever - .expect_compute_merkle_map_from_block_range_roots() - .return_once(move |_| { - Ok(test_data::compute_mk_map_from_block_ranges_map( - transactions_group_by_block_range(&[]), - )) - }); - block_range_root_retriever - }; - - let prover = MithrilProverService::new_with_filter( - Arc::new(transaction_retriever), - Arc::new(block_range_root_retriever), - 1, - slog_scope::logger(), - Box::new(transactions_hash_filter), - ); - - prover.compute_cache(123456).await.unwrap(); - - prover - .compute_transactions_proofs(99999, &transactions_hashes) - .await - .unwrap(); - } - #[tokio::test] async fn compute_proof_for_one_set_of_three_certified_transactions() { let transactions = CardanoTransactionsBuilder::new() @@ -506,10 +374,7 @@ mod tests { let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .withf(move |transactions_hashes, beacon| { - equivalent_to(transactions_hashes, &transaction_hashes_to_prove) - && *beacon == test_data.beacon - }) + .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -539,9 +404,9 @@ mod tests { .unwrap(); assert_eq!(transactions_set_proof.len(), 1); - assert_equivalent( + assert_eq!( transactions_set_proof[0].transactions_hashes(), - &test_data.transaction_hashes_to_prove, + test_data.transaction_hashes_to_prove ); transactions_set_proof[0].verify().unwrap(); } @@ -560,10 +425,7 @@ mod tests { let transaction_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .withf(move |transactions_hashes, beacon| { - equivalent_to(transactions_hashes, &transaction_hashes_to_prove) - && *beacon == test_data.beacon - }) + .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) .return_once(move |_, _| Ok(vec![])); transaction_retriever_mock .expect_get_by_block_ranges() @@ -606,10 +468,7 @@ mod tests { let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .withf(move |transactions_hashes, beacon| { - equivalent_to(transactions_hashes, &transaction_hashes_to_prove) - && *beacon == test_data.beacon - }) + .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -660,11 +519,11 @@ mod tests { .concat(); let prover = build_prover( |transaction_retriever_mock| { - let transactions_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); + let transaction_hashes_to_prove = test_data.transaction_hashes_to_prove.clone(); let transactions_to_prove = transactions_to_prove.clone(); transaction_retriever_mock .expect_get_by_hashes() - .withf(move |hashes, _| equivalent_to(hashes, &transactions_hashes_to_prove)) + .with(eq(transaction_hashes_to_prove), eq(test_data.beacon)) .return_once(move |_, _| Ok(transactions_to_prove)); let block_ranges_to_prove = test_data.block_ranges_to_prove.clone(); @@ -694,9 +553,9 @@ mod tests { .unwrap(); assert_eq!(transactions_set_proof.len(), 1); - assert_equivalent( + assert_eq!( transactions_set_proof[0].transactions_hashes(), - &transaction_hashes_known, + transaction_hashes_known ); transactions_set_proof[0].verify().unwrap(); } @@ -764,96 +623,4 @@ mod tests { .await .expect_err("Should have failed because of block range root retriever failure"); } - - #[test] - fn transactions_hash_filter_to_prove_apply_remove_empty_hashes() { - let transactions_hashes = vec!["a".repeat(64), "".to_string(), "b".repeat(64)]; - let filter = TransactionsHashFilterToProve::default(); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); - } - - #[test] - fn transactions_hash_filter_to_prove_apply_deduplicate_hashes() { - let transactions_hashes = vec![ - "a".repeat(64), - "b".repeat(64), - "b".repeat(64), - "c".repeat(64), - "b".repeat(64), - ]; - let filter = TransactionsHashFilterToProve::default(); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent( - vec!["a".repeat(64), "b".repeat(64), "c".repeat(64)], - valid_hashes, - ); - } - - #[test] - fn transactions_hash_filter_to_prove_apply_keep_only_hash_that_have_a_size_of_64() { - let transactions_hashes = vec!["a".repeat(64), "b".repeat(65), "c".repeat(64)]; - let filter = TransactionsHashFilterToProve::default(); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent(vec!["a".repeat(64), "c".repeat(64)], valid_hashes); - } - - #[test] - fn transactions_hash_filter_to_prove_apply_keep_only_hexadecimal_characters() { - let transactions_hashes = vec![ - "a".repeat(63) + "a", - "a".repeat(63) + "g", - "a".repeat(63) + "f", - "a".repeat(63) + "x", - "a".repeat(63) + ";", - "a".repeat(63) + " ", - "a".repeat(63) + "à", - "a".repeat(63) + "9", - ]; - let filter = TransactionsHashFilterToProve::default(); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent( - vec![ - "a".repeat(63) + "a", - "a".repeat(63) + "f", - "a".repeat(63) + "9", - ], - valid_hashes, - ); - } - - #[test] - fn transactions_hash_filter_to_prove_apply_with_limit_on_transactions_hashes_number() { - let transactions_hashes = vec!["a".repeat(64), "b".repeat(64), "c".repeat(64)]; - let filter = TransactionsHashFilterToProve::new(2); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); - } - - #[test] - fn transactions_hash_filter_to_prove_apply_with_limit_on_transactions_hashes_number_only_on_valid_hashes( - ) { - let transactions_hashes = vec![ - "zz".to_string(), - "a".repeat(64), - "xx".to_string(), - "b".repeat(64), - "c".repeat(64), - ]; - let filter = TransactionsHashFilterToProve::new(2); - - let valid_hashes = filter.apply(&transactions_hashes); - - assert_equivalent(vec!["a".repeat(64), "b".repeat(64)], valid_hashes); - } } From 9a4fc2d2f5af2d40fc3ee120cda6c6889b6c08b3 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 24 Jun 2024 17:22:25 +0200 Subject: [PATCH 12/17] Handle Cardano transactions prover capability in explorer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- .../CardanoTransactionsFormInput.test.js | 22 +++++++++++++++++-- .../CardanoTransactionsSnapshotsList/index.js | 15 +++++++++++-- .../CardanoTransactionsFormInput.js | 7 ++++-- mithril-explorer/src/constants.js | 3 +++ mithril-explorer/src/store/settingsSlice.js | 2 ++ 5 files changed, 43 insertions(+), 6 deletions(-) diff --git a/mithril-explorer/__tests__/CardanoTransactionsFormInput.test.js b/mithril-explorer/__tests__/CardanoTransactionsFormInput.test.js index d70ee3502ac..16e376c7eff 100644 --- a/mithril-explorer/__tests__/CardanoTransactionsFormInput.test.js +++ b/mithril-explorer/__tests__/CardanoTransactionsFormInput.test.js @@ -2,8 +2,10 @@ import { fireEvent, render, screen } from "@testing-library/react"; import "@testing-library/jest-dom"; import CardanoTransactionsFormInput from "#/CardanoTransactionsFormInput"; -function setup() { - const utils = [render()]; +function setup(maxHashesAllowedByRequest = 100) { + const utils = [ + render(), + ]; return { input: screen.getByRole("textbox"), ...utils, @@ -83,4 +85,20 @@ describe("CardanoTransactionsFormInput", () => { expect(input.checkValidity()).toBeFalsy(); expect(input.value).toBe(transactions); }); + + it.each([ + ["Max 'one'", 1], + ["Max 'zero' should be equivalent to max 'one'", 0], + ["Max 'two'", 2], + ])("Setting more than max hashes allowed by request: %s", (_, maxHashesAllowedByRequest) => { + const { input } = setup(maxHashesAllowedByRequest); + const transactions = + "c44d1f8df92bc75fe74d69b236b1c0ebcab1e3c350ee5ac70e8a8ef53c1e5918,b45d2f9df92bc75fe74e69b236b1c0ebcab1e3c350ee5ac71e8a8ef53c1e5918,a60d2f9df92bc75fe74e69b224b1c0bacab1e3c350ee5ac71e8a8ef65c1e5918"; + fireEvent.change(input, { + target: { value: transactions }, + }); + + expect(input.checkValidity()).toBeFalsy(); + expect(input.value).toBe(transactions); + }); }); diff --git a/mithril-explorer/src/components/Artifacts/CardanoTransactionsSnapshotsList/index.js b/mithril-explorer/src/components/Artifacts/CardanoTransactionsSnapshotsList/index.js index 183dacf8135..deafd4dce8a 100644 --- a/mithril-explorer/src/components/Artifacts/CardanoTransactionsSnapshotsList/index.js +++ b/mithril-explorer/src/components/Artifacts/CardanoTransactionsSnapshotsList/index.js @@ -6,7 +6,8 @@ import LocalDateTime from "#/LocalDateTime"; import CardanoTransactionsFormInput from "#/CardanoTransactionsFormInput"; import CertificateModal from "#/CertificateModal"; import CertifyCardanoTransactionsModal from "#/CertifyCardanoTransactionsModal"; -import { selectedAggregator } from "@/store/settingsSlice"; +import { defaultAggregatorCapabilities } from "@/constants"; +import { selectedAggregator, selectedAggregatorCapabilities } from "@/store/settingsSlice"; export default function CardanoTransactionsSnapshotsList(props) { const [cardanoTransactionsSnapshots, setCardanoTransactionsSnapshots] = useState([]); @@ -19,6 +20,9 @@ export default function CardanoTransactionsSnapshotsList(props) { ); const autoUpdate = useSelector((state) => state.settings.autoUpdate); const updateInterval = useSelector((state) => state.settings.updateInterval); + const currentAggregatorCapabilities = useSelector((state) => + selectedAggregatorCapabilities(state), + ); useEffect(() => { if (!autoUpdate) { @@ -96,7 +100,14 @@ export default function CardanoTransactionsSnapshotsList(props) { noValidate validated={showCertificationFormValidation}> - + diff --git a/mithril-explorer/src/components/CardanoTransactionsFormInput.js b/mithril-explorer/src/components/CardanoTransactionsFormInput.js index 725fdfda940..23b1816e2c7 100644 --- a/mithril-explorer/src/components/CardanoTransactionsFormInput.js +++ b/mithril-explorer/src/components/CardanoTransactionsFormInput.js @@ -1,7 +1,10 @@ import { Button, Form, FormGroup, InputGroup } from "react-bootstrap"; import React from "react"; -export default function CardanoTransactionsFormInput() { +export default function CardanoTransactionsFormInput({ maxAllowedHashesByRequest }) { + const maxHashesAllowed = Math.max(maxAllowedHashesByRequest, 1); + const validationPattern = ` *(\\w+ *, *){0,${maxHashesAllowed - 1}}\\w+,? *`; + return ( @@ -11,7 +14,7 @@ export default function CardanoTransactionsFormInput() { type="text" placeholder="comma-separated list of transactions hashes" required - pattern=" *(\w+ *, *)*\w+,? *" + pattern={validationPattern} /> Please provide a comma-separated list of transactions hashes. diff --git a/mithril-explorer/src/constants.js b/mithril-explorer/src/constants.js index a9ab4660cbc..bb723a91ab0 100644 --- a/mithril-explorer/src/constants.js +++ b/mithril-explorer/src/constants.js @@ -9,4 +9,7 @@ export const signedEntityType = { export const defaultAggregatorCapabilities = { signed_entity_types: [], + cardano_transactions_prover: { + max_hashes_allowed_by_request: 100, + }, }; diff --git a/mithril-explorer/src/store/settingsSlice.js b/mithril-explorer/src/store/settingsSlice.js index 4476a1af772..99cb6e8a22b 100644 --- a/mithril-explorer/src/store/settingsSlice.js +++ b/mithril-explorer/src/store/settingsSlice.js @@ -87,6 +87,8 @@ export const { } = settingsSlice.actions; export const selectedAggregator = (state) => state.settings.selectedAggregator; +export const selectedAggregatorCapabilities = (state) => + state.settings?.aggregatorCapabilities ?? defaultAggregatorCapabilities; export const selectedAggregatorSignedEntities = (state) => state.settings?.aggregatorCapabilities?.signed_entity_types ?? []; From 845a532bdd061a51f0d83fba58736a9dc05bc54c Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 25 Jun 2024 10:44:29 +0200 Subject: [PATCH 13/17] Fix wrong key for `cardano_transactions_database_connection_pool_size` parameter to build the default configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-aggregator/src/configuration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 7446fbfdd55..95bd460cd46 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -471,7 +471,7 @@ impl Source for DefaultConfiguration { into_value(myself.cardano_transactions_prover_cache_pool_size), ); result.insert( - "cardano_transactions_prover_cache_pool_size".to_string(), + "cardano_transactions_database_connection_pool_size".to_string(), into_value(myself.cardano_transactions_database_connection_pool_size), ); result.insert( From 8316a800da869a3b4d13179995483c1f587e61a8 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:35:32 +0200 Subject: [PATCH 14/17] Refacto validator error messages and generic types in `ClientError` constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- .../prover_transactions_hash_validator.rs | 16 ++++++++-------- mithril-common/src/entities/http_server_error.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs index c8c03b2c204..d4f84d6f305 100644 --- a/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs +++ b/mithril-aggregator/src/http_server/validators/prover_transactions_hash_validator.rs @@ -16,7 +16,7 @@ impl ProverTransactionsHashValidator { return Err(ClientError::new( Self::LABEL, format!( - "Transaction hashes list contains more than maximum allowed hashes: '{}'", + "Transaction hashes list contains more than maximum allowed number of hashes: '{}'", self.max_hashes ), )); @@ -26,21 +26,21 @@ impl ProverTransactionsHashValidator { if hash.is_empty() { return Err(ClientError::new( Self::LABEL, - "Transaction hashes cannot be empty", + "Transaction hash cannot be empty", )); } if hash.chars().count() != 64 { return Err(ClientError::new( Self::LABEL, - "Transaction hashes must have 64 characters", + "Transaction hash must have 64 characters", )); } if !hash.chars().all(|c| c.is_ascii_hexdigit()) { return Err(ClientError::new( Self::LABEL, - "Transaction hashes must contain only hexadecimal characters", + "Transaction hash must contain only hexadecimal characters", )); } } @@ -70,7 +70,7 @@ mod tests { error, ClientError::new( "invalid_transaction_hashes", - "Transaction hashes cannot be empty" + "Transaction hash cannot be empty" ) ); } @@ -85,7 +85,7 @@ mod tests { error, ClientError::new( "invalid_transaction_hashes", - "Transaction hashes must have 64 characters" + "Transaction hash must have 64 characters" ) ); } @@ -102,7 +102,7 @@ mod tests { error, ClientError::new( "invalid_transaction_hashes", - "Transaction hashes must contain only hexadecimal characters" + "Transaction hash must contain only hexadecimal characters" ), "Invalid hash: {}", hash @@ -131,7 +131,7 @@ mod tests { ClientError::new( "invalid_transaction_hashes", format!( - "Transaction hashes list contains more than maximum allowed hashes: '{}'", + "Transaction hashes list contains more than maximum allowed number of hashes: '{}'", validator.max_hashes ) ) diff --git a/mithril-common/src/entities/http_server_error.rs b/mithril-common/src/entities/http_server_error.rs index 4fff1b31eb6..7f62db87ce4 100644 --- a/mithril-common/src/entities/http_server_error.rs +++ b/mithril-common/src/entities/http_server_error.rs @@ -45,7 +45,7 @@ pub struct ClientError { impl ClientError { /// ClientError factory - pub fn new, T2: Into>(label: T1, message: T2) -> ClientError { + pub fn new, M: Into>(label: L, message: M) -> ClientError { ClientError { label: label.into(), message: message.into(), From 7a9e6ea17c70f2935867749cec8e240d6133ec3c Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:23:11 +0200 Subject: [PATCH 15/17] Move deduplicate responsability to `CardanoTransactionProofQueryParams` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- .../src/http_server/routes/proof_routes.rs | 68 ++++++++++--------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/proof_routes.rs b/mithril-aggregator/src/http_server/routes/proof_routes.rs index 4921883c4fe..bb033b8ac3f 100644 --- a/mithril-aggregator/src/http_server/routes/proof_routes.rs +++ b/mithril-aggregator/src/http_server/routes/proof_routes.rs @@ -11,8 +11,18 @@ struct CardanoTransactionProofQueryParams { } impl CardanoTransactionProofQueryParams { - pub fn split_transactions_hashes(&self) -> Vec<&str> { - self.transaction_hashes.split(',').collect() + pub fn split_transactions_hashes(&self) -> Vec { + self.transaction_hashes + .split(',') + .map(|s| s.to_string()) + .collect() + } + + pub fn sanitize(&self) -> Vec { + let mut transaction_hashes = self.split_transactions_hashes(); + transaction_hashes.sort(); + transaction_hashes.dedup(); + transaction_hashes } } @@ -66,11 +76,7 @@ mod handlers { validator: ProverTransactionsHashValidator, prover_service: Arc, ) -> Result { - let mut transaction_hashes = transaction_parameters - .split_transactions_hashes() - .iter() - .map(|s| s.to_string()) - .collect::>(); + let transaction_hashes = transaction_parameters.split_transactions_hashes(); debug!( "⇄ HTTP SERVER: proof_cardano_transaction?transaction_hashes={}", transaction_parameters.transaction_hashes @@ -81,8 +87,7 @@ mod handlers { return Ok(reply::bad_request(error.label, error.message)); } - transaction_hashes.sort(); - transaction_hashes.dedup(); + let sanitized_hashes = transaction_parameters.sanitize(); match unwrap_to_internal_server_error!( signed_entity_service @@ -92,7 +97,7 @@ mod handlers { ) { Some(signed_entity) => { let message = unwrap_to_internal_server_error!( - build_response_message(prover_service, signed_entity, transaction_hashes).await, + build_response_message(prover_service, signed_entity, sanitized_hashes).await, "proof_cardano_transaction" ); Ok(reply::json(&message, StatusCode::OK)) @@ -137,7 +142,7 @@ mod tests { use mithril_common::{ entities::{CardanoTransactionsSetProof, CardanoTransactionsSnapshot, SignedEntity}, - test_utils::{apispec::APISpec, fake_data}, + test_utils::{apispec::APISpec, assert_equivalent, fake_data}, }; use crate::services::MockSignedEntityService; @@ -330,6 +335,7 @@ mod tests { #[tokio::test] async fn proof_cardano_transaction_route_deduplicate_hashes() { + let tx = fake_data::transaction_hashes()[0].to_string(); let config = Configuration::new_sample(); let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); @@ -340,42 +346,38 @@ mod tests { dependency_manager.signed_entity_service = Arc::new(mock_signed_entity_service); let mut mock_prover_service = MockProverService::new(); + let txs_expected = vec![tx.clone()]; mock_prover_service .expect_compute_transactions_proofs() - .withf(|_, transaction_hashes| { - transaction_hashes.len() == 2 - && transaction_hashes.contains(&fake_data::transaction_hashes()[0].to_string()) - && transaction_hashes.contains(&fake_data::transaction_hashes()[1].to_string()) - }) + .withf(move |_, transaction_hashes| transaction_hashes == txs_expected) .returning(|_, _| Ok(vec![CardanoTransactionsSetProof::dummy()])); dependency_manager.prover_service = Arc::new(mock_prover_service); let method = Method::GET.as_str(); let path = "/proof/cardano-transaction"; - // We are testing on an unordered list of transaction hashes let response = request() .method(method) .path(&format!( - "/{SERVER_BASE_PATH}{path}?transaction_hashes={},{},{},{},{}", - fake_data::transaction_hashes()[0], - fake_data::transaction_hashes()[1], - fake_data::transaction_hashes()[1], - fake_data::transaction_hashes()[0], - fake_data::transaction_hashes()[1] + "/{SERVER_BASE_PATH}{path}?transaction_hashes={tx},{tx}", )) .reply(&setup_router(Arc::new(dependency_manager))) .await; - APISpec::verify_conformity( - APISpec::get_all_spec_files(), - method, - path, - "application/json", - &Null, - &response, - &StatusCode::OK, - ) - .unwrap(); + assert_eq!(StatusCode::OK, response.status()); + } + + #[test] + fn sanitize_cardano_transaction_proof_query_params_remove_duplicate() { + let tx1 = fake_data::transaction_hashes()[0].to_string(); + let tx2 = fake_data::transaction_hashes()[1].to_string(); + + // We are testing on an unordered list of transaction hashes + // as some rust dedup methods only remove consecutive duplicates + let params = CardanoTransactionProofQueryParams { + transaction_hashes: format!("{tx1},{tx2},{tx2},{tx1},{tx2}",), + }; + + assert_equivalent(params.sanitize(), vec![tx1, tx2]); } } From 20c74ffcee591a9fb8da01638783868df4fabbdb Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:31:35 +0200 Subject: [PATCH 16/17] Bump `openapi.yml` and `mithril-explorer` versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- mithril-explorer/package-lock.json | 4 ++-- mithril-explorer/package.json | 2 +- openapi.yaml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mithril-explorer/package-lock.json b/mithril-explorer/package-lock.json index e8d3554a88a..03e8fe828ad 100644 --- a/mithril-explorer/package-lock.json +++ b/mithril-explorer/package-lock.json @@ -1,12 +1,12 @@ { "name": "mithril-explorer", - "version": "0.7.1", + "version": "0.7.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "mithril-explorer", - "version": "0.7.1", + "version": "0.7.2", "dependencies": { "@mithril-dev/mithril-client-wasm": "file:../mithril-client-wasm/pkg", "@popperjs/core": "^2.11.8", diff --git a/mithril-explorer/package.json b/mithril-explorer/package.json index 20b88b2a544..6181025a715 100644 --- a/mithril-explorer/package.json +++ b/mithril-explorer/package.json @@ -1,6 +1,6 @@ { "name": "mithril-explorer", - "version": "0.7.1", + "version": "0.7.2", "private": true, "scripts": { "dev": "next dev", diff --git a/openapi.yaml b/openapi.yaml index 0a6e2c1a38e..9d27f2fbd94 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -4,7 +4,7 @@ info: # `mithril-common/src/lib.rs` file. If you plan to update it # here to reflect changes in the API, please also update the constant in the # Rust file. - version: 0.1.23 + version: 0.1.24 title: Mithril Aggregator Server description: | The REST API provided by a Mithril Aggregator Node in a Mithril network. From 9f634909d873fffac90dc666f66d67e87231710b Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:33:49 +0200 Subject: [PATCH 17/17] Bump crates versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DJO Co-authored-by: Sébastien Fauvel --- Cargo.lock | 4 ++-- mithril-aggregator/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ae53c86789..75104cb53ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3538,7 +3538,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.5.25" +version = "0.5.26" dependencies = [ "anyhow", "async-trait", @@ -3694,7 +3694,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.4.19" +version = "0.4.20" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 14faa18c52e..917c116e190 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.5.25" +version = "0.5.26" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index c850f632a86..4c22406dfe2 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.4.19" +version = "0.4.20" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true }