Skip to content

Commit

Permalink
fix: fix unit tests (#4740)
Browse files Browse the repository at this point in the history
Signed-off-by: Shanin Roman <[email protected]>
  • Loading branch information
Erigara authored Jun 20, 2024
1 parent b3c8f8e commit 230e0bb
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 64 deletions.
4 changes: 2 additions & 2 deletions cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ mod tests {
async fn iroha_should_notify_on_panic() {
let notify = Arc::new(Notify::new());
let hook = panic::take_hook();
<crate::Iroha<ToriiNotStarted>>::prepare_panic_hook(Arc::clone(&notify));
crate::Iroha::prepare_panic_hook(Arc::clone(&notify));
let waiters: Vec<_> = repeat(()).take(10).map(|_| Arc::clone(&notify)).collect();
let handles: Vec<_> = waiters.iter().map(|waiter| waiter.notified()).collect();
thread::spawn(move || {
Expand All @@ -837,7 +837,7 @@ mod tests {

let mut table = toml::Table::new();
iroha_config::base::toml::Writer::new(&mut table)
.write("chain_id", "0")
.write("chain", "0")
.write("public_key", pubkey)
.write("private_key", ExposedPrivateKey(privkey))
.write(["network", "address"], socket_addr!(127.0.0.1:1337))
Expand Down
2 changes: 1 addition & 1 deletion config/base/tests/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub mod sample_config {
.finish_with_origin();

let max_content_len = reader
.read_parameter::<u64>(["max_content_length"])
.read_parameter::<u64>(["max_content_len"])
.value_or_else(|| 1024)
.finish();

Expand Down
83 changes: 42 additions & 41 deletions core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,12 +615,15 @@ mod valid {
}

#[cfg(test)]
pub(crate) fn new_dummy() -> Self {
Self::new_dummy_and_modify_payload(|_| {})
pub(crate) fn new_dummy(leader_private_key: &PrivateKey) -> Self {
Self::new_dummy_and_modify_payload(leader_private_key, |_| {})
}

#[cfg(test)]
pub(crate) fn new_dummy_and_modify_payload(f: impl FnOnce(&mut BlockPayload)) -> Self {
pub(crate) fn new_dummy_and_modify_payload(
leader_private_key: &PrivateKey,
f: impl FnOnce(&mut BlockPayload),
) -> Self {
let mut payload = BlockPayload {
header: BlockHeader {
height: 2,
Expand All @@ -638,7 +641,7 @@ mod valid {
};
f(&mut payload);
BlockBuilder(Chained(payload))
.sign(iroha_crypto::KeyPair::random().private_key())
.sign(leader_private_key)
.unpack(|_| {})
}
}
Expand Down Expand Up @@ -671,17 +674,25 @@ mod valid {
let peers = test_peers![0, 1, 2, 3, 4, 5, 6: key_pairs_iter];
let topology = Topology::new(peers);

let mut block = ValidBlock::new_dummy();
let mut block = ValidBlock::new_dummy(key_pairs[0].private_key());
let payload = block.0.payload().clone();
key_pairs
.iter()
.map(|key_pair| {
BlockSignature(0, SignatureOf::new(key_pair.private_key(), &payload))
.enumerate()
// Include only peers in validator set
.take(topology.min_votes_for_commit())
// Skip leader since already singed
.skip(1)
.filter(|(i, _)| *i != 4) // Skip proxy tail
.map(|(i, key_pair)| {
BlockSignature(i as u64, SignatureOf::new(key_pair.private_key(), &payload))
})
.try_for_each(|signature| block.add_signature(signature, &topology))
.expect("Failed to add signatures");

assert!(block.commit(&topology).unpack(|_| {}).is_ok());
block.sign(&key_pairs[4], &topology);

let _ = block.commit(&topology).unpack(|_| {}).unwrap();
}

#[test]
Expand All @@ -693,15 +704,7 @@ mod valid {
let peers = test_peers![0,: key_pairs_iter];
let topology = Topology::new(peers);

let mut block = ValidBlock::new_dummy();
let payload = block.0.payload().clone();
key_pairs
.iter()
.map(|key_pair| {
BlockSignature(0, SignatureOf::new(key_pair.private_key(), &payload))
})
.try_for_each(|signature| block.add_signature(signature, &topology))
.expect("Failed to add signatures");
let block = ValidBlock::new_dummy(key_pairs[0].private_key());

assert!(block.commit(&topology).unpack(|_| {}).is_ok());
}
Expand All @@ -716,18 +719,13 @@ mod valid {
let peers = test_peers![0, 1, 2, 3, 4, 5, 6: key_pairs_iter];
let topology = Topology::new(peers);

let mut block = ValidBlock::new_dummy();
let payload = block.0.payload().clone();
let proxy_tail_signature =
BlockSignature(0, SignatureOf::new(key_pairs[4].private_key(), &payload));
block
.add_signature(proxy_tail_signature, &topology)
.expect("Failed to add signature");
let mut block = ValidBlock::new_dummy(key_pairs[0].private_key());
block.sign(&key_pairs[4], &topology);

assert_eq!(
block.commit(&topology).unpack(|_| {}).unwrap_err().1,
SignatureVerificationError::NotEnoughSignatures {
votes_count: 1,
votes_count: 2,
min_votes_for_commit: topology.min_votes_for_commit(),
}
.into()
Expand All @@ -744,14 +742,18 @@ mod valid {
let peers = test_peers![0, 1, 2, 3, 4, 5, 6: key_pairs_iter];
let topology = Topology::new(peers);

let mut block = ValidBlock::new_dummy();
let mut block = ValidBlock::new_dummy(key_pairs[0].private_key());
let payload = block.0.payload().clone();
key_pairs
.iter()
.enumerate()
// Include only peers in validator set
.take(topology.min_votes_for_commit())
// Skip leader since already singed
.skip(1)
.filter(|(i, _)| *i != 4) // Skip proxy tail
.map(|(_, key_pair)| {
BlockSignature(0, SignatureOf::new(key_pair.private_key(), &payload))
.map(|(i, key_pair)| {
BlockSignature(i as u64, SignatureOf::new(key_pair.private_key(), &payload))
})
.try_for_each(|signature| block.add_signature(signature, &topology))
.expect("Failed to add signatures");
Expand Down Expand Up @@ -916,15 +918,18 @@ mod tests {
use super::*;
use crate::{
kura::Kura, query::store::LiveQueryStore, smartcontracts::isi::Registrable as _,
state::State, tx::SignatureVerificationFail,
state::State,
};

#[test]
pub fn committed_and_valid_block_hashes_are_equal() {
let valid_block = ValidBlock::new_dummy();
let (peer_public_key, _) = KeyPair::random().into_parts();
let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), peer_public_key);
let peer_key_pair = KeyPair::random();
let peer_id = PeerId::new(
"127.0.0.1:8080".parse().unwrap(),
peer_key_pair.public_key().clone(),
);
let topology = Topology::new(vec![peer_id]);
let valid_block = ValidBlock::new_dummy(peer_key_pair.private_key());
let committed_block = valid_block
.clone()
.commit(&topology)
Expand Down Expand Up @@ -1176,15 +1181,11 @@ mod tests {

// Create genesis transaction
// Sign with `genesis_wrong_key` as peer which has incorrect genesis key pair
// Bypass `accept_genesis` check to allow signing with wrong key
let tx = TransactionBuilder::new(chain_id.clone(), genesis_wrong_account_id.clone())
.with_instructions([isi])
.sign(genesis_wrong_key.private_key());
let tx = AcceptedTransaction::accept_genesis(
iroha_genesis::GenesisTransaction(tx),
&chain_id,
&SAMPLE_GENESIS_ACCOUNT_ID,
)
.expect("Valid");
let tx = AcceptedTransaction(tx);

// Create genesis block
let transactions = vec![tx];
Expand Down Expand Up @@ -1213,8 +1214,8 @@ mod tests {
assert!(matches!(
error,
BlockValidationError::TransactionValidation(TransactionValidationError::Accept(
AcceptTransactionFail::SignatureVerification(SignatureVerificationFail { reason, .. })
)) if reason == "Signature doesn't correspond to genesis public key"
));
AcceptTransactionFail::UnexpectedGenesisAccountSignature
))
))
}
}
9 changes: 5 additions & 4 deletions core/src/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ impl<T> AddErrContextExt<T> for Result<T, std::io::Error> {
#[cfg(test)]
mod tests {

use iroha_crypto::KeyPair;
use tempfile::TempDir;

use super::*;
Expand Down Expand Up @@ -934,7 +935,7 @@ mod tests {
let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked);
block_store.create_files_if_they_do_not_exist().unwrap();

let dummy_block = ValidBlock::new_dummy().into();
let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into();

let append_count = 35;
for _ in 0..append_count {
Expand All @@ -950,7 +951,7 @@ mod tests {
let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked);
block_store.create_files_if_they_do_not_exist().unwrap();

let dummy_block = ValidBlock::new_dummy().into();
let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into();

let append_count = 35;
for _ in 0..append_count {
Expand All @@ -966,7 +967,7 @@ mod tests {
let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked);
block_store.create_files_if_they_do_not_exist().unwrap();

let dummy_block = ValidBlock::new_dummy().into();
let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into();

let append_count = 35;
for _ in 0..append_count {
Expand All @@ -986,7 +987,7 @@ mod tests {
let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked);
block_store.create_files_if_they_do_not_exist().unwrap();

let dummy_block = ValidBlock::new_dummy().into();
let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into();

let append_count = 35;
for _ in 0..append_count {
Expand Down
10 changes: 6 additions & 4 deletions core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,12 +409,12 @@ mod tests {
let mut transactions = vec![valid_tx; valid_tx_per_block];
transactions.append(&mut vec![invalid_tx; invalid_tx_per_block]);

let (peer_public_key, _) = KeyPair::random().into_parts();
let (peer_public_key, peer_private_key) = KeyPair::random().into_parts();
let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), peer_public_key);
let topology = Topology::new(vec![peer_id]);
let first_block = BlockBuilder::new(transactions.clone(), topology.clone(), Vec::new())
.chain(0, &mut state_block)
.sign(ALICE_KEYPAIR.private_key())
.sign(&peer_private_key)
.unpack(|_| {})
.commit(&topology)
.unpack(|_| {})
Expand All @@ -426,7 +426,7 @@ mod tests {
for _ in 1u64..blocks {
let block = BlockBuilder::new(transactions.clone(), topology.clone(), Vec::new())
.chain(0, &mut state_block)
.sign(ALICE_KEYPAIR.private_key())
.sign(&peer_private_key)
.unpack(|_| {})
.commit(&topology)
.unpack(|_| {})
Expand Down Expand Up @@ -481,7 +481,9 @@ mod tests {
let blocks = FindAllBlocks.execute(&state.view())?.collect::<Vec<_>>();

assert_eq!(blocks.len() as u64, num_blocks);
assert!(blocks.windows(2).all(|wnd| wnd[0] >= wnd[1]));
assert!(blocks
.windows(2)
.all(|wnd| wnd[0].header() >= wnd[1].header()));

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1837,11 +1837,11 @@ mod tests {

/// Used to inject faulty payload for testing
fn new_dummy_block_with_payload(f: impl FnOnce(&mut BlockPayload)) -> CommittedBlock {
let (leader_public_key, _) = iroha_crypto::KeyPair::random().into_parts();
let (leader_public_key, leader_private_key) = iroha_crypto::KeyPair::random().into_parts();
let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), leader_public_key);
let topology = Topology::new(vec![peer_id]);

ValidBlock::new_dummy_and_modify_payload(f)
ValidBlock::new_dummy_and_modify_payload(&leader_private_key, f)
.commit(&topology)
.unpack(|_| {})
.unwrap()
Expand Down
12 changes: 6 additions & 6 deletions core/src/sumeragi/network_topology.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ mod tests {
let peers = test_peers![0, 1, 2, 3, 4, 5, 6: key_pairs_iter];
let topology = Topology::new(peers);

let dummy_block = ValidBlock::new_dummy();
let dummy_block = ValidBlock::new_dummy(key_pairs[0].private_key());
let dummy_signature = &dummy_block.as_ref().signatures().next().unwrap().1;
let dummy_signatures = (0..key_pairs.len())
.map(|i| BlockSignature(i as u64, dummy_signature.clone()))
Expand Down Expand Up @@ -400,7 +400,7 @@ mod tests {
let peers = test_peers![0: key_pairs_iter];
let topology = Topology::new(peers);

let dummy_block = ValidBlock::new_dummy();
let dummy_block = ValidBlock::new_dummy(key_pairs[0].private_key());
let dummy_signature = &dummy_block.as_ref().signatures().next().unwrap().1;
let dummy_signatures = (0..key_pairs.len())
.map(|i| BlockSignature(i as u64, dummy_signature.clone()))
Expand Down Expand Up @@ -434,7 +434,7 @@ mod tests {
let peers = test_peers![0, 1: key_pairs_iter];
let topology = Topology::new(peers);

let dummy_block = ValidBlock::new_dummy();
let dummy_block = ValidBlock::new_dummy(key_pairs[0].private_key());
let dummy_signature = &dummy_block.as_ref().signatures().next().unwrap().1;
let dummy_signatures = (0..key_pairs.len())
.map(|i| BlockSignature(i as u64, dummy_signature.clone()))
Expand Down Expand Up @@ -470,7 +470,7 @@ mod tests {
let peers = test_peers![0, 1, 2: key_pairs_iter];
let topology = Topology::new(peers);

let dummy_block = ValidBlock::new_dummy();
let dummy_block = ValidBlock::new_dummy(key_pairs[0].private_key());
let dummy_signature = &dummy_block.as_ref().signatures().next().unwrap().1;
let dummy_signatures = (0..key_pairs.len())
.map(|i| BlockSignature(i as u64, dummy_signature.clone()))
Expand Down Expand Up @@ -742,7 +742,7 @@ mod tests {
.is_consensus_required()
.as_ref()
.map(ConsensusTopology::validating_peers),
None,
Some::<&[_]>(&[]),
);
}

Expand All @@ -756,7 +756,7 @@ mod tests {
.is_consensus_required()
.as_ref()
.map(ConsensusTopology::observing_peers),
None,
Some::<&[_]>(&[]),
);
}
}
4 changes: 4 additions & 0 deletions core/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ impl AcceptedTransaction {
}));
}

if *iroha_genesis::GENESIS_DOMAIN_ID == *tx.authority().domain() {
return Err(AcceptTransactionFail::UnexpectedGenesisAccountSignature);
}

match &tx.instructions() {
Executable::Instructions(instructions) => {
let instruction_count = instructions.len();
Expand Down
1 change: 0 additions & 1 deletion crypto/src/signature/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ mod tests {
#[test]
fn signature_serialized_representation() {
let input = json!({
"public_key": "e701210312273E8810581E58948D3FB8F9E8AD53AAA21492EBB8703915BBB565A21B7FCC",
"payload": "3A7991AF1ABB77F3FD27CC148404A6AE4439D095A63591B77C788D53F708A02A1509A611AD6D97B01D871E58ED00C8FD7C3917B6CA61A8C2833A19E000AAC2E4"
});

Expand Down
3 changes: 1 addition & 2 deletions schema/gen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ types!(
BTreeMap<AccountId, Account>,
BTreeMap<AssetDefinitionId, AssetDefinition>,
BTreeMap<AssetDefinitionId, Numeric>,
BTreeMap<AssetId, Asset>,
BTreeMap<AssetDefinitionId, Asset>,
BTreeMap<Name, MetadataValueBox>,
BTreeSet<Permission>,
BTreeSet<PermissionId>,
Expand Down Expand Up @@ -244,7 +244,6 @@ types!(
Option<BlockStatus>,
Option<DomainId>,
Option<Duration>,
Option<HashOf<MerkleTree<SignedTransaction>>>,
Option<HashOf<SignedBlock>>,
Option<HashOf<SignedTransaction>>,
Option<IpfsPath>,
Expand Down
2 changes: 1 addition & 1 deletion tools/parity_scale_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ mod tests {
is_coloring_supported()
);
assert!(try_with("--terminal-colors")?);
assert!(try_with("--terminal-colors=false")?);
assert!(!try_with("--terminal-colors=false")?);
assert!(try_with("--terminal-colors=true")?);
assert!(try_with("--terminal-colors=random").is_err());

Expand Down

0 comments on commit 230e0bb

Please sign in to comment.