From c12967b25549b7310fadc4753993d236894d8e61 Mon Sep 17 00:00:00 2001 From: Oleksandr Anyshchenko Date: Fri, 10 Jan 2025 11:38:09 +0000 Subject: [PATCH] chore: add some optimisations to the AccountId creation methods (#985) ## Description The PR introduces some optimisations to the creation methods of the `AccounId` struct. Namely, it gets rid of redundant heap allocations. ## Performance / NEAR gas cost considerations There is no changes in the performance. ## Testing The existing tests cover the changes. --- engine-precompiles/src/native.rs | 12 ++++++---- engine-precompiles/src/xcc.rs | 10 +++++++-- engine-standalone-storage/src/sync/types.rs | 2 +- engine-types/src/account_id.rs | 25 ++++++++++++--------- engine/src/contract_methods/xcc.rs | 2 +- engine/src/xcc.rs | 4 ++-- 6 files changed, 35 insertions(+), 20 deletions(-) diff --git a/engine-precompiles/src/native.rs b/engine-precompiles/src/native.rs index e4e23578f..25944912e 100644 --- a/engine-precompiles/src/native.rs +++ b/engine-precompiles/src/native.rs @@ -51,7 +51,7 @@ mod costs { } pub mod events { - use crate::prelude::{types::Address, vec, String, ToString, H160, H256, U256}; + use crate::prelude::{types::Address, vec, String, ToString, H256, U256}; /// Derived from event signature (see `tests::test_exit_signatures`) pub const EXIT_TO_NEAR_SIGNATURE: H256 = crate::make_h256( @@ -68,7 +68,7 @@ pub mod events { /// which ERC-20 token is being withdrawn. However, ETH is not an ERC-20 token /// So we need to have some other address to fill this field. This constant is /// used for this purpose. - pub const ETH_ADDRESS: Address = Address::new(H160([0; 20])); + pub const ETH_ADDRESS: Address = Address::zero(); /// `ExitToNear`( /// Address indexed sender, @@ -655,7 +655,11 @@ impl Precompile for ExitToEthereum { if input.len() == 20 { // Parse ethereum address in hex - let eth_recipient = hex::encode(input); + let mut buffer = [0; 40]; + hex::encode_to_slice(input, &mut buffer).unwrap(); + let recipient_in_hex = str::from_utf8(&buffer).map_err(|_| { + ExitError::Other(Cow::from("ERR_INVALID_RECIPIENT_ADDRESS")) + })?; // unwrap cannot fail since we checked the length already let recipient_address = Address::try_from_slice(input).map_err(|_| { ExitError::Other(crate::prelude::Cow::from("ERR_WRONG_ADDRESS")) @@ -668,7 +672,7 @@ impl Precompile for ExitToEthereum { format!( r#"{{"amount": "{}", "recipient": "{}"}}"#, amount.as_u128(), - eth_recipient + recipient_in_hex ) .into_bytes(), events::ExitToEth { diff --git a/engine-precompiles/src/xcc.rs b/engine-precompiles/src/xcc.rs index f587231ba..f6f616a24 100644 --- a/engine-precompiles/src/xcc.rs +++ b/engine-precompiles/src/xcc.rs @@ -9,6 +9,7 @@ use aurora_engine_types::{ borsh::{self, BorshDeserialize}, format, parameters::{CrossContractCallArgs, PromiseCreateArgs}, + str, types::{balance::ZERO_YOCTO, Address, EthGas, NearGas}, vec, Cow, Vec, H160, H256, U256, }; @@ -299,8 +300,13 @@ fn create_target_account_id( sender: H160, engine_account_id: &str, ) -> Result { - format!("{}.{}", hex::encode(sender.as_bytes()), engine_account_id) - .parse() + let mut buffer = [0; 40]; + hex::encode_to_slice(sender.as_bytes(), &mut buffer) + .map_err(|_| revert_with_message(consts::ERR_XCC_ACCOUNT_ID))?; + let sender_in_hex = + str::from_utf8(&buffer).map_err(|_| revert_with_message(consts::ERR_XCC_ACCOUNT_ID))?; + + AccountId::try_from(format!("{sender_in_hex}.{engine_account_id}")) .map_err(|_| revert_with_message(consts::ERR_XCC_ACCOUNT_ID)) } diff --git a/engine-standalone-storage/src/sync/types.rs b/engine-standalone-storage/src/sync/types.rs index c36cc8aed..6efdbb7e6 100644 --- a/engine-standalone-storage/src/sync/types.rs +++ b/engine-standalone-storage/src/sync/types.rs @@ -381,7 +381,7 @@ impl TransactionKind { ) } Self::WithdrawWnearToRouter(args) => { - let recipient = AccountId::new(&format!( + let recipient = AccountId::try_from(format!( "{}.{}", args.target.encode(), engine_account.as_ref() diff --git a/engine-types/src/account_id.rs b/engine-types/src/account_id.rs index 67d0adbc3..f196a7daf 100644 --- a/engine-types/src/account_id.rs +++ b/engine-types/src/account_id.rs @@ -94,7 +94,8 @@ impl BorshDeserialize for AccountId { return Ok(Self::default()); } - Self::new(&account).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string())) + Self::try_from(account) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string())) } } @@ -105,7 +106,7 @@ impl<'de> Deserialize<'de> for AccountId { D::Error: serde::de::Error, { let account = ::deserialize(deserializer)?; - Self::new(&account).map_err(serde::de::Error::custom) + Self::try_from(account).map_err(serde::de::Error::custom) } } @@ -113,7 +114,10 @@ impl TryFrom for AccountId { type Error = ParseAccountError; fn try_from(account_id: String) -> Result { - Self::new(&account_id) + let account_id = account_id.into_boxed_str(); + Self::validate(&account_id)?; + + Ok(Self(account_id)) } } @@ -130,7 +134,9 @@ impl TryFrom> for AccountId { type Error = ParseAccountError; fn try_from(account_id: Vec) -> Result { - Self::try_from(&account_id[..]) + String::from_utf8(account_id) + .map_err(|_| ParseAccountError::Invalid) + .and_then(Self::try_from) } } @@ -138,8 +144,7 @@ impl FromStr for AccountId { type Err = ParseAccountError; fn from_str(account_id: &str) -> Result { - Self::validate(account_id)?; - Ok(Self(account_id.into())) + Self::new(account_id) } } @@ -156,14 +161,14 @@ impl fmt::Display for AccountId { } impl From for Box { - fn from(value: AccountId) -> Self { - value.0 + fn from(account_id: AccountId) -> Self { + account_id.0 } } impl From for Vec { fn from(account_id: AccountId) -> Self { - account_id.as_bytes().to_vec() + account_id.0.into_boxed_bytes().into_vec() } } @@ -202,7 +207,7 @@ impl AsRef<[u8]> for ParseAccountError { impl fmt::Display for ParseAccountError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let msg = String::from_utf8(self.as_ref().to_vec()).unwrap(); + let msg = str::from_utf8(self.as_ref()).map_err(|_| fmt::Error)?; write!(f, "{msg}") } } diff --git a/engine/src/contract_methods/xcc.rs b/engine/src/contract_methods/xcc.rs index 91bef1d47..4184d83c7 100644 --- a/engine/src/contract_methods/xcc.rs +++ b/engine/src/contract_methods/xcc.rs @@ -34,7 +34,7 @@ pub fn withdraw_wnear_to_router( } let args: WithdrawWnearToRouterArgs = io.read_input_borsh()?; let current_account_id = env.current_account_id(); - let recipient = AccountId::new(&format!( + let recipient = AccountId::try_from(format!( "{}.{}", args.target.encode(), current_account_id.as_ref() diff --git a/engine/src/xcc.rs b/engine/src/xcc.rs index 31578ddd9..5b8e9a55a 100644 --- a/engine/src/xcc.rs +++ b/engine/src/xcc.rs @@ -77,7 +77,7 @@ where E: Env, { let current_account_id = env.current_account_id(); - let target_account_id = AccountId::new(&format!( + let target_account_id = AccountId::try_from(format!( "{}.{}", args.target.encode(), current_account_id.as_ref() @@ -95,7 +95,7 @@ where if let AddressVersionStatus::DeployNeeded { create_needed } = deploy_needed { let code = get_router_code(io).0.into_owned(); // wnear_account is needed for initialization so we must assume it is set - // in the Engine or we need to accept it as input. + // in the Engine, or we need to accept it as input. let wnear_account = if let Some(wnear_account) = args.wnear_account_id { wnear_account } else {