Skip to content

Commit

Permalink
Merge pull request #1795 from input-output-hk/djo/1785/last_tx_not_pr…
Browse files Browse the repository at this point in the history
…oved

Bugfix: Can't generate proof for transactions on the last signed block number
  • Loading branch information
Alenar authored Jul 4, 2024
2 parents f10e4eb + 10e2a20 commit f04a629
Show file tree
Hide file tree
Showing 18 changed files with 515 additions and 238 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion internal/mithril-persistence/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-persistence"
version = "0.2.14"
version = "0.2.15"
description = "Common types, interfaces, and utilities to persist data for Mithril nodes."
authors = { workspace = true }
edition = { workspace = true }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,13 @@ mod tests {
use mithril_common::crypto_helper::MKTreeNode;
use mithril_common::entities::BlockRange;

use crate::database::query::{GetBlockRangeRootQuery, InsertBlockRangeRootQuery};
use crate::database::query::block_range_root::test_helper::insert_block_range_roots;
use crate::database::query::GetBlockRangeRootQuery;
use crate::database::test_helper::cardano_tx_db_connection;
use crate::sqlite::{ConnectionExtensions, SqliteConnection};
use crate::sqlite::ConnectionExtensions;

use super::*;

fn insert_block_range_roots(connection: &SqliteConnection, records: Vec<BlockRangeRootRecord>) {
connection
.fetch_first(InsertBlockRangeRootQuery::insert_many(records).unwrap())
.unwrap();
}

fn block_range_root_dataset() -> Vec<BlockRangeRootRecord> {
[
(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use mithril_common::entities::BlockNumber;
use sqlite::Value;

use mithril_common::entities::BlockNumber;

use crate::database::record::BlockRangeRootRecord;
use crate::sqlite::{Query, SourceAlias, SqLiteEntity, WhereCondition};

Expand All @@ -16,12 +17,9 @@ impl GetBlockRangeRootQuery {
}
}

pub fn up_to_block_number(up_to_or_equal_end_block_number: BlockNumber) -> Self {
pub fn contains_or_below_block_number(block_number: BlockNumber) -> Self {
Self {
condition: WhereCondition::new(
"end <= ?*",
vec![Value::Integer(up_to_or_equal_end_block_number as i64)],
),
condition: WhereCondition::new("start < ?*", vec![Value::Integer(block_number as i64)]),
}
}
}
Expand All @@ -40,3 +38,100 @@ impl Query for GetBlockRangeRootQuery {
format!("select {projection} from block_range_root where {condition} order by start, end")
}
}

#[cfg(test)]
mod tests {
use mithril_common::crypto_helper::MKTreeNode;
use mithril_common::entities::BlockRange;

use crate::database::query::block_range_root::test_helper::insert_block_range_roots;
use crate::database::query::GetBlockRangeRootQuery;
use crate::database::test_helper::cardano_tx_db_connection;
use crate::sqlite::ConnectionExtensions;

use super::*;

fn block_range_root_dataset() -> Vec<BlockRangeRootRecord> {
[
(
BlockRange::from_block_number(15),
MKTreeNode::from_hex("AAAA").unwrap(),
),
(
BlockRange::from_block_number(30),
MKTreeNode::from_hex("BBBB").unwrap(),
),
(
BlockRange::from_block_number(45),
MKTreeNode::from_hex("CCCC").unwrap(),
),
]
.into_iter()
.map(BlockRangeRootRecord::from)
.collect()
}

#[test]
fn test_get_contains_or_below_block_number_with_empty_db() {
let connection = cardano_tx_db_connection().unwrap();

let cursor: Vec<BlockRangeRootRecord> = connection
.fetch_collect(GetBlockRangeRootQuery::contains_or_below_block_number(100))
.unwrap();
assert_eq!(Vec::<BlockRangeRootRecord>::new(), cursor);
}

#[test]
fn test_get_contains_or_below_block_number_higher_than_the_highest_stored_block_range() {
let connection = cardano_tx_db_connection().unwrap();
let dataset = block_range_root_dataset();
insert_block_range_roots(&connection, dataset.clone());

let cursor: Vec<BlockRangeRootRecord> = connection
.fetch_collect(GetBlockRangeRootQuery::contains_or_below_block_number(
10_000,
))
.unwrap();

assert_eq!(dataset, cursor);
}

#[test]
fn test_get_contains_or_below_block_number_below_end_of_the_third_block_range() {
let connection = cardano_tx_db_connection().unwrap();
let dataset = block_range_root_dataset();
insert_block_range_roots(&connection, dataset.clone());

let cursor: Vec<BlockRangeRootRecord> = connection
.fetch_collect(GetBlockRangeRootQuery::contains_or_below_block_number(44))
.unwrap();

assert_eq!(&dataset[0..2], &cursor);
}

#[test]
fn test_get_contains_or_below_block_number_equal_to_end_of_the_third_block_range() {
let connection = cardano_tx_db_connection().unwrap();
let dataset = block_range_root_dataset();
insert_block_range_roots(&connection, dataset.clone());

let cursor: Vec<BlockRangeRootRecord> = connection
.fetch_collect(GetBlockRangeRootQuery::contains_or_below_block_number(45))
.unwrap();

assert_eq!(&dataset[0..2], &cursor);
}

#[test]
fn test_get_contains_or_below_block_number_after_end_of_the_third_block_range() {
let connection = cardano_tx_db_connection().unwrap();
let dataset = block_range_root_dataset();
insert_block_range_roots(&connection, dataset.clone());

let cursor: Vec<BlockRangeRootRecord> = connection
.fetch_collect(GetBlockRangeRootQuery::contains_or_below_block_number(46))
.unwrap();

assert_eq!(dataset, cursor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,20 @@ pub use delete_block_range_root::*;
pub use get_block_range_root::*;
pub use get_interval_without_block_range::*;
pub use insert_block_range::*;

#[cfg(test)]
mod test_helper {
use crate::database::record::BlockRangeRootRecord;
use crate::sqlite::{ConnectionExtensions, SqliteConnection};

use super::*;

pub fn insert_block_range_roots(
connection: &SqliteConnection,
records: Vec<BlockRangeRootRecord>,
) {
connection
.fetch_first(InsertBlockRangeRootQuery::insert_many(records).unwrap())
.unwrap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,17 @@ impl CardanoTransactionRepository {
)
}

/// Retrieve all the Block Range Roots in database up to the given end block number included.
/// Retrieve all the Block Range Roots in database up to the block range that contains the given
/// block number.
pub async fn retrieve_block_range_roots_up_to(
&self,
up_to_or_equal_end_block_number: BlockNumber,
block_number: BlockNumber,
) -> StdResult<Box<dyn Iterator<Item = (BlockRange, MKTreeNode)> + '_>> {
let block_range_roots = self
.connection_pool
.connection()?
.fetch(GetBlockRangeRootQuery::up_to_block_number(
up_to_or_equal_end_block_number,
.fetch(GetBlockRangeRootQuery::contains_or_below_block_number(
block_number,
))?
.map(|record| -> (BlockRange, MKTreeNode) { record.into() })
.collect::<Vec<_>>(); // TODO: remove this collect to return the iterator directly
Expand Down Expand Up @@ -325,13 +326,9 @@ impl CardanoTransactionRepository {
impl BlockRangeRootRetriever for CardanoTransactionRepository {
async fn retrieve_block_range_roots<'a>(
&'a self,
up_to_or_equal_beacon: BlockNumber,
up_to_beacon: BlockNumber,
) -> StdResult<Box<dyn Iterator<Item = (BlockRange, MKTreeNode)> + 'a>> {
let iterator = self
.retrieve_block_range_roots_up_to(up_to_or_equal_beacon)
.await?;

Ok(Box::new(iterator))
self.retrieve_block_range_roots_up_to(up_to_beacon).await
}
}

Expand Down Expand Up @@ -973,50 +970,14 @@ mod tests {
.await
.unwrap();

// Retrieve with a block far higher than the highest block range - should return all
{
let retrieved_block_ranges = repository
.retrieve_block_range_roots_up_to(1000)
.await
.unwrap();
assert_eq!(
block_range_roots,
retrieved_block_ranges.collect::<Vec<_>>()
);
}
// Retrieve with a block bellow than the smallest block range - should return none
{
let retrieved_block_ranges = repository
.retrieve_block_range_roots_up_to(2)
.await
.unwrap();
assert_eq!(
Vec::<(BlockRange, MKTreeNode)>::new(),
retrieved_block_ranges.collect::<Vec<_>>()
);
}
// Right below the end of the second block range - should return first of the three
{
let retrieved_block_ranges = repository
.retrieve_block_range_roots_up_to(44)
.await
.unwrap();
assert_eq!(
vec![block_range_roots[0].clone()],
retrieved_block_ranges.collect::<Vec<_>>()
);
}
// The given block is matched to the end (included) - should return the two of the three
{
let retrieved_block_ranges = repository
.retrieve_block_range_roots_up_to(45)
.await
.unwrap();
assert_eq!(
block_range_roots[0..=1].to_vec(),
retrieved_block_ranges.collect::<Vec<_>>()
);
}
let retrieved_block_ranges = repository
.retrieve_block_range_roots_up_to(45)
.await
.unwrap();
assert_eq!(
block_range_roots[0..2].to_vec(),
retrieved_block_ranges.collect::<Vec<_>>()
);
}

#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion mithril-aggregator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-aggregator"
version = "0.5.34"
version = "0.5.35"
description = "A Mithril Aggregator server"
authors = { workspace = true }
edition = { workspace = true }
Expand Down
5 changes: 3 additions & 2 deletions mithril-aggregator/src/services/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl ProverService for MithrilProverService {

// 4 - Enrich the Merkle map with the block ranges Merkle trees
for (block_range, mk_tree) in mk_trees {
mk_map.insert(block_range, mk_tree.into())?;
mk_map.replace(block_range, mk_tree.into())?;
}

// 5 - Compute the proof for all transactions
Expand All @@ -167,7 +167,8 @@ impl ProverService for MithrilProverService {
let pool_size = self.mk_map_pool.size();
info!(
self.logger,
"Prover starts computing the Merkle map pool resource of size {pool_size}"
"Prover starts computing the Merkle map pool resource of size {pool_size}";
"up_to_block_number" => up_to,
);
let mk_map_cache = self
.block_range_root_retriever
Expand Down
6 changes: 3 additions & 3 deletions mithril-aggregator/tests/create_certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ async fn create_certificate() {

comment!(
"Increase cardano chain block number to 185,
the state machine should be signing CardanoTransactions for block 180"
the state machine should be signing CardanoTransactions for block 179"
);
tester.increase_block_number(85, 185).await.unwrap();
cycle!(tester, "signing");
Expand All @@ -166,7 +166,7 @@ async fn create_certificate() {
.map(|s| s.signer_with_stake.clone().into())
.collect::<Vec<_>>(),
fixture.compute_and_encode_avk(),
SignedEntityType::CardanoTransactions(Epoch(1), 180),
SignedEntityType::CardanoTransactions(Epoch(1), 179),
ExpectedCertificate::genesis_identifier(&CardanoDbBeacon::new(
"devnet".to_string(),
1,
Expand Down Expand Up @@ -201,7 +201,7 @@ async fn create_certificate() {
.map(|s| s.signer_with_stake.clone().into())
.collect::<Vec<_>>(),
fixture.compute_and_encode_avk(),
SignedEntityType::CardanoTransactions(Epoch(1), 120),
SignedEntityType::CardanoTransactions(Epoch(1), 119),
ExpectedCertificate::genesis_identifier(&CardanoDbBeacon::new(
"devnet".to_string(),
1,
Expand Down
Loading

0 comments on commit f04a629

Please sign in to comment.