Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Can't generate proof for transactions on the last signed block number #1795

Merged
merged 9 commits into from
Jul 4, 2024
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
Loading