Skip to content

Commit a869994

Browse files
sfauvelsfauvel
committed
Address PR review comments
1 parent a0bbfb5 commit a869994

File tree

3 files changed

+35
-60
lines changed

3 files changed

+35
-60
lines changed

‎mithril-aggregator/src/database/provider/cardano_transaction.rs

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ impl SqLiteEntity for CardanoTransactionRecord {
6666
where
6767
Self: Sized,
6868
{
69-
// TODO generalize this method
69+
// TODO: generalize this method to other hydrator
7070
fn try_to_u64(field: &str, value: i64) -> Result<u64, HydrationError> {
7171
u64::try_from(value)
7272
.map_err(|e| HydrationError::InvalidData(format!("Integer field cardano_tx.{field} (value={value}) is incompatible with u64 representation. Error = {e}")))
@@ -167,31 +167,29 @@ impl<'client> InsertCardanoTransactionProvider<'client> {
167167
&self,
168168
transactions_records: Vec<CardanoTransactionRecord>,
169169
) -> StdResult<WhereCondition> {
170-
fn map_record(record: CardanoTransactionRecord) -> StdResult<Vec<Value>> {
171-
Ok(vec![
172-
Value::String(record.transaction_hash),
173-
Value::Integer(record.block_number.try_into().unwrap()),
174-
Value::Integer(record.slot_number.try_into()?),
175-
Value::String(record.block_hash.clone()),
176-
Value::Integer(record.immutable_file_number.try_into().unwrap()),
177-
])
178-
}
179-
180170
let columns =
181171
"(transaction_hash, block_number, slot_number, block_hash, immutable_file_number)";
182172
let values_columns: Vec<&str> = repeat("(?*, ?*, ?*, ?*, ?*)")
183173
.take(transactions_records.len())
184174
.collect();
185175

186-
// TODO see if we can find another way to do it
187-
let values: StdResult<Vec<Vec<Value>>> =
188-
transactions_records.into_iter().map(map_record).collect();
189-
190-
let values: Vec<Value> = values?.into_iter().flatten().collect();
176+
let values: StdResult<Vec<Value>> =
177+
transactions_records
178+
.into_iter()
179+
.try_fold(vec![], |mut vec, record| {
180+
vec.append(&mut vec![
181+
Value::String(record.transaction_hash),
182+
Value::Integer(record.block_number.try_into()?),
183+
Value::Integer(record.slot_number.try_into()?),
184+
Value::String(record.block_hash.clone()),
185+
Value::Integer(record.immutable_file_number.try_into()?),
186+
]);
187+
Ok(vec)
188+
});
191189

192190
Ok(WhereCondition::new(
193191
format!("{columns} values {}", values_columns.join(", ")).as_str(),
194-
values,
192+
values?,
195193
))
196194
}
197195
}
@@ -336,7 +334,6 @@ mod tests {
336334
.unwrap()
337335
}
338336

339-
// TODO Is it really a useful test ?
340337
#[test]
341338
fn cardano_transaction_projection() {
342339
let projection = CardanoTransactionRecord::get_projection();
@@ -398,7 +395,6 @@ mod tests {
398395
.unwrap()
399396
.expand();
400397

401-
// TODO Why this assert ?
402398
assert_eq!(
403399
"(transaction_hash, block_number, slot_number, block_hash, immutable_file_number) values (?1, ?2, ?3, ?4, ?5)"
404400
.to_string(),
@@ -418,7 +414,6 @@ mod tests {
418414
);
419415
}
420416

421-
// TODO: Is it useful ?
422417
#[test]
423418
fn insert_provider_many_condition() {
424419
let connection = Connection::open_thread_safe(":memory:").unwrap();
@@ -477,10 +472,7 @@ mod tests {
477472
.create_transaction("tx-hash-456", 11, 51, "block_hash-456", 100)
478473
.await
479474
.unwrap();
480-
let transaction_result = repository
481-
.get_transaction(&"tx-hash-123".to_string())
482-
.await
483-
.unwrap();
475+
let transaction_result = repository.get_transaction("tx-hash-123").await.unwrap();
484476

485477
assert_eq!(
486478
Some(CardanoTransactionRecord {
@@ -650,10 +642,7 @@ mod tests {
650642
.await
651643
.unwrap();
652644

653-
let transaction_result = repository
654-
.get_transaction(&"tx-hash-000".to_string())
655-
.await
656-
.unwrap();
645+
let transaction_result = repository.get_transaction("tx-hash-000").await.unwrap();
657646

658647
assert_eq!(
659648
Some(CardanoTransactionRecord {

‎mithril-common/src/signable_builder/cardano_transactions.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ mod tests {
219219
}
220220

221221
{
222-
// In a same block range Transactions in a different order returns a different merkle root.
222+
// In a same block range Transactions in a different order return a different merkle root.
223223
let transactions_set = vec![
224224
transaction_1.clone(),
225225
transaction_3.clone(),
@@ -250,9 +250,6 @@ mod tests {
250250
.compute_merkle_root(&[transaction_1.clone(), transaction_2.clone()])
251251
.unwrap();
252252

253-
// Transactions in two different block range compute the same merkle root as long as their
254-
// order in their block range is the same but the order of appearance of the block range
255-
// doesn't matter.
256253
let mk_root = cardano_transaction_signable_builder
257254
.compute_merkle_root(&[transaction_2.clone(), transaction_1.clone()])
258255
.unwrap();

‎mithril-signer/src/database/provider/cardano_transaction.rs

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl SqLiteEntity for CardanoTransactionRecord {
6464
where
6565
Self: Sized,
6666
{
67-
// TODO generalize this method
67+
// TODO: generalize this method to other hydrator
6868
fn try_to_u64(field: &str, value: i64) -> Result<u64, HydrationError> {
6969
u64::try_from(value)
7070
.map_err(|e| HydrationError::InvalidData(format!("Integer field cardano_tx.{field} (value={value}) is incompatible with u64 representation. Error = {e}")))
@@ -165,31 +165,29 @@ impl<'client> InsertCardanoTransactionProvider<'client> {
165165
&self,
166166
transactions_records: Vec<CardanoTransactionRecord>,
167167
) -> StdResult<WhereCondition> {
168-
fn map_record(record: CardanoTransactionRecord) -> StdResult<Vec<Value>> {
169-
Ok(vec![
170-
Value::String(record.transaction_hash),
171-
Value::Integer(record.block_number.try_into().unwrap()),
172-
Value::Integer(record.slot_number.try_into()?),
173-
Value::String(record.block_hash.clone()),
174-
Value::Integer(record.immutable_file_number.try_into().unwrap()),
175-
])
176-
}
177-
178168
let columns =
179169
"(transaction_hash, block_number, slot_number, block_hash, immutable_file_number)";
180170
let values_columns: Vec<&str> = repeat("(?*, ?*, ?*, ?*, ?*)")
181171
.take(transactions_records.len())
182172
.collect();
183173

184-
// TODO see if we can find another way to do it
185-
let values: StdResult<Vec<Vec<Value>>> =
186-
transactions_records.into_iter().map(map_record).collect();
187-
188-
let values: Vec<Value> = values?.into_iter().flatten().collect();
174+
let values: StdResult<Vec<Value>> =
175+
transactions_records
176+
.into_iter()
177+
.try_fold(vec![], |mut vec, record| {
178+
vec.append(&mut vec![
179+
Value::String(record.transaction_hash),
180+
Value::Integer(record.block_number.try_into()?),
181+
Value::Integer(record.slot_number.try_into()?),
182+
Value::String(record.block_hash.clone()),
183+
Value::Integer(record.immutable_file_number.try_into()?),
184+
]);
185+
Ok(vec)
186+
});
189187

190188
Ok(WhereCondition::new(
191189
format!("{columns} values {}", values_columns.join(", ")).as_str(),
192-
values,
190+
values?,
193191
))
194192
}
195193
}
@@ -327,7 +325,6 @@ mod tests {
327325
.unwrap()
328326
}
329327

330-
// TODO Is it really a useful test ?
331328
#[test]
332329
fn cardano_transaction_projection() {
333330
let projection = CardanoTransactionRecord::get_projection();
@@ -389,7 +386,6 @@ mod tests {
389386
.unwrap()
390387
.expand();
391388

392-
// TODO Why this assert ?
393389
assert_eq!(
394390
"(transaction_hash, block_number, slot_number, block_hash, immutable_file_number) values (?1, ?2, ?3, ?4, ?5)"
395391
.to_string(),
@@ -409,7 +405,6 @@ mod tests {
409405
);
410406
}
411407

412-
// TODO: Is it useful ?
413408
#[test]
414409
fn insert_provider_many_condition() {
415410
let connection = Connection::open_thread_safe(":memory:").unwrap();
@@ -468,10 +463,7 @@ mod tests {
468463
.create_transaction("tx-hash-456", 11, 51, "block_hash-456", 100)
469464
.await
470465
.unwrap();
471-
let transaction_result = repository
472-
.get_transaction(&"tx-hash-123".to_string())
473-
.await
474-
.unwrap();
466+
let transaction_result = repository.get_transaction("tx-hash-123").await.unwrap();
475467

476468
assert_eq!(
477469
Some(CardanoTransactionRecord {
@@ -597,10 +589,7 @@ mod tests {
597589
.await
598590
.unwrap();
599591

600-
let transaction_result = repository
601-
.get_transaction(&"tx-hash-000".to_string())
602-
.await
603-
.unwrap();
592+
let transaction_result = repository.get_transaction("tx-hash-000").await.unwrap();
604593

605594
assert_eq!(
606595
Some(CardanoTransactionRecord {

0 commit comments

Comments
 (0)