From f2d2dd854b1560417ae18dea85886738bddf33cd Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 6 Dec 2022 21:03:16 +0100 Subject: [PATCH] primitives: get rid of Cursor As per the TODO, take `&[u8; 36]` in ValueRef::decode and stop using Cursor. This makes the method considerably simpler without making the call site more complex. While dealing with ValueRef, stop using Cursor in account_id_to_shard_id as well. Issue: https://github.com/near/nearcore/issues/7327 --- Cargo.lock | 2 +- core/primitives/Cargo.toml | 2 +- core/primitives/src/shard_layout.rs | 8 +++----- core/primitives/src/state.rs | 16 +++++----------- core/store/src/flat_state.rs | 15 +++++++++------ 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d8b07fd1b6..068068b358f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3412,7 +3412,6 @@ dependencies = [ "assert_matches", "bencher", "borsh", - "byteorder", "bytesize", "cfg-if 1.0.0", "chrono", @@ -3425,6 +3424,7 @@ dependencies = [ "near-o11y", "near-primitives-core", "near-rpc-error-macro", + "near-stdx", "near-vm-errors", "num-rational", "once_cell", diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 34d37428d87..516fb381123 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -15,7 +15,6 @@ This crate provides the base set of primitives used by other nearcore crates [dependencies] arbitrary.workspace = true borsh.workspace = true -byteorder.workspace = true bytesize.workspace = true cfg-if.workspace = true chrono.workspace = true @@ -31,6 +30,7 @@ reed-solomon-erasure.workspace = true serde.workspace = true serde_json.workspace = true smart-default.workspace = true +stdx.workspace = true strum.workspace = true thiserror.workspace = true tracing.workspace = true diff --git a/core/primitives/src/shard_layout.rs b/core/primitives/src/shard_layout.rs index ad460595df9..7d1fcef4b80 100644 --- a/core/primitives/src/shard_layout.rs +++ b/core/primitives/src/shard_layout.rs @@ -1,13 +1,10 @@ use std::cmp::Ordering::Greater; use std::{fmt, str}; -use byteorder::{LittleEndian, ReadBytesExt}; use serde::{Deserialize, Serialize}; -use near_primitives_core::hash::hash; use near_primitives_core::types::ShardId; -use crate::borsh::maybestd::io::Cursor; use crate::hash::CryptoHash; use crate::types::{AccountId, NumShards}; use std::collections::HashMap; @@ -251,8 +248,9 @@ impl ShardLayout { pub fn account_id_to_shard_id(account_id: &AccountId, shard_layout: &ShardLayout) -> ShardId { match shard_layout { ShardLayout::V0(ShardLayoutV0 { num_shards, .. }) => { - let mut cursor = Cursor::new(hash(account_id.as_ref().as_bytes()).0); - cursor.read_u64::().expect("Must not happened") % (num_shards) + let hash = CryptoHash::hash_bytes(account_id.as_ref().as_bytes()); + let (bytes, _) = stdx::split_array::<32, 8, 24>(hash.as_bytes()); + u64::from_le_bytes(*bytes) % num_shards } ShardLayout::V1(ShardLayoutV1 { fixed_shards, boundary_accounts, .. }) => { for (shard_id, fixed_account) in fixed_shards.iter().enumerate() { diff --git a/core/primitives/src/state.rs b/core/primitives/src/state.rs index 797d4990fdb..374c7b974cc 100644 --- a/core/primitives/src/state.rs +++ b/core/primitives/src/state.rs @@ -1,8 +1,6 @@ use borsh::{BorshDeserialize, BorshSerialize}; -use byteorder::{LittleEndian, ReadBytesExt}; use near_primitives_core::hash::{hash, CryptoHash}; -use std::io::{Cursor, Read}; /// State value reference. Used to charge fees for value length before retrieving the value itself. #[derive(BorshSerialize, BorshDeserialize, Clone, PartialEq, Eq, Debug)] @@ -22,14 +20,10 @@ impl ValueRef { } /// Decode value reference from the raw byte array. - /// TODO (#7327): use &[u8; 36] and get rid of Cursor; also check that there are no leftover bytes - pub fn decode(bytes: &[u8]) -> Result { - let mut cursor = Cursor::new(bytes); - let value_length = cursor.read_u32::()?; - let mut arr = [0; 32]; - cursor.read_exact(&mut arr)?; - let value_hash = CryptoHash(arr); - Ok(ValueRef { length: value_length, hash: value_hash }) + pub fn decode(bytes: &[u8; 36]) -> Self { + let (length, hash) = stdx::split_array(bytes); + let length = u32::from_le_bytes(*length); + ValueRef { length, hash: CryptoHash(*hash) } } } @@ -45,7 +39,7 @@ mod tests { let mut value_ref_ser = [0u8; 36]; value_ref_ser[0..4].copy_from_slice(&old_value_ref.length.to_le_bytes()); value_ref_ser[4..36].copy_from_slice(&old_value_ref.hash.0); - let value_ref = ValueRef::decode(&value_ref_ser).unwrap(); + let value_ref = ValueRef::decode(&value_ref_ser); assert_eq!(value_ref.length, value.len() as u32); assert_eq!(value_ref.hash, hash(&value)); } diff --git a/core/store/src/flat_state.rs b/core/store/src/flat_state.rs index 2415786eeb5..f56010c398f 100644 --- a/core/store/src/flat_state.rs +++ b/core/store/src/flat_state.rs @@ -561,12 +561,15 @@ pub mod store_helper { pub(crate) fn get_ref(store: &Store, key: &[u8]) -> Result, FlatStorageError> { let raw_ref = store .get(crate::DBCol::FlatState, key) - .map_err(|_| FlatStorageError::StorageInternalError); - match raw_ref? { - Some(bytes) => ValueRef::decode(&bytes) - .map(Some) - .map_err(|_| FlatStorageError::StorageInternalError), - None => Ok(None), + .map_err(|_| FlatStorageError::StorageInternalError)?; + if let Some(raw_ref) = raw_ref { + let bytes = raw_ref + .as_slice() + .try_into() + .map_err(|_| FlatStorageError::StorageInternalError)?; + Ok(Some(ValueRef::decode(bytes))) + } else { + Ok(None) } }