Skip to content

Commit

Permalink
deprecates Hash::new in favor of Hash::new_from_array (#3617)
Browse files Browse the repository at this point in the history
Hash::new lacks type-safety and may panic.
  • Loading branch information
behzadnouri authored Nov 14, 2024
1 parent 8b720f7 commit d2299be
Show file tree
Hide file tree
Showing 29 changed files with 103 additions and 71 deletions.
2 changes: 1 addition & 1 deletion account-decoder/src/parse_sysvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ mod test {

#[test]
fn test_parse_sysvars() {
let hash = Hash::new(&[1; 32]);
let hash = Hash::new_from_array([1; 32]);

let clock_sysvar = create_account_for_test(&Clock::default());
assert_eq!(
Expand Down
7 changes: 5 additions & 2 deletions accounts-db/src/accounts_db/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ define_accounts_db_test!(test_maybe_unref_accounts_already_in_ancient, |db| {
rent_epoch: 0,
};
let offset = 3 * std::mem::size_of::<u64>();
let hash = AccountHash(Hash::new(&[2; 32]));
let hash = AccountHash(Hash::new_from_array([2; 32]));
let stored_meta = StoredMeta {
// global write version
write_version_obsolete: 0,
Expand Down Expand Up @@ -2465,7 +2465,10 @@ fn test_verify_accounts_hash() {

db.set_accounts_hash(
some_slot,
(AccountsHash(Hash::new(&[0xca; HASH_BYTES])), capitalization),
(
AccountsHash(Hash::new_from_array([0xca; HASH_BYTES])),
capitalization,
),
);

assert_matches!(
Expand Down
29 changes: 17 additions & 12 deletions accounts-db/src/accounts_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1472,7 +1472,9 @@ mod tests {
// 0 hashes
let mut file = AccountHashesFile::new(0, dir_for_temp_cache_files.path());
assert!(file.get_reader().is_none());
let hashes = (0..2).map(|i| Hash::new(&[i; 32])).collect::<Vec<_>>();
let hashes = (0..2)
.map(|i| Hash::new_from_array([i; 32]))
.collect::<Vec<_>>();

// 1 hash
let mut file = AccountHashesFile::new(1, dir_for_temp_cache_files.path());
Expand All @@ -1493,7 +1495,9 @@ mod tests {
fn test_cumulative_hashes_from_files() {
let dir_for_temp_cache_files = tempdir().unwrap();
(0..4).for_each(|permutation| {
let hashes = (0..2).map(|i| Hash::new(&[i + 1; 32])).collect::<Vec<_>>();
let hashes = (0..2)
.map(|i| Hash::new_from_array([i + 1; 32]))
.collect::<Vec<_>>();

let mut combined = Vec::default();

Expand Down Expand Up @@ -1585,7 +1589,7 @@ mod tests {
let mut account_maps = Vec::new();

let pubkey = Pubkey::from([11u8; 32]);
let hash = AccountHash(Hash::new(&[1u8; 32]));
let hash = AccountHash(Hash::new_from_array([1u8; 32]));
let val = CalculateHashIntermediate {
hash,
lamports: 88,
Expand All @@ -1595,7 +1599,7 @@ mod tests {

// 2nd key - zero lamports, so will be removed
let pubkey = Pubkey::from([12u8; 32]);
let hash = AccountHash(Hash::new(&[2u8; 32]));
let hash = AccountHash(Hash::new_from_array([2u8; 32]));
let val = CalculateHashIntermediate {
hash,
lamports: 0,
Expand All @@ -1615,7 +1619,7 @@ mod tests {

// 3rd key - with pubkey value before 1st key so it will be sorted first
let pubkey = Pubkey::from([10u8; 32]);
let hash = AccountHash(Hash::new(&[2u8; 32]));
let hash = AccountHash(Hash::new_from_array([2u8; 32]));
let val = CalculateHashIntermediate {
hash,
lamports: 20,
Expand All @@ -1633,7 +1637,7 @@ mod tests {

// 3rd key - with later slot
let pubkey = Pubkey::from([10u8; 32]);
let hash = AccountHash(Hash::new(&[99u8; 32]));
let hash = AccountHash(Hash::new_from_array([99u8; 32]));
let val = CalculateHashIntermediate {
hash,
lamports: 30,
Expand Down Expand Up @@ -1729,7 +1733,7 @@ mod tests {
let key_b = Pubkey::from([2u8; 32]);
let key_c = Pubkey::from([3u8; 32]);
const COUNT: usize = 6;
let hashes = (0..COUNT).map(|i| AccountHash(Hash::new(&[i as u8; 32])));
let hashes = (0..COUNT).map(|i| AccountHash(Hash::new_from_array([i as u8; 32])));
// create this vector
// abbbcc
let keys = [key_a, key_b, key_b, key_b, key_c, key_c];
Expand Down Expand Up @@ -2392,7 +2396,8 @@ mod tests {
let mut input: Vec<_> = (0..count)
.map(|i| {
let key = Pubkey::from([(pass * iterations + count) as u8; 32]);
let hash = Hash::new(&[(pass * iterations + count + i + 1) as u8; 32]);
let hash =
Hash::new_from_array([(pass * iterations + count + i + 1) as u8; 32]);
(key, hash)
})
.collect();
Expand Down Expand Up @@ -2436,12 +2441,12 @@ mod tests {
let offset = 2;
let input = vec![
CalculateHashIntermediate {
hash: AccountHash(Hash::new(&[1u8; 32])),
hash: AccountHash(Hash::new_from_array([1u8; 32])),
lamports: u64::MAX - offset,
pubkey: Pubkey::new_unique(),
},
CalculateHashIntermediate {
hash: AccountHash(Hash::new(&[2u8; 32])),
hash: AccountHash(Hash::new_from_array([2u8; 32])),
lamports: offset + 1,
pubkey: Pubkey::new_unique(),
},
Expand Down Expand Up @@ -2470,12 +2475,12 @@ mod tests {
let offset = 2;
let input = vec![
vec![CalculateHashIntermediate {
hash: AccountHash(Hash::new(&[1u8; 32])),
hash: AccountHash(Hash::new_from_array([1u8; 32])),
lamports: u64::MAX - offset,
pubkey: Pubkey::new_unique(),
}],
vec![CalculateHashIntermediate {
hash: AccountHash(Hash::new(&[2u8; 32])),
hash: AccountHash(Hash::new_from_array([2u8; 32])),
lamports: offset + 1,
pubkey: Pubkey::new_unique(),
}],
Expand Down
2 changes: 1 addition & 1 deletion accounts-db/src/ancient_append_vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ pub mod tests {
rent_epoch: 0,
};
let offset = 3 * std::mem::size_of::<u64>();
let hash = AccountHash(Hash::new(&[2; 32]));
let hash = AccountHash(Hash::new_from_array([2; 32]));
let stored_meta = StoredMeta {
// global write version
write_version_obsolete: 0,
Expand Down
2 changes: 1 addition & 1 deletion cli-output/src/cli_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3272,7 +3272,7 @@ mod tests {
));

let signers = vec![present.as_ref(), absent.as_ref(), bad.as_ref()];
let blockhash = Hash::new(&[7u8; 32]);
let blockhash = Hash::new_from_array([7u8; 32]);
tx.try_partial_sign(&signers, blockhash).unwrap();
let res = return_signers(&tx, &OutputFormat::JsonCompact).unwrap();
let sign_only = parse_sign_only_reply_string(&res);
Expand Down
2 changes: 1 addition & 1 deletion cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2563,7 +2563,7 @@ mod tests {
);

//Test Transfer Subcommand, offline sign
let blockhash = Hash::new(&[1u8; 32]);
let blockhash = Hash::new_from_array([1u8; 32]);
let blockhash_string = blockhash.to_string();
let test_transfer = test_commands.clone().get_matches_from(vec![
"test",
Expand Down
4 changes: 2 additions & 2 deletions cli/src/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ mod tests {
let mut nonce_account = nonce_account::create_account(1).into_inner();
assert_eq!(state_from_account(&nonce_account), Ok(State::Uninitialized));

let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]));
let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([42u8; 32]));
let data = nonce::state::Data::new(Pubkey::from([1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&Versions::new(State::Initialized(data.clone())))
Expand Down Expand Up @@ -1168,7 +1168,7 @@ mod tests {
Err(Error::InvalidStateForOperation)
);

let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]));
let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([42u8; 32]));
let data = nonce::state::Data::new(Pubkey::from([1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&Versions::new(State::Initialized(data.clone())))
Expand Down
4 changes: 2 additions & 2 deletions cli/src/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4127,7 +4127,7 @@ mod tests {
let offline_string = offline_pubkey.to_string();
let offline_sig = offline.sign_message(&[3u8]);
let offline_signer = format!("{offline_pubkey}={offline_sig}");
let nonce_hash = Hash::new(&[4u8; 32]);
let nonce_hash = Hash::new_from_array([4u8; 32]);
let nonce_hash_string = nonce_hash.to_string();
let test_create_stake_account2 = test_commands.clone().get_matches_from(vec![
"test",
Expand Down Expand Up @@ -4984,7 +4984,7 @@ mod tests {
let stake_auth_string = stake_auth_pubkey.to_string();
let stake_sig = stake_auth.sign_message(&[0u8]);
let stake_signer = format!("{stake_auth_pubkey}={stake_sig}");
let nonce_hash = Hash::new(&[4u8; 32]);
let nonce_hash = Hash::new_from_array([4u8; 32]);
let nonce_hash_string = nonce_hash.to_string();

let test_split_stake_account = test_commands.clone().get_matches_from(vec![
Expand Down
2 changes: 1 addition & 1 deletion core/src/optimistic_confirmation_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ mod test {
let snapshot_start_slot = 0;
let mut optimistic_confirmation_verifier =
OptimisticConfirmationVerifier::new(snapshot_start_slot);
let bad_bank_hash = Hash::new(&[42u8; 32]);
let bad_bank_hash = Hash::new_from_array([42u8; 32]);
let blockstore_path = get_tmp_ledger_path_auto_delete!();
let blockstore = Blockstore::open(blockstore_path.path()).unwrap();
let optimistic_slots = vec![(1, bad_bank_hash), (3, Hash::default())];
Expand Down
4 changes: 3 additions & 1 deletion entry/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,9 @@ impl EntrySlice for [Entry] {
.all(|(j, ref_entry)| {
let start = j * HASH_BYTES;
let end = start + HASH_BYTES;
let hash = Hash::new(&chunk[start..end]);
let hash = <[u8; HASH_BYTES]>::try_from(&chunk[start..end])
.map(Hash::new_from_array)
.unwrap();
compare_hashes(hash, ref_entry)
})
})
Expand Down
2 changes: 1 addition & 1 deletion faucet/src/faucet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ mod tests {
#[test]
fn test_process_faucet_request() {
let to = solana_sdk::pubkey::new_rand();
let blockhash = Hash::new(to.as_ref());
let blockhash = Hash::new_from_array(to.to_bytes());
let lamports = 50;
let req = FaucetRequest::GetAirdrop {
lamports,
Expand Down
2 changes: 1 addition & 1 deletion faucet/tests/local-faucet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn test_local_faucet() {
let keypair = Keypair::new();
let to = solana_sdk::pubkey::new_rand();
let lamports = 50;
let blockhash = Hash::new(to.as_ref());
let blockhash = Hash::new_from_array(to.to_bytes());
let create_instruction = system_instruction::transfer(&keypair.pubkey(), &to, lamports);
let message = Message::new(&[create_instruction], Some(&keypair.pubkey()));
let expected_tx = Transaction::new(&[&keypair], message, blockhash);
Expand Down
4 changes: 2 additions & 2 deletions gossip/src/crds_gossip_pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,8 @@ pub(crate) mod tests {

#[test]
fn test_hash_as_u64() {
let arr: Vec<u8> = (0..HASH_BYTES).map(|i| i as u8 + 1).collect();
let hash = Hash::new(&arr);
let arr: [u8; HASH_BYTES] = std::array::from_fn(|i| i as u8 + 1);
let hash = Hash::new_from_array(arr);
assert_eq!(CrdsFilter::hash_as_u64(&hash), 0x807060504030201);
}

Expand Down
12 changes: 8 additions & 4 deletions ledger/src/shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,11 @@ pub mod layout {
.ok()?;
shred
.get(offset..offset + SIZE_OF_MERKLE_ROOT)
.map(Hash::new)
.map(|merkle_root| {
<[u8; SIZE_OF_MERKLE_ROOT]>::try_from(merkle_root)
.map(Hash::new_from_array)
.unwrap()
})
}

fn get_retransmitter_signature_offset(shred: &[u8]) -> Result<usize, Error> {
Expand Down Expand Up @@ -1404,19 +1408,19 @@ mod tests {
0x5a, 0x5a, 0xa5, 0xa5, 0x5a, 0x5a, 0xa5, 0xa5, 0x5a, 0x5a, 0xa5, 0xa5, 0x5a, 0x5a,
0xa5, 0xa5, 0x5a, 0x5a,
];
let version = shred_version::version_from_hash(&Hash::new(&hash));
let version = shred_version::version_from_hash(&Hash::new_from_array(hash));
assert_eq!(version, 1);
let hash = [
0xa5u8, 0xa5, 0x5a, 0x5a, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
];
let version = shred_version::version_from_hash(&Hash::new(&hash));
let version = shred_version::version_from_hash(&Hash::new_from_array(hash));
assert_eq!(version, 0xffff);
let hash = [
0xa5u8, 0xa5, 0x5a, 0x5a, 0xa5, 0xa5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
];
let version = shred_version::version_from_hash(&Hash::new(&hash));
let version = shred_version::version_from_hash(&Hash::new_from_array(hash));
assert_eq!(version, 0x5a5b);
}

Expand Down
6 changes: 5 additions & 1 deletion ledger/src/shred/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,11 @@ macro_rules! impl_merkle_shred {
let offset = self.chained_merkle_root_offset()?;
self.payload
.get(offset..offset + SIZE_OF_MERKLE_ROOT)
.map(Hash::new)
.map(|chained_merkle_root| {
<[u8; SIZE_OF_MERKLE_ROOT]>::try_from(chained_merkle_root)
.map(Hash::new_from_array)
.unwrap()
})
.ok_or(Error::InvalidPayloadSize(self.payload.len()))
}

Expand Down
6 changes: 4 additions & 2 deletions merkle-tree/src/merkle_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl MerkleTree {

#[cfg(test)]
mod tests {
use super::*;
use {super::*, solana_hash::HASH_BYTES};

const TEST: &[&[u8]] = &[
b"my", b"very", b"eager", b"mother", b"just", b"served", b"us", b"nine", b"pizzas",
Expand Down Expand Up @@ -209,7 +209,9 @@ mod tests {
// changes
let bytes = hex::decode("b40c847546fdceea166f927fc46c5ca33c3638236a36275c1346d3dffb84e1bc")
.unwrap();
let expected = Hash::new(&bytes);
let expected = <[u8; HASH_BYTES]>::try_from(bytes)
.map(Hash::new_from_array)
.unwrap();
assert_eq!(mt.get_root(), Some(&expected));
}

Expand Down
2 changes: 1 addition & 1 deletion perf/src/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ mod tests {
#[test]
fn test_to_packet_batches() {
let keypair = Keypair::new();
let hash = Hash::new(&[1; 32]);
let hash = Hash::new_from_array([1; 32]);
let tx = system_transaction::transfer(&keypair, &keypair.pubkey(), 1, hash);
let rv = to_packet_batches_for_tests(&[tx.clone(); 1]);
assert_eq!(rv.len(), 1);
Expand Down
2 changes: 1 addition & 1 deletion programs/bpf_loader/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3518,7 +3518,7 @@ mod tests {
let mut src_rewards = create_filled_type::<EpochRewards>(false);
src_rewards.distribution_starting_block_height = 42;
src_rewards.num_partitions = 2;
src_rewards.parent_blockhash = Hash::new(&[3; 32]);
src_rewards.parent_blockhash = Hash::new_from_array([3; 32]);
src_rewards.total_points = 4;
src_rewards.total_rewards = 100;
src_rewards.distributed_rewards = 10;
Expand Down
2 changes: 1 addition & 1 deletion rpc-client-nonce-utils/src/blockhash_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ mod tests {
.get_blockhash(&rpc_client, CommitmentConfig::default())
.is_err());

let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[2u8; 32]));
let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([2u8; 32]));
let nonce_blockhash = *durable_nonce.as_hash();
let nonce_fee_calc = FeeCalculator::new(4242);
let data = nonce::state::Data {
Expand Down
2 changes: 1 addition & 1 deletion rpc-client-nonce-utils/src/nonblocking/blockhash_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ mod tests {
.await
.is_err());

let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[2u8; 32]));
let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([2u8; 32]));
let nonce_blockhash = *durable_nonce.as_hash();
let nonce_fee_calc = FeeCalculator::new(4242);
let data = nonce::state::Data {
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/transaction_status_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ pub(crate) mod tests {
let pubkey = Pubkey::new_unique();

let mut nonce_account = nonce_account::create_account(1).into_inner();
let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]));
let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([42u8; 32]));
let data = nonce::state::Data::new(Pubkey::from([1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&nonce::state::Versions::new(nonce::State::Initialized(
Expand Down
4 changes: 2 additions & 2 deletions runtime/benches/status_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use {
solana_accounts_db::ancestors::Ancestors,
solana_runtime::{bank::BankStatusCache, status_cache::*},
solana_sdk::{
hash::{hash, Hash},
hash::{hash, Hash, HASH_BYTES},
signature::{Signature, SIGNATURE_BYTES},
},
test::Bencher,
Expand All @@ -19,7 +19,7 @@ fn bench_status_cache_serialize(bencher: &mut Bencher) {
status_cache.add_root(0);
status_cache.clear();
for hash_index in 0..100 {
let blockhash = Hash::new(&vec![hash_index; std::mem::size_of::<Hash>()]);
let blockhash = Hash::new_from_array([hash_index; HASH_BYTES]);
let mut id = blockhash;
for _ in 0..100 {
id = hash(id.as_ref());
Expand Down
Loading

0 comments on commit d2299be

Please sign in to comment.