From 62f66348ae7871e2c5879a60395f4003a9e9bbf1 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 23 Aug 2022 11:31:30 +0200 Subject: [PATCH] =?UTF-8?q?base=20=E2=86=92=20base58=20(#7462)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Referring to base58 encoding as ‘base’ is quite confusing. It’s not only ambiguous but also by just using ‘base’ it’s not even clear that code is referring to binary-to-text encoding schema. Rename to_base and from_base methods to to_base58 and from_base58 respectively. Furthermore, for CryptoHash prefer to_string rather than to_base58. --- chain/chain-primitives/src/error.rs | 3 +-- chain/chain/src/test_utils.rs | 11 +++++----- .../fuzz/fuzz_targets/fuzz_target_1.rs | 2 +- .../jsonrpc-tests/tests/rpc_transactions.rs | 4 ++-- chain/jsonrpc/src/api/query.rs | 8 +++---- core/primitives-core/src/account.rs | 3 +-- core/primitives-core/src/logging.rs | 4 ++-- core/primitives-core/src/serialize.rs | 14 ++++++------ core/primitives/src/transaction.rs | 3 +-- core/store/src/lib.rs | 22 ++++++++++--------- integration-tests/src/user/rpc_user.rs | 4 ++-- nearcore/src/migrations.rs | 9 ++++---- runtime/near-vm-logic/src/context.rs | 4 ++-- runtime/near-vm-logic/src/logic.rs | 8 +++---- tools/state-viewer/src/commands.rs | 11 +++++----- 15 files changed, 54 insertions(+), 56 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 7372ff9b457..e12f5f5b568 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -6,7 +6,6 @@ use near_primitives::time::Utc; use near_primitives::block::BlockValidityError; use near_primitives::challenge::{ChunkProofs, ChunkState}; use near_primitives::errors::{EpochError, StorageError}; -use near_primitives::serialize::to_base; use near_primitives::shard_layout::ShardLayoutError; use near_primitives::sharding::{ChunkHash, ShardChunkHeader}; use near_primitives::types::{BlockHeight, EpochId, ShardId}; @@ -299,7 +298,7 @@ impl From for Error { fn from(error: EpochError) -> Self { match error { EpochError::EpochOutOfBounds(epoch_id) => Error::EpochOutOfBounds(epoch_id), - EpochError::MissingBlock(h) => Error::DBNotFoundErr(to_base(&h)), + EpochError::MissingBlock(h) => Error::DBNotFoundErr(h.to_string()), err => Error::ValidatorError(err.to_string()), } } diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index af7eeb094ff..1dddf3a44b0 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -24,7 +24,6 @@ use near_primitives::epoch_manager::epoch_info::EpochInfo; use near_primitives::errors::{EpochError, InvalidTxError}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum}; -use near_primitives::serialize::to_base; use near_primitives::shard_layout; use near_primitives::shard_layout::{ShardLayout, ShardUId}; use near_primitives::sharding::ChunkHash; @@ -305,7 +304,7 @@ impl KeyValueRuntime { } let prev_block_header = self .get_block_header(&prev_hash)? - .ok_or_else(|| Error::DBNotFoundErr(to_base(&prev_hash)))?; + .ok_or_else(|| Error::DBNotFoundErr(prev_hash.to_string()))?; let mut hash_to_epoch = self.hash_to_epoch.write().unwrap(); let mut hash_to_next_epoch_approvals_req = @@ -1274,7 +1273,7 @@ impl RuntimeAdapter for KeyValueRuntime { loop { let header = self .get_block_header(&candidate_hash)? - .ok_or_else(|| Error::DBNotFoundErr(to_base(&candidate_hash)))?; + .ok_or_else(|| Error::DBNotFoundErr(candidate_hash.to_string()))?; candidate_hash = *header.prev_hash(); if self.is_next_block_epoch_start(&candidate_hash)? { break Ok(self.get_epoch_and_valset(candidate_hash)?.0); @@ -1394,8 +1393,10 @@ pub fn setup_with_validators( (chain, runtime, signers) } -pub fn format_hash(h: CryptoHash) -> String { - to_base(&h)[..6].to_string() +pub fn format_hash(hash: CryptoHash) -> String { + let mut hash = hash.to_string(); + hash.truncate(6); + hash } /// Displays chain from given store. diff --git a/chain/jsonrpc/fuzz/fuzz_targets/fuzz_target_1.rs b/chain/jsonrpc/fuzz/fuzz_targets/fuzz_target_1.rs index b84fa815887..8049538272f 100644 --- a/chain/jsonrpc/fuzz/fuzz_targets/fuzz_target_1.rs +++ b/chain/jsonrpc/fuzz/fuzz_targets/fuzz_target_1.rs @@ -122,7 +122,7 @@ impl Serialize for Base58String { where S: Serializer, { - let serialised = near_primitives::serialize::to_base(&self.0); + let serialised = near_primitives::serialize::to_base58(&self.0); serializer.serialize_newtype_struct("Base58String", &serialised) } } diff --git a/chain/jsonrpc/jsonrpc-tests/tests/rpc_transactions.rs b/chain/jsonrpc/jsonrpc-tests/tests/rpc_transactions.rs index a171768ce82..913a457ebaf 100644 --- a/chain/jsonrpc/jsonrpc-tests/tests/rpc_transactions.rs +++ b/chain/jsonrpc/jsonrpc-tests/tests/rpc_transactions.rs @@ -10,7 +10,7 @@ use near_jsonrpc::client::new_client; use near_logger_utils::{init_integration_logger, init_test_logger}; use near_network::test_utils::WaitOrTimeoutActor; use near_primitives::hash::{hash, CryptoHash}; -use near_primitives::serialize::{to_base, to_base64}; +use near_primitives::serialize::to_base64; use near_primitives::transaction::SignedTransaction; use near_primitives::types::BlockReference; use near_primitives::views::FinalExecutionStatus; @@ -190,7 +190,7 @@ fn test_replay_protection() { #[test] fn test_tx_status_missing_tx() { test_with_client!(test_utils::NodeType::Validator, client, async move { - match client.tx(to_base(&CryptoHash::default()), "test1".parse().unwrap()).await { + match client.tx(CryptoHash::new().to_string(), "test1".parse().unwrap()).await { Err(e) => { let s = serde_json::to_string(&e.data.unwrap()).unwrap(); assert_eq!(s, "\"Transaction 11111111111111111111111111111111 doesn't exist\""); diff --git a/chain/jsonrpc/src/api/query.rs b/chain/jsonrpc/src/api/query.rs index 7de116e7db2..4498ce4843b 100644 --- a/chain/jsonrpc/src/api/query.rs +++ b/chain/jsonrpc/src/api/query.rs @@ -14,14 +14,14 @@ const QUERY_DATA_MAX_SIZE: usize = 10 * 1024; impl RpcRequest for RpcQueryRequest { fn parse(value: Option) -> Result { - let query_request = if let Ok((path, data)) = - parse_params::<(String, String)>(value.clone()) - { + let params = parse_params::<(String, String)>(value.clone()); + let query_request = if let Ok((path, data)) = params { // Handle a soft-deprecated version of the query API, which is based on // positional arguments with a "path"-style first argument. // // This whole block can be removed one day, when the new API is 100% adopted. - let data = serialize::from_base(&data).map_err(|err| RpcParseError(err.to_string()))?; + let data = + serialize::from_base58(&data).map_err(|err| RpcParseError(err.to_string()))?; let query_data_size = path.len() + data.len(); if query_data_size > QUERY_DATA_MAX_SIZE { return Err(RpcParseError(format!( diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 39fc35bc8fb..c38a3e7e9dc 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -214,7 +214,6 @@ mod tests { use borsh::BorshSerialize; use crate::hash::hash; - use crate::serialize::to_base; use super::*; @@ -222,7 +221,7 @@ mod tests { fn test_account_serialization() { let acc = Account::new(1_000_000, 1_000_000, CryptoHash::default(), 100); let bytes = acc.try_to_vec().unwrap(); - assert_eq!(to_base(&hash(&bytes)), "EVk5UaxBe8LQ8r8iD5EAxVBs6TJcMDKqyH7PBuho6bBJ"); + assert_eq!(hash(&bytes).to_string(), "EVk5UaxBe8LQ8r8iD5EAxVBs6TJcMDKqyH7PBuho6bBJ"); } #[test] diff --git a/core/primitives-core/src/logging.rs b/core/primitives-core/src/logging.rs index 70e6c11c097..fb1a570c2c3 100644 --- a/core/primitives-core/src/logging.rs +++ b/core/primitives-core/src/logging.rs @@ -1,6 +1,6 @@ use std::fmt::Debug; -use crate::serialize::to_base; +use crate::serialize::to_base58; const VECTOR_MAX_LENGTH: usize = 5; const STRING_PRINT_LEN: usize = 128; @@ -37,7 +37,7 @@ pub fn pretty_utf8(buf: &[u8]) -> String { Ok(s) => pretty_hash(s), Err(_) => { if buf.len() <= STRING_PRINT_LEN { - pretty_hash(&to_base(buf)) + pretty_hash(&to_base58(buf)) } else { pretty_vec(buf) } diff --git a/core/primitives-core/src/serialize.rs b/core/primitives-core/src/serialize.rs index ff191c1325a..cc10dc4af43 100644 --- a/core/primitives-core/src/serialize.rs +++ b/core/primitives-core/src/serialize.rs @@ -1,8 +1,8 @@ -pub fn to_base>(input: T) -> String { +pub fn to_base58>(input: T) -> String { bs58::encode(input).into_string() } -pub fn from_base(s: &str) -> Result, Box> { +pub fn from_base58(s: &str) -> Result, Box> { bs58::decode(s).into_vec().map_err(|err| err.into()) } @@ -92,17 +92,17 @@ fn test_option_base64_format() { assert_round_trip("{\"field\":null}", Test { field: None }); } -pub mod base_bytes_format { +pub mod base58_format { use serde::de; use serde::{Deserialize, Deserializer, Serializer}; - use super::{from_base, to_base}; + use super::{from_base58, to_base58}; pub fn serialize(data: &[u8], serializer: S) -> Result where S: Serializer, { - serializer.serialize_str(&to_base(data)) + serializer.serialize_str(&to_base58(data)) } pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> @@ -110,7 +110,7 @@ pub mod base_bytes_format { D: Deserializer<'de>, { let s = String::deserialize(deserializer)?; - from_base(&s).map_err(|err| de::Error::custom(err.to_string())) + from_base58(&s).map_err(|err| de::Error::custom(err.to_string())) } } @@ -118,7 +118,7 @@ pub mod base_bytes_format { fn test_base_bytes_format() { #[derive(PartialEq, Debug, serde::Deserialize, serde::Serialize)] struct Test { - #[serde(with = "base_bytes_format")] + #[serde(with = "base58_format")] field: Vec, } diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index b6269118204..c06cd8a1f2b 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -465,7 +465,6 @@ mod tests { use near_crypto::{InMemorySigner, KeyType, Signature, Signer}; use crate::account::{AccessKeyPermission, FunctionCallPermission}; - use crate::serialize::to_base; use super::*; @@ -537,7 +536,7 @@ mod tests { SignedTransaction::try_from_slice(&signed_tx.try_to_vec().unwrap()).unwrap(); assert_eq!( - to_base(&new_signed_tx.get_hash()), + new_signed_tx.get_hash().to_string(), "4GXvjMFN6wSxnU9jEVT8HbXP5Yk6yELX9faRSKp6n9fX" ); } diff --git a/core/store/src/lib.rs b/core/store/src/lib.rs index eed1aab1cd5..824a0cd908f 100644 --- a/core/store/src/lib.rs +++ b/core/store/src/lib.rs @@ -19,7 +19,7 @@ use near_primitives::contract::ContractCode; pub use near_primitives::errors::StorageError; use near_primitives::hash::CryptoHash; use near_primitives::receipt::{DelayedReceiptIndices, Receipt, ReceivedData}; -use near_primitives::serialize::to_base; +use near_primitives::serialize::to_base58; pub use near_primitives::shard_layout::ShardUId; use near_primitives::trie_key::{trie_key_parsers, TrieKey}; use near_primitives::types::{AccountId, CompiledContractCache, StateRoot}; @@ -90,7 +90,7 @@ impl Store { target: "store", db_op = "get", col = %column, - key = %to_base(key), + key = %to_base58(key), size = value.as_ref().map(Vec::len) ); Ok(value) @@ -387,16 +387,16 @@ impl StoreUpdate { for op in &self.transaction.ops { match op { DBOp::Insert { col, key, value } => { - tracing::trace!(target: "store", db_op = "insert", col = %col, key = %to_base(key), size = value.len()) + tracing::trace!(target: "store", db_op = "insert", col = %col, key = %to_base58(key), size = value.len()) } DBOp::Set { col, key, value } => { - tracing::trace!(target: "store", db_op = "set", col = %col, key = %to_base(key), size = value.len()) + tracing::trace!(target: "store", db_op = "set", col = %col, key = %to_base58(key), size = value.len()) } DBOp::UpdateRefcount { col, key, value } => { - tracing::trace!(target: "store", db_op = "update_rc", col = %col, key = %to_base(key), size = value.len()) + tracing::trace!(target: "store", db_op = "update_rc", col = %col, key = %to_base58(key), size = value.len()) } DBOp::Delete { col, key } => { - tracing::trace!(target: "store", db_op = "delete", col = %col, key = %to_base(key)) + tracing::trace!(target: "store", db_op = "delete", col = %col, key = %to_base58(key)) } DBOp::DeleteAll { col } => { tracing::trace!(target: "store", db_op = "delete_all", col = %col) @@ -412,10 +412,12 @@ impl fmt::Debug for StoreUpdate { writeln!(f, "Store Update {{")?; for op in self.transaction.ops.iter() { match op { - DBOp::Insert { col, key, .. } => writeln!(f, " + {col} {}", to_base(key))?, - DBOp::Set { col, key, .. } => writeln!(f, " = {col} {}", to_base(key))?, - DBOp::UpdateRefcount { col, key, .. } => writeln!(f, " ± {col} {}", to_base(key))?, - DBOp::Delete { col, key } => writeln!(f, " - {col} {}", to_base(key))?, + DBOp::Insert { col, key, .. } => writeln!(f, " + {col} {}", to_base58(key))?, + DBOp::Set { col, key, .. } => writeln!(f, " = {col} {}", to_base58(key))?, + DBOp::UpdateRefcount { col, key, .. } => { + writeln!(f, " ± {col} {}", to_base58(key))? + } + DBOp::Delete { col, key } => writeln!(f, " - {col} {}", to_base58(key))?, DBOp::DeleteAll { col } => writeln!(f, " - {col} (all)")?, } } diff --git a/integration-tests/src/user/rpc_user.rs b/integration-tests/src/user/rpc_user.rs index 5fc711e863b..7ee0ba0f220 100644 --- a/integration-tests/src/user/rpc_user.rs +++ b/integration-tests/src/user/rpc_user.rs @@ -13,7 +13,7 @@ use near_jsonrpc_primitives::errors::ServerError; use near_jsonrpc_primitives::types::query::RpcQueryResponse; use near_primitives::hash::CryptoHash; use near_primitives::receipt::Receipt; -use near_primitives::serialize::{to_base, to_base64}; +use near_primitives::serialize::{to_base58, to_base64}; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{ AccountId, BlockHeight, BlockId, BlockReference, MaybeBlockId, ShardId, @@ -51,7 +51,7 @@ impl RpcUser { } pub fn query(&self, path: String, data: &[u8]) -> Result { - let data = to_base(data); + let data = to_base58(data); self.actix(move |client| client.query_by_path(path, data).map_err(|err| err.to_string())) } diff --git a/nearcore/src/migrations.rs b/nearcore/src/migrations.rs index 0d1ea961f30..6bacb2551f7 100644 --- a/nearcore/src/migrations.rs +++ b/nearcore/src/migrations.rs @@ -68,14 +68,12 @@ mod tests { use near_mainnet_res::mainnet_restored_receipts; use near_mainnet_res::mainnet_storage_usage_delta; use near_primitives::hash::hash; - use near_primitives::serialize::to_base; #[test] fn test_migration_data() { assert_eq!( - to_base(&hash( - serde_json::to_string(&mainnet_storage_usage_delta()).unwrap().as_bytes() - )), + hash(serde_json::to_string(&mainnet_storage_usage_delta()).unwrap().as_bytes()) + .to_string(), "2fEgaLFBBJZqgLQEvHPsck4NS3sFzsgyKaMDqTw5HVvQ" ); let mainnet_migration_data = load_migration_data("mainnet"); @@ -87,7 +85,8 @@ mod tests { #[test] fn test_restored_receipts_data() { assert_eq!( - to_base(&hash(serde_json::to_string(&mainnet_restored_receipts()).unwrap().as_bytes())), + hash(serde_json::to_string(&mainnet_restored_receipts()).unwrap().as_bytes()) + .to_string(), "48ZMJukN7RzvyJSW9MJ5XmyQkQFfjy2ZxPRaDMMHqUcT" ); let mainnet_migration_data = load_migration_data("mainnet"); diff --git a/runtime/near-vm-logic/src/context.rs b/runtime/near-vm-logic/src/context.rs index f740801f8c5..c3993ef2d22 100644 --- a/runtime/near-vm-logic/src/context.rs +++ b/runtime/near-vm-logic/src/context.rs @@ -14,7 +14,7 @@ pub struct VMContext { /// The account id of that signed the original transaction that led to this /// execution. pub signer_account_id: AccountId, - #[serde(with = "serialize::base_bytes_format")] + #[serde(with = "serialize::base58_format")] /// The public key that was used to sign the original transaction that led to /// this execution. pub signer_account_pk: PublicKey, @@ -51,7 +51,7 @@ pub struct VMContext { pub attached_deposit: Balance, /// The gas attached to the call that can be used to pay for the gas fees. pub prepaid_gas: Gas, - #[serde(with = "serialize::base_bytes_format")] + #[serde(with = "serialize::base58_format")] /// Initial seed for randomness pub random_seed: Vec, /// If Some, it means that execution is made in a view mode and defines its configuration. diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 07af1075d2e..3287b044a02 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -2348,7 +2348,7 @@ impl<'a> VMLogic<'a> { near_o11y::io_trace!( storage_op = "write", - key = %near_primitives::serialize::to_base(key.clone()), + key = %near_primitives::serialize::to_base58(&key), size = value_len, evicted_len = evicted.as_ref().map(Vec::len), tn_mem_reads = nodes_delta.mem_reads, @@ -2439,7 +2439,7 @@ impl<'a> VMLogic<'a> { near_o11y::io_trace!( storage_op = "read", - key = %near_primitives::serialize::to_base(key.clone()), + key = %near_primitives::serialize::to_base58(&key), size = read.as_ref().map(Vec::len), tn_db_reads = nodes_delta.db_reads, tn_mem_reads = nodes_delta.mem_reads, @@ -2499,7 +2499,7 @@ impl<'a> VMLogic<'a> { near_o11y::io_trace!( storage_op = "remove", - key = %near_primitives::serialize::to_base(key.clone()), + key = %near_primitives::serialize::to_base58(&key), evicted_len = removed.as_ref().map(Vec::len), tn_mem_reads = nodes_delta.mem_reads, tn_db_reads = nodes_delta.db_reads, @@ -2555,7 +2555,7 @@ impl<'a> VMLogic<'a> { near_o11y::io_trace!( storage_op = "exists", - key = %near_primitives::serialize::to_base(key.clone()), + key = %near_primitives::serialize::to_base58(&key), tn_mem_reads = nodes_delta.mem_reads, tn_db_reads = nodes_delta.db_reads, ); diff --git a/tools/state-viewer/src/commands.rs b/tools/state-viewer/src/commands.rs index 051d6f3c3b2..5848eebb069 100644 --- a/tools/state-viewer/src/commands.rs +++ b/tools/state-viewer/src/commands.rs @@ -15,7 +15,6 @@ use near_network::iter_peers_from_store; use near_primitives::account::id::AccountId; use near_primitives::block::{Block, BlockHeader}; use near_primitives::hash::CryptoHash; -use near_primitives::serialize::to_base; use near_primitives::shard_layout::ShardUId; use near_primitives::sharding::ChunkHash; use near_primitives::state_record::StateRecord; @@ -746,12 +745,12 @@ fn load_trie_stop_at_height( (runtime, state_roots, last_block.header().clone()) } -pub fn format_hash(h: CryptoHash, show_full_hashes: bool) -> String { - if show_full_hashes { - to_base(&h).to_string() - } else { - to_base(&h)[..7].to_string() +fn format_hash(h: CryptoHash, show_full_hashes: bool) -> String { + let mut hash = h.to_string(); + if !show_full_hashes { + hash.truncate(7); } + hash } pub fn chunk_mask_to_str(mask: &[bool]) -> String {