Skip to content

Commit

Permalink
refactor: use Cow for Account contract fn (#12902)
Browse files Browse the repository at this point in the history
Currently we return owned instance of `AccountContract` which forces
cloning of `AccountId` for `GlobalByAccount` variant. We cannot return
ref since for `AccountV1` we need to create instance of
`AccountContract` to wrap `code_hash`. Using `Cow` solves the issue.
  • Loading branch information
pugachAG authored Feb 10, 2025
1 parent 5e89eb9 commit a4d9451
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 12 deletions.
13 changes: 10 additions & 3 deletions core/primitives-core/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use borsh::{BorshDeserialize, BorshSerialize};
pub use near_account_id as id;
use near_account_id::AccountId;
use near_schema_checker_lib::ProtocolSchema;
use std::borrow::Cow;
use std::io;

#[derive(
Expand Down Expand Up @@ -109,6 +110,10 @@ impl AccountContract {
AccountContract::Local(code_hash)
}
}

pub fn is_none(&self) -> bool {
matches!(self, Self::None)
}
}

#[derive(
Expand Down Expand Up @@ -179,10 +184,12 @@ impl Account {
}

#[inline]
pub fn contract(&self) -> AccountContract {
pub fn contract(&self) -> Cow<AccountContract> {
match self {
Self::V1(account) => AccountContract::from_local_code_hash(account.code_hash),
Self::V2(account) => account.contract.clone(),
Self::V1(account) => {
Cow::Owned(AccountContract::from_local_code_hash(account.code_hash))
}
Self::V2(account) => Cow::Borrowed(&account.contract),
}
}

Expand Down
11 changes: 6 additions & 5 deletions core/primitives/src/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,12 @@ pub struct ContractCodeView {

impl From<&Account> for AccountView {
fn from(account: &Account) -> Self {
let (global_contract_hash, global_contract_account_id) = match account.contract() {
AccountContract::Global(contract) => (Some(contract), None),
AccountContract::GlobalByAccount(account_id) => (None, Some(account_id)),
AccountContract::Local(_) | AccountContract::None => (None, None),
};
let (global_contract_hash, global_contract_account_id) =
match account.contract().into_owned() {
AccountContract::Global(contract) => (Some(contract), None),
AccountContract::GlobalByAccount(account_id) => (None, Some(account_id)),
AccountContract::Local(_) | AccountContract::None => (None, None),
};
AccountView {
amount: account.amount(),
locked: account.locked(),
Expand Down
8 changes: 4 additions & 4 deletions tools/amend-genesis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl AccountRecords {
// records. Set the storage usage to reflect whatever's in the original records, and at the
// end we will add to the storage usage with any extra keys added for this account
account.set_storage_usage(existing.storage_usage());
account.set_contract(existing.contract());
account.set_contract(existing.contract().into_owned());
if self.amount_needed {
set_total_balance(account, existing);
}
Expand Down Expand Up @@ -171,7 +171,7 @@ fn parse_extra_records(
near_chain_configs::stream_records_from_file(reader, |r| {
match r {
StateRecord::Account { account_id, account } => {
if account.contract() != AccountContract::None {
if !account.contract().is_none() {
result = Err(anyhow::anyhow!(
"FIXME: accounts in --extra-records with code_hash set not supported"
));
Expand Down Expand Up @@ -517,7 +517,7 @@ mod test {
(
account.amount(),
account.locked(),
account.contract(),
account.contract().into_owned(),
account.storage_usage(),
),
)
Expand Down Expand Up @@ -555,7 +555,7 @@ mod test {
(
account.amount(),
account.locked(),
account.contract(),
account.contract().into_owned(),
account.storage_usage(),
),
);
Expand Down

0 comments on commit a4d9451

Please sign in to comment.