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

Implemented test for unauthenticated input notes #408

Merged
merged 14 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions crates/block-producer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ tracing-subscriber = { workspace = true }
[dev-dependencies]
figment = { version = "0.10", features = ["toml", "env", "test"] }
miden-air = { workspace = true }
miden-lib = { workspace = true, features = ["testing"] }
miden-node-test-macro = { path = "../test-macro" }
miden-objects = { workspace = true, features = ["testing"] }
miden-tx = { workspace = true, features = ["testing"] }
once_cell = { version = "1.18" }
rand_chacha = { version = "0.3", default-features = false }
tokio = { version = "1.38", features = ["test-util"] }
winterfell = { version = "0.8" }
160 changes: 151 additions & 9 deletions crates/block-producer/src/batch_builder/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ impl TransactionBatch {

// Populate batch produced nullifiers and match output notes with corresponding
// unauthenticated input notes in the same batch, which are removed from the unauthenticated
// input notes set. We also don't add nullifiers for such output notes to the produced
// nullifiers set.
// input notes set.
//
// One thing to note:
// This still allows transaction `A` to consume an unauthenticated note `x` and output note `y`
Expand All @@ -97,13 +96,13 @@ impl TransactionBatch {
continue;
}

match found_unauthenticated_notes {
Some(ref found_notes) => match found_notes.get(&input_note_header.id()) {
Some(_path) => input_note.nullifier().into(),
None => input_note.clone(),
},
None => input_note.clone(),
}
// If an unauthenticated note was found in the store, transform it to an authenticated one
// (i.e. erase additional note details except the nullifier)
found_unauthenticated_notes
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
.as_ref()
.and_then(|found_notes| found_notes.get(&input_note_header.id()))
.map(|_path| InputNoteCommitment::from(input_note.nullifier()))
.unwrap_or_else(|| input_note.clone())
},
None => input_note.clone(),
};
Expand Down Expand Up @@ -193,6 +192,7 @@ impl TransactionBatch {
}
}

#[derive(Debug)]
struct OutputNoteTracker {
output_notes: Vec<Option<OutputNote>>,
output_note_index: BTreeMap<NoteId, usize>,
Expand Down Expand Up @@ -244,3 +244,145 @@ impl OutputNoteTracker {
self.output_notes.into_iter().flatten().collect()
}
}

#[cfg(test)]
mod tests {
Comment on lines +250 to +251
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's add TESTS section separator above this line.

use super::*;
use crate::test_utils::{
mock_proven_tx,
note::{mock_note, mock_output_note, mock_unauthenticated_note_commitment},
};

#[test]
fn test_output_note_tracker_duplicate_output_notes() {
let mut txs = mock_proven_txs();

OutputNoteTracker::new(&txs).expect("Something went wrong, no error expected");
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved

let duplicate_output_note = txs[1].output_notes().get_note(1).clone();

txs.push(mock_proven_tx(
3,
vec![],
vec![duplicate_output_note.clone(), mock_output_note(8), mock_output_note(4)],
));

match OutputNoteTracker::new(&txs) {
Err(BuildBatchError::DuplicateOutputNote(note_id, _)) => {
assert_eq!(note_id, duplicate_output_note.id())
},
res => panic!("Unexpected result: {res:?}"),
}
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
fn test_output_note_tracker_remove_in_place_consumed_note() {
let txs = mock_proven_txs();
let mut tracker = OutputNoteTracker::new(&txs).unwrap();

let note_to_remove = mock_note(4);

assert!(tracker.remove_note(note_to_remove.header(), &txs).unwrap());
assert!(!tracker.remove_note(note_to_remove.header(), &txs).unwrap());

// Check that output notes are in the expected order and consumed note was removed
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(
tracker.into_notes(),
vec![
mock_output_note(2),
mock_output_note(3),
mock_output_note(6),
mock_output_note(7),
mock_output_note(8),
]
);
}

#[test]
fn test_duplicate_unauthenticated_notes() {
let mut txs = mock_proven_txs();
let duplicate_note = mock_note(5);
txs.push(mock_proven_tx(4, vec![duplicate_note.clone()], vec![mock_output_note(9)]));
match TransactionBatch::new(txs, None) {
Err(BuildBatchError::DuplicateUnauthenticatedNote(note_id, _)) => {
assert_eq!(note_id, duplicate_note.id())
},
res => panic!("Unexpected result: {res:?}"),
}
}

#[test]
fn test_consume_notes_in_place() {
let mut txs = mock_proven_txs();
let note_to_consume = mock_note(3);
txs.push(mock_proven_tx(
3,
vec![mock_note(11), note_to_consume, mock_note(13)],
vec![mock_output_note(9), mock_output_note(10)],
));

let batch = TransactionBatch::new(txs, None).unwrap();

// One of the unauthenticated notes must be removed from the batch due to the consumption
// of the corresponding output note
let expected_input_notes = vec![
mock_unauthenticated_note_commitment(1),
mock_unauthenticated_note_commitment(5),
mock_unauthenticated_note_commitment(11),
mock_unauthenticated_note_commitment(13),
];
assert_eq!(batch.input_notes.len(), expected_input_notes.len());
assert_eq!(batch.input_notes, expected_input_notes);

// One of the output notes must be removed from the batch due to the consumption
// by the corresponding unauthenticated note
let expected_output_notes = vec![
mock_output_note(2),
mock_output_note(4),
mock_output_note(6),
mock_output_note(7),
mock_output_note(8),
mock_output_note(9),
mock_output_note(10),
];
assert_eq!(batch.output_notes.len(), expected_output_notes.len());
assert_eq!(batch.output_notes, expected_output_notes);

// Ensure all nullifiers match the corresponding input notes' nullifiers
let expected_nullifiers: Vec<_> =
batch.input_notes().iter().map(InputNoteCommitment::nullifier).collect();
let actual_nullifiers: Vec<_> = batch.produced_nullifiers().collect();
assert_eq!(actual_nullifiers, expected_nullifiers);
}

#[test]
fn test_convert_unauthenticated_note_to_authenticated() {
let txs = mock_proven_txs();
let found_unauthenticated_notes =
BTreeMap::from_iter([(mock_note(5).id(), Default::default())]);
let batch = TransactionBatch::new(txs, Some(found_unauthenticated_notes)).unwrap();

let expected_input_notes =
vec![mock_unauthenticated_note_commitment(1), mock_note(5).nullifier().into()];
assert_eq!(batch.input_notes.len(), expected_input_notes.len());
assert_eq!(batch.input_notes, expected_input_notes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for the lengths to match seems a bit redundant as if the actual vectors don't match, the lengths won't match either.

}

// UTILITIES
// =============================================================================================

fn mock_proven_txs() -> Vec<ProvenTransaction> {
vec![
mock_proven_tx(
1,
vec![mock_note(1)],
vec![mock_output_note(2), mock_output_note(3), mock_output_note(4)],
),
mock_proven_tx(
2,
vec![mock_note(5)],
vec![mock_output_note(6), mock_output_note(7), mock_output_note(8)],
),
]
}
}
159 changes: 157 additions & 2 deletions crates/block-producer/src/batch_builder/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ use tokio::sync::RwLock;

use super::*;
use crate::{
block_builder::DefaultBlockBuilder,
errors::BuildBlockError,
test_utils::{MockProvenTxBuilder, MockStoreSuccessBuilder},
test_utils::{
note::mock_note, MockPrivateAccount, MockProvenTxBuilder, MockStoreSuccessBuilder,
},
};

// STRUCTS
// ================================================================================================

Expand Down Expand Up @@ -146,6 +148,159 @@ async fn test_batches_added_back_to_queue_on_block_build_failure() {
assert_eq!(internal_ready_batches.read().await.len(), 3);
}

#[tokio::test]
async fn test_batch_builder_find_dangling_notes() {
let store = Arc::new(MockStoreSuccessBuilder::from_accounts(iter::empty()).build());
let block_builder = Arc::new(BlockBuilderSuccess::default());

let batch_builder = Arc::new(DefaultBatchBuilder::new(
store,
block_builder,
DefaultBatchBuilderOptions {
block_frequency: Duration::from_millis(20),
max_batches_per_block: 2,
},
));

let note_1 = mock_note(1);
let note_2 = mock_note(2);
let tx1 = MockProvenTxBuilder::with_account_index(1)
.output_notes(vec![OutputNote::Full(note_1.clone())])
.build();
let tx2 = MockProvenTxBuilder::with_account_index(1)
.unauthenticated_notes(vec![note_1.clone()])
.output_notes(vec![OutputNote::Full(note_2.clone())])
.build();

let txs = vec![tx1, tx2];

let dangling_notes = batch_builder.find_dangling_notes(&txs).await;
assert_eq!(dangling_notes, vec![], "Note must be presented in the same batch");

batch_builder.build_batch(txs.clone()).await.unwrap();

let dangling_notes = batch_builder.find_dangling_notes(&txs).await;
assert_eq!(dangling_notes, vec![], "Note must be presented in the same batch");

let note_3 = mock_note(3);

let tx1 = MockProvenTxBuilder::with_account_index(1)
.unauthenticated_notes(vec![note_2.clone()])
.build();
let tx2 = MockProvenTxBuilder::with_account_index(1)
.unauthenticated_notes(vec![note_3.clone()])
.build();

let txs = vec![tx1, tx2];

let dangling_notes = batch_builder.find_dangling_notes(&txs).await;
assert_eq!(
dangling_notes,
vec![note_3.id()],
"Only one dangling node must be found before block is built"
);

batch_builder.try_build_block().await;

let dangling_notes = batch_builder.find_dangling_notes(&txs).await;
assert_eq!(
dangling_notes,
vec![note_2.id(), note_3.id()],
"Two dangling notes must be found after block is built"
);
}

#[tokio::test]
async fn test_block_builder_no_missing_notes() {
let account_1: MockPrivateAccount<3> = MockPrivateAccount::from(1);
let account_2: MockPrivateAccount<3> = MockPrivateAccount::from(2);
let store = Arc::new(
MockStoreSuccessBuilder::from_accounts(
[account_1, account_2].iter().map(|account| (account.id, account.states[0])),
)
.build(),
);
let block_builder = Arc::new(DefaultBlockBuilder::new(Arc::clone(&store), Arc::clone(&store)));
let batch_builder = Arc::new(DefaultBatchBuilder::new(
store,
Arc::clone(&block_builder),
DefaultBatchBuilderOptions {
block_frequency: Duration::from_millis(20),
max_batches_per_block: 2,
},
));

let note_1 = mock_note(1);
let note_2 = mock_note(2);

let tx1 = MockProvenTxBuilder::with_account_index(1)
.output_notes(vec![OutputNote::Full(note_1.clone())])
.build();

let tx2 = MockProvenTxBuilder::with_account_index(2)
.unauthenticated_notes(vec![note_1.clone()])
.output_notes(vec![OutputNote::Full(note_2.clone())])
.build();

let txs = vec![tx1, tx2];

batch_builder.build_batch(txs.clone()).await.unwrap();

let build_block_result = batch_builder
.block_builder
.build_block(&batch_builder.ready_batches.read().await)
.await;
assert_eq!(build_block_result, Ok(()));
}

#[tokio::test]
async fn test_block_builder_missing_notes() {
polydez marked this conversation as resolved.
Show resolved Hide resolved
let accounts: Vec<_> = (1..=4).map(MockPrivateAccount::<3>::from).collect();
let notes: Vec<_> = (1..=6).map(mock_note).collect();

let store = Arc::new(
MockStoreSuccessBuilder::from_accounts(
accounts.iter().map(|account| (account.id, account.states[0])),
)
.initial_notes([vec![OutputNote::Full(notes[0].clone())]].iter())
.build(),
);
let block_builder = Arc::new(DefaultBlockBuilder::new(Arc::clone(&store), Arc::clone(&store)));
let batch_builder = Arc::new(DefaultBatchBuilder::new(
store,
Arc::clone(&block_builder),
DefaultBatchBuilderOptions {
block_frequency: Duration::from_millis(20),
max_batches_per_block: 2,
},
));

let tx1 = MockProvenTxBuilder::with_account_index(1)
.output_notes(vec![OutputNote::Full(notes[1].clone())])
.build();

let tx2 = MockProvenTxBuilder::with_account_index(2)
.unauthenticated_notes(vec![notes[0].clone()])
.output_notes(vec![OutputNote::Full(notes[2].clone()), OutputNote::Full(notes[3].clone())])
.build();

let tx3 = MockProvenTxBuilder::with_account_index(3)
.unauthenticated_notes(notes.iter().skip(1).cloned().collect())
.build();

let txs = vec![tx1, tx2, tx3];

let batch = TransactionBatch::new(txs.clone(), None).unwrap();
let build_block_result = batch_builder.block_builder.build_block(&[batch]).await;
assert_eq!(
build_block_result,
Err(BuildBlockError::UnauthenticatedNotesNotFound(vec![
notes[4].id(),
notes[5].id()
]))
);
}

// HELPERS
// ================================================================================================

Expand Down
Loading
Loading