From d81c640b27a42d6b627e768dd49fdd1979d3f0ef Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 17:23:44 +0100 Subject: [PATCH 01/43] global contract support --- core/primitives-core/src/account.rs | 76 +++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 045113427da..e0a7eee46de 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -3,6 +3,7 @@ use crate::serialize::dec_format; use crate::types::{Balance, Nonce, StorageUsage}; use borsh::{BorshDeserialize, BorshSerialize}; pub use near_account_id as id; +use near_account_id::AccountId; use near_schema_checker_lib::ProtocolSchema; use std::io; @@ -69,6 +70,8 @@ impl AccountV1 { locked: self.locked, code_hash: self.code_hash, storage_usage: self.storage_usage, + global_contract_hash: None, + global_contract_account_id: None, } } } @@ -94,6 +97,9 @@ pub struct AccountV2 { code_hash: CryptoHash, /// Storage used by the given account, includes account id, this struct, access keys and other data. storage_usage: StorageUsage, + /// Global contracts fields + global_contract_hash: Option, + global_contract_account_id: Option, } impl Account { @@ -154,6 +160,22 @@ impl Account { } } + #[inline] + pub fn global_contract_hash(&self) -> Option { + match self { + Self::V1(_) => None, + Self::V2(account) => account.global_contract_hash, + } + } + + #[inline] + pub fn global_contract_account_id(&self) -> Option { + match self { + Self::V1(_) => None, + Self::V2(account) => account.global_contract_account_id.clone(), + } + } + #[inline] pub fn set_amount(&mut self, amount: Balance) { match self { @@ -200,6 +222,9 @@ struct SerdeAccount { /// Version of Account in re migrations and similar. #[serde(default)] version: AccountVersion, + /// Global contracts fields + global_contract_hash: Option, + global_contract_account_id: Option, } impl<'de> serde::Deserialize<'de> for Account { @@ -208,6 +233,15 @@ impl<'de> serde::Deserialize<'de> for Account { D: serde::Deserializer<'de>, { let account_data = SerdeAccount::deserialize(deserializer)?; + if account_data.code_hash != CryptoHash::default() + && (account_data.global_contract_hash.is_some() + || account_data.global_contract_account_id.is_some()) + { + return Err(serde::de::Error::custom( + "An Account can't contain both a local and global contract", + )); + } + match account_data.version { AccountVersion::V1 => Ok(Account::V1(AccountV1 { amount: account_data.amount, @@ -220,6 +254,8 @@ impl<'de> serde::Deserialize<'de> for Account { locked: account_data.locked, code_hash: account_data.code_hash, storage_usage: account_data.storage_usage, + global_contract_hash: account_data.global_contract_hash, + global_contract_account_id: account_data.global_contract_account_id, })), } } @@ -237,6 +273,8 @@ impl serde::Serialize for Account { code_hash: self.code_hash(), storage_usage: self.storage_usage(), version, + global_contract_hash: self.global_contract_hash(), + global_contract_account_id: self.global_contract_account_id(), }; repr.serialize(serializer) } @@ -401,6 +439,8 @@ mod tests { code_hash: old_account.code_hash, storage_usage: old_account.storage_usage, version: AccountVersion::V1, + global_contract_hash: None, + global_contract_account_id: None, }; let actual_serde_repr: SerdeAccount = serde_json::from_str(&serialized_account).unwrap(); assert_eq!(actual_serde_repr, expected_serde_repr); @@ -439,6 +479,8 @@ mod tests { locked: 100_000, code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, + global_contract_hash: None, + global_contract_account_id: None, }; let account = Account::V2(account_v2.clone()); @@ -449,6 +491,8 @@ mod tests { code_hash: account_v2.code_hash, storage_usage: account_v2.storage_usage, version: AccountVersion::V2, + global_contract_hash: None, + global_contract_account_id: None, }; let actual_serde_repr: SerdeAccount = serde_json::from_str(&serialized_account).unwrap(); assert_eq!(actual_serde_repr, expected_serde_repr); @@ -464,6 +508,8 @@ mod tests { locked: 100_000, code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, + global_contract_hash: None, + global_contract_account_id: None, }; let account = Account::V2(account_v2); let serialized_account = borsh::to_vec(&account).unwrap(); @@ -471,4 +517,34 @@ mod tests { ::deserialize(&mut &serialized_account[..]).unwrap(); assert_eq!(deserialized_account, account); } + + #[test] + fn test_account_v2_serde_deserialization_cannot_have_both_local_and_global_contract() { + let id = AccountId::try_from("test.near".to_string()).unwrap(); + let account_v2 = AccountV2 { + amount: 10_000_000, + locked: 100_000, + code_hash: CryptoHash::hash_bytes(&[42]), + storage_usage: 1000, + global_contract_hash: Some(CryptoHash::hash_bytes(&[42])), + global_contract_account_id: Some(id.clone()), + }; + let account = Account::V2(account_v2.clone()); + + let serialized_account = serde_json::to_string(&account).unwrap(); + let expected_serde_repr = SerdeAccount { + amount: account_v2.amount, + locked: account_v2.locked, + code_hash: account_v2.code_hash, + storage_usage: account_v2.storage_usage, + version: AccountVersion::V2, + global_contract_hash: Some(CryptoHash::hash_bytes(&[42])), + global_contract_account_id: Some(id), + }; + let actual_serde_repr: SerdeAccount = serde_json::from_str(&serialized_account).unwrap(); + assert_eq!(actual_serde_repr, expected_serde_repr); + + let deserialization_attempt: Result = serde_json::from_str(&serialized_account); + assert!(deserialization_attempt.is_err()); + } } From 05887a24b5ca25a91dc6326774e55fb6aec4dcbd Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 17:28:29 +0100 Subject: [PATCH 02/43] remove duplicate --- core/primitives-core/src/account.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index e0a7eee46de..8f0634677d0 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -530,19 +530,7 @@ mod tests { global_contract_account_id: Some(id.clone()), }; let account = Account::V2(account_v2.clone()); - let serialized_account = serde_json::to_string(&account).unwrap(); - let expected_serde_repr = SerdeAccount { - amount: account_v2.amount, - locked: account_v2.locked, - code_hash: account_v2.code_hash, - storage_usage: account_v2.storage_usage, - version: AccountVersion::V2, - global_contract_hash: Some(CryptoHash::hash_bytes(&[42])), - global_contract_account_id: Some(id), - }; - let actual_serde_repr: SerdeAccount = serde_json::from_str(&serialized_account).unwrap(); - assert_eq!(actual_serde_repr, expected_serde_repr); let deserialization_attempt: Result = serde_json::from_str(&serialized_account); assert!(deserialization_attempt.is_err()); From f2fd27b2c126da8f8eeb1fa0e611a0002f63504a Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 17:42:07 +0100 Subject: [PATCH 03/43] clippy --- core/primitives-core/src/account.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 8f0634677d0..2003decd281 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -527,9 +527,9 @@ mod tests { code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, global_contract_hash: Some(CryptoHash::hash_bytes(&[42])), - global_contract_account_id: Some(id.clone()), + global_contract_account_id: Some(id), }; - let account = Account::V2(account_v2.clone()); + let account = Account::V2(account_v2); let serialized_account = serde_json::to_string(&account).unwrap(); let deserialization_attempt: Result = serde_json::from_str(&serialized_account); From 498a9ad8bb3379852b377ccf4b22d94668c04cc5 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 22:51:38 +0100 Subject: [PATCH 04/43] . --- core/primitives-core/src/account.rs | 131 ++++++++++++++++++------ integration-tests/src/node/mod.rs | 2 +- runtime/runtime/src/actions.rs | 2 +- runtime/runtime/src/verifier.rs | 2 +- test-utils/testlib/src/runtime_utils.rs | 2 +- tools/amend-genesis/src/lib.rs | 2 +- 6 files changed, 104 insertions(+), 37 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 2003decd281..b28c8fb4968 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -68,15 +68,45 @@ impl AccountV1 { AccountV2 { amount: self.amount, locked: self.locked, - code_hash: self.code_hash, storage_usage: self.storage_usage, - global_contract_hash: None, - global_contract_account_id: None, + account_contract: if self.code_hash == CryptoHash::default() { + AccountContract::None + } else { + AccountContract::LocalContract(self.code_hash) + }, + } + } +} + +#[derive( + BorshSerialize, + BorshDeserialize, + serde::Serialize, + serde::Deserialize, + PartialEq, + Eq, + Debug, + Clone, + ProtocolSchema, +)] +pub enum AccountContract { + None, + LocalContract(CryptoHash), + GlobalContractHash(CryptoHash), + GlobalContractByAccount(AccountId, CryptoHash), +} + +impl From for CryptoHash { + fn from(contract: AccountContract) -> Self { + match contract { + AccountContract::None => CryptoHash::default(), + AccountContract::LocalContract(hash) => hash, + AccountContract::GlobalContractHash(hash) => hash, + AccountContract::GlobalContractByAccount(_, hash) => hash, } } } -// TODO(global-contract): add new field #[derive( BorshSerialize, BorshDeserialize, @@ -93,13 +123,10 @@ pub struct AccountV2 { amount: Balance, /// The amount locked due to staking. locked: Balance, - /// Hash of the code stored in the storage for this account. - code_hash: CryptoHash, /// Storage used by the given account, includes account id, this struct, access keys and other data. storage_usage: StorageUsage, - /// Global contracts fields - global_contract_hash: Option, - global_contract_account_id: Option, + /// Type of contract deployed to this account, if any. + account_contract: AccountContract, } impl Account { @@ -140,7 +167,7 @@ impl Account { pub fn code_hash(&self) -> CryptoHash { match self { Self::V1(account) => account.code_hash, - Self::V2(account) => account.code_hash, + Self::V2(account) => account.account_contract.clone().into(), } } @@ -164,7 +191,11 @@ impl Account { pub fn global_contract_hash(&self) -> Option { match self { Self::V1(_) => None, - Self::V2(account) => account.global_contract_hash, + Self::V2(account) => match account.account_contract.clone() { + AccountContract::GlobalContractHash(hash) + | AccountContract::GlobalContractByAccount(_, hash) => Some(hash), + _ => None, + }, } } @@ -172,7 +203,24 @@ impl Account { pub fn global_contract_account_id(&self) -> Option { match self { Self::V1(_) => None, - Self::V2(account) => account.global_contract_account_id.clone(), + Self::V2(account) => match account.account_contract.clone() { + AccountContract::GlobalContractByAccount(account_id, _) => Some(account_id), + _ => None, + }, + } + } + + #[inline] + pub fn account_contract(&self) -> AccountContract { + match self { + Self::V1(account) => { + if account.code_hash == CryptoHash::default() { + AccountContract::None + } else { + AccountContract::LocalContract(account.code_hash) + } + } + Self::V2(account) => account.account_contract.clone(), } } @@ -193,10 +241,15 @@ impl Account { } #[inline] - pub fn set_code_hash(&mut self, code_hash: CryptoHash) { + pub fn set_local_code_hash(&mut self, code_hash: CryptoHash) { match self { Self::V1(account) => account.code_hash = code_hash, - Self::V2(account) => account.code_hash = code_hash, + Self::V2(account) => match account.account_contract { + AccountContract::None | AccountContract::LocalContract(_) => { + account.account_contract = AccountContract::LocalContract(code_hash) + } + _ => panic!("Cannot set local code hash for a global contract"), + }, } } @@ -249,14 +302,31 @@ impl<'de> serde::Deserialize<'de> for Account { code_hash: account_data.code_hash, storage_usage: account_data.storage_usage, })), - AccountVersion::V2 => Ok(Account::V2(AccountV2 { - amount: account_data.amount, - locked: account_data.locked, - code_hash: account_data.code_hash, - storage_usage: account_data.storage_usage, - global_contract_hash: account_data.global_contract_hash, - global_contract_account_id: account_data.global_contract_account_id, - })), + AccountVersion::V2 => { + let account_contract = account_data.global_contract_hash.map_or_else( + || { + if account_data.code_hash == CryptoHash::default() { + AccountContract::None + } else { + AccountContract::LocalContract(account_data.code_hash) + } + }, + |hash| { + account_data + .global_contract_account_id + .map_or(AccountContract::GlobalContractHash(hash), |account_id| { + AccountContract::GlobalContractByAccount(account_id, hash) + }) + }, + ); + + Ok(Account::V2(AccountV2 { + amount: account_data.amount, + locked: account_data.locked, + storage_usage: account_data.storage_usage, + account_contract, + })) + } } } } @@ -477,10 +547,8 @@ mod tests { let account_v2 = AccountV2 { amount: 10_000_000, locked: 100_000, - code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, - global_contract_hash: None, - global_contract_account_id: None, + account_contract: AccountContract::LocalContract(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2.clone()); @@ -488,7 +556,7 @@ mod tests { let expected_serde_repr = SerdeAccount { amount: account_v2.amount, locked: account_v2.locked, - code_hash: account_v2.code_hash, + code_hash: account_v2.account_contract.into(), storage_usage: account_v2.storage_usage, version: AccountVersion::V2, global_contract_hash: None, @@ -506,10 +574,8 @@ mod tests { let account_v2 = AccountV2 { amount: 10_000_000, locked: 100_000, - code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, - global_contract_hash: None, - global_contract_account_id: None, + account_contract: AccountContract::GlobalContractHash(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2); let serialized_account = borsh::to_vec(&account).unwrap(); @@ -524,10 +590,11 @@ mod tests { let account_v2 = AccountV2 { amount: 10_000_000, locked: 100_000, - code_hash: CryptoHash::hash_bytes(&[42]), storage_usage: 1000, - global_contract_hash: Some(CryptoHash::hash_bytes(&[42])), - global_contract_account_id: Some(id), + account_contract: AccountContract::GlobalContractByAccount( + id, + CryptoHash::hash_bytes(&[42]), + ), }; let account = Account::V2(account_v2); let serialized_account = serde_json::to_string(&account).unwrap(); diff --git a/integration-tests/src/node/mod.rs b/integration-tests/src/node/mod.rs index 8f44eb5e7b2..84e9ed41562 100644 --- a/integration-tests/src/node/mod.rs +++ b/integration-tests/src/node/mod.rs @@ -171,7 +171,7 @@ pub fn create_nodes_from_seeds(seeds: Vec) -> Vec { if let StateRecord::Account { account_id, account } = record { if account_id == &seed { found_account_record = true; - account.set_code_hash(*ContractCode::new(code.to_vec(), None).hash()); + account.set_local_code_hash(*ContractCode::new(code.to_vec(), None).hash()); } } } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 99c1b59cc91..525f7636419 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -598,7 +598,7 @@ pub(crate) fn action_deploy_contract( )) })?, ); - account.set_code_hash(*code.hash()); + account.set_local_code_hash(*code.hash()); // Legacy: populate the mapping from `AccountId => sha256(code)` thus making contracts part of // The State. For the time being we are also relying on the `TrieUpdate` to actually write the // contracts into the storage as part of the commit routine, however no code should be relying diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index daae0d4df69..e93052abf03 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -714,7 +714,7 @@ mod tests { account_id.clone(), &ContractCode::new(code.clone(), Some(code_hash)), ); - initial_account.set_code_hash(code_hash); + initial_account.set_local_code_hash(code_hash); initial_account.set_storage_usage( initial_account.storage_usage().checked_add(code.len() as u64).unwrap(), ); diff --git a/test-utils/testlib/src/runtime_utils.rs b/test-utils/testlib/src/runtime_utils.rs index eafbf8396be..1cc24ecb6e8 100644 --- a/test-utils/testlib/src/runtime_utils.rs +++ b/test-utils/testlib/src/runtime_utils.rs @@ -39,7 +39,7 @@ pub fn add_contract(genesis: &mut Genesis, account_id: &AccountId, code: Vec if let StateRecord::Account { account_id: record_account_id, ref mut account } = record { if record_account_id == account_id { is_account_record_found = true; - account.set_code_hash(hash); + account.set_local_code_hash(hash); } } } diff --git a/tools/amend-genesis/src/lib.rs b/tools/amend-genesis/src/lib.rs index eb36a93e27e..a7cd6fae9b4 100644 --- a/tools/amend-genesis/src/lib.rs +++ b/tools/amend-genesis/src/lib.rs @@ -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_code_hash(existing.code_hash()); + account.set_local_code_hash(existing.code_hash()); if self.amount_needed { set_total_balance(account, existing); } From a95fe4592270506a17f1c8560ebe21c9ad377512 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 22:56:07 +0100 Subject: [PATCH 05/43] simplify --- core/primitives-core/src/account.rs | 77 +++++++++++++---------------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index b28c8fb4968..5eaccd22968 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -69,11 +69,7 @@ impl AccountV1 { amount: self.amount, locked: self.locked, storage_usage: self.storage_usage, - account_contract: if self.code_hash == CryptoHash::default() { - AccountContract::None - } else { - AccountContract::LocalContract(self.code_hash) - }, + account_contract: convert_code_hash(self.code_hash), } } } @@ -89,24 +85,32 @@ impl AccountV1 { Clone, ProtocolSchema, )] -pub enum AccountContract { +pub enum AccountContractType { None, LocalContract(CryptoHash), - GlobalContractHash(CryptoHash), + GlobalContract(CryptoHash), GlobalContractByAccount(AccountId, CryptoHash), } -impl From for CryptoHash { - fn from(contract: AccountContract) -> Self { +impl From for CryptoHash { + fn from(contract: AccountContractType) -> Self { match contract { - AccountContract::None => CryptoHash::default(), - AccountContract::LocalContract(hash) => hash, - AccountContract::GlobalContractHash(hash) => hash, - AccountContract::GlobalContractByAccount(_, hash) => hash, + AccountContractType::None => CryptoHash::default(), + AccountContractType::LocalContract(hash) => hash, + AccountContractType::GlobalContract(hash) => hash, + AccountContractType::GlobalContractByAccount(_, hash) => hash, } } } +fn convert_code_hash(code_hash: CryptoHash) -> AccountContractType { + if code_hash == CryptoHash::default() { + AccountContractType::None + } else { + AccountContractType::LocalContract(code_hash) + } +} + #[derive( BorshSerialize, BorshDeserialize, @@ -126,7 +130,7 @@ pub struct AccountV2 { /// Storage used by the given account, includes account id, this struct, access keys and other data. storage_usage: StorageUsage, /// Type of contract deployed to this account, if any. - account_contract: AccountContract, + account_contract: AccountContractType, } impl Account { @@ -192,8 +196,8 @@ impl Account { match self { Self::V1(_) => None, Self::V2(account) => match account.account_contract.clone() { - AccountContract::GlobalContractHash(hash) - | AccountContract::GlobalContractByAccount(_, hash) => Some(hash), + AccountContractType::GlobalContract(hash) + | AccountContractType::GlobalContractByAccount(_, hash) => Some(hash), _ => None, }, } @@ -204,22 +208,16 @@ impl Account { match self { Self::V1(_) => None, Self::V2(account) => match account.account_contract.clone() { - AccountContract::GlobalContractByAccount(account_id, _) => Some(account_id), + AccountContractType::GlobalContractByAccount(account_id, _) => Some(account_id), _ => None, }, } } #[inline] - pub fn account_contract(&self) -> AccountContract { + pub fn account_contract(&self) -> AccountContractType { match self { - Self::V1(account) => { - if account.code_hash == CryptoHash::default() { - AccountContract::None - } else { - AccountContract::LocalContract(account.code_hash) - } - } + Self::V1(account) => convert_code_hash(account.code_hash), Self::V2(account) => account.account_contract.clone(), } } @@ -245,8 +243,8 @@ impl Account { match self { Self::V1(account) => account.code_hash = code_hash, Self::V2(account) => match account.account_contract { - AccountContract::None | AccountContract::LocalContract(_) => { - account.account_contract = AccountContract::LocalContract(code_hash) + AccountContractType::None | AccountContractType::LocalContract(_) => { + account.account_contract = AccountContractType::LocalContract(code_hash) } _ => panic!("Cannot set local code hash for a global contract"), }, @@ -304,19 +302,14 @@ impl<'de> serde::Deserialize<'de> for Account { })), AccountVersion::V2 => { let account_contract = account_data.global_contract_hash.map_or_else( - || { - if account_data.code_hash == CryptoHash::default() { - AccountContract::None - } else { - AccountContract::LocalContract(account_data.code_hash) - } - }, + || convert_code_hash(account_data.code_hash), |hash| { - account_data - .global_contract_account_id - .map_or(AccountContract::GlobalContractHash(hash), |account_id| { - AccountContract::GlobalContractByAccount(account_id, hash) - }) + account_data.global_contract_account_id.map_or( + AccountContractType::GlobalContract(hash), + |account_id| { + AccountContractType::GlobalContractByAccount(account_id, hash) + }, + ) }, ); @@ -548,7 +541,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContract::LocalContract(CryptoHash::hash_bytes(&[42])), + account_contract: AccountContractType::LocalContract(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2.clone()); @@ -575,7 +568,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContract::GlobalContractHash(CryptoHash::hash_bytes(&[42])), + account_contract: AccountContractType::GlobalContract(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2); let serialized_account = borsh::to_vec(&account).unwrap(); @@ -591,7 +584,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContract::GlobalContractByAccount( + account_contract: AccountContractType::GlobalContractByAccount( id, CryptoHash::hash_bytes(&[42]), ), From 625d5929e4c051761cfa44fa4bd4c1d0ba8a6a79 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 22:58:44 +0100 Subject: [PATCH 06/43] . --- core/primitives-core/src/account.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 5eaccd22968..7b52f8a6f5d 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -96,9 +96,9 @@ impl From for CryptoHash { fn from(contract: AccountContractType) -> Self { match contract { AccountContractType::None => CryptoHash::default(), - AccountContractType::LocalContract(hash) => hash, - AccountContractType::GlobalContract(hash) => hash, - AccountContractType::GlobalContractByAccount(_, hash) => hash, + AccountContractType::LocalContract(hash) + | AccountContractType::GlobalContract(hash) + | AccountContractType::GlobalContractByAccount(_, hash) => hash, } } } From e4b30e6849daabd5204a786c3444802b191813d7 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 5 Feb 2025 23:13:34 +0100 Subject: [PATCH 07/43] fix --- core/primitives-core/src/account.rs | 8 -------- core/store/src/db/colddb.rs | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 7b52f8a6f5d..1e4f174a526 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -214,14 +214,6 @@ impl Account { } } - #[inline] - pub fn account_contract(&self) -> AccountContractType { - match self { - Self::V1(account) => convert_code_hash(account.code_hash), - Self::V2(account) => account.account_contract.clone(), - } - } - #[inline] pub fn set_amount(&mut self, amount: Balance) { match self { diff --git a/core/store/src/db/colddb.rs b/core/store/src/db/colddb.rs index c69a6ce7e7e..c9d2a1f9eb5 100644 --- a/core/store/src/db/colddb.rs +++ b/core/store/src/db/colddb.rs @@ -208,7 +208,7 @@ mod test { } } fn hash(chunk: &[u8]) -> String { - crate::CryptoHash::from(chunk.try_into().unwrap()).to_string() + crate::CryptoHash::try_from(chunk).unwrap().to_string() } match key.len() { 8 => chunk(key), From b2d8efa5478e141843195bed4add4f6e5f475898 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 00:20:44 +0100 Subject: [PATCH 08/43] schema --- tools/protocol-schema-check/res/protocol_schema.toml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/protocol-schema-check/res/protocol_schema.toml b/tools/protocol-schema-check/res/protocol_schema.toml index 2a0471983a9..7cba4e05165 100644 --- a/tools/protocol-schema-check/res/protocol_schema.toml +++ b/tools/protocol-schema-check/res/protocol_schema.toml @@ -1,8 +1,9 @@ AccessKey = 433079403 AccessKeyPermission = 885623561 -Account = 1842241023 +Account = 1022111109 +AccountContractType = 844412571 AccountV1 = 3570440720 -AccountV2 = 337859929 +AccountV2 = 3213928698 AccountVersion = 3672019478 Action = 708080604 ActionCosts = 3115555891 @@ -225,7 +226,7 @@ RoutedMessageBody = 2711205418 RoutingTableUpdate = 2987752645 Secp256K1PublicKey = 4117078281 Secp256K1Signature = 3687154735 -SerdeAccount = 4024521980 +SerdeAccount = 1519554694 ServerError = 3111148975 ShardChunk = 1084327441 ShardChunkHeader = 2471921769 From e73e9d9a3ed6a28d037406e9ff4b1e0a5c77eaa6 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 12:18:31 +0100 Subject: [PATCH 09/43] address comments --- core/primitives-core/src/account.rs | 139 ++++++++++++++---------- core/primitives/src/views.rs | 2 +- core/store/src/genesis/state_applier.rs | 2 +- integration-tests/src/node/mod.rs | 5 +- runtime/runtime/src/actions.rs | 10 +- runtime/runtime/src/lib.rs | 6 +- runtime/runtime/src/pipelining.rs | 2 +- runtime/runtime/src/state_viewer/mod.rs | 17 ++- runtime/runtime/src/verifier.rs | 3 +- test-utils/testlib/src/runtime_utils.rs | 2 +- tools/amend-genesis/src/lib.rs | 9 +- 11 files changed, 114 insertions(+), 83 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 1e4f174a526..2db430379e8 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -69,7 +69,7 @@ impl AccountV1 { amount: self.amount, locked: self.locked, storage_usage: self.storage_usage, - account_contract: convert_code_hash(self.code_hash), + account_contract: AccountContract::from_local_code_hash(self.code_hash), } } } @@ -85,29 +85,27 @@ impl AccountV1 { Clone, ProtocolSchema, )] -pub enum AccountContractType { +pub enum AccountContract { None, - LocalContract(CryptoHash), - GlobalContract(CryptoHash), - GlobalContractByAccount(AccountId, CryptoHash), + Local(CryptoHash), + Global(CryptoHash), + GlobalByAccount(AccountId), } -impl From for CryptoHash { - fn from(contract: AccountContractType) -> Self { - match contract { - AccountContractType::None => CryptoHash::default(), - AccountContractType::LocalContract(hash) - | AccountContractType::GlobalContract(hash) - | AccountContractType::GlobalContractByAccount(_, hash) => hash, +impl AccountContract { + pub fn to_code_hash(&self) -> CryptoHash { + match self { + AccountContract::None | AccountContract::GlobalByAccount(_) => CryptoHash::default(), + AccountContract::Local(hash) | AccountContract::Global(hash) => *hash, } } -} -fn convert_code_hash(code_hash: CryptoHash) -> AccountContractType { - if code_hash == CryptoHash::default() { - AccountContractType::None - } else { - AccountContractType::LocalContract(code_hash) + fn from_local_code_hash(code_hash: CryptoHash) -> AccountContract { + if code_hash == CryptoHash::default() { + AccountContract::None + } else { + AccountContract::Local(code_hash) + } } } @@ -130,7 +128,7 @@ pub struct AccountV2 { /// Storage used by the given account, includes account id, this struct, access keys and other data. storage_usage: StorageUsage, /// Type of contract deployed to this account, if any. - account_contract: AccountContractType, + account_contract: AccountContract, } impl Account { @@ -168,10 +166,10 @@ impl Account { } #[inline] - pub fn code_hash(&self) -> CryptoHash { + pub fn contract(&self) -> AccountContract { match self { - Self::V1(account) => account.code_hash, - Self::V2(account) => account.account_contract.clone().into(), + Self::V1(account) => AccountContract::from_local_code_hash(account.code_hash), + Self::V2(account) => account.account_contract.clone(), } } @@ -196,19 +194,18 @@ impl Account { match self { Self::V1(_) => None, Self::V2(account) => match account.account_contract.clone() { - AccountContractType::GlobalContract(hash) - | AccountContractType::GlobalContractByAccount(_, hash) => Some(hash), + AccountContract::Global(hash) => Some(hash), _ => None, }, } } #[inline] - pub fn global_contract_account_id(&self) -> Option { + pub fn global_contract_account_id(&self) -> Option<&AccountId> { match self { Self::V1(_) => None, - Self::V2(account) => match account.account_contract.clone() { - AccountContractType::GlobalContractByAccount(account_id, _) => Some(account_id), + Self::V2(account) => match &account.account_contract { + AccountContract::GlobalByAccount(account_id) => Some(account_id), _ => None, }, } @@ -231,15 +228,20 @@ impl Account { } #[inline] - pub fn set_local_code_hash(&mut self, code_hash: CryptoHash) { + pub fn set_contract(&mut self, contract: AccountContract) { match self { - Self::V1(account) => account.code_hash = code_hash, - Self::V2(account) => match account.account_contract { - AccountContractType::None | AccountContractType::LocalContract(_) => { - account.account_contract = AccountContractType::LocalContract(code_hash) - } - _ => panic!("Cannot set local code hash for a global contract"), - }, + Self::V1(_) => { + let account_v2 = AccountV2 { + amount: self.amount(), + locked: self.locked(), + storage_usage: self.storage_usage(), + account_contract: contract, + }; + *self = Self::V2(account_v2); + } + Self::V2(account) => { + account.account_contract = contract; + } } } @@ -266,7 +268,9 @@ struct SerdeAccount { #[serde(default)] version: AccountVersion, /// Global contracts fields + #[serde(default, skip_serializing_if = "Option::is_none")] global_contract_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] global_contract_account_id: Option, } @@ -284,6 +288,13 @@ impl<'de> serde::Deserialize<'de> for Account { "An Account can't contain both a local and global contract", )); } + if account_data.global_contract_hash.is_some() + && account_data.global_contract_account_id.is_some() + { + return Err(serde::de::Error::custom( + "An Account can't contain both types of global contracts", + )); + } match account_data.version { AccountVersion::V1 => Ok(Account::V1(AccountV1 { @@ -293,17 +304,13 @@ impl<'de> serde::Deserialize<'de> for Account { storage_usage: account_data.storage_usage, })), AccountVersion::V2 => { - let account_contract = account_data.global_contract_hash.map_or_else( - || convert_code_hash(account_data.code_hash), - |hash| { - account_data.global_contract_account_id.map_or( - AccountContractType::GlobalContract(hash), - |account_id| { - AccountContractType::GlobalContractByAccount(account_id, hash) - }, - ) + let account_contract = match account_data.global_contract_account_id { + Some(account_id) => AccountContract::GlobalByAccount(account_id), + None => match account_data.global_contract_hash { + Some(hash) => AccountContract::Global(hash), + None => AccountContract::from_local_code_hash(account_data.code_hash), }, - ); + }; Ok(Account::V2(AccountV2 { amount: account_data.amount, @@ -325,11 +332,11 @@ impl serde::Serialize for Account { let repr = SerdeAccount { amount: self.amount(), locked: self.locked(), - code_hash: self.code_hash(), + code_hash: self.contract().to_code_hash(), storage_usage: self.storage_usage(), version, global_contract_hash: self.global_contract_hash(), - global_contract_account_id: self.global_contract_account_id(), + global_contract_account_id: self.global_contract_account_id().cloned(), }; repr.serialize(serializer) } @@ -533,7 +540,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContractType::LocalContract(CryptoHash::hash_bytes(&[42])), + account_contract: AccountContract::Local(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2.clone()); @@ -541,7 +548,7 @@ mod tests { let expected_serde_repr = SerdeAccount { amount: account_v2.amount, locked: account_v2.locked, - code_hash: account_v2.account_contract.into(), + code_hash: account_v2.account_contract.to_code_hash(), storage_usage: account_v2.storage_usage, version: AccountVersion::V2, global_contract_hash: None, @@ -560,7 +567,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContractType::GlobalContract(CryptoHash::hash_bytes(&[42])), + account_contract: AccountContract::Global(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2); let serialized_account = borsh::to_vec(&account).unwrap(); @@ -570,21 +577,35 @@ mod tests { } #[test] - fn test_account_v2_serde_deserialization_cannot_have_both_local_and_global_contract() { + fn test_account_v2_serde_deserialization_fails() { let id = AccountId::try_from("test.near".to_string()).unwrap(); - let account_v2 = AccountV2 { + let code_hash = CryptoHash::hash_bytes(&[42]); + + let mut serde_repr = SerdeAccount { amount: 10_000_000, locked: 100_000, + code_hash, storage_usage: 1000, - account_contract: AccountContractType::GlobalContractByAccount( - id, - CryptoHash::hash_bytes(&[42]), - ), + version: AccountVersion::V2, + global_contract_hash: None, + global_contract_account_id: Some(id.clone()), }; - let account = Account::V2(account_v2); - let serialized_account = serde_json::to_string(&account).unwrap(); - let deserialization_attempt: Result = serde_json::from_str(&serialized_account); + let serde_string = serde_json::to_string(&serde_repr).unwrap(); + let deserialization_attempt: Result = serde_json::from_str(&serde_string); + assert!(deserialization_attempt.is_err()); + + serde_repr.global_contract_hash = Some(code_hash); + serde_repr.global_contract_account_id = None; + let serde_string = serde_json::to_string(&serde_repr).unwrap(); + let deserialization_attempt: Result = serde_json::from_str(&serde_string); + assert!(deserialization_attempt.is_err()); + + serde_repr.code_hash = CryptoHash::default(); + serde_repr.global_contract_hash = Some(code_hash); + serde_repr.global_contract_account_id = Some(id); + let serde_string = serde_json::to_string(&serde_repr).unwrap(); + let deserialization_attempt: Result = serde_json::from_str(&serde_string); assert!(deserialization_attempt.is_err()); } } diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 7c78e4b5261..1d4c93ca6c8 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -87,7 +87,7 @@ impl From<&Account> for AccountView { AccountView { amount: account.amount(), locked: account.locked(), - code_hash: account.code_hash(), + code_hash: account.contract().to_code_hash(), storage_usage: account.storage_usage(), storage_paid_at: 0, } diff --git a/core/store/src/genesis/state_applier.rs b/core/store/src/genesis/state_applier.rs index 7c944764133..839c6f66e99 100644 --- a/core/store/src/genesis/state_applier.rs +++ b/core/store/src/genesis/state_applier.rs @@ -206,7 +206,7 @@ impl GenesisStateApplier { get_account(state_update, account_id).expect("Failed to read state") { state_update.set_code(account_id.clone(), &code); - assert_eq!(*code.hash(), acc.code_hash()); + assert_eq!(*code.hash(), acc.contract().to_code_hash()); } else { tracing::error!( target: "runtime", diff --git a/integration-tests/src/node/mod.rs b/integration-tests/src/node/mod.rs index 84e9ed41562..4b3c8714b9d 100644 --- a/integration-tests/src/node/mod.rs +++ b/integration-tests/src/node/mod.rs @@ -9,6 +9,7 @@ use near_chain_configs::Genesis; use near_chain_configs::MutableConfigValue; use near_crypto::Signer; use near_jsonrpc_primitives::errors::ServerError; +use near_primitives::account::AccountContract; use near_primitives::num_rational::Ratio; use near_primitives::state_record::StateRecord; use near_primitives::transaction::SignedTransaction; @@ -171,7 +172,9 @@ pub fn create_nodes_from_seeds(seeds: Vec) -> Vec { if let StateRecord::Account { account_id, account } = record { if account_id == &seed { found_account_record = true; - account.set_local_code_hash(*ContractCode::new(code.to_vec(), None).hash()); + account.set_contract(AccountContract::Local( + *ContractCode::new(code.to_vec(), None).hash(), + )); } } } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 525f7636419..5244e978645 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -7,7 +7,7 @@ use crate::receipt_manager::ReceiptManager; use crate::{metrics, ActionResult, ApplyState}; use near_crypto::PublicKey; use near_parameters::{AccountCreationConfig, ActionCosts, RuntimeConfig, RuntimeFeesConfig}; -use near_primitives::account::{AccessKey, AccessKeyPermission, Account}; +use near_primitives::account::{AccessKey, AccessKeyPermission, Account, AccountContract}; use near_primitives::action::delegate::{DelegateAction, SignedDelegateAction}; use near_primitives::action::{ DeployGlobalContractAction, GlobalContractDeployMode, GlobalContractIdentifier, @@ -584,7 +584,7 @@ pub(crate) fn action_deploy_contract( let prev_code_len = get_code_len_or_default( state_update, account_id.clone(), - account.code_hash(), + account.contract().to_code_hash(), current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); @@ -598,7 +598,7 @@ pub(crate) fn action_deploy_contract( )) })?, ); - account.set_local_code_hash(*code.hash()); + account.set_contract(AccountContract::Local(*code.hash())); // Legacy: populate the mapping from `AccountId => sha256(code)` thus making contracts part of // The State. For the time being we are also relying on the `TrieUpdate` to actually write the // contracts into the storage as part of the commit routine, however no code should be relying @@ -665,7 +665,7 @@ pub(crate) fn action_delete_account( let code_len = get_code_len_or_default( state_update, account_id.clone(), - account.code_hash(), + account.contract().to_code_hash(), current_protocol_version, )?; debug_assert!(code_len == 0 || account_storage_usage > code_len, @@ -1349,7 +1349,7 @@ mod tests { assert!(res.is_ok()); test_delete_large_account( &account_id, - &account.code_hash(), + &account.contract().to_code_hash(), storage_usage, &mut state_update, ) diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 7356932a73c..8fc3a879a8b 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -490,7 +490,7 @@ impl Runtime { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); let contract = preparation_pipeline.get_contract( receipt, - account.code_hash(), + account.contract().to_code_hash(), action_index, None, ); @@ -506,7 +506,7 @@ impl Runtime { account_id, function_call, action_hash, - account.code_hash(), + account.contract().to_code_hash(), &apply_state.config, is_last_action, epoch_info_provider, @@ -1587,7 +1587,7 @@ impl Runtime { // Recompute contract code hash. let code = ContractCode::new(code, None); state_update.set_code(account_id, &code); - assert_eq!(*code.hash(), acc.code_hash()); + assert_eq!(*code.hash(), acc.contract().to_code_hash()); } StateRecord::AccessKey { account_id, public_key, access_key } => { set_access_key(state_update, account_id, public_key, &access_key); diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index a093403c239..e0b41f688fe 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -136,7 +136,7 @@ impl ReceiptPreparationPipeline { } Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; - let code_hash = account.code_hash(); + let code_hash = account.contract().to_code_hash(); let key = PrepareTaskKey { receipt_id: receipt.get_hash(), action_index }; let gas_counter = self.gas_counter(view_config.as_ref(), function_call.gas); let entry = match self.map.entry(key) { diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index e83f5027bea..8035e021724 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -92,11 +92,11 @@ impl TrieViewer { account_id: &AccountId, ) -> Result { let account = self.view_account(state_update, account_id)?; - state_update.get_code(account_id.clone(), account.code_hash())?.ok_or_else(|| { - errors::ViewContractCodeError::NoContractCode { + state_update.get_code(account_id.clone(), account.contract().to_code_hash())?.ok_or_else( + || errors::ViewContractCodeError::NoContractCode { contract_account_id: account_id.clone(), - } - }) + }, + ) } pub fn view_access_key( @@ -150,7 +150,7 @@ impl TrieViewer { match get_account(state_update, account_id)? { Some(account) => { let code_len = state_update - .get_code_len(account_id.clone(), account.code_hash())? + .get_code_len(account_id.clone(), account.contract().to_code_hash())? .unwrap_or_default() as u64; if let Some(limit) = self.state_size_limit { if account.storage_usage().saturating_sub(code_len) > limit { @@ -255,7 +255,12 @@ impl TrieViewer { state_update.contract_storage(), ); let view_config = Some(ViewConfig { max_gas_burnt: self.max_gas_burnt_view }); - let contract = pipeline.get_contract(&receipt, account.code_hash(), 0, view_config.clone()); + let contract = pipeline.get_contract( + &receipt, + account.contract().to_code_hash(), + 0, + view_config.clone(), + ); let mut runtime_ext = RuntimeExt::new( &mut state_update, diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index e93052abf03..2704f589976 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -714,7 +714,8 @@ mod tests { account_id.clone(), &ContractCode::new(code.clone(), Some(code_hash)), ); - initial_account.set_local_code_hash(code_hash); + initial_account + .set_contract(near_primitives::account::AccountContract::Local(code_hash)); initial_account.set_storage_usage( initial_account.storage_usage().checked_add(code.len() as u64).unwrap(), ); diff --git a/test-utils/testlib/src/runtime_utils.rs b/test-utils/testlib/src/runtime_utils.rs index 1cc24ecb6e8..b8acea4ff74 100644 --- a/test-utils/testlib/src/runtime_utils.rs +++ b/test-utils/testlib/src/runtime_utils.rs @@ -39,7 +39,7 @@ pub fn add_contract(genesis: &mut Genesis, account_id: &AccountId, code: Vec if let StateRecord::Account { account_id: record_account_id, ref mut account } = record { if record_account_id == account_id { is_account_record_found = true; - account.set_local_code_hash(hash); + account.set_contract(near_primitives::account::AccountContract::Local(hash)); } } } diff --git a/tools/amend-genesis/src/lib.rs b/tools/amend-genesis/src/lib.rs index a7cd6fae9b4..9b8573c7fd8 100644 --- a/tools/amend-genesis/src/lib.rs +++ b/tools/amend-genesis/src/lib.rs @@ -2,6 +2,7 @@ use anyhow::Context; use near_chain_configs::{Genesis, GenesisValidationMode, NEAR_BASE}; use near_crypto::PublicKey; +use near_primitives::account::AccountContract; use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; @@ -73,7 +74,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_local_code_hash(existing.code_hash()); + account.set_contract(existing.contract()); if self.amount_needed { set_total_balance(account, existing); } @@ -171,7 +172,7 @@ fn parse_extra_records( near_chain_configs::stream_records_from_file(reader, |r| { match r { StateRecord::Account { account_id, account } => { - if account.code_hash() != CryptoHash::default() { + if account.contract() != AccountContract::None { result = Err(anyhow::anyhow!( "FIXME: accounts in --extra-records with code_hash set not supported" )); @@ -517,7 +518,7 @@ mod test { ( account.amount(), account.locked(), - account.code_hash(), + account.contract(), account.storage_usage(), ), ) @@ -555,7 +556,7 @@ mod test { ( account.amount(), account.locked(), - account.code_hash(), + account.contract(), account.storage_usage(), ), ); From 506d96408f07f649c97aa58c33fa35b3fd677ef7 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 12:53:27 +0100 Subject: [PATCH 10/43] schema --- tools/protocol-schema-check/res/protocol_schema.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/protocol-schema-check/res/protocol_schema.toml b/tools/protocol-schema-check/res/protocol_schema.toml index 7cba4e05165..19459cd2d10 100644 --- a/tools/protocol-schema-check/res/protocol_schema.toml +++ b/tools/protocol-schema-check/res/protocol_schema.toml @@ -1,9 +1,9 @@ AccessKey = 433079403 AccessKeyPermission = 885623561 -Account = 1022111109 -AccountContractType = 844412571 +Account = 1423723558 +AccountContract = 2377656612 AccountV1 = 3570440720 -AccountV2 = 3213928698 +AccountV2 = 4150394471 AccountVersion = 3672019478 Action = 708080604 ActionCosts = 3115555891 From a285285dfdab08f9cbe4e268f33c7ad6843e11ab Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 14:30:16 +0100 Subject: [PATCH 11/43] fix --- core/primitives-core/src/account.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 2db430379e8..913a0162fb5 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -329,10 +329,16 @@ impl serde::Serialize for Account { S: serde::Serializer, { let version = self.version(); + let code_hash = match self.contract() { + AccountContract::None + | AccountContract::Global(_) + | AccountContract::GlobalByAccount(_) => CryptoHash::default(), + AccountContract::Local(code_hash) => code_hash, + }; let repr = SerdeAccount { amount: self.amount(), locked: self.locked(), - code_hash: self.contract().to_code_hash(), + code_hash, storage_usage: self.storage_usage(), version, global_contract_hash: self.global_contract_hash(), From 28c6ea112096f148a7d082a62078fd7d1c989a53 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 16:14:03 +0100 Subject: [PATCH 12/43] comments --- Cargo.lock | 1 + chain/chain/src/test_utils/kv_runtime.rs | 2 +- chain/jsonrpc/jsonrpc-tests/Cargo.toml | 1 + .../jsonrpc/jsonrpc-tests/tests/rpc_query.rs | 6 +- chain/rosetta-rpc/src/adapters/mod.rs | 13 +++-- chain/rosetta-rpc/src/lib.rs | 2 +- core/chain-configs/src/genesis_validate.rs | 2 +- core/chain-configs/src/test_genesis.rs | 4 +- core/chain-configs/src/test_utils.rs | 2 +- core/primitives-core/src/account.rs | 56 +++++++++++++++---- core/primitives/src/test_utils.rs | 2 +- core/primitives/src/views.rs | 20 ++++++- core/store/src/genesis/state_applier.rs | 2 +- .../genesis-csv-to-json/src/csv_parser.rs | 2 +- genesis-tools/genesis-populate/src/lib.rs | 2 +- .../client/features/stateless_validation.rs | 2 +- .../tests/client/features/wallet_contract.rs | 2 +- .../src/tests/runtime/state_viewer.rs | 4 +- .../src/tests/standard_cases/mod.rs | 4 +- runtime/runtime/src/actions.rs | 18 +++--- runtime/runtime/src/lib.rs | 6 +- runtime/runtime/src/pipelining.rs | 2 +- runtime/runtime/src/state_viewer/mod.rs | 6 +- runtime/runtime/src/verifier.rs | 5 +- .../runtime/tests/runtime_group_tools/mod.rs | 2 +- test-utils/testlib/src/runtime_utils.rs | 4 +- tools/amend-genesis/src/lib.rs | 4 +- tools/fork-network/src/cli.rs | 2 +- 28 files changed, 113 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8efbf3cfd98..449873b7379 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4678,6 +4678,7 @@ dependencies = [ "near-network", "near-o11y", "near-primitives", + "near-primitives-core", "near-store", "near-time", "serde", diff --git a/chain/chain/src/test_utils/kv_runtime.rs b/chain/chain/src/test_utils/kv_runtime.rs index f220b141f58..684a79a88a8 100644 --- a/chain/chain/src/test_utils/kv_runtime.rs +++ b/chain/chain/src/test_utils/kv_runtime.rs @@ -1208,7 +1208,7 @@ impl RuntimeAdapter for KeyValueRuntime { match request { QueryRequest::ViewAccount { account_id, .. } => Ok(QueryResponse { kind: QueryResponseKind::ViewAccount( - Account::new( + Account::new_v1( self.state.read().unwrap().get(state_root).map_or_else( || 0, |state| *state.amounts.get(account_id).unwrap_or(&0), diff --git a/chain/jsonrpc/jsonrpc-tests/Cargo.toml b/chain/jsonrpc/jsonrpc-tests/Cargo.toml index d38561b6cc8..abacbb26571 100644 --- a/chain/jsonrpc/jsonrpc-tests/Cargo.toml +++ b/chain/jsonrpc/jsonrpc-tests/Cargo.toml @@ -33,6 +33,7 @@ near-jsonrpc-primitives.workspace = true [dev-dependencies] near-actix-test-utils.workspace = true +near-primitives-core.workspace = true [features] test_features = ["near-jsonrpc/test_features"] diff --git a/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs b/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs index ba71d59cdcb..76c2904a6d5 100644 --- a/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs +++ b/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs @@ -13,7 +13,7 @@ use near_jsonrpc_primitives::types::query::QueryResponseKind; use near_jsonrpc_primitives::types::validator::RpcValidatorsOrderedRequest; use near_network::test_utils::wait_or_timeout; use near_o11y::testonly::init_test_logger; -use near_primitives::account::{AccessKey, AccessKeyPermission}; +use near_primitives::account::{AccessKey, AccessKeyPermission, AccountContract}; use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, EpochId, ShardId, SyncCheckpoint}; use near_primitives::views::QueryRequest; @@ -147,7 +147,7 @@ fn test_query_by_path_account() { panic!("queried account, but received something else: {:?}", query_response.kind); }; assert_eq!(account_info.amount, 0); - assert_eq!(account_info.code_hash.as_ref(), &[0; 32]); + assert_eq!(account_info.account_contract, AccountContract::None); assert_eq!(account_info.locked, 0); assert_eq!(account_info.storage_paid_at, 0); assert_eq!(account_info.storage_usage, 0); @@ -192,7 +192,7 @@ fn test_query_account() { panic!("queried account, but received something else: {:?}", query_response.kind); }; assert_eq!(account_info.amount, 0); - assert_eq!(account_info.code_hash.as_ref(), &[0; 32]); + assert_eq!(account_info.account_contract, AccountContract::None); assert_eq!(account_info.locked, 0); assert_eq!(account_info.storage_paid_at, 0); assert_eq!(account_info.storage_usage, 0); diff --git a/chain/rosetta-rpc/src/adapters/mod.rs b/chain/rosetta-rpc/src/adapters/mod.rs index 709224917e7..bd61f29c5d5 100644 --- a/chain/rosetta-rpc/src/adapters/mod.rs +++ b/chain/rosetta-rpc/src/adapters/mod.rs @@ -838,6 +838,7 @@ mod tests { use near_client::test_utils::setup_no_network; use near_crypto::{KeyType, SecretKey}; use near_parameters::{RuntimeConfig, RuntimeConfigView}; + use near_primitives::account::AccountContract; use near_primitives::action::delegate::{DelegateAction, SignedDelegateAction}; use near_primitives::transaction::{Action, TransferAction}; use near_time::Clock; @@ -865,7 +866,7 @@ mod tests { account_id: "nfvalidator1.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 5000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, @@ -880,7 +881,7 @@ mod tests { account_id: "nfvalidator1.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 4000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, @@ -893,7 +894,7 @@ mod tests { account_id: "nfvalidator2.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 7000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, @@ -908,7 +909,7 @@ mod tests { account_id: "nfvalidator2.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 8000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, @@ -921,7 +922,7 @@ mod tests { "nfvalidator1.near".parse().unwrap(), near_primitives::views::AccountView { amount: 4000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, @@ -931,7 +932,7 @@ mod tests { "nfvalidator2.near".parse().unwrap(), near_primitives::views::AccountView { amount: 6000000000000000000, - code_hash: near_primitives::hash::CryptoHash::default(), + account_contract: AccountContract::None, locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, diff --git a/chain/rosetta-rpc/src/lib.rs b/chain/rosetta-rpc/src/lib.rs index 2ff0fec70e1..0c57160bb26 100644 --- a/chain/rosetta-rpc/src/lib.rs +++ b/chain/rosetta-rpc/src/lib.rs @@ -369,7 +369,7 @@ async fn account_balance( Err(crate::errors::ErrorKind::NotFound(_)) => ( block.header.hash, block.header.height, - near_primitives::account::Account::new(0, 0, Default::default(), 0).into(), + near_primitives::account::Account::new_v1(0, 0, Default::default(), 0).into(), ), Err(err) => return Err(err.into()), }; diff --git a/core/chain-configs/src/genesis_validate.rs b/core/chain-configs/src/genesis_validate.rs index b2674cbbae2..058dadabe86 100644 --- a/core/chain-configs/src/genesis_validate.rs +++ b/core/chain-configs/src/genesis_validate.rs @@ -204,7 +204,7 @@ mod test { const VALID_ED25519_RISTRETTO_KEY: &str = "ed25519:KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7"; fn create_account() -> Account { - Account::new(100, 10, Default::default(), 0) + Account::new_v1(100, 10, Default::default(), 0) } #[test] diff --git a/core/chain-configs/src/test_genesis.rs b/core/chain-configs/src/test_genesis.rs index 93044a35370..ef7e2d2c9d0 100644 --- a/core/chain-configs/src/test_genesis.rs +++ b/core/chain-configs/src/test_genesis.rs @@ -444,7 +444,7 @@ impl TestGenesisBuilder { total_supply += user_account.balance; records.push(StateRecord::Account { account_id: user_account.account_id.clone(), - account: Account::new( + account: Account::new_v1( user_account.balance, validator_stake.remove(&user_account.account_id).unwrap_or(0), CryptoHash::default(), @@ -465,7 +465,7 @@ impl TestGenesisBuilder { for (account_id, balance) in validator_stake { records.push(StateRecord::Account { account_id, - account: Account::new(0, balance, CryptoHash::default(), 0), + account: Account::new_v1(0, balance, CryptoHash::default(), 0), }); } diff --git a/core/chain-configs/src/test_utils.rs b/core/chain-configs/src/test_utils.rs index aca9b93a047..5f3f9170074 100644 --- a/core/chain-configs/src/test_utils.rs +++ b/core/chain-configs/src/test_utils.rs @@ -185,7 +185,7 @@ pub fn add_account_with_key( ) { records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new(amount, staked, code_hash, 0), + account: Account::new_v1(amount, staked, code_hash, 0), }); records.push(StateRecord::AccessKey { account_id, diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 913a0162fb5..113ea7a0254 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -93,7 +93,7 @@ pub enum AccountContract { } impl AccountContract { - pub fn to_code_hash(&self) -> CryptoHash { + pub fn local_code(&self) -> CryptoHash { match self { AccountContract::None | AccountContract::GlobalByAccount(_) => CryptoHash::default(), AccountContract::Local(hash) | AccountContract::Global(hash) => *hash, @@ -140,7 +140,7 @@ impl Account { /// differentiate AccountVersion V1 from newer versions. const SERIALIZATION_SENTINEL: u128 = u128::MAX; - pub fn new( + pub fn new_v1( amount: Balance, locked: Balance, code_hash: CryptoHash, @@ -149,6 +149,15 @@ impl Account { Self::V1(AccountV1 { amount, locked, code_hash, storage_usage }) } + pub fn new_v2( + amount: Balance, + locked: Balance, + account_contract: AccountContract, + storage_usage: StorageUsage, + ) -> Self { + Self::V2(AccountV2 { amount, locked, storage_usage, account_contract }) + } + #[inline] pub fn amount(&self) -> Balance { match self { @@ -230,15 +239,16 @@ impl Account { #[inline] pub fn set_contract(&mut self, contract: AccountContract) { match self { - Self::V1(_) => { - let account_v2 = AccountV2 { - amount: self.amount(), - locked: self.locked(), - storage_usage: self.storage_usage(), - account_contract: contract, - }; - *self = Self::V2(account_v2); - } + Self::V1(account) => match contract { + AccountContract::None | AccountContract::Local(_) => { + account.code_hash = contract.local_code(); + } + _ => { + let mut account_v2 = account.to_v2(); + account_v2.account_contract = contract; + *self = Self::V2(account_v2); + } + }, Self::V2(account) => { account.account_contract = contract; } @@ -554,7 +564,7 @@ mod tests { let expected_serde_repr = SerdeAccount { amount: account_v2.amount, locked: account_v2.locked, - code_hash: account_v2.account_contract.to_code_hash(), + code_hash: account_v2.account_contract.local_code(), storage_usage: account_v2.storage_usage, version: AccountVersion::V2, global_contract_hash: None, @@ -614,4 +624,26 @@ mod tests { let deserialization_attempt: Result = serde_json::from_str(&serde_string); assert!(deserialization_attempt.is_err()); } + + #[test] + fn test_account_version_upgrade_behaviour() { + let account_v1 = AccountV1 { + amount: 100, + locked: 200, + code_hash: CryptoHash::hash_bytes(&[42]), + storage_usage: 300, + }; + let mut account = Account::V1(account_v1); + let contract = AccountContract::Local(CryptoHash::hash_bytes(&[42])); + account.set_contract(contract); + assert!(matches!(account, Account::V1(_))); + + let contract = AccountContract::None; + account.set_contract(contract); + assert!(matches!(account, Account::V1(_))); + + let contract = AccountContract::Global(CryptoHash::hash_bytes(&[42])); + account.set_contract(contract); + assert!(matches!(account, Account::V2(_))); + } } diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs index dd86e8f7c33..714f6161625 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -26,7 +26,7 @@ use std::collections::HashMap; use std::sync::Arc; pub fn account_new(amount: Balance, code_hash: CryptoHash) -> Account { - Account::new(amount, 0, code_hash, std::mem::size_of::() as u64) + Account::new_v1(amount, 0, code_hash, std::mem::size_of::() as u64) } impl Transaction { diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 1d4c93ca6c8..036c81db4aa 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -47,6 +47,7 @@ use near_fmt::{AbbrBytes, Slice}; use near_parameters::config::CongestionControlConfig; use near_parameters::view::CongestionControlConfigView; use near_parameters::{ActionCosts, ExtCosts}; +use near_primitives_core::account::AccountContract; use near_schema_checker_lib::ProtocolSchema; use near_time::Utc; use serde_with::base64::Base64; @@ -65,7 +66,7 @@ pub struct AccountView { pub amount: Balance, #[serde(with = "dec_format")] pub locked: Balance, - pub code_hash: CryptoHash, + pub account_contract: AccountContract, pub storage_usage: StorageUsage, /// TODO(2271): deprecated. #[serde(default)] @@ -87,7 +88,7 @@ impl From<&Account> for AccountView { AccountView { amount: account.amount(), locked: account.locked(), - code_hash: account.contract().to_code_hash(), + account_contract: account.contract(), storage_usage: account.storage_usage(), storage_paid_at: 0, } @@ -102,7 +103,20 @@ impl From for AccountView { impl From<&AccountView> for Account { fn from(view: &AccountView) -> Self { - Account::new(view.amount, view.locked, view.code_hash, view.storage_usage) + match view.account_contract { + AccountContract::None => { + Account::new_v1(view.amount, view.locked, CryptoHash::default(), view.storage_usage) + } + AccountContract::Local(hash) => { + Account::new_v1(view.amount, view.locked, hash, view.storage_usage) + } + _ => Account::new_v2( + view.amount, + view.locked, + view.account_contract.clone(), + view.storage_usage, + ), + } } } diff --git a/core/store/src/genesis/state_applier.rs b/core/store/src/genesis/state_applier.rs index 839c6f66e99..65daabcb66a 100644 --- a/core/store/src/genesis/state_applier.rs +++ b/core/store/src/genesis/state_applier.rs @@ -206,7 +206,7 @@ impl GenesisStateApplier { get_account(state_update, account_id).expect("Failed to read state") { state_update.set_code(account_id.clone(), &code); - assert_eq!(*code.hash(), acc.contract().to_code_hash()); + assert_eq!(*code.hash(), acc.contract().local_code()); } else { tracing::error!( target: "runtime", diff --git a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs index 73f752f0d9b..f3ed91dedb6 100644 --- a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs +++ b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs @@ -188,7 +188,7 @@ fn account_records(row: &Row, gas_price: Balance) -> Vec { let mut res = vec![StateRecord::Account { account_id: row.account_id.clone(), - account: Account::new(row.amount, row.validator_stake, smart_contract_hash, 0), + account: Account::new_v1(row.amount, row.validator_stake, smart_contract_hash, 0), }]; // Add restricted access keys. diff --git a/genesis-tools/genesis-populate/src/lib.rs b/genesis-tools/genesis-populate/src/lib.rs index 2605ad49a80..95942c0dc7a 100644 --- a/genesis-tools/genesis-populate/src/lib.rs +++ b/genesis-tools/genesis-populate/src/lib.rs @@ -336,7 +336,7 @@ impl GenesisBuilder { self.state_updates.remove(&shard_id).expect("State update should have been added"); let signer = InMemorySigner::test_signer(&account_id); - let account = Account::new( + let account = Account::new_v1( testing_init_balance, testing_init_stake, self.additional_accounts_code_hash, diff --git a/integration-tests/src/tests/client/features/stateless_validation.rs b/integration-tests/src/tests/client/features/stateless_validation.rs index 7f90b4723cb..aa9fa1d9870 100644 --- a/integration-tests/src/tests/client/features/stateless_validation.rs +++ b/integration-tests/src/tests/client/features/stateless_validation.rs @@ -110,7 +110,7 @@ fn run_chunk_validation_test( let staked = if i < num_validators { validator_stake } else { 0 }; records.push(StateRecord::Account { account_id: account.clone(), - account: Account::new(initial_balance, staked, CryptoHash::default(), 0), + account: Account::new_v1(initial_balance, staked, CryptoHash::default(), 0), }); records.push(StateRecord::AccessKey { account_id: account.clone(), diff --git a/integration-tests/src/tests/client/features/wallet_contract.rs b/integration-tests/src/tests/client/features/wallet_contract.rs index 56686bba11e..5307ddfdf1e 100644 --- a/integration-tests/src/tests/client/features/wallet_contract.rs +++ b/integration-tests/src/tests/client/features/wallet_contract.rs @@ -117,7 +117,7 @@ fn test_eth_implicit_account_creation() { match view_request(&env, request).kind { QueryResponseKind::ViewAccount(view) => { assert_eq!(view.amount, 0); - assert_eq!(view.code_hash, *magic_bytes.hash()); + assert_eq!(view.account_contract.local_code(), *magic_bytes.hash()); assert!(view.storage_usage <= ZERO_BALANCE_ACCOUNT_STORAGE_LIMIT) } _ => panic!("wrong query response"), diff --git a/integration-tests/src/tests/runtime/state_viewer.rs b/integration-tests/src/tests/runtime/state_viewer.rs index 85472b5dba6..11c8d329d6f 100644 --- a/integration-tests/src/tests/runtime/state_viewer.rs +++ b/integration-tests/src/tests/runtime/state_viewer.rs @@ -363,7 +363,7 @@ fn test_view_state_too_large() { set_account( &mut state_update, alice_account(), - &Account::new(0, 0, CryptoHash::default(), 50_001), + &Account::new_v1(0, 0, CryptoHash::default(), 50_001), ); let trie_viewer = TrieViewer::new(Some(50_000), None); let result = trie_viewer.view_state(&state_update, &alice_account(), b"", false); @@ -378,7 +378,7 @@ fn test_view_state_with_large_contract() { set_account( &mut state_update, alice_account(), - &Account::new(0, 0, sha256(&contract_code), 50_001), + &Account::new_v1(0, 0, sha256(&contract_code), 50_001), ); state_update.set(TrieKey::ContractCode { account_id: alice_account() }, contract_code); let trie_viewer = TrieViewer::new(Some(50_000), None); diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index aab034ba800..b16b519b837 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -284,7 +284,7 @@ pub fn test_upload_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(&eve_dot_alice_account()).unwrap(); - assert_eq!(account.code_hash, hash(wasm_binary)); + assert_eq!(account.account_contract.local_code(), hash(wasm_binary)); let code = node_user.view_contract_code(&eve_dot_alice_account()).unwrap(); assert_eq!(code.code, wasm_binary.to_vec()); @@ -302,7 +302,7 @@ pub fn test_redeploy_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(account_id).unwrap(); - assert_eq!(account.code_hash, hash(test_binary)); + assert_eq!(account.account_contract.local_code(), hash(test_binary)); } pub fn test_send_money(node: impl Node) { diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 5244e978645..de20bd16462 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -479,7 +479,7 @@ pub(crate) fn action_create_account( } *actor_id = account_id.clone(); - *account = Some(Account::new( + *account = Some(Account::new_v1( 0, 0, CryptoHash::default(), @@ -518,7 +518,7 @@ pub(crate) fn action_implicit_account_creation_transfer( // unwrap: here it's safe because the `account_id` has already been determined to be implicit by `get_account_type` let public_key = PublicKey::from_near_implicit_account(account_id).unwrap(); - *account = Some(Account::new( + *account = Some(Account::new_v1( deposit, 0, CryptoHash::default(), @@ -546,7 +546,7 @@ pub(crate) fn action_implicit_account_creation_transfer( + fee_config.storage_usage_config.num_extra_bytes_record; let contract_hash = *magic_bytes.hash(); - *account = Some(Account::new(deposit, 0, contract_hash, storage_usage)); + *account = Some(Account::new_v1(deposit, 0, contract_hash, storage_usage)); state_update.set_code(account_id.clone(), &magic_bytes); // Precompile Wallet Contract and store result (compiled code or error) in the database. @@ -584,7 +584,7 @@ pub(crate) fn action_deploy_contract( let prev_code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().to_code_hash(), + account.contract().local_code(), current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); @@ -665,7 +665,7 @@ pub(crate) fn action_delete_account( let code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().to_code_hash(), + account.contract().local_code(), current_protocol_version, )?; debug_assert!(code_len == 0 || account_storage_usage > code_len, @@ -1285,7 +1285,7 @@ mod tests { storage_usage: u64, state_update: &mut TrieUpdate, ) -> ActionResult { - let mut account = Some(Account::new(100, 0, *code_hash, storage_usage)); + let mut account = Some(Account::new_v1(100, 0, *code_hash, storage_usage)); let mut actor_id = account_id.clone(); let mut action_result = ActionResult::default(); let receipt = Receipt::new_balance_refund( @@ -1335,7 +1335,7 @@ mod tests { tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); let account_id = "alice".parse::().unwrap(); let deploy_action = DeployContractAction { code: [0; 10_000].to_vec() }; - let mut account = Account::new(100, 0, CryptoHash::default(), storage_usage); + let mut account = Account::new_v1(100, 0, CryptoHash::default(), storage_usage); let apply_state = create_apply_state(0); let res = action_deploy_contract( &mut state_update, @@ -1349,7 +1349,7 @@ mod tests { assert!(res.is_ok()); test_delete_large_account( &account_id, - &account.contract().to_code_hash(), + &account.contract().local_code(), storage_usage, &mut state_update, ) @@ -1445,7 +1445,7 @@ mod tests { let tries = TestTriesBuilder::new().build(); let mut state_update = tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); - let account = Account::new(100, 0, CryptoHash::default(), 100); + let account = Account::new_v1(100, 0, CryptoHash::default(), 100); set_account(&mut state_update, account_id.clone(), &account); set_access_key(&mut state_update, account_id.clone(), public_key.clone(), access_key); diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 8fc3a879a8b..5c9d4b77e0a 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -490,7 +490,7 @@ impl Runtime { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); let contract = preparation_pipeline.get_contract( receipt, - account.contract().to_code_hash(), + account.contract().local_code(), action_index, None, ); @@ -506,7 +506,7 @@ impl Runtime { account_id, function_call, action_hash, - account.contract().to_code_hash(), + account.contract().local_code(), &apply_state.config, is_last_action, epoch_info_provider, @@ -1587,7 +1587,7 @@ impl Runtime { // Recompute contract code hash. let code = ContractCode::new(code, None); state_update.set_code(account_id, &code); - assert_eq!(*code.hash(), acc.contract().to_code_hash()); + assert_eq!(*code.hash(), acc.contract().local_code()); } StateRecord::AccessKey { account_id, public_key, access_key } => { set_access_key(state_update, account_id, public_key, &access_key); diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index e0b41f688fe..0f8ea1148ce 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -136,7 +136,7 @@ impl ReceiptPreparationPipeline { } Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; - let code_hash = account.contract().to_code_hash(); + let code_hash = account.contract().local_code(); let key = PrepareTaskKey { receipt_id: receipt.get_hash(), action_index }; let gas_counter = self.gas_counter(view_config.as_ref(), function_call.gas); let entry = match self.map.entry(key) { diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 8035e021724..4d209fdb13c 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -92,7 +92,7 @@ impl TrieViewer { account_id: &AccountId, ) -> Result { let account = self.view_account(state_update, account_id)?; - state_update.get_code(account_id.clone(), account.contract().to_code_hash())?.ok_or_else( + state_update.get_code(account_id.clone(), account.contract().local_code())?.ok_or_else( || errors::ViewContractCodeError::NoContractCode { contract_account_id: account_id.clone(), }, @@ -150,7 +150,7 @@ impl TrieViewer { match get_account(state_update, account_id)? { Some(account) => { let code_len = state_update - .get_code_len(account_id.clone(), account.contract().to_code_hash())? + .get_code_len(account_id.clone(), account.contract().local_code())? .unwrap_or_default() as u64; if let Some(limit) = self.state_size_limit { if account.storage_usage().saturating_sub(code_len) > limit { @@ -257,7 +257,7 @@ impl TrieViewer { let view_config = Some(ViewConfig { max_gas_burnt: self.max_gas_burnt_view }); let contract = pipeline.get_contract( &receipt, - account.contract().to_code_hash(), + account.contract().local_code(), 0, view_config.clone(), ); diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 2704f589976..5f00450da42 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -618,7 +618,7 @@ mod tests { use std::sync::Arc; use near_crypto::{InMemorySigner, KeyType, PublicKey, Signature, Signer}; - use near_primitives::account::{AccessKey, FunctionCallPermission}; + use near_primitives::account::{AccessKey, AccountContract, FunctionCallPermission}; use near_primitives::action::delegate::{DelegateAction, NonDelegateAction}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::ReceiptPriority; @@ -714,8 +714,7 @@ mod tests { account_id.clone(), &ContractCode::new(code.clone(), Some(code_hash)), ); - initial_account - .set_contract(near_primitives::account::AccountContract::Local(code_hash)); + initial_account.set_contract(AccountContract::Local(code_hash)); initial_account.set_storage_usage( initial_account.storage_usage().checked_add(code.len() as u64).unwrap(), ); diff --git a/runtime/runtime/tests/runtime_group_tools/mod.rs b/runtime/runtime/tests/runtime_group_tools/mod.rs index b5056dd6683..8419a29e216 100644 --- a/runtime/runtime/tests/runtime_group_tools/mod.rs +++ b/runtime/runtime/tests/runtime_group_tools/mod.rs @@ -267,7 +267,7 @@ impl RuntimeGroup { if (i as u64) < num_existing_accounts { state_records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new(TESTING_INIT_BALANCE, TESTING_INIT_STAKE, code_hash, 0), + account: Account::new_v1(TESTING_INIT_BALANCE, TESTING_INIT_STAKE, code_hash, 0), }); state_records.push(StateRecord::AccessKey { account_id: account_id.clone(), diff --git a/test-utils/testlib/src/runtime_utils.rs b/test-utils/testlib/src/runtime_utils.rs index b8acea4ff74..5c66fdd946f 100644 --- a/test-utils/testlib/src/runtime_utils.rs +++ b/test-utils/testlib/src/runtime_utils.rs @@ -46,7 +46,7 @@ pub fn add_contract(genesis: &mut Genesis, account_id: &AccountId, code: Vec if !is_account_record_found { records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new(0, 0, hash, 0), + account: Account::new_v1(0, 0, hash, 0), }); } records.push(StateRecord::Contract { account_id: account_id.clone(), code }); @@ -63,7 +63,7 @@ pub fn add_account_with_access_key( let records = genesis.force_read_records().as_mut(); records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new(balance, 0, Default::default(), 0), + account: Account::new_v1(balance, 0, Default::default(), 0), }); records.push(StateRecord::AccessKey { account_id, public_key, access_key }); } diff --git a/tools/amend-genesis/src/lib.rs b/tools/amend-genesis/src/lib.rs index 9b8573c7fd8..83840ade21f 100644 --- a/tools/amend-genesis/src/lib.rs +++ b/tools/amend-genesis/src/lib.rs @@ -63,7 +63,7 @@ impl AccountRecords { fn set_account(&mut self, amount: Balance, locked: Balance, num_bytes_account: u64) { assert!(self.account.is_none()); - let account = Account::new(amount, locked, CryptoHash::default(), num_bytes_account); + let account = Account::new_v1(amount, locked, CryptoHash::default(), num_bytes_account); self.account = Some(account); } @@ -460,7 +460,7 @@ mod test { match &self { Self::Account { account_id, amount, locked, storage_usage } => { let account = - Account::new(*amount, *locked, CryptoHash::default(), *storage_usage); + Account::new_v1(*amount, *locked, CryptoHash::default(), *storage_usage); StateRecord::Account { account_id: account_id.parse().unwrap(), account } } Self::AccessKey { account_id, public_key } => StateRecord::AccessKey { diff --git a/tools/fork-network/src/cli.rs b/tools/fork-network/src/cli.rs index 441655af755..98771589894 100644 --- a/tools/fork-network/src/cli.rs +++ b/tools/fork-network/src/cli.rs @@ -854,7 +854,7 @@ impl ForkNetworkCommand { new_validator_accounts.push(validator_account.clone()); storage_mutator.set_account( &validator_account.account_id, - Account::new( + Account::new_v1( liquid_balance, validator_account.amount, CryptoHash::default(), From 115b53d2aea378b43e34b742e6a12d3cef76a836 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 16:15:52 +0100 Subject: [PATCH 13/43] fmt --- runtime/runtime/tests/runtime_group_tools/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/runtime/runtime/tests/runtime_group_tools/mod.rs b/runtime/runtime/tests/runtime_group_tools/mod.rs index 8419a29e216..60e03cb24c8 100644 --- a/runtime/runtime/tests/runtime_group_tools/mod.rs +++ b/runtime/runtime/tests/runtime_group_tools/mod.rs @@ -267,7 +267,12 @@ impl RuntimeGroup { if (i as u64) < num_existing_accounts { state_records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new_v1(TESTING_INIT_BALANCE, TESTING_INIT_STAKE, code_hash, 0), + account: Account::new_v1( + TESTING_INIT_BALANCE, + TESTING_INIT_STAKE, + code_hash, + 0, + ), }); state_records.push(StateRecord::AccessKey { account_id: account_id.clone(), From 6baaef7d474cd016beccb5a7bbb6d7a69c202542 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 16:18:55 +0100 Subject: [PATCH 14/43] style --- chain/jsonrpc/jsonrpc-tests/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chain/jsonrpc/jsonrpc-tests/Cargo.toml b/chain/jsonrpc/jsonrpc-tests/Cargo.toml index abacbb26571..94a173b3961 100644 --- a/chain/jsonrpc/jsonrpc-tests/Cargo.toml +++ b/chain/jsonrpc/jsonrpc-tests/Cargo.toml @@ -46,6 +46,7 @@ nightly = [ "near-jsonrpc/nightly", "near-network/nightly", "near-o11y/nightly", + "near-primitives-core/nightly", "near-primitives/nightly", "near-store/nightly", "nightly_protocol", @@ -59,6 +60,7 @@ nightly_protocol = [ "near-jsonrpc/nightly_protocol", "near-network/nightly_protocol", "near-o11y/nightly_protocol", + "near-primitives-core/nightly_protocol", "near-primitives/nightly_protocol", "near-store/nightly_protocol", ] From 0cc098a28c2052e40101296d4bd0b853551cd927 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 16:50:52 +0100 Subject: [PATCH 15/43] . --- core/primitives-core/src/account.rs | 12 +++++++----- core/store/src/genesis/state_applier.rs | 5 ++++- .../src/tests/client/features/wallet_contract.rs | 2 +- integration-tests/src/tests/standard_cases/mod.rs | 4 ++-- runtime/runtime/src/actions.rs | 6 +++--- runtime/runtime/src/lib.rs | 13 +++++-------- runtime/runtime/src/pipelining.rs | 2 +- runtime/runtime/src/state_viewer/mod.rs | 15 +++++++++------ 8 files changed, 32 insertions(+), 27 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 113ea7a0254..cc71a63efff 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -93,10 +93,12 @@ pub enum AccountContract { } impl AccountContract { - pub fn local_code(&self) -> CryptoHash { + pub fn local_code(&self) -> Option { match self { - AccountContract::None | AccountContract::GlobalByAccount(_) => CryptoHash::default(), - AccountContract::Local(hash) | AccountContract::Global(hash) => *hash, + AccountContract::None + | AccountContract::GlobalByAccount(_) + | AccountContract::Global(_) => None, + AccountContract::Local(hash) => Some(*hash), } } @@ -241,7 +243,7 @@ impl Account { match self { Self::V1(account) => match contract { AccountContract::None | AccountContract::Local(_) => { - account.code_hash = contract.local_code(); + account.code_hash = contract.local_code().unwrap_or_default(); } _ => { let mut account_v2 = account.to_v2(); @@ -564,7 +566,7 @@ mod tests { let expected_serde_repr = SerdeAccount { amount: account_v2.amount, locked: account_v2.locked, - code_hash: account_v2.account_contract.local_code(), + code_hash: account_v2.account_contract.local_code().unwrap_or_default(), storage_usage: account_v2.storage_usage, version: AccountVersion::V2, global_contract_hash: None, diff --git a/core/store/src/genesis/state_applier.rs b/core/store/src/genesis/state_applier.rs index 65daabcb66a..23f041ff37d 100644 --- a/core/store/src/genesis/state_applier.rs +++ b/core/store/src/genesis/state_applier.rs @@ -206,7 +206,10 @@ impl GenesisStateApplier { get_account(state_update, account_id).expect("Failed to read state") { state_update.set_code(account_id.clone(), &code); - assert_eq!(*code.hash(), acc.contract().local_code()); + assert_eq!( + *code.hash(), + acc.contract().local_code().unwrap_or_default() + ); } else { tracing::error!( target: "runtime", diff --git a/integration-tests/src/tests/client/features/wallet_contract.rs b/integration-tests/src/tests/client/features/wallet_contract.rs index 5307ddfdf1e..f9947b118f7 100644 --- a/integration-tests/src/tests/client/features/wallet_contract.rs +++ b/integration-tests/src/tests/client/features/wallet_contract.rs @@ -117,7 +117,7 @@ fn test_eth_implicit_account_creation() { match view_request(&env, request).kind { QueryResponseKind::ViewAccount(view) => { assert_eq!(view.amount, 0); - assert_eq!(view.account_contract.local_code(), *magic_bytes.hash()); + assert_eq!(view.account_contract.local_code().unwrap_or_default(), *magic_bytes.hash()); assert!(view.storage_usage <= ZERO_BALANCE_ACCOUNT_STORAGE_LIMIT) } _ => panic!("wrong query response"), diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index b16b519b837..ca43fd787a5 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -284,7 +284,7 @@ pub fn test_upload_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(&eve_dot_alice_account()).unwrap(); - assert_eq!(account.account_contract.local_code(), hash(wasm_binary)); + assert_eq!(account.account_contract.local_code().unwrap_or_default(), hash(wasm_binary)); let code = node_user.view_contract_code(&eve_dot_alice_account()).unwrap(); assert_eq!(code.code, wasm_binary.to_vec()); @@ -302,7 +302,7 @@ pub fn test_redeploy_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(account_id).unwrap(); - assert_eq!(account.account_contract.local_code(), hash(test_binary)); + assert_eq!(account.account_contract.local_code().unwrap_or_default(), hash(test_binary)); } pub fn test_send_money(node: impl Node) { diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index de20bd16462..7f12f99fadb 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -584,7 +584,7 @@ pub(crate) fn action_deploy_contract( let prev_code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().local_code(), + account.contract().local_code().unwrap_or_default(), current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); @@ -665,7 +665,7 @@ pub(crate) fn action_delete_account( let code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().local_code(), + account.contract().local_code().unwrap_or_default(), current_protocol_version, )?; debug_assert!(code_len == 0 || account_storage_usage > code_len, @@ -1349,7 +1349,7 @@ mod tests { assert!(res.is_ok()); test_delete_large_account( &account_id, - &account.contract().local_code(), + &account.contract().local_code().unwrap_or_default(), storage_usage, &mut state_update, ) diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 5c9d4b77e0a..027109422f9 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -488,12 +488,9 @@ impl Runtime { } Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); - let contract = preparation_pipeline.get_contract( - receipt, - account.contract().local_code(), - action_index, - None, - ); + let code_hash = account.contract().local_code().unwrap_or_default(); + let contract = + preparation_pipeline.get_contract(receipt, code_hash, action_index, None); let is_last_action = action_index + 1 == actions.len(); action_function_call( state_update, @@ -506,7 +503,7 @@ impl Runtime { account_id, function_call, action_hash, - account.contract().local_code(), + code_hash, &apply_state.config, is_last_action, epoch_info_provider, @@ -1587,7 +1584,7 @@ impl Runtime { // Recompute contract code hash. let code = ContractCode::new(code, None); state_update.set_code(account_id, &code); - assert_eq!(*code.hash(), acc.contract().local_code()); + assert_eq!(*code.hash(), acc.contract().local_code().unwrap_or_default()); } StateRecord::AccessKey { account_id, public_key, access_key } => { set_access_key(state_update, account_id, public_key, &access_key); diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index 0f8ea1148ce..3ccf3d8ba20 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -136,7 +136,7 @@ impl ReceiptPreparationPipeline { } Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; - let code_hash = account.contract().local_code(); + let Some(code_hash) = account.contract().local_code() else { continue }; // TODO: deal with global contract call let key = PrepareTaskKey { receipt_id: receipt.get_hash(), action_index }; let gas_counter = self.gas_counter(view_config.as_ref(), function_call.gas); let entry = match self.map.entry(key) { diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 4d209fdb13c..461498a046f 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -92,11 +92,11 @@ impl TrieViewer { account_id: &AccountId, ) -> Result { let account = self.view_account(state_update, account_id)?; - state_update.get_code(account_id.clone(), account.contract().local_code())?.ok_or_else( - || errors::ViewContractCodeError::NoContractCode { + state_update + .get_code(account_id.clone(), account.contract().local_code().unwrap_or_default())? + .ok_or_else(|| errors::ViewContractCodeError::NoContractCode { contract_account_id: account_id.clone(), - }, - ) + }) } pub fn view_access_key( @@ -150,7 +150,10 @@ impl TrieViewer { match get_account(state_update, account_id)? { Some(account) => { let code_len = state_update - .get_code_len(account_id.clone(), account.contract().local_code())? + .get_code_len( + account_id.clone(), + account.contract().local_code().unwrap_or_default(), + )? .unwrap_or_default() as u64; if let Some(limit) = self.state_size_limit { if account.storage_usage().saturating_sub(code_len) > limit { @@ -257,7 +260,7 @@ impl TrieViewer { let view_config = Some(ViewConfig { max_gas_burnt: self.max_gas_burnt_view }); let contract = pipeline.get_contract( &receipt, - account.contract().local_code(), + account.contract().local_code().unwrap_or_default(), 0, view_config.clone(), ); From 4d9bbc95b45ffaac702ead53e19cbe8675f50fd5 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 17:01:33 +0100 Subject: [PATCH 16/43] reference todo --- runtime/runtime/src/pipelining.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index 3ccf3d8ba20..87bb47b1561 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -136,7 +136,9 @@ impl ReceiptPreparationPipeline { } Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; - let Some(code_hash) = account.contract().local_code() else { continue }; // TODO: deal with global contract call + let Some(code_hash) = account.contract().local_code() else { + todo!("#12884: global contracts pipelining support") + }; let key = PrepareTaskKey { receipt_id: receipt.get_hash(), action_index }; let gas_counter = self.gas_counter(view_config.as_ref(), function_call.gas); let entry = match self.map.entry(key) { From e73e6272766e8713d2128d5457de8355b5d16d5b Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 17:48:16 +0100 Subject: [PATCH 17/43] . --- runtime/runtime/src/pipelining.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index 87bb47b1561..999bac12c8b 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -137,7 +137,8 @@ impl ReceiptPreparationPipeline { Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; let Some(code_hash) = account.contract().local_code() else { - todo!("#12884: global contracts pipelining support") + // TODO(#12884): support global contracts pipelining + continue; }; let key = PrepareTaskKey { receipt_id: receipt.get_hash(), action_index }; let gas_counter = self.gas_counter(view_config.as_ref(), function_call.gas); From 897985e71d9d640616aaa2cc1070a3115af99887 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 6 Feb 2025 21:38:51 +0100 Subject: [PATCH 18/43] comments --- chain/chain/src/test_utils/kv_runtime.rs | 6 +-- .../jsonrpc/jsonrpc-tests/tests/rpc_query.rs | 10 +++-- chain/rosetta-rpc/src/adapters/mod.rs | 26 +++++++++---- chain/rosetta-rpc/src/lib.rs | 4 +- core/chain-configs/src/genesis_validate.rs | 4 +- core/chain-configs/src/test_genesis.rs | 9 ++--- core/chain-configs/src/test_utils.rs | 4 +- core/primitives-core/src/account.rs | 26 +++++++------ core/primitives/src/test_utils.rs | 8 +++- core/primitives/src/views.rs | 37 +++++++++++-------- .../genesis-csv-to-json/src/csv_parser.rs | 8 +++- genesis-tools/genesis-populate/src/lib.rs | 6 +-- .../client/features/stateless_validation.rs | 5 +-- .../tests/client/features/wallet_contract.rs | 2 +- .../src/tests/runtime/state_viewer.rs | 6 +-- .../src/tests/standard_cases/mod.rs | 4 +- runtime/runtime/src/actions.rs | 26 +++++++++---- .../runtime/tests/runtime_group_tools/mod.rs | 6 +-- test-utils/testlib/src/runtime_utils.rs | 6 +-- tools/amend-genesis/src/lib.rs | 7 ++-- tools/fork-network/src/cli.rs | 6 +-- 21 files changed, 129 insertions(+), 87 deletions(-) diff --git a/chain/chain/src/test_utils/kv_runtime.rs b/chain/chain/src/test_utils/kv_runtime.rs index 684a79a88a8..3ca5693a786 100644 --- a/chain/chain/src/test_utils/kv_runtime.rs +++ b/chain/chain/src/test_utils/kv_runtime.rs @@ -14,7 +14,7 @@ use near_crypto::{KeyType, PublicKey, SecretKey}; use near_epoch_manager::EpochManagerAdapter; use near_parameters::RuntimeConfig; use near_pool::types::TransactionGroupIterator; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::apply::ApplyChunkReason; use near_primitives::bandwidth_scheduler::BandwidthRequests; use near_primitives::block::Tip; @@ -1208,13 +1208,13 @@ impl RuntimeAdapter for KeyValueRuntime { match request { QueryRequest::ViewAccount { account_id, .. } => Ok(QueryResponse { kind: QueryResponseKind::ViewAccount( - Account::new_v1( + Account::new( self.state.read().unwrap().get(state_root).map_or_else( || 0, |state| *state.amounts.get(account_id).unwrap_or(&0), ), 0, - CryptoHash::default(), + AccountContract::None, 0, ) .into(), diff --git a/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs b/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs index 76c2904a6d5..f18106840ce 100644 --- a/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs +++ b/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs @@ -13,7 +13,7 @@ use near_jsonrpc_primitives::types::query::QueryResponseKind; use near_jsonrpc_primitives::types::validator::RpcValidatorsOrderedRequest; use near_network::test_utils::wait_or_timeout; use near_o11y::testonly::init_test_logger; -use near_primitives::account::{AccessKey, AccessKeyPermission, AccountContract}; +use near_primitives::account::{AccessKey, AccessKeyPermission}; use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, EpochId, ShardId, SyncCheckpoint}; use near_primitives::views::QueryRequest; @@ -147,10 +147,12 @@ fn test_query_by_path_account() { panic!("queried account, but received something else: {:?}", query_response.kind); }; assert_eq!(account_info.amount, 0); - assert_eq!(account_info.account_contract, AccountContract::None); + assert_eq!(account_info.code_hash, CryptoHash::default()); assert_eq!(account_info.locked, 0); assert_eq!(account_info.storage_paid_at, 0); assert_eq!(account_info.storage_usage, 0); + assert_eq!(account_info.global_contract_hash, None); + assert_eq!(account_info.global_contract_account_id, None); }); } @@ -192,10 +194,12 @@ fn test_query_account() { panic!("queried account, but received something else: {:?}", query_response.kind); }; assert_eq!(account_info.amount, 0); - assert_eq!(account_info.account_contract, AccountContract::None); + assert_eq!(account_info.code_hash, CryptoHash::default()); assert_eq!(account_info.locked, 0); assert_eq!(account_info.storage_paid_at, 0); assert_eq!(account_info.storage_usage, 0); + assert_eq!(account_info.global_contract_hash, None); + assert_eq!(account_info.global_contract_account_id, None); } }); } diff --git a/chain/rosetta-rpc/src/adapters/mod.rs b/chain/rosetta-rpc/src/adapters/mod.rs index bd61f29c5d5..1436468996c 100644 --- a/chain/rosetta-rpc/src/adapters/mod.rs +++ b/chain/rosetta-rpc/src/adapters/mod.rs @@ -838,8 +838,8 @@ mod tests { use near_client::test_utils::setup_no_network; use near_crypto::{KeyType, SecretKey}; use near_parameters::{RuntimeConfig, RuntimeConfigView}; - use near_primitives::account::AccountContract; use near_primitives::action::delegate::{DelegateAction, SignedDelegateAction}; + use near_primitives::hash::CryptoHash; use near_primitives::transaction::{Action, TransferAction}; use near_time::Clock; @@ -866,10 +866,12 @@ mod tests { account_id: "nfvalidator1.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 5000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, }, }, @@ -881,10 +883,12 @@ mod tests { account_id: "nfvalidator1.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 4000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, }, }, @@ -894,10 +898,12 @@ mod tests { account_id: "nfvalidator2.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 7000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, }, }, @@ -909,10 +915,12 @@ mod tests { account_id: "nfvalidator2.near".parse().unwrap(), account: near_primitives::views::AccountView { amount: 8000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, }, }, @@ -922,20 +930,24 @@ mod tests { "nfvalidator1.near".parse().unwrap(), near_primitives::views::AccountView { amount: 4000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, ); accounts_previous_state.insert( "nfvalidator2.near".parse().unwrap(), near_primitives::views::AccountView { amount: 6000000000000000000, - account_contract: AccountContract::None, + code_hash: CryptoHash::default(), locked: 400000000000000000000000000000, storage_paid_at: 0, storage_usage: 200000, + global_contract_hash: None, + global_contract_account_id: None, }, ); let transactions = super::transactions::convert_block_changes_to_transactions( diff --git a/chain/rosetta-rpc/src/lib.rs b/chain/rosetta-rpc/src/lib.rs index 0c57160bb26..a3c486b435e 100644 --- a/chain/rosetta-rpc/src/lib.rs +++ b/chain/rosetta-rpc/src/lib.rs @@ -17,7 +17,7 @@ pub use config::RosettaRpcConfig; use near_chain_configs::Genesis; use near_client::{ClientActor, ViewClientActor}; use near_o11y::WithSpanContextExt; -use near_primitives::borsh::BorshDeserialize; +use near_primitives::{account::AccountContract, borsh::BorshDeserialize}; mod adapters; mod config; @@ -369,7 +369,7 @@ async fn account_balance( Err(crate::errors::ErrorKind::NotFound(_)) => ( block.header.hash, block.header.height, - near_primitives::account::Account::new_v1(0, 0, Default::default(), 0).into(), + near_primitives::account::Account::new(0, 0, AccountContract::None, 0).into(), ), Err(err) => return Err(err.into()), }; diff --git a/core/chain-configs/src/genesis_validate.rs b/core/chain-configs/src/genesis_validate.rs index 058dadabe86..158f21a8ee1 100644 --- a/core/chain-configs/src/genesis_validate.rs +++ b/core/chain-configs/src/genesis_validate.rs @@ -198,13 +198,13 @@ mod test { use crate::GenesisRecords; use near_crypto::{KeyType, PublicKey}; - use near_primitives::account::{AccessKey, Account}; + use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::types::AccountInfo; const VALID_ED25519_RISTRETTO_KEY: &str = "ed25519:KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7"; fn create_account() -> Account { - Account::new_v1(100, 10, Default::default(), 0) + Account::new(100, 10, AccountContract::None, 0) } #[test] diff --git a/core/chain-configs/src/test_genesis.rs b/core/chain-configs/src/test_genesis.rs index ef7e2d2c9d0..2faa54c28c2 100644 --- a/core/chain-configs/src/test_genesis.rs +++ b/core/chain-configs/src/test_genesis.rs @@ -2,9 +2,8 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; use near_crypto::PublicKey; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::epoch_manager::{EpochConfig, EpochConfigStore}; -use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; use near_primitives::test_utils::{create_test_signer, create_user_test_signer}; @@ -444,10 +443,10 @@ impl TestGenesisBuilder { total_supply += user_account.balance; records.push(StateRecord::Account { account_id: user_account.account_id.clone(), - account: Account::new_v1( + account: Account::new( user_account.balance, validator_stake.remove(&user_account.account_id).unwrap_or(0), - CryptoHash::default(), + AccountContract::None, 0, ), }); @@ -465,7 +464,7 @@ impl TestGenesisBuilder { for (account_id, balance) in validator_stake { records.push(StateRecord::Account { account_id, - account: Account::new_v1(0, balance, CryptoHash::default(), 0), + account: Account::new(0, balance, AccountContract::None, 0), }); } diff --git a/core/chain-configs/src/test_utils.rs b/core/chain-configs/src/test_utils.rs index 5f3f9170074..b04d6f03f9b 100644 --- a/core/chain-configs/src/test_utils.rs +++ b/core/chain-configs/src/test_utils.rs @@ -1,5 +1,5 @@ use near_crypto::{InMemorySigner, PublicKey}; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; @@ -185,7 +185,7 @@ pub fn add_account_with_key( ) { records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new_v1(amount, staked, code_hash, 0), + account: Account::new(amount, staked, AccountContract::from_local_code_hash(code_hash), 0), }); records.push(StateRecord::AccessKey { account_id, diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index cc71a63efff..8a38111c1c3 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -102,7 +102,7 @@ impl AccountContract { } } - fn from_local_code_hash(code_hash: CryptoHash) -> AccountContract { + pub fn from_local_code_hash(code_hash: CryptoHash) -> AccountContract { if code_hash == CryptoHash::default() { AccountContract::None } else { @@ -142,22 +142,24 @@ impl Account { /// differentiate AccountVersion V1 from newer versions. const SERIALIZATION_SENTINEL: u128 = u128::MAX; - pub fn new_v1( - amount: Balance, - locked: Balance, - code_hash: CryptoHash, - storage_usage: StorageUsage, - ) -> Self { - Self::V1(AccountV1 { amount, locked, code_hash, storage_usage }) - } - - pub fn new_v2( + pub fn new( amount: Balance, locked: Balance, account_contract: AccountContract, storage_usage: StorageUsage, ) -> Self { - Self::V2(AccountV2 { amount, locked, storage_usage, account_contract }) + match account_contract { + AccountContract::None => Self::V1(AccountV1 { + amount, + locked, + code_hash: CryptoHash::default(), + storage_usage, + }), + AccountContract::Local(code_hash) => { + Self::V1(AccountV1 { amount, locked, code_hash, storage_usage }) + } + _ => Self::V2(AccountV2 { amount, locked, storage_usage, account_contract }), + } } #[inline] diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs index 714f6161625..f0669872b82 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -21,12 +21,18 @@ use crate::version::PROTOCOL_VERSION; use crate::views::{ExecutionStatusView, FinalExecutionOutcomeView, FinalExecutionStatus}; use near_crypto::vrf::Value; use near_crypto::{EmptySigner, PublicKey, SecretKey, Signature, Signer}; +use near_primitives_core::account::AccountContract; use near_primitives_core::types::{BlockHeight, MerkleHash, ProtocolVersion}; use std::collections::HashMap; use std::sync::Arc; pub fn account_new(amount: Balance, code_hash: CryptoHash) -> Account { - Account::new_v1(amount, 0, code_hash, std::mem::size_of::() as u64) + Account::new( + amount, + 0, + AccountContract::from_local_code_hash(code_hash), + std::mem::size_of::() as u64, + ) } impl Transaction { diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 036c81db4aa..5604e16a419 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -66,11 +66,15 @@ pub struct AccountView { pub amount: Balance, #[serde(with = "dec_format")] pub locked: Balance, - pub account_contract: AccountContract, + pub code_hash: CryptoHash, pub storage_usage: StorageUsage, /// TODO(2271): deprecated. #[serde(default)] pub storage_paid_at: BlockHeight, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub global_contract_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub global_contract_account_id: Option, } /// A view of the contract code. @@ -85,12 +89,19 @@ 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)), + _ => (None, None), + }; AccountView { amount: account.amount(), locked: account.locked(), - account_contract: account.contract(), + code_hash: account.contract().local_code().unwrap_or_default(), storage_usage: account.storage_usage(), storage_paid_at: 0, + global_contract_hash, + global_contract_account_id, } } } @@ -103,20 +114,14 @@ impl From for AccountView { impl From<&AccountView> for Account { fn from(view: &AccountView) -> Self { - match view.account_contract { - AccountContract::None => { - Account::new_v1(view.amount, view.locked, CryptoHash::default(), view.storage_usage) - } - AccountContract::Local(hash) => { - Account::new_v1(view.amount, view.locked, hash, view.storage_usage) - } - _ => Account::new_v2( - view.amount, - view.locked, - view.account_contract.clone(), - view.storage_usage, - ), - } + let account_contract = match &view.global_contract_account_id { + Some(account_id) => AccountContract::GlobalByAccount(account_id.clone()), + None => match view.global_contract_hash { + Some(hash) => AccountContract::Global(hash), + None => AccountContract::from_local_code_hash(view.code_hash), + }, + }; + Account::new(view.amount, view.locked, account_contract, view.storage_usage) } } diff --git a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs index f3ed91dedb6..61fedf48a09 100644 --- a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs +++ b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs @@ -4,6 +4,7 @@ use chrono::Utc; use csv::ReaderBuilder; use near_crypto::{KeyType, PublicKey}; use near_network::types::PeerInfo; +use near_primitives::account::AccountContract; use near_primitives::account::{AccessKey, AccessKeyPermission, Account, FunctionCallPermission}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::ReceiptV0; @@ -188,7 +189,12 @@ fn account_records(row: &Row, gas_price: Balance) -> Vec { let mut res = vec![StateRecord::Account { account_id: row.account_id.clone(), - account: Account::new_v1(row.amount, row.validator_stake, smart_contract_hash, 0), + account: Account::new( + row.amount, + row.validator_stake, + AccountContract::from_local_code_hash(smart_contract_hash), + 0, + ), }]; // Add restricted access keys. diff --git a/genesis-tools/genesis-populate/src/lib.rs b/genesis-tools/genesis-populate/src/lib.rs index 95942c0dc7a..df37fde6498 100644 --- a/genesis-tools/genesis-populate/src/lib.rs +++ b/genesis-tools/genesis-populate/src/lib.rs @@ -10,7 +10,7 @@ use near_chain::{Block, Chain, ChainStore}; use near_chain_configs::Genesis; use near_crypto::InMemorySigner; use near_epoch_manager::{EpochManager, EpochManagerAdapter, EpochManagerHandle}; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::block::{genesis_chunks, Tip}; use near_primitives::congestion_info::CongestionInfo; use near_primitives::epoch_block_info::BlockInfo; @@ -336,10 +336,10 @@ impl GenesisBuilder { self.state_updates.remove(&shard_id).expect("State update should have been added"); let signer = InMemorySigner::test_signer(&account_id); - let account = Account::new_v1( + let account = Account::new( testing_init_balance, testing_init_stake, - self.additional_accounts_code_hash, + AccountContract::from_local_code_hash(self.additional_accounts_code_hash), 0, ); set_account(&mut state_update, account_id.clone(), &account); diff --git a/integration-tests/src/tests/client/features/stateless_validation.rs b/integration-tests/src/tests/client/features/stateless_validation.rs index aa9fa1d9870..cca1d265335 100644 --- a/integration-tests/src/tests/client/features/stateless_validation.rs +++ b/integration-tests/src/tests/client/features/stateless_validation.rs @@ -10,7 +10,7 @@ use near_crypto::{InMemorySigner, KeyType, SecretKey}; use near_epoch_manager::{EpochManager, EpochManagerAdapter}; use near_o11y::testonly::init_integration_logger; use near_primitives::account::id::AccountIdRef; -use near_primitives::account::{AccessKeyPermission, FunctionCallPermission}; +use near_primitives::account::{AccessKeyPermission, AccountContract, FunctionCallPermission}; use near_primitives::action::{Action, AddKeyAction, TransferAction}; use near_primitives::epoch_manager::AllEpochConfigTestOverrides; use near_primitives::num_rational::Rational32; @@ -25,7 +25,6 @@ use near_primitives::version::ProtocolFeature; use near_primitives::version::{ProtocolVersion, PROTOCOL_VERSION}; use near_primitives::views::FinalExecutionStatus; use near_primitives_core::account::{AccessKey, Account}; -use near_primitives_core::hash::CryptoHash; use near_primitives_core::types::{AccountId, NumSeats}; use near_store::test_utils::create_test_store; use nearcore::test_utils::TestEnvNightshadeSetupExt; @@ -110,7 +109,7 @@ fn run_chunk_validation_test( let staked = if i < num_validators { validator_stake } else { 0 }; records.push(StateRecord::Account { account_id: account.clone(), - account: Account::new_v1(initial_balance, staked, CryptoHash::default(), 0), + account: Account::new(initial_balance, staked, AccountContract::None, 0), }); records.push(StateRecord::AccessKey { account_id: account.clone(), diff --git a/integration-tests/src/tests/client/features/wallet_contract.rs b/integration-tests/src/tests/client/features/wallet_contract.rs index f9947b118f7..56686bba11e 100644 --- a/integration-tests/src/tests/client/features/wallet_contract.rs +++ b/integration-tests/src/tests/client/features/wallet_contract.rs @@ -117,7 +117,7 @@ fn test_eth_implicit_account_creation() { match view_request(&env, request).kind { QueryResponseKind::ViewAccount(view) => { assert_eq!(view.amount, 0); - assert_eq!(view.account_contract.local_code().unwrap_or_default(), *magic_bytes.hash()); + assert_eq!(view.code_hash, *magic_bytes.hash()); assert!(view.storage_usage <= ZERO_BALANCE_ACCOUNT_STORAGE_LIMIT) } _ => panic!("wrong query response"), diff --git a/integration-tests/src/tests/runtime/state_viewer.rs b/integration-tests/src/tests/runtime/state_viewer.rs index 11c8d329d6f..92dec8613dc 100644 --- a/integration-tests/src/tests/runtime/state_viewer.rs +++ b/integration-tests/src/tests/runtime/state_viewer.rs @@ -4,7 +4,7 @@ use borsh::BorshDeserialize; use crate::runtime_utils::{get_runtime_and_trie, get_test_trie_viewer, TEST_SHARD_UID}; use near_primitives::{ - account::Account, + account::{Account, AccountContract}, hash::{hash as sha256, CryptoHash}, serialize::to_base64, trie_key::trie_key_parsers, @@ -363,7 +363,7 @@ fn test_view_state_too_large() { set_account( &mut state_update, alice_account(), - &Account::new_v1(0, 0, CryptoHash::default(), 50_001), + &Account::new(0, 0, AccountContract::None, 50_001), ); let trie_viewer = TrieViewer::new(Some(50_000), None); let result = trie_viewer.view_state(&state_update, &alice_account(), b"", false); @@ -378,7 +378,7 @@ fn test_view_state_with_large_contract() { set_account( &mut state_update, alice_account(), - &Account::new_v1(0, 0, sha256(&contract_code), 50_001), + &Account::new(0, 0, AccountContract::from_local_code_hash(sha256(&contract_code)), 50_001), ); state_update.set(TrieKey::ContractCode { account_id: alice_account() }, contract_code); let trie_viewer = TrieViewer::new(Some(50_000), None); diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index ca43fd787a5..aab034ba800 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -284,7 +284,7 @@ pub fn test_upload_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(&eve_dot_alice_account()).unwrap(); - assert_eq!(account.account_contract.local_code().unwrap_or_default(), hash(wasm_binary)); + assert_eq!(account.code_hash, hash(wasm_binary)); let code = node_user.view_contract_code(&eve_dot_alice_account()).unwrap(); assert_eq!(code.code, wasm_binary.to_vec()); @@ -302,7 +302,7 @@ pub fn test_redeploy_contract(node: impl Node) { let new_root = node_user.get_state_root(); assert_ne!(root, new_root); let account = node_user.view_account(account_id).unwrap(); - assert_eq!(account.account_contract.local_code().unwrap_or_default(), hash(test_binary)); + assert_eq!(account.code_hash, hash(test_binary)); } pub fn test_send_money(node: impl Node) { diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 7f12f99fadb..6b84c5f1e74 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -479,10 +479,10 @@ pub(crate) fn action_create_account( } *actor_id = account_id.clone(); - *account = Some(Account::new_v1( + *account = Some(Account::new( 0, 0, - CryptoHash::default(), + AccountContract::None, fee_config.storage_usage_config.num_bytes_account, )); } @@ -518,10 +518,10 @@ pub(crate) fn action_implicit_account_creation_transfer( // unwrap: here it's safe because the `account_id` has already been determined to be implicit by `get_account_type` let public_key = PublicKey::from_near_implicit_account(account_id).unwrap(); - *account = Some(Account::new_v1( + *account = Some(Account::new( deposit, 0, - CryptoHash::default(), + AccountContract::None, fee_config.storage_usage_config.num_bytes_account + public_key.len() as u64 + borsh::object_length(&access_key).unwrap() as u64 @@ -546,7 +546,12 @@ pub(crate) fn action_implicit_account_creation_transfer( + fee_config.storage_usage_config.num_extra_bytes_record; let contract_hash = *magic_bytes.hash(); - *account = Some(Account::new_v1(deposit, 0, contract_hash, storage_usage)); + *account = Some(Account::new( + deposit, + 0, + AccountContract::from_local_code_hash(contract_hash), + storage_usage, + )); state_update.set_code(account_id.clone(), &magic_bytes); // Precompile Wallet Contract and store result (compiled code or error) in the database. @@ -1285,7 +1290,12 @@ mod tests { storage_usage: u64, state_update: &mut TrieUpdate, ) -> ActionResult { - let mut account = Some(Account::new_v1(100, 0, *code_hash, storage_usage)); + let mut account = Some(Account::new( + 100, + 0, + AccountContract::from_local_code_hash(*code_hash), + storage_usage, + )); let mut actor_id = account_id.clone(); let mut action_result = ActionResult::default(); let receipt = Receipt::new_balance_refund( @@ -1335,7 +1345,7 @@ mod tests { tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); let account_id = "alice".parse::().unwrap(); let deploy_action = DeployContractAction { code: [0; 10_000].to_vec() }; - let mut account = Account::new_v1(100, 0, CryptoHash::default(), storage_usage); + let mut account = Account::new(100, 0, AccountContract::None, storage_usage); let apply_state = create_apply_state(0); let res = action_deploy_contract( &mut state_update, @@ -1445,7 +1455,7 @@ mod tests { let tries = TestTriesBuilder::new().build(); let mut state_update = tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); - let account = Account::new_v1(100, 0, CryptoHash::default(), 100); + let account = Account::new(100, 0, AccountContract::None, 100); set_account(&mut state_update, account_id.clone(), &account); set_access_key(&mut state_update, account_id.clone(), public_key.clone(), access_key); diff --git a/runtime/runtime/tests/runtime_group_tools/mod.rs b/runtime/runtime/tests/runtime_group_tools/mod.rs index 60e03cb24c8..741fff9595d 100644 --- a/runtime/runtime/tests/runtime_group_tools/mod.rs +++ b/runtime/runtime/tests/runtime_group_tools/mod.rs @@ -1,7 +1,7 @@ use near_chain_configs::{get_initial_supply, Genesis, GenesisConfig, GenesisRecords}; use near_crypto::{InMemorySigner, Signer}; use near_parameters::ActionCosts; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::apply::ApplyChunkReason; use near_primitives::bandwidth_scheduler::BlockBandwidthRequests; use near_primitives::congestion_info::{BlockCongestionInfo, ExtendedCongestionInfo}; @@ -267,10 +267,10 @@ impl RuntimeGroup { if (i as u64) < num_existing_accounts { state_records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new_v1( + account: Account::new( TESTING_INIT_BALANCE, TESTING_INIT_STAKE, - code_hash, + AccountContract::from_local_code_hash(code_hash), 0, ), }); diff --git a/test-utils/testlib/src/runtime_utils.rs b/test-utils/testlib/src/runtime_utils.rs index 5c66fdd946f..95967a3afc8 100644 --- a/test-utils/testlib/src/runtime_utils.rs +++ b/test-utils/testlib/src/runtime_utils.rs @@ -1,6 +1,6 @@ use near_chain_configs::Genesis; use near_crypto::PublicKey; -use near_primitives::account::{AccessKey, Account}; +use near_primitives::account::{AccessKey, Account, AccountContract}; use near_primitives::hash::hash; use near_primitives::state_record::StateRecord; use near_primitives::types::{AccountId, Balance}; @@ -46,7 +46,7 @@ pub fn add_contract(genesis: &mut Genesis, account_id: &AccountId, code: Vec if !is_account_record_found { records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new_v1(0, 0, hash, 0), + account: Account::new(0, 0, AccountContract::from_local_code_hash(hash), 0), }); } records.push(StateRecord::Contract { account_id: account_id.clone(), code }); @@ -63,7 +63,7 @@ pub fn add_account_with_access_key( let records = genesis.force_read_records().as_mut(); records.push(StateRecord::Account { account_id: account_id.clone(), - account: Account::new_v1(balance, 0, Default::default(), 0), + account: Account::new(balance, 0, AccountContract::None, 0), }); records.push(StateRecord::AccessKey { account_id, public_key, access_key }); } diff --git a/tools/amend-genesis/src/lib.rs b/tools/amend-genesis/src/lib.rs index 83840ade21f..0cd63303cd3 100644 --- a/tools/amend-genesis/src/lib.rs +++ b/tools/amend-genesis/src/lib.rs @@ -3,7 +3,6 @@ use anyhow::Context; use near_chain_configs::{Genesis, GenesisValidationMode, NEAR_BASE}; use near_crypto::PublicKey; use near_primitives::account::AccountContract; -use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; use near_primitives::types::{AccountId, AccountInfo}; @@ -63,7 +62,7 @@ impl AccountRecords { fn set_account(&mut self, amount: Balance, locked: Balance, num_bytes_account: u64) { assert!(self.account.is_none()); - let account = Account::new_v1(amount, locked, CryptoHash::default(), num_bytes_account); + let account = Account::new(amount, locked, AccountContract::None, num_bytes_account); self.account = Some(account); } @@ -407,7 +406,7 @@ pub fn amend_genesis( mod test { use anyhow::Context; use near_chain_configs::{get_initial_supply, Genesis, GenesisConfig, NEAR_BASE}; - use near_primitives::hash::CryptoHash; + use near_primitives::account::AccountContract; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; use near_primitives::types::{AccountId, AccountInfo}; @@ -460,7 +459,7 @@ mod test { match &self { Self::Account { account_id, amount, locked, storage_usage } => { let account = - Account::new_v1(*amount, *locked, CryptoHash::default(), *storage_usage); + Account::new(*amount, *locked, AccountContract::None, *storage_usage); StateRecord::Account { account_id: account_id.parse().unwrap(), account } } Self::AccessKey { account_id, public_key } => StateRecord::AccessKey { diff --git a/tools/fork-network/src/cli.rs b/tools/fork-network/src/cli.rs index 98771589894..618cc2ffdfe 100644 --- a/tools/fork-network/src/cli.rs +++ b/tools/fork-network/src/cli.rs @@ -13,7 +13,7 @@ use near_o11y::default_subscriber_with_opentelemetry; use near_o11y::env_filter::make_env_filter; use near_parameters::{RuntimeConfig, RuntimeConfigStore}; use near_primitives::account::id::AccountType; -use near_primitives::account::{AccessKey, AccessKeyPermission, Account}; +use near_primitives::account::{AccessKey, AccessKeyPermission, Account, AccountContract}; use near_primitives::borsh; use near_primitives::epoch_manager::{EpochConfig, EpochConfigStore}; use near_primitives::hash::CryptoHash; @@ -854,10 +854,10 @@ impl ForkNetworkCommand { new_validator_accounts.push(validator_account.clone()); storage_mutator.set_account( &validator_account.account_id, - Account::new_v1( + Account::new( liquid_balance, validator_account.amount, - CryptoHash::default(), + AccountContract::None, storage_bytes, ), )?; From 102e4a97c6ab10e71d0fe1114020a3246e5631c2 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 11:47:54 +0100 Subject: [PATCH 19/43] comments --- core/primitives-core/src/account.rs | 90 +++++++++++++++---------- core/primitives/src/views.rs | 4 +- core/store/src/db/colddb.rs | 2 +- test-utils/testlib/src/runtime_utils.rs | 2 +- 4 files changed, 57 insertions(+), 41 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 8a38111c1c3..5f49f007348 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -69,7 +69,7 @@ impl AccountV1 { amount: self.amount, locked: self.locked, storage_usage: self.storage_usage, - account_contract: AccountContract::from_local_code_hash(self.code_hash), + contract: AccountContract::from_local_code_hash(self.code_hash), } } } @@ -130,7 +130,7 @@ pub struct AccountV2 { /// Storage used by the given account, includes account id, this struct, access keys and other data. storage_usage: StorageUsage, /// Type of contract deployed to this account, if any. - account_contract: AccountContract, + contract: AccountContract, } impl Account { @@ -145,10 +145,10 @@ impl Account { pub fn new( amount: Balance, locked: Balance, - account_contract: AccountContract, + contract: AccountContract, storage_usage: StorageUsage, ) -> Self { - match account_contract { + match contract { AccountContract::None => Self::V1(AccountV1 { amount, locked, @@ -158,7 +158,7 @@ impl Account { AccountContract::Local(code_hash) => { Self::V1(AccountV1 { amount, locked, code_hash, storage_usage }) } - _ => Self::V2(AccountV2 { amount, locked, storage_usage, account_contract }), + _ => Self::V2(AccountV2 { amount, locked, storage_usage, contract }), } } @@ -182,7 +182,7 @@ impl Account { pub fn contract(&self) -> AccountContract { match self { Self::V1(account) => AccountContract::from_local_code_hash(account.code_hash), - Self::V2(account) => account.account_contract.clone(), + Self::V2(account) => account.contract.clone(), } } @@ -205,22 +205,18 @@ impl Account { #[inline] pub fn global_contract_hash(&self) -> Option { match self { - Self::V1(_) => None, - Self::V2(account) => match account.account_contract.clone() { - AccountContract::Global(hash) => Some(hash), - _ => None, - }, + Self::V2(AccountV2 { contract: AccountContract::Global(hash), .. }) => Some(*hash), + Self::V1(_) | Self::V2(_) => None, } } #[inline] pub fn global_contract_account_id(&self) -> Option<&AccountId> { match self { - Self::V1(_) => None, - Self::V2(account) => match &account.account_contract { - AccountContract::GlobalByAccount(account_id) => Some(account_id), - _ => None, - }, + Self::V2(AccountV2 { contract: AccountContract::GlobalByAccount(account), .. }) => { + Some(account) + } + Self::V1(_) | Self::V2(_) => None, } } @@ -249,12 +245,12 @@ impl Account { } _ => { let mut account_v2 = account.to_v2(); - account_v2.account_contract = contract; + account_v2.contract = contract; *self = Self::V2(account_v2); } }, Self::V2(account) => { - account.account_contract = contract; + account.contract = contract; } } } @@ -318,7 +314,7 @@ impl<'de> serde::Deserialize<'de> for Account { storage_usage: account_data.storage_usage, })), AccountVersion::V2 => { - let account_contract = match account_data.global_contract_account_id { + let contract = match account_data.global_contract_account_id { Some(account_id) => AccountContract::GlobalByAccount(account_id), None => match account_data.global_contract_hash { Some(hash) => AccountContract::Global(hash), @@ -330,7 +326,7 @@ impl<'de> serde::Deserialize<'de> for Account { amount: account_data.amount, locked: account_data.locked, storage_usage: account_data.storage_usage, - account_contract, + contract, })) } } @@ -505,6 +501,22 @@ pub struct FunctionCallPermission { mod tests { use super::*; + fn create_serde_account( + code_hash: CryptoHash, + global_contract_hash: Option, + global_contract_account_id: Option, + ) -> SerdeAccount { + SerdeAccount { + amount: 10_000_000, + locked: 100_000, + code_hash, + storage_usage: 1000, + version: AccountVersion::V2, + global_contract_hash, + global_contract_account_id, + } + } + #[test] fn test_v1_account_serde_serialization() { let old_account = AccountV1 { @@ -560,7 +572,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContract::Local(CryptoHash::hash_bytes(&[42])), + contract: AccountContract::Local(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2.clone()); @@ -568,7 +580,7 @@ mod tests { let expected_serde_repr = SerdeAccount { amount: account_v2.amount, locked: account_v2.locked, - code_hash: account_v2.account_contract.local_code().unwrap_or_default(), + code_hash: account_v2.contract.local_code().unwrap_or_default(), storage_usage: account_v2.storage_usage, version: AccountVersion::V2, global_contract_hash: None, @@ -587,7 +599,7 @@ mod tests { amount: 10_000_000, locked: 100_000, storage_usage: 1000, - account_contract: AccountContract::Global(CryptoHash::hash_bytes(&[42])), + contract: AccountContract::Global(CryptoHash::hash_bytes(&[42])), }; let account = Account::V2(account_v2); let serialized_account = borsh::to_vec(&account).unwrap(); @@ -601,29 +613,33 @@ mod tests { let id = AccountId::try_from("test.near".to_string()).unwrap(); let code_hash = CryptoHash::hash_bytes(&[42]); - let mut serde_repr = SerdeAccount { - amount: 10_000_000, - locked: 100_000, - code_hash, - storage_usage: 1000, - version: AccountVersion::V2, - global_contract_hash: None, - global_contract_account_id: Some(id.clone()), - }; + let serde_repr = create_serde_account(code_hash, None, Some(id)); let serde_string = serde_json::to_string(&serde_repr).unwrap(); let deserialization_attempt: Result = serde_json::from_str(&serde_string); assert!(deserialization_attempt.is_err()); + } + + #[test] + fn test_account_v2_serde_deserialization_fails_with_local_and_global_contracts() { + let code_hash = CryptoHash::hash_bytes(&[42]); + + let serde_repr = create_serde_account(code_hash, Some(code_hash), None); - serde_repr.global_contract_hash = Some(code_hash); - serde_repr.global_contract_account_id = None; let serde_string = serde_json::to_string(&serde_repr).unwrap(); let deserialization_attempt: Result = serde_json::from_str(&serde_string); assert!(deserialization_attempt.is_err()); + } + + #[test] + fn test_account_v2_serde_deserialization_fails_if_both_types_of_global_contract_are_present() { + let id = AccountId::try_from("test.near".to_string()).unwrap(); + let serde_repr = create_serde_account( + CryptoHash::default(), + Some(CryptoHash::hash_bytes(&[42])), + Some(id), + ); - serde_repr.code_hash = CryptoHash::default(); - serde_repr.global_contract_hash = Some(code_hash); - serde_repr.global_contract_account_id = Some(id); let serde_string = serde_json::to_string(&serde_repr).unwrap(); let deserialization_attempt: Result = serde_json::from_str(&serde_string); assert!(deserialization_attempt.is_err()); diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 5604e16a419..de72903030a 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -114,14 +114,14 @@ impl From for AccountView { impl From<&AccountView> for Account { fn from(view: &AccountView) -> Self { - let account_contract = match &view.global_contract_account_id { + let contract = match &view.global_contract_account_id { Some(account_id) => AccountContract::GlobalByAccount(account_id.clone()), None => match view.global_contract_hash { Some(hash) => AccountContract::Global(hash), None => AccountContract::from_local_code_hash(view.code_hash), }, }; - Account::new(view.amount, view.locked, account_contract, view.storage_usage) + Account::new(view.amount, view.locked, contract, view.storage_usage) } } diff --git a/core/store/src/db/colddb.rs b/core/store/src/db/colddb.rs index c9d2a1f9eb5..c69a6ce7e7e 100644 --- a/core/store/src/db/colddb.rs +++ b/core/store/src/db/colddb.rs @@ -208,7 +208,7 @@ mod test { } } fn hash(chunk: &[u8]) -> String { - crate::CryptoHash::try_from(chunk).unwrap().to_string() + crate::CryptoHash::from(chunk.try_into().unwrap()).to_string() } match key.len() { 8 => chunk(key), diff --git a/test-utils/testlib/src/runtime_utils.rs b/test-utils/testlib/src/runtime_utils.rs index 95967a3afc8..6427011aa5f 100644 --- a/test-utils/testlib/src/runtime_utils.rs +++ b/test-utils/testlib/src/runtime_utils.rs @@ -39,7 +39,7 @@ pub fn add_contract(genesis: &mut Genesis, account_id: &AccountId, code: Vec if let StateRecord::Account { account_id: record_account_id, ref mut account } = record { if record_account_id == account_id { is_account_record_found = true; - account.set_contract(near_primitives::account::AccountContract::Local(hash)); + account.set_contract(AccountContract::Local(hash)); } } } From b4f5ed9301ac9d662ee84e083b04fd271dcb6e7a Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 11:52:39 +0100 Subject: [PATCH 20/43] introduce local_contract_hash --- core/primitives-core/src/account.rs | 17 +++++++++++------ core/primitives/src/views.rs | 2 +- runtime/runtime/src/actions.rs | 6 +++--- runtime/runtime/src/lib.rs | 2 +- runtime/runtime/src/pipelining.rs | 2 +- runtime/runtime/src/state_viewer/mod.rs | 6 +++--- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 5f49f007348..9750e9b7a5b 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -220,6 +220,16 @@ impl Account { } } + #[inline] + pub fn local_contract_hash(&self) -> Option { + match self.contract() { + AccountContract::None + | AccountContract::Global(_) + | AccountContract::GlobalByAccount(_) => None, + AccountContract::Local(code_hash) => Some(code_hash), + } + } + #[inline] pub fn set_amount(&mut self, amount: Balance) { match self { @@ -339,12 +349,7 @@ impl serde::Serialize for Account { S: serde::Serializer, { let version = self.version(); - let code_hash = match self.contract() { - AccountContract::None - | AccountContract::Global(_) - | AccountContract::GlobalByAccount(_) => CryptoHash::default(), - AccountContract::Local(code_hash) => code_hash, - }; + let code_hash = self.local_contract_hash().unwrap_or_default(); let repr = SerdeAccount { amount: self.amount(), locked: self.locked(), diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index de72903030a..be1daf681f2 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -97,7 +97,7 @@ impl From<&Account> for AccountView { AccountView { amount: account.amount(), locked: account.locked(), - code_hash: account.contract().local_code().unwrap_or_default(), + code_hash: account.local_contract_hash().unwrap_or_default(), storage_usage: account.storage_usage(), storage_paid_at: 0, global_contract_hash, diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 6b84c5f1e74..cc4c3b0ec5e 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -589,7 +589,7 @@ pub(crate) fn action_deploy_contract( let prev_code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().local_code().unwrap_or_default(), + account.local_contract_hash().unwrap_or_default(), current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); @@ -670,7 +670,7 @@ pub(crate) fn action_delete_account( let code_len = get_code_len_or_default( state_update, account_id.clone(), - account.contract().local_code().unwrap_or_default(), + account.local_contract_hash().unwrap_or_default(), current_protocol_version, )?; debug_assert!(code_len == 0 || account_storage_usage > code_len, @@ -1359,7 +1359,7 @@ mod tests { assert!(res.is_ok()); test_delete_large_account( &account_id, - &account.contract().local_code().unwrap_or_default(), + &account.local_contract_hash().unwrap_or_default(), storage_usage, &mut state_update, ) diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 027109422f9..03309179ae8 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -488,7 +488,7 @@ impl Runtime { } Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); - let code_hash = account.contract().local_code().unwrap_or_default(); + let code_hash = account.local_contract_hash().unwrap_or_default(); let contract = preparation_pipeline.get_contract(receipt, code_hash, action_index, None); let is_last_action = action_index + 1 == actions.len(); diff --git a/runtime/runtime/src/pipelining.rs b/runtime/runtime/src/pipelining.rs index 999bac12c8b..c8df4fa2cbf 100644 --- a/runtime/runtime/src/pipelining.rs +++ b/runtime/runtime/src/pipelining.rs @@ -136,7 +136,7 @@ impl ReceiptPreparationPipeline { } Action::FunctionCall(function_call) => { let Some(account) = &**account else { continue }; - let Some(code_hash) = account.contract().local_code() else { + let Some(code_hash) = account.local_contract_hash() else { // TODO(#12884): support global contracts pipelining continue; }; diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 461498a046f..9c3a8de6c7a 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -93,7 +93,7 @@ impl TrieViewer { ) -> Result { let account = self.view_account(state_update, account_id)?; state_update - .get_code(account_id.clone(), account.contract().local_code().unwrap_or_default())? + .get_code(account_id.clone(), account.local_contract_hash().unwrap_or_default())? .ok_or_else(|| errors::ViewContractCodeError::NoContractCode { contract_account_id: account_id.clone(), }) @@ -152,7 +152,7 @@ impl TrieViewer { let code_len = state_update .get_code_len( account_id.clone(), - account.contract().local_code().unwrap_or_default(), + account.local_contract_hash().unwrap_or_default(), )? .unwrap_or_default() as u64; if let Some(limit) = self.state_size_limit { @@ -260,7 +260,7 @@ impl TrieViewer { let view_config = Some(ViewConfig { max_gas_burnt: self.max_gas_burnt_view }); let contract = pipeline.get_contract( &receipt, - account.contract().local_code().unwrap_or_default(), + account.local_contract_hash().unwrap_or_default(), 0, view_config.clone(), ); From ad1f7811daa4507ffe390f95ec4f041e8dbf0f16 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 12:03:19 +0100 Subject: [PATCH 21/43] rename tests --- core/primitives-core/src/account.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/primitives-core/src/account.rs b/core/primitives-core/src/account.rs index 9750e9b7a5b..f9c20ff5aaf 100644 --- a/core/primitives-core/src/account.rs +++ b/core/primitives-core/src/account.rs @@ -614,7 +614,7 @@ mod tests { } #[test] - fn test_account_v2_serde_deserialization_fails() { + fn test_account_v2_serde_deserialization_fails_with_local_hash_and_global_account_id() { let id = AccountId::try_from("test.near".to_string()).unwrap(); let code_hash = CryptoHash::hash_bytes(&[42]); @@ -626,7 +626,7 @@ mod tests { } #[test] - fn test_account_v2_serde_deserialization_fails_with_local_and_global_contracts() { + fn test_account_v2_serde_deserialization_fails_with_local_and_global_hashes() { let code_hash = CryptoHash::hash_bytes(&[42]); let serde_repr = create_serde_account(code_hash, Some(code_hash), None); From 0d2447922940e4e589716567f658da9f62fa1ee6 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 12:13:16 +0100 Subject: [PATCH 22/43] wip --- chain/chain/src/runtime/mod.rs | 3 +++ core/primitives/src/errors.rs | 8 ++++++++ core/primitives/src/trie_key.rs | 15 ++++++++++++++- integration-tests/src/user/runtime_user.rs | 3 +++ runtime/runtime/src/actions.rs | 19 ++++++++++++++----- 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/chain/chain/src/runtime/mod.rs b/chain/chain/src/runtime/mod.rs index 9bf0885c13f..39e5f7cd8eb 100644 --- a/chain/chain/src/runtime/mod.rs +++ b/chain/chain/src/runtime/mod.rs @@ -336,6 +336,9 @@ impl NightshadeRuntime { // TODO(#2152): process gracefully RuntimeError::ReceiptValidationError(e) => panic!("{}", e), RuntimeError::ValidatorError(e) => e.into(), + RuntimeError::GlobalContractError(_e) => { + todo!() + } })?; let elapsed = instant.elapsed(); diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 7e43f2bb525..1808d8fcc49 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -1,3 +1,4 @@ +use crate::action::GlobalContractIdentifier; use crate::hash::CryptoHash; use crate::serialize::dec_format; use crate::shard_layout::ShardLayoutError; @@ -67,6 +68,8 @@ pub enum RuntimeError { ReceiptValidationError(ReceiptValidationError), /// Error when accessing validator information. Happens inside epoch manager. ValidatorError(EpochError), + /// Global contract errors. + GlobalContractError(GlobalContractError), } impl std::fmt::Display for RuntimeError { @@ -935,6 +938,11 @@ impl Display for ActionErrorKind { } } +#[derive(Eq, PartialEq, Clone, Debug)] +pub enum GlobalContractError { + IdentifierNotFound(GlobalContractIdentifier), +} + #[derive(Eq, PartialEq, Clone)] pub enum EpochError { /// Error calculating threshold from given stakes for given number of seats. diff --git a/core/primitives/src/trie_key.rs b/core/primitives/src/trie_key.rs index 4499a752cc0..97467c8e504 100644 --- a/core/primitives/src/trie_key.rs +++ b/core/primitives/src/trie_key.rs @@ -1,5 +1,5 @@ -use crate::hash::CryptoHash; use crate::types::AccountId; +use crate::{action::GlobalContractIdentifier, hash::CryptoHash}; use borsh::{to_vec, BorshDeserialize, BorshSerialize}; use near_crypto::PublicKey; use near_primitives_core::types::ShardId; @@ -124,6 +124,19 @@ impl GlobalContractCodeIdentifier { } } +impl From for GlobalContractCodeIdentifier { + fn from(identifier: GlobalContractIdentifier) -> Self { + match identifier { + GlobalContractIdentifier::CodeHash(hash) => { + GlobalContractCodeIdentifier::CodeHash(hash) + } + GlobalContractIdentifier::AccountId(account_id) => { + GlobalContractCodeIdentifier::AccountId(account_id) + } + } + } +} + /// Describes the key of a specific key-value record in a state trie. #[derive(Debug, Clone, PartialEq, Eq, BorshDeserialize, BorshSerialize, ProtocolSchema)] pub enum TrieKey { diff --git a/integration-tests/src/user/runtime_user.rs b/integration-tests/src/user/runtime_user.rs index 4d88feabe3e..95a903ddd1f 100644 --- a/integration-tests/src/user/runtime_user.rs +++ b/integration-tests/src/user/runtime_user.rs @@ -130,6 +130,9 @@ impl RuntimeUser { } RuntimeError::ReceiptValidationError(e) => panic!("{}", e), RuntimeError::ValidatorError(e) => panic!("{}", e), + RuntimeError::GlobalContractError(_e) => { + todo!() + } })?; for outcome_with_id in apply_result.outcomes { self.transaction_results diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 6b84c5f1e74..22276889065 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -15,7 +15,9 @@ use near_primitives::action::{ }; use near_primitives::checked_feature; use near_primitives::config::ViewConfig; -use near_primitives::errors::{ActionError, ActionErrorKind, InvalidAccessKeyError, RuntimeError}; +use near_primitives::errors::{ + ActionError, ActionErrorKind, GlobalContractError, InvalidAccessKeyError, RuntimeError, +}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::{ ActionReceipt, DataReceipt, Receipt, ReceiptEnum, ReceiptPriority, ReceiptV0, @@ -24,6 +26,7 @@ use near_primitives::transaction::{ Action, AddKeyAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, FunctionCallAction, StakeAction, }; +use near_primitives::trie_key::TrieKey; use near_primitives::types::validator_stake::ValidatorStake; use near_primitives::types::{ AccountId, Balance, BlockHeight, EpochInfoProvider, Gas, StorageUsage, TrieCacheMode, @@ -645,12 +648,18 @@ pub(crate) fn action_deploy_global_contract( } pub(crate) fn action_use_global_contract( - _state_update: &mut TrieUpdate, - _account: &mut Account, - _action: &UseGlobalContractAction, + state_update: &mut TrieUpdate, + account: &mut Account, + action: &UseGlobalContractAction, ) -> Result<(), RuntimeError> { let _span = tracing::debug_span!(target: "runtime", "action_use_global_contract").entered(); - // TODO(#12716): implement global contract usage + let key = TrieKey::GlobalContractCode { identifier: action.contract_identifier.clone().into() }; + if !state_update.contains_key(&key)? { + return Err(RuntimeError::GlobalContractError(GlobalContractError::IdentifierNotFound( + action.contract_identifier.clone(), + ))); + } + if account.contract() != AccountContract::None {} Ok(()) } From dd266a7b4b9ddf5f717ea8ab501be654d61ff545 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 12:39:24 +0100 Subject: [PATCH 23/43] wip --- core/primitives/src/errors.rs | 1 + core/store/src/trie/update.rs | 9 +++++++++ runtime/runtime/src/actions.rs | 26 +++++++++++++++++++++++++- runtime/runtime/src/lib.rs | 10 +++++++++- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 1808d8fcc49..ffdbb2bd896 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -941,6 +941,7 @@ impl Display for ActionErrorKind { #[derive(Eq, PartialEq, Clone, Debug)] pub enum GlobalContractError { IdentifierNotFound(GlobalContractIdentifier), + CodeNotFound(GlobalContractIdentifier), } #[derive(Eq, PartialEq, Clone)] diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index dd57a75cdaf..ba749e1f542 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -5,6 +5,7 @@ use crate::contract::ContractStorage; use crate::trie::TrieAccess; use crate::trie::{KeyLookupMode, TrieChanges}; use crate::StorageError; +use near_primitives::action::GlobalContractIdentifier; use near_primitives::apply::ApplyChunkReason; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::stateless_validation::contract_distribution::ContractUpdates; @@ -164,6 +165,14 @@ impl TrieUpdate { self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, Some(code_hash)))) } + pub fn get_global_code( + &self, + identifier: GlobalContractIdentifier, + ) -> Result, StorageError> { + let key = TrieKey::GlobalContractCode { identifier: identifier.into() }; + self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, None))) + } + /// Returns the size (in num bytes) of the contract code for the given account. /// /// This is different from `get_code` in that it does not read the code from storage. diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index e0e57fe67f5..2804430a61c 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -606,6 +606,7 @@ pub(crate) fn action_deploy_contract( )) })?, ); + // TODO: handle GlobalContract account.set_contract(AccountContract::Local(*code.hash())); // Legacy: populate the mapping from `AccountId => sha256(code)` thus making contracts part of // The State. For the time being we are also relying on the `TrieUpdate` to actually write the @@ -649,8 +650,12 @@ pub(crate) fn action_deploy_global_contract( pub(crate) fn action_use_global_contract( state_update: &mut TrieUpdate, + account_id: &AccountId, account: &mut Account, action: &UseGlobalContractAction, + config: Arc, + cache: Option<&dyn ContractRuntimeCache>, + current_protocol_version: ProtocolVersion, ) -> Result<(), RuntimeError> { let _span = tracing::debug_span!(target: "runtime", "action_use_global_contract").entered(); let key = TrieKey::GlobalContractCode { identifier: action.contract_identifier.clone().into() }; @@ -659,7 +664,26 @@ pub(crate) fn action_use_global_contract( action.contract_identifier.clone(), ))); } - if account.contract() != AccountContract::None {} + if account.contract() != AccountContract::None { + let code = state_update + .get_global_code(action.contract_identifier.clone())? + .ok_or_else(|| { + RuntimeError::GlobalContractError(GlobalContractError::CodeNotFound( + action.contract_identifier.clone(), + )) + })? + .into_code(); + let action = DeployContractAction { code }; + action_deploy_contract( + state_update, + account, + account_id, + &action, + config, + cache, + current_protocol_version, + )?; + } Ok(()) } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 03309179ae8..b43190bc430 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -484,7 +484,15 @@ impl Runtime { } Action::UseGlobalContract(use_global_contract) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); - action_use_global_contract(state_update, account, use_global_contract)?; + action_use_global_contract( + state_update, + account_id, + account, + use_global_contract, + Arc::clone(&apply_state.config.wasm_config), + apply_state.cache.as_deref(), + apply_state.current_protocol_version, + )?; } Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); From e981c09bde6a19d9ed7bbe06c5cdfec5e54cc7f3 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 7 Feb 2025 14:04:20 +0100 Subject: [PATCH 24/43] deploy global contract --- runtime/runtime/src/actions.rs | 54 ++++++++++++++++++++-------------- runtime/runtime/src/lib.rs | 1 + 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 2804430a61c..0e29083b089 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -587,6 +587,7 @@ pub(crate) fn action_deploy_contract( config: Arc, cache: Option<&dyn ContractRuntimeCache>, current_protocol_version: ProtocolVersion, + global_contract_identifier: Option, ) -> Result<(), StorageError> { let _span = tracing::debug_span!(target: "runtime", "action_deploy_contract").entered(); let prev_code_len = get_code_len_or_default( @@ -606,8 +607,17 @@ pub(crate) fn action_deploy_contract( )) })?, ); - // TODO: handle GlobalContract - account.set_contract(AccountContract::Local(*code.hash())); + match global_contract_identifier { + Some(GlobalContractIdentifier::CodeHash(hash)) => { + account.set_contract(AccountContract::Global(hash)); + } + Some(GlobalContractIdentifier::AccountId(id)) => { + account.set_contract(AccountContract::GlobalByAccount(id)); + } + None => { + account.set_contract(AccountContract::from_local_code_hash(*code.hash())); + } + }; // Legacy: populate the mapping from `AccountId => sha256(code)` thus making contracts part of // The State. For the time being we are also relying on the `TrieUpdate` to actually write the // contracts into the storage as part of the commit routine, however no code should be relying @@ -664,26 +674,25 @@ pub(crate) fn action_use_global_contract( action.contract_identifier.clone(), ))); } - if account.contract() != AccountContract::None { - let code = state_update - .get_global_code(action.contract_identifier.clone())? - .ok_or_else(|| { - RuntimeError::GlobalContractError(GlobalContractError::CodeNotFound( - action.contract_identifier.clone(), - )) - })? - .into_code(); - let action = DeployContractAction { code }; - action_deploy_contract( - state_update, - account, - account_id, - &action, - config, - cache, - current_protocol_version, - )?; - } + let code = state_update + .get_global_code(action.contract_identifier.clone())? + .ok_or_else(|| { + RuntimeError::GlobalContractError(GlobalContractError::CodeNotFound( + action.contract_identifier.clone(), + )) + })? + .into_code(); + let deploy_action = DeployContractAction { code }; + action_deploy_contract( + state_update, + account, + account_id, + &deploy_action, + config, + cache, + current_protocol_version, + Some(action.contract_identifier.clone()), + )?; Ok(()) } @@ -1388,6 +1397,7 @@ mod tests { Arc::clone(&apply_state.config.wasm_config), None, apply_state.current_protocol_version, + None, ); assert!(res.is_ok()); test_delete_large_account( diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index b43190bc430..939f9640488 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -477,6 +477,7 @@ impl Runtime { Arc::clone(&apply_state.config.wasm_config), apply_state.cache.as_deref(), apply_state.current_protocol_version, + None, )?; } Action::DeployGlobalContract(deploy_global_contract) => { From f747bb82ba592f22188f70c6685106f3c2f3f77c Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Mon, 10 Feb 2025 11:53:32 +0100 Subject: [PATCH 25/43] . --- runtime/runtime/src/actions.rs | 37 ++++++++++++++++------------------ runtime/runtime/src/lib.rs | 2 -- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index e7ec6de2e64..f2c976cd823 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -652,8 +652,6 @@ pub(crate) fn action_use_global_contract( account_id: &AccountId, account: &mut Account, action: &UseGlobalContractAction, - config: Arc, - cache: Option<&dyn ContractRuntimeCache>, current_protocol_version: ProtocolVersion, ) -> Result<(), RuntimeError> { let _span = tracing::debug_span!(target: "runtime", "action_use_global_contract").entered(); @@ -663,24 +661,23 @@ pub(crate) fn action_use_global_contract( action.contract_identifier.clone(), ))); } - let code = state_update - .get_global_code(action.contract_identifier.clone())? - .ok_or_else(|| { - RuntimeError::GlobalContractError(GlobalContractError::IdentifierNotFound( - action.contract_identifier.clone(), - )) - })? - .into_code(); - let deploy_action = DeployContractAction { code }; - action_deploy_contract( - state_update, - account, - account_id, - &deploy_action, - config, - cache, - current_protocol_version, - )?; + if let AccountContract::Local(code_hash) = account.contract() { + let prev_code_len = get_code_len_or_default( + state_update, + account_id.clone(), + code_hash, + current_protocol_version, + )?; + account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); + } + match &action.contract_identifier { + GlobalContractIdentifier::CodeHash(code_hash) => { + account.set_contract(AccountContract::Global(*code_hash)); + } + GlobalContractIdentifier::AccountId(id) => { + account.set_contract(AccountContract::GlobalByAccount(id.clone())); + } + }; Ok(()) } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index aa054deb21c..6321b48afe5 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -480,8 +480,6 @@ impl Runtime { account_id, account, use_global_contract, - Arc::clone(&apply_state.config.wasm_config), - apply_state.cache.as_deref(), apply_state.current_protocol_version, )?; } From 067c5bb546e7b7668652b0e6f2ced662b62530ce Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Mon, 10 Feb 2025 11:56:55 +0100 Subject: [PATCH 26/43] remove useless fn --- core/store/src/trie/update.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index ba749e1f542..cc72ec47132 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -165,14 +165,6 @@ impl TrieUpdate { self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, Some(code_hash)))) } - pub fn get_global_code( - &self, - identifier: GlobalContractIdentifier, - ) -> Result, StorageError> { - let key = TrieKey::GlobalContractCode { identifier: identifier.into() }; - self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, None))) - } - /// Returns the size (in num bytes) of the contract code for the given account. /// /// This is different from `get_code` in that it does not read the code from storage. From 9db880db67f899069f82efe6e480ddd1f07ff2af Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Mon, 10 Feb 2025 12:11:22 +0100 Subject: [PATCH 27/43] impl errors --- chain/chain-primitives/src/error.rs | 5 +++++ chain/chain/src/runtime/mod.rs | 4 +--- core/primitives/src/errors.rs | 7 +++++++ core/store/src/trie/update.rs | 1 - integration-tests/src/user/runtime_user.rs | 4 +--- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 147ea8aaa30..6f7448522cc 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -251,6 +251,9 @@ pub enum Error { /// Invalid chunk header version for protocol version #[error(transparent)] BadHeaderForProtocolVersion(#[from] BadHeaderForProtocolVersionError), + /// Global contract error. + #[error("Global Contract Error: {0}")] + GlobalContractError(String), /// Anything else #[error("Other Error: {0}")] Other(String), @@ -289,6 +292,7 @@ impl Error { | Error::StorageError(_) | Error::GCError(_) | Error::ReshardingError(_) + | Error::GlobalContractError(_) | Error::DBNotFoundErr(_) => false, Error::InvalidBlockPastTime(_, _) | Error::InvalidBlockFutureTime(_) @@ -424,6 +428,7 @@ impl Error { Error::NotAChunkValidator => "not_a_chunk_validator", Error::InvalidChallengeRoot => "invalid_challenge_root", Error::ReshardingError(_) => "resharding_error", + Error::GlobalContractError(_) => "global_contract_error", Error::BadHeaderForProtocolVersion(_) => "bad_header_for_protocol_version", } } diff --git a/chain/chain/src/runtime/mod.rs b/chain/chain/src/runtime/mod.rs index cf60e8b39ed..377c7100a6b 100644 --- a/chain/chain/src/runtime/mod.rs +++ b/chain/chain/src/runtime/mod.rs @@ -336,9 +336,7 @@ impl NightshadeRuntime { // TODO(#2152): process gracefully RuntimeError::ReceiptValidationError(e) => panic!("{}", e), RuntimeError::ValidatorError(e) => e.into(), - RuntimeError::GlobalContractError(_e) => { - todo!() - } + RuntimeError::GlobalContractError(e) => Error::Other(e.to_string()), })?; let elapsed = instant.elapsed(); diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 1808d8fcc49..3c177ab9a66 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -943,6 +943,13 @@ pub enum GlobalContractError { IdentifierNotFound(GlobalContractIdentifier), } +impl Display for GlobalContractError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + let Self::IdentifierNotFound(id) = self; + write!(f, "GlobalContractError: {:?}", id) + } +} + #[derive(Eq, PartialEq, Clone)] pub enum EpochError { /// Error calculating threshold from given stakes for given number of seats. diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index cc72ec47132..dd57a75cdaf 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -5,7 +5,6 @@ use crate::contract::ContractStorage; use crate::trie::TrieAccess; use crate::trie::{KeyLookupMode, TrieChanges}; use crate::StorageError; -use near_primitives::action::GlobalContractIdentifier; use near_primitives::apply::ApplyChunkReason; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::stateless_validation::contract_distribution::ContractUpdates; diff --git a/integration-tests/src/user/runtime_user.rs b/integration-tests/src/user/runtime_user.rs index 95a903ddd1f..34c250f8415 100644 --- a/integration-tests/src/user/runtime_user.rs +++ b/integration-tests/src/user/runtime_user.rs @@ -130,9 +130,7 @@ impl RuntimeUser { } RuntimeError::ReceiptValidationError(e) => panic!("{}", e), RuntimeError::ValidatorError(e) => panic!("{}", e), - RuntimeError::GlobalContractError(_e) => { - todo!() - } + RuntimeError::GlobalContractError(e) => panic!("{}", e), })?; for outcome_with_id in apply_result.outcomes { self.transaction_results From 037611a94035758a1c75185b6f2d6acb495c2f93 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Mon, 10 Feb 2025 16:17:11 +0100 Subject: [PATCH 28/43] Revert "remove useless fn" This reverts commit 067c5bb546e7b7668652b0e6f2ced662b62530ce. --- core/store/src/trie/update.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index dd57a75cdaf..d908df8e1ed 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -164,6 +164,14 @@ impl TrieUpdate { self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, Some(code_hash)))) } + pub fn get_global_code( + &self, + identifier: GlobalContractIdentifier, + ) -> Result, StorageError> { + let key = TrieKey::GlobalContractCode { identifier: identifier.into() }; + self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, None))) + } + /// Returns the size (in num bytes) of the contract code for the given account. /// /// This is different from `get_code` in that it does not read the code from storage. From 139f5ff3fe3b14023fe532e5d13904356de6deb7 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 10:05:14 +0100 Subject: [PATCH 29/43] mvp --- core/store/src/trie/update.rs | 18 +++++++++++++++--- runtime/runtime/src/actions.rs | 2 ++ runtime/runtime/src/lib.rs | 24 ++++++++++++++++++++---- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index d908df8e1ed..a9626148702 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -5,10 +5,12 @@ use crate::contract::ContractStorage; use crate::trie::TrieAccess; use crate::trie::{KeyLookupMode, TrieChanges}; use crate::StorageError; +use near_primitives::account::AccountContract; +use near_primitives::action::GlobalContractIdentifier; use near_primitives::apply::ApplyChunkReason; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::stateless_validation::contract_distribution::ContractUpdates; -use near_primitives::trie_key::TrieKey; +use near_primitives::trie_key::{GlobalContractCodeIdentifier, TrieKey}; use near_primitives::types::{ AccountId, RawStateChange, RawStateChanges, RawStateChangesWithTrieKey, StateChangeCause, StateRoot, TrieCacheMode, @@ -323,6 +325,7 @@ impl TrieUpdate { &self, account_id: AccountId, code_hash: CryptoHash, + account_contract: AccountContract, apply_reason: ApplyChunkReason, protocol_version: ProtocolVersion, ) -> Result<(), StorageError> { @@ -341,10 +344,19 @@ impl TrieUpdate { // Only record the call if trie contains the contract (with the given hash) being called deployed to the given account. // This avoids recording contracts that do not exist or are newly-deployed to the account. // Note that the check below to see if the contract exists has no side effects (not charging gas or recording trie nodes) - if code_hash == CryptoHash::default() { + if account_contract == AccountContract::None { return Ok(()); } - let trie_key = TrieKey::ContractCode { account_id }; + let trie_key = match account_contract { + AccountContract::None => return Ok(()), + AccountContract::Local(_) => TrieKey::ContractCode { account_id }, + AccountContract::Global(code_hash) => TrieKey::GlobalContractCode { + identifier: GlobalContractCodeIdentifier::CodeHash(code_hash), + }, + AccountContract::GlobalByAccount(account_id) => TrieKey::GlobalContractCode { + identifier: GlobalContractCodeIdentifier::AccountId(account_id), + }, + }; let contract_ref = self .trie .get_optimized_ref_no_side_effects(&trie_key.to_vec(), KeyLookupMode::FlatStorage) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index f2c976cd823..477ff0a4771 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -180,6 +180,7 @@ pub(crate) fn action_function_call( is_last_action: bool, epoch_info_provider: &dyn EpochInfoProvider, contract: Box, + account_contract: AccountContract, ) -> Result<(), RuntimeError> { if account.amount().checked_add(function_call.deposit).is_none() { return Err(StorageError::StorageInconsistentState( @@ -191,6 +192,7 @@ pub(crate) fn action_function_call( state_update.record_contract_call( account_id.clone(), code_hash, + account_contract, apply_state.apply_reason.clone(), apply_state.current_protocol_version, )?; diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 6321b48afe5..ee97601aed2 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -19,15 +19,15 @@ use metrics::ApplyMetrics; pub use near_crypto; use near_parameters::{ActionCosts, RuntimeConfig}; pub use near_primitives; -use near_primitives::account::Account; +use near_primitives::account::{Account, AccountContract}; use near_primitives::action::GlobalContractIdentifier; use near_primitives::bandwidth_scheduler::{BandwidthRequests, BlockBandwidthRequests}; use near_primitives::checked_feature; use near_primitives::chunk_apply_stats::{BalanceStats, ChunkApplyStatsV0}; use near_primitives::congestion_info::{BlockCongestionInfo, CongestionInfo}; use near_primitives::errors::{ - ActionError, ActionErrorKind, EpochError, IntegerOverflowError, InvalidTxError, RuntimeError, - TxExecutionError, + ActionError, ActionErrorKind, EpochError, GlobalContractError, IntegerOverflowError, + InvalidTxError, RuntimeError, TxExecutionError, }; use near_primitives::hash::CryptoHash; use near_primitives::receipt::{ @@ -485,7 +485,22 @@ impl Runtime { } Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); - let code_hash = account.local_contract_hash().unwrap_or_default(); + let account_contract = account.contract(); + let code_hash = match &account_contract { + AccountContract::None => CryptoHash::default(), + AccountContract::Local(code_hash) | AccountContract::Global(code_hash) => { + *code_hash + } + AccountContract::GlobalByAccount(account_id) => { + let identifier = GlobalContractIdentifier::AccountId(account_id.clone()); + let code = state_update.get_global_code(identifier.clone())?.ok_or( + RuntimeError::GlobalContractError( + GlobalContractError::IdentifierNotFound(identifier), + ), + )?; + *code.hash() + } + }; let contract = preparation_pipeline.get_contract(receipt, code_hash, action_index, None); let is_last_action = action_index + 1 == actions.len(); @@ -505,6 +520,7 @@ impl Runtime { is_last_action, epoch_info_provider, contract, + account_contract, )?; } Action::Transfer(TransferAction { deposit }) => { From 5f10308ff7be9744d729f6c1a62c590333027adc Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 11:02:23 +0100 Subject: [PATCH 30/43] test wip failing --- core/store/src/trie/update.rs | 8 +-- .../contract_distribution_cross_shard.rs | 63 +++++++++++++++++- .../src/test_loop/utils/transactions.rs | 66 +++++++++++++++++++ runtime/runtime/src/actions.rs | 9 +-- runtime/runtime/src/lib.rs | 6 +- 5 files changed, 141 insertions(+), 11 deletions(-) diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index a9626148702..afeac7a1dbe 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -325,7 +325,7 @@ impl TrieUpdate { &self, account_id: AccountId, code_hash: CryptoHash, - account_contract: AccountContract, + account_contract: &AccountContract, apply_reason: ApplyChunkReason, protocol_version: ProtocolVersion, ) -> Result<(), StorageError> { @@ -344,17 +344,17 @@ impl TrieUpdate { // Only record the call if trie contains the contract (with the given hash) being called deployed to the given account. // This avoids recording contracts that do not exist or are newly-deployed to the account. // Note that the check below to see if the contract exists has no side effects (not charging gas or recording trie nodes) - if account_contract == AccountContract::None { + if account_contract.is_none() { return Ok(()); } let trie_key = match account_contract { AccountContract::None => return Ok(()), AccountContract::Local(_) => TrieKey::ContractCode { account_id }, AccountContract::Global(code_hash) => TrieKey::GlobalContractCode { - identifier: GlobalContractCodeIdentifier::CodeHash(code_hash), + identifier: GlobalContractCodeIdentifier::CodeHash(*code_hash), }, AccountContract::GlobalByAccount(account_id) => TrieKey::GlobalContractCode { - identifier: GlobalContractCodeIdentifier::AccountId(account_id), + identifier: GlobalContractCodeIdentifier::AccountId(account_id.clone()), }, }; let contract_ref = self diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index af89ff790f8..295c4c6270d 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -4,6 +4,8 @@ use near_chain_configs::test_genesis::{ build_genesis_and_epoch_config_store, GenesisAndEpochConfigParams, ValidatorsSpec, }; use near_o11y::testonly::init_test_logger; +use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; +use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::types::AccountId; use near_primitives::version::PROTOCOL_VERSION; @@ -17,7 +19,8 @@ use crate::test_loop::utils::contract_distribution::{ }; use crate::test_loop::utils::get_head_height; use crate::test_loop::utils::transactions::{ - call_contract, check_txs, deploy_contract, make_accounts, + call_contract, check_txs, deploy_contract, deploy_global_contract, make_accounts, + use_global_contract, }; const EPOCH_LENGTH: u64 = 10; @@ -29,6 +32,64 @@ const NUM_RPC: usize = 1; const NUM_VALIDATORS: usize = NUM_BLOCK_AND_CHUNK_PRODUCERS + NUM_CHUNK_VALIDATORS_ONLY; const NUM_ACCOUNTS: usize = NUM_VALIDATORS + NUM_RPC; +#[test] +fn test_global_contracts() { + init_test_logger(); + tracing::info!("QQP START"); + let accounts = make_accounts(NUM_ACCOUNTS); + + let (mut env, rpc_id) = setup(&accounts); + + let mut nonce = 1; + let rpc_index = 8; + assert_eq!(accounts[rpc_index], rpc_id); + + let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); + let deploy_mode = GlobalContractDeployMode::CodeHash; + let deploy_tx = deploy_global_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + &accounts[0], + contract.code().into(), + deploy_mode, + nonce, + ); + nonce += 1; + env.test_loop.run_for(Duration::seconds(3)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); + panic!("QQP PANIC"); + let code_hash = CryptoHash::hash_bytes(contract.code()); + let identifier = GlobalContractIdentifier::CodeHash(code_hash); + // test on accounts from different shards + for account in [&accounts[1], &accounts[6]] { + let use_tx = use_global_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + account, + identifier.clone(), + nonce, + ); + nonce += 1; + env.test_loop.run_for(Duration::seconds(3)); + // check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); + let call_tx = call_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + account, + account, + "log_something".to_owned(), + vec![], + nonce, + ); + env.test_loop.run_for(Duration::seconds(3)); + // check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); + } + env.shutdown_and_drain_remaining_events(Duration::seconds(20)); +} + /// Tests a scenario that different contracts are deployed to a number of accounts and /// these contracts are called from a set of accounts. /// Test setup: 2 shards with 9 accounts, for 8 validators and 1 RPC node. diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 0a652adc167..4ede32188d6 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -13,6 +13,10 @@ use near_client::test_utils::test_loop::ClientQueries; use near_client::{Client, ProcessTxResponse}; use near_crypto::Signer; use near_network::client::ProcessTxRequest; +use near_primitives::action::{ + Action, DeployGlobalContractAction, GlobalContractDeployMode, GlobalContractIdentifier, + UseGlobalContractAction, +}; use near_primitives::block::Tip; use near_primitives::errors::InvalidTxError; use near_primitives::hash::CryptoHash; @@ -303,6 +307,68 @@ pub fn deploy_contract( tx_hash } +pub fn deploy_global_contract( + test_loop: &mut TestLoopV2, + node_datas: &[TestData], + rpc_id: &AccountId, + account_id: &AccountId, + code: Vec, + deploy_mode: GlobalContractDeployMode, + nonce: u64, +) -> CryptoHash { + let block_hash = get_shared_block_hash(node_datas, &test_loop.data); + let signer = create_user_test_signer(&account_id); + + let tx = SignedTransaction::from_actions( + nonce, + account_id.clone(), + account_id.clone(), + &signer, + vec![Action::DeployGlobalContract(DeployGlobalContractAction { + code: code.into(), + deploy_mode, + })], + block_hash, + 0, + ); + let tx_hash = tx.get_hash(); + submit_tx(node_datas, rpc_id, tx); + + println!("QQP"); + tracing::info!(target: "test", ?account_id, ?tx_hash, "QQP deployed global contract"); + tx_hash +} + +pub fn use_global_contract( + test_loop: &mut TestLoopV2, + node_datas: &[TestData], + rpc_id: &AccountId, + account_id: &AccountId, + // global_code_hash: CryptoHash, + global_contract_identifier: GlobalContractIdentifier, + nonce: u64, +) -> CryptoHash { + let block_hash = get_shared_block_hash(node_datas, &test_loop.data); + let signer = create_user_test_signer(&account_id); + + let tx = SignedTransaction::from_actions( + nonce, + account_id.clone(), + account_id.clone(), + &signer, + vec![Action::UseGlobalContract(Box::new(UseGlobalContractAction { + contract_identifier: global_contract_identifier, + }))], + block_hash, + 0, + ); + let tx_hash = tx.get_hash(); + submit_tx(node_datas, rpc_id, tx); + + tracing::debug!(target: "test", ?account_id, ?tx_hash, "used global contract"); + tx_hash +} + /// Call the contract deployed at contract id from the sender id. /// /// This function does not wait until the transactions is executed. diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 1c27c67d099..e6c85e85ab0 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -180,7 +180,6 @@ pub(crate) fn action_function_call( is_last_action: bool, epoch_info_provider: &dyn EpochInfoProvider, contract: Box, - account_contract: AccountContract, ) -> Result<(), RuntimeError> { if account.amount().checked_add(function_call.deposit).is_none() { return Err(StorageError::StorageInconsistentState( @@ -189,10 +188,11 @@ pub(crate) fn action_function_call( .into()); } + let account_contract = account.contract(); state_update.record_contract_call( account_id.clone(), code_hash, - account_contract, + account_contract.as_ref(), apply_state.apply_reason.clone(), apply_state.current_protocol_version, )?; @@ -634,6 +634,7 @@ pub(crate) fn action_deploy_global_contract( result: &mut ActionResult, ) { let _span = tracing::debug_span!(target: "runtime", "action_deploy_global_contract").entered(); + tracing::info!("QQP Deploying Global Contract: {:#?}", deploy_contract); let storage_cost = apply_state .config @@ -681,11 +682,11 @@ pub(crate) fn action_use_global_contract( action.contract_identifier.clone(), ))); } - if let AccountContract::Local(code_hash) = account.contract() { + if let AccountContract::Local(code_hash) = account.contract().as_ref() { let prev_code_len = get_code_len_or_default( state_update, account_id.clone(), - code_hash, + *code_hash, current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 7eba345f212..6f6d627e630 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -471,6 +471,8 @@ impl Runtime { )?; } Action::DeployGlobalContract(deploy_global_contract) => { + tracing::info!("QQP Processing Action::DeployGlobalContract"); + println!("QQP Processing Action::DeployGlobalContract"); let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); action_deploy_global_contract( account, @@ -493,7 +495,7 @@ impl Runtime { Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); let account_contract = account.contract(); - let code_hash = match &account_contract { + let code_hash = match account_contract.as_ref() { AccountContract::None => CryptoHash::default(), AccountContract::Local(code_hash) | AccountContract::Global(code_hash) => { *code_hash @@ -527,7 +529,6 @@ impl Runtime { is_last_action, epoch_info_provider, contract, - account_contract, )?; } Action::Transfer(TransferAction { deposit }) => { @@ -672,6 +673,7 @@ impl Runtime { apply_state.block_height, action_index, ); + tracing::info!("QQP Applying action {:?}", action); let mut new_result = self.apply_action( action, state_update, From c4e603ab10acfc3925a673419570838482b38d24 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 11:15:48 +0100 Subject: [PATCH 31/43] remove debug logs --- .../test_loop/tests/contract_distribution_cross_shard.rs | 6 +++--- runtime/runtime/src/actions.rs | 1 - runtime/runtime/src/lib.rs | 6 ++---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 295c4c6270d..225adb85f4b 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -58,7 +58,7 @@ fn test_global_contracts() { nonce += 1; env.test_loop.run_for(Duration::seconds(3)); check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); - panic!("QQP PANIC"); + // panic!("QQP PANIC"); let code_hash = CryptoHash::hash_bytes(contract.code()); let identifier = GlobalContractIdentifier::CodeHash(code_hash); // test on accounts from different shards @@ -73,7 +73,7 @@ fn test_global_contracts() { ); nonce += 1; env.test_loop.run_for(Duration::seconds(3)); - // check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); let call_tx = call_contract( &mut env.test_loop, &env.datas, @@ -85,7 +85,7 @@ fn test_global_contracts() { nonce, ); env.test_loop.run_for(Duration::seconds(3)); - // check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); } env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index e6c85e85ab0..0d9d7fdd28d 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -634,7 +634,6 @@ pub(crate) fn action_deploy_global_contract( result: &mut ActionResult, ) { let _span = tracing::debug_span!(target: "runtime", "action_deploy_global_contract").entered(); - tracing::info!("QQP Deploying Global Contract: {:#?}", deploy_contract); let storage_cost = apply_state .config diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 6f6d627e630..39196b566b9 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -471,8 +471,6 @@ impl Runtime { )?; } Action::DeployGlobalContract(deploy_global_contract) => { - tracing::info!("QQP Processing Action::DeployGlobalContract"); - println!("QQP Processing Action::DeployGlobalContract"); let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); action_deploy_global_contract( account, @@ -673,7 +671,6 @@ impl Runtime { apply_state.block_height, action_index, ); - tracing::info!("QQP Applying action {:?}", action); let mut new_result = self.apply_action( action, state_update, @@ -2203,7 +2200,8 @@ impl Runtime { &receipt_sink.outgoing_receipts(), &stats.balance, ) { - panic!( + // panic!( + tracing::error!( "The runtime's balance_checker failed for shard {} at height {} with block hash {} and protocol version {}: {}", apply_state.shard_id, apply_state.block_height, From d3cf5ed413e282d5dad90c27d12aefb14590c806 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 11:32:38 +0100 Subject: [PATCH 32/43] remove logs --- .../src/test_loop/tests/contract_distribution_cross_shard.rs | 2 -- integration-tests/src/test_loop/utils/transactions.rs | 2 -- 2 files changed, 4 deletions(-) diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 225adb85f4b..2936e057cd5 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -35,7 +35,6 @@ const NUM_ACCOUNTS: usize = NUM_VALIDATORS + NUM_RPC; #[test] fn test_global_contracts() { init_test_logger(); - tracing::info!("QQP START"); let accounts = make_accounts(NUM_ACCOUNTS); let (mut env, rpc_id) = setup(&accounts); @@ -58,7 +57,6 @@ fn test_global_contracts() { nonce += 1; env.test_loop.run_for(Duration::seconds(3)); check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); - // panic!("QQP PANIC"); let code_hash = CryptoHash::hash_bytes(contract.code()); let identifier = GlobalContractIdentifier::CodeHash(code_hash); // test on accounts from different shards diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 4ede32188d6..b4368e3e682 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -334,8 +334,6 @@ pub fn deploy_global_contract( let tx_hash = tx.get_hash(); submit_tx(node_datas, rpc_id, tx); - println!("QQP"); - tracing::info!(target: "test", ?account_id, ?tx_hash, "QQP deployed global contract"); tx_hash } From 713d0b6dc8ebfc0dc3aaf66637125d266da5dffe Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 11:46:48 +0100 Subject: [PATCH 33/43] tests --- .../contract_distribution_cross_shard.rs | 116 +++++++++++------- 1 file changed, 69 insertions(+), 47 deletions(-) diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 2936e057cd5..249eb25e8c1 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -33,7 +33,27 @@ const NUM_VALIDATORS: usize = NUM_BLOCK_AND_CHUNK_PRODUCERS + NUM_CHUNK_VALIDATO const NUM_ACCOUNTS: usize = NUM_VALIDATORS + NUM_RPC; #[test] -fn test_global_contracts() { +fn test_global_contract_by_hash() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::CodeHash; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); +} + +#[test] +fn test_global_contract_by_account_id() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::AccountId; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); +} + +/// Tests a scenario that different contracts are deployed to a number of accounts and +/// these contracts are called from a set of accounts. +/// Test setup: 2 shards with 9 accounts, for 8 validators and 1 RPC node. +/// Deploys contract to one account from each shard. +/// Make 2 accounts from each shard make calls to these contracts. +#[cfg_attr(not(feature = "test_features"), ignore)] +#[test] +fn test_contract_distribution_cross_shard() { init_test_logger(); let accounts = make_accounts(NUM_ACCOUNTS); @@ -43,8 +63,55 @@ fn test_global_contracts() { let rpc_index = 8; assert_eq!(accounts[rpc_index], rpc_id); + // Deploy a contract for each shard (account0 from first one, and account4 from second one). + // Then take two accounts from each shard (one with a contract deployed and one without) and + // make them call both the contracts, so we cover same-shard and cross-shard contract calls. + let contract_ids = [&accounts[0], &accounts[4]]; + let sender_ids = [&accounts[0], &accounts[1], &accounts[4], &accounts[5]]; + + let start_height = get_head_height(&mut env); + + // First deploy and call the contracts as described above. + // Next, clear the compiled contract cache and repeat the same contract calls. + let contracts = deploy_contracts(&mut env, &rpc_id, &contract_ids, &mut nonce); + + for contract in contracts.into_iter() { + run_until_caches_contain_contract(&mut env, contract.hash()); + } + + call_contracts(&mut env, &rpc_id, &contract_ids, &sender_ids, &mut nonce); + + clear_compiled_contract_caches(&mut env); + + call_contracts(&mut env, &rpc_id, &contract_ids, &sender_ids, &mut nonce); + + let end_height = get_head_height(&mut env); + assert_all_chunk_endorsements_received(&mut env, start_height, end_height); + + env.shutdown_and_drain_remaining_events(Duration::seconds(20)); +} + +fn setup_global_contract_test() -> (TestLoopEnv, Vec, ContractCode, AccountId) { + init_test_logger(); + let accounts = make_accounts(NUM_ACCOUNTS); + + let (env, rpc_id) = setup(&accounts); + + let rpc_index = 8; + assert_eq!(accounts[rpc_index], rpc_id); + let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); - let deploy_mode = GlobalContractDeployMode::CodeHash; + (env, accounts, contract, rpc_id) +} + +fn test_global_contract( + mut env: TestLoopEnv, + deploy_mode: GlobalContractDeployMode, + accounts: &[AccountId], + contract: &ContractCode, + rpc_id: AccountId, +) { + let mut nonce = 1; let deploy_tx = deploy_global_contract( &mut env.test_loop, &env.datas, @@ -88,51 +155,6 @@ fn test_global_contracts() { env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } -/// Tests a scenario that different contracts are deployed to a number of accounts and -/// these contracts are called from a set of accounts. -/// Test setup: 2 shards with 9 accounts, for 8 validators and 1 RPC node. -/// Deploys contract to one account from each shard. -/// Make 2 accounts from each shard make calls to these contracts. -#[cfg_attr(not(feature = "test_features"), ignore)] -#[test] -fn test_contract_distribution_cross_shard() { - init_test_logger(); - let accounts = make_accounts(NUM_ACCOUNTS); - - let (mut env, rpc_id) = setup(&accounts); - - let mut nonce = 1; - let rpc_index = 8; - assert_eq!(accounts[rpc_index], rpc_id); - - // Deploy a contract for each shard (account0 from first one, and account4 from second one). - // Then take two accounts from each shard (one with a contract deployed and one without) and - // make them call both the contracts, so we cover same-shard and cross-shard contract calls. - let contract_ids = [&accounts[0], &accounts[4]]; - let sender_ids = [&accounts[0], &accounts[1], &accounts[4], &accounts[5]]; - - let start_height = get_head_height(&mut env); - - // First deploy and call the contracts as described above. - // Next, clear the compiled contract cache and repeat the same contract calls. - let contracts = deploy_contracts(&mut env, &rpc_id, &contract_ids, &mut nonce); - - for contract in contracts.into_iter() { - run_until_caches_contain_contract(&mut env, contract.hash()); - } - - call_contracts(&mut env, &rpc_id, &contract_ids, &sender_ids, &mut nonce); - - clear_compiled_contract_caches(&mut env); - - call_contracts(&mut env, &rpc_id, &contract_ids, &sender_ids, &mut nonce); - - let end_height = get_head_height(&mut env); - assert_all_chunk_endorsements_received(&mut env, start_height, end_height); - - env.shutdown_and_drain_remaining_events(Duration::seconds(20)); -} - fn setup(accounts: &Vec) -> (TestLoopEnv, AccountId) { let builder = TestLoopBuilder::new(); From 6500dde58ea5302dbdf38ffe13775508c2d51fba Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 11:48:22 +0100 Subject: [PATCH 34/43] remove comment --- integration-tests/src/test_loop/utils/transactions.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index b4368e3e682..2e78d69ae20 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -342,7 +342,6 @@ pub fn use_global_contract( node_datas: &[TestData], rpc_id: &AccountId, account_id: &AccountId, - // global_code_hash: CryptoHash, global_contract_identifier: GlobalContractIdentifier, nonce: u64, ) -> CryptoHash { From b4c4c2fc0495de8e01e9bde1c78918a50809fdab Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 12:50:42 +0100 Subject: [PATCH 35/43] fix --- .../tests/contract_distribution_cross_shard.rs | 13 ++++++++++--- runtime/runtime/src/lib.rs | 1 + 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 249eb25e8c1..14235c6663f 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -118,14 +118,21 @@ fn test_global_contract( &rpc_id, &accounts[0], contract.code().into(), - deploy_mode, + deploy_mode.clone(), nonce, ); nonce += 1; env.test_loop.run_for(Duration::seconds(3)); check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); - let code_hash = CryptoHash::hash_bytes(contract.code()); - let identifier = GlobalContractIdentifier::CodeHash(code_hash); + let identifier = match deploy_mode { + GlobalContractDeployMode::CodeHash => { + let code_hash = CryptoHash::hash_bytes(contract.code()); + GlobalContractIdentifier::CodeHash(code_hash) + } + GlobalContractDeployMode::AccountId => { + GlobalContractIdentifier::AccountId(accounts[0].clone()) + } + }; // test on accounts from different shards for account in [&accounts[1], &accounts[6]] { let use_tx = use_global_contract( diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 39196b566b9..56be6e05bde 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -493,6 +493,7 @@ impl Runtime { Action::FunctionCall(function_call) => { let account = account.as_mut().expect(EXPECT_ACCOUNT_EXISTS); let account_contract = account.contract(); + let code_hash = match account_contract.as_ref() { AccountContract::None => CryptoHash::default(), AccountContract::Local(code_hash) | AccountContract::Global(code_hash) => { From a1eed5b3dbc62b50f0e2c4febc1779b35a070b9e Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Tue, 11 Feb 2025 13:19:29 +0100 Subject: [PATCH 36/43] move tests behind nightly_protocol feature --- .../contract_distribution_cross_shard.rs | 155 +++++++++--------- .../src/test_loop/utils/transactions.rs | 3 + 2 files changed, 83 insertions(+), 75 deletions(-) diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 14235c6663f..0c259edf8bc 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -4,8 +4,6 @@ use near_chain_configs::test_genesis::{ build_genesis_and_epoch_config_store, GenesisAndEpochConfigParams, ValidatorsSpec, }; use near_o11y::testonly::init_test_logger; -use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; -use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::types::AccountId; use near_primitives::version::PROTOCOL_VERSION; @@ -19,8 +17,7 @@ use crate::test_loop::utils::contract_distribution::{ }; use crate::test_loop::utils::get_head_height; use crate::test_loop::utils::transactions::{ - call_contract, check_txs, deploy_contract, deploy_global_contract, make_accounts, - use_global_contract, + call_contract, check_txs, deploy_contract, make_accounts, }; const EPOCH_LENGTH: u64 = 10; @@ -32,20 +29,6 @@ const NUM_RPC: usize = 1; const NUM_VALIDATORS: usize = NUM_BLOCK_AND_CHUNK_PRODUCERS + NUM_CHUNK_VALIDATORS_ONLY; const NUM_ACCOUNTS: usize = NUM_VALIDATORS + NUM_RPC; -#[test] -fn test_global_contract_by_hash() { - let (env, accounts, contract, rpc_id) = setup_global_contract_test(); - let deploy_mode = GlobalContractDeployMode::CodeHash; - test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); -} - -#[test] -fn test_global_contract_by_account_id() { - let (env, accounts, contract, rpc_id) = setup_global_contract_test(); - let deploy_mode = GlobalContractDeployMode::AccountId; - test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); -} - /// Tests a scenario that different contracts are deployed to a number of accounts and /// these contracts are called from a set of accounts. /// Test setup: 2 shards with 9 accounts, for 8 validators and 1 RPC node. @@ -91,75 +74,97 @@ fn test_contract_distribution_cross_shard() { env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } -fn setup_global_contract_test() -> (TestLoopEnv, Vec, ContractCode, AccountId) { - init_test_logger(); - let accounts = make_accounts(NUM_ACCOUNTS); +#[cfg(feature = "nightly_protocol")] +mod global_contracts_tests { + use super::*; + use crate::test_loop::utils::transactions::{deploy_global_contract, use_global_contract}; + use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; + use near_primitives::hash::CryptoHash; - let (env, rpc_id) = setup(&accounts); + #[test] + fn test_global_contract_by_hash() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::CodeHash; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); + } - let rpc_index = 8; - assert_eq!(accounts[rpc_index], rpc_id); + #[test] + fn test_global_contract_by_account_id() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::AccountId; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); + } - let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); - (env, accounts, contract, rpc_id) -} + fn setup_global_contract_test() -> (TestLoopEnv, Vec, ContractCode, AccountId) { + init_test_logger(); + let accounts = make_accounts(NUM_ACCOUNTS); -fn test_global_contract( - mut env: TestLoopEnv, - deploy_mode: GlobalContractDeployMode, - accounts: &[AccountId], - contract: &ContractCode, - rpc_id: AccountId, -) { - let mut nonce = 1; - let deploy_tx = deploy_global_contract( - &mut env.test_loop, - &env.datas, - &rpc_id, - &accounts[0], - contract.code().into(), - deploy_mode.clone(), - nonce, - ); - nonce += 1; - env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); - let identifier = match deploy_mode { - GlobalContractDeployMode::CodeHash => { - let code_hash = CryptoHash::hash_bytes(contract.code()); - GlobalContractIdentifier::CodeHash(code_hash) - } - GlobalContractDeployMode::AccountId => { - GlobalContractIdentifier::AccountId(accounts[0].clone()) - } - }; - // test on accounts from different shards - for account in [&accounts[1], &accounts[6]] { - let use_tx = use_global_contract( + let (env, rpc_id) = setup(&accounts); + + let rpc_index = 8; + assert_eq!(accounts[rpc_index], rpc_id); + + let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); + (env, accounts, contract, rpc_id) + } + + fn test_global_contract( + mut env: TestLoopEnv, + deploy_mode: GlobalContractDeployMode, + accounts: &[AccountId], + contract: &ContractCode, + rpc_id: AccountId, + ) { + let mut nonce = 1; + let deploy_tx = deploy_global_contract( &mut env.test_loop, &env.datas, &rpc_id, - account, - identifier.clone(), + &accounts[0], + contract.code().into(), + deploy_mode.clone(), nonce, ); nonce += 1; env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); - let call_tx = call_contract( - &mut env.test_loop, - &env.datas, - &rpc_id, - account, - account, - "log_something".to_owned(), - vec![], - nonce, - ); - env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); + let identifier = match deploy_mode { + GlobalContractDeployMode::CodeHash => { + let code_hash = CryptoHash::hash_bytes(contract.code()); + GlobalContractIdentifier::CodeHash(code_hash) + } + GlobalContractDeployMode::AccountId => { + GlobalContractIdentifier::AccountId(accounts[0].clone()) + } + }; + // test on accounts from different shards + for account in [&accounts[1], &accounts[6]] { + let use_tx = use_global_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + account, + identifier.clone(), + nonce, + ); + nonce += 1; + env.test_loop.run_for(Duration::seconds(3)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); + let call_tx = call_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + account, + account, + "log_something".to_owned(), + vec![], + nonce, + ); + env.test_loop.run_for(Duration::seconds(3)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); + } + env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } - env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } fn setup(accounts: &Vec) -> (TestLoopEnv, AccountId) { diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 2e78d69ae20..1289e26373f 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -13,6 +13,7 @@ use near_client::test_utils::test_loop::ClientQueries; use near_client::{Client, ProcessTxResponse}; use near_crypto::Signer; use near_network::client::ProcessTxRequest; +#[cfg(feature = "nightly_protocol")] use near_primitives::action::{ Action, DeployGlobalContractAction, GlobalContractDeployMode, GlobalContractIdentifier, UseGlobalContractAction, @@ -307,6 +308,7 @@ pub fn deploy_contract( tx_hash } +#[cfg(feature = "nightly_protocol")] pub fn deploy_global_contract( test_loop: &mut TestLoopV2, node_datas: &[TestData], @@ -337,6 +339,7 @@ pub fn deploy_global_contract( tx_hash } +#[cfg(feature = "nightly_protocol")] pub fn use_global_contract( test_loop: &mut TestLoopV2, node_datas: &[TestData], From be89a14c79fee2422c6e0846440b25a8d067eaba Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 12 Feb 2025 11:42:36 +0100 Subject: [PATCH 37/43] remove get_global_code fn and update errors --- chain/chain-primitives/src/error.rs | 4 ++-- chain/chain/src/runtime/mod.rs | 2 +- core/store/src/trie/update.rs | 9 --------- runtime/runtime/src/lib.rs | 16 +++++++++------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 6f7448522cc..0b9a5cf699f 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -1,6 +1,6 @@ use near_primitives::block::BlockValidityError; use near_primitives::challenge::{ChunkProofs, ChunkState, MaybeEncodedShardChunk}; -use near_primitives::errors::{ChunkAccessError, EpochError, StorageError}; +use near_primitives::errors::{ChunkAccessError, EpochError, GlobalContractError, StorageError}; use near_primitives::shard_layout::ShardLayoutError; use near_primitives::sharding::{BadHeaderForProtocolVersionError, ChunkHash, ShardChunkHeader}; use near_primitives::types::{BlockHeight, EpochId, ShardId, ShardIndex}; @@ -253,7 +253,7 @@ pub enum Error { BadHeaderForProtocolVersion(#[from] BadHeaderForProtocolVersionError), /// Global contract error. #[error("Global Contract Error: {0}")] - GlobalContractError(String), + GlobalContractError(GlobalContractError), /// Anything else #[error("Other Error: {0}")] Other(String), diff --git a/chain/chain/src/runtime/mod.rs b/chain/chain/src/runtime/mod.rs index 377c7100a6b..04aba5d79dd 100644 --- a/chain/chain/src/runtime/mod.rs +++ b/chain/chain/src/runtime/mod.rs @@ -336,7 +336,7 @@ impl NightshadeRuntime { // TODO(#2152): process gracefully RuntimeError::ReceiptValidationError(e) => panic!("{}", e), RuntimeError::ValidatorError(e) => e.into(), - RuntimeError::GlobalContractError(e) => Error::Other(e.to_string()), + RuntimeError::GlobalContractError(e) => Error::GlobalContractError(e), })?; let elapsed = instant.elapsed(); diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index b12ef5e1aff..69ddca2aa10 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -6,7 +6,6 @@ use crate::trie::TrieAccess; use crate::trie::{KeyLookupMode, TrieChanges}; use crate::StorageError; use near_primitives::account::AccountContract; -use near_primitives::action::GlobalContractIdentifier; use near_primitives::apply::ApplyChunkReason; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::stateless_validation::contract_distribution::ContractUpdates; @@ -190,14 +189,6 @@ impl TrieUpdate { self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, Some(code_hash)))) } - pub fn get_global_code( - &self, - identifier: GlobalContractIdentifier, - ) -> Result, StorageError> { - let key = TrieKey::GlobalContractCode { identifier: identifier.into() }; - self.get(&key).map(|opt| opt.map(|code| ContractCode::new(code, None))) - } - /// Returns the size (in num bytes) of the contract code for the given account. /// /// This is different from `get_code` in that it does not read the code from storage. diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 0b53fa9a6bb..987c7a05002 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -60,8 +60,8 @@ use near_store::{ get, get_account, get_postponed_receipt, get_promise_yield_receipt, get_pure, get_received_data, has_received_data, remove_account, remove_postponed_receipt, remove_promise_yield_receipt, set, set_access_key, set_account, set_postponed_receipt, - set_promise_yield_receipt, set_received_data, PartialStorage, StorageError, Trie, TrieAccess, - TrieChanges, TrieUpdate, + set_promise_yield_receipt, set_received_data, KeyLookupMode, PartialStorage, StorageError, + Trie, TrieAccess, TrieChanges, TrieUpdate, }; use near_vm_runner::logic::types::PromiseResult; use near_vm_runner::logic::ReturnData; @@ -501,12 +501,14 @@ impl Runtime { } AccountContract::GlobalByAccount(account_id) => { let identifier = GlobalContractIdentifier::AccountId(account_id.clone()); - let code = state_update.get_global_code(identifier.clone())?.ok_or( - RuntimeError::GlobalContractError( + let key = + TrieKey::GlobalContractCode { identifier: identifier.clone().into() }; + let value_ref = state_update + .get_ref_no_side_effects(&key, KeyLookupMode::FlatStorage)? + .ok_or(RuntimeError::GlobalContractError( GlobalContractError::IdentifierNotFound(identifier), - ), - )?; - *code.hash() + ))?; + value_ref.value_hash() } }; let contract = From 71cca0809d6154e7cbbe2b18dcbe1f291e6c9b16 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Wed, 12 Feb 2025 14:30:09 +0100 Subject: [PATCH 38/43] . --- runtime/runtime/src/balance_checker.rs | 1 + runtime/runtime/src/lib.rs | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 7d26fc1c528..313271fc3ee 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -93,6 +93,7 @@ fn total_accounts_balance( None => return Ok(accumulator), Some(account) => (account.amount(), account.locked()), }; + tracing::info!("ACC {:#?} | BAL {:#?} | LCK {:#?}", account_id, amount, locked); Ok(safe_add_balance_apply!(accumulator, amount, locked)) }) } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 987c7a05002..017ce6eaa01 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -2203,8 +2203,7 @@ impl Runtime { &receipt_sink.outgoing_receipts(), &stats.balance, ) { - // panic!( - tracing::error!( + panic!( "The runtime's balance_checker failed for shard {} at height {} with block hash {} and protocol version {}: {}", apply_state.shard_id, apply_state.block_height, From fc319ce98850c618d6e094a57e7f0e3d925cd450 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Thu, 13 Feb 2025 12:49:24 +0100 Subject: [PATCH 39/43] consider global contract deployment receipt cost --- runtime/runtime/src/balance_checker.rs | 46 ++++++++++++++++---------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 313271fc3ee..b96fd40c4ab 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -43,6 +43,7 @@ fn get_delayed_receipts( fn receipt_cost( config: &RuntimeConfig, receipt: &Receipt, + is_outgoing: bool, ) -> Result { Ok(match receipt.receipt() { ReceiptEnum::Action(action_receipt) | ReceiptEnum::PromiseYield(action_receipt) => { @@ -66,9 +67,19 @@ fn receipt_cost( } total_cost } - ReceiptEnum::GlobalContractDistribution(_) - | ReceiptEnum::Data(_) - | ReceiptEnum::PromiseResume(_) => 0, + ReceiptEnum::GlobalContractDistribution(contract_data) => { + // Cost is only paid when the outgoing receipt is generated + if is_outgoing { + config + .fees + .storage_usage_config + .global_contract_storage_amount_per_byte + .saturating_mul(contract_data.code.len() as u128) + } else { + 0 + } + } + ReceiptEnum::Data(_) | ReceiptEnum::PromiseResume(_) => 0, }) } @@ -76,9 +87,10 @@ fn receipt_cost( fn total_receipts_cost( config: &RuntimeConfig, receipts: &[Receipt], + is_outgoing: bool, ) -> Result { receipts.iter().try_fold(0, |accumulator, receipt| { - let cost = receipt_cost(config, receipt)?; + let cost = receipt_cost(config, receipt, is_outgoing)?; safe_add_balance(accumulator, cost) }) } @@ -93,7 +105,6 @@ fn total_accounts_balance( None => return Ok(accumulator), Some(account) => (account.amount(), account.locked()), }; - tracing::info!("ACC {:#?} | BAL {:#?} | LCK {:#?}", account_id, amount, locked); Ok(safe_add_balance_apply!(accumulator, amount, locked)) }) } @@ -117,13 +128,13 @@ fn total_postponed_receipts_cost( PostponedReceiptType::Action => { match get_postponed_receipt(state, account_id, *lookup_id)? { None => return Ok(total), - Some(receipt) => receipt_cost(config, &receipt)?, + Some(receipt) => receipt_cost(config, &receipt, false)?, } } PostponedReceiptType::PromiseYield => { match get_promise_yield_receipt(state, account_id, *lookup_id)? { None => return Ok(total), - Some(receipt) => receipt_cost(config, &receipt)?, + Some(receipt) => receipt_cost(config, &receipt, false)?, } } }; @@ -321,16 +332,17 @@ pub(crate) fn check_balance( validator_accounts_update.as_ref().map(validator_rewards).transpose()?.unwrap_or(0); // Receipts - let receipts_cost = |receipts: &[Receipt]| -> Result { - total_receipts_cost(config, receipts) - }; + let receipts_cost = + |receipts: &[Receipt], is_outgoing: bool| -> Result { + total_receipts_cost(config, receipts, is_outgoing) + }; let incoming_receipts_balance = - receipts_cost(incoming_receipts)? + receipts_cost(yield_timeout_receipts)?; - let outgoing_receipts_balance = receipts_cost(outgoing_receipts)?; - let processed_delayed_receipts_balance = receipts_cost(&processed_delayed_receipts)?; - let new_delayed_receipts_balance = receipts_cost(&new_delayed_receipts)?; - let forwarded_buffered_receipts_balance = receipts_cost(&forwarded_receipts)?; - let new_buffered_receipts_balance = receipts_cost(&new_buffered_receipts)?; + receipts_cost(incoming_receipts, false)? + receipts_cost(yield_timeout_receipts, false)?; + let outgoing_receipts_balance = receipts_cost(outgoing_receipts, true)?; + let processed_delayed_receipts_balance = receipts_cost(&processed_delayed_receipts, false)?; + let new_delayed_receipts_balance = receipts_cost(&new_delayed_receipts, false)?; + let forwarded_buffered_receipts_balance = receipts_cost(&forwarded_receipts, false)?; + let new_buffered_receipts_balance = receipts_cost(&new_buffered_receipts, false)?; // Postponed actions receipts. let all_potential_postponed_receipt_ids = potential_postponed_receipt_ids( @@ -344,8 +356,8 @@ pub(crate) fn check_balance( total_postponed_receipts_cost(initial_state, config, &all_potential_postponed_receipt_ids)?; let final_postponed_receipts_balance = total_postponed_receipts_cost(final_state, config, &all_potential_postponed_receipt_ids)?; - // Sum it up + // Sum it up let initial_balance = safe_add_balance_apply!( incoming_validator_rewards, initial_accounts_balance, From f771c8921333253110e43cefeef1fc314c522ad0 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 14 Feb 2025 12:00:01 +0100 Subject: [PATCH 40/43] address comments --- core/primitives/src/test_utils.rs | 50 ++++++ core/store/src/trie/update.rs | 3 - .../contract_distribution_cross_shard.rs | 151 +++++++++--------- .../src/test_loop/utils/transactions.rs | 30 +--- runtime/runtime/src/actions.rs | 13 +- runtime/runtime/src/lib.rs | 2 +- 6 files changed, 139 insertions(+), 110 deletions(-) diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs index f0669872b82..ae35b47abde 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -1,4 +1,8 @@ use crate::account::{AccessKey, AccessKeyPermission, Account}; +use crate::action::{ + DeployGlobalContractAction, GlobalContractDeployMode, GlobalContractIdentifier, + UseGlobalContractAction, +}; use crate::block::Block; use crate::block_body::{BlockBody, ChunkEndorsementSignatures}; use crate::block_header::BlockHeader; @@ -273,6 +277,52 @@ impl SignedTransaction { ) } + pub fn deploy_global_contract( + nonce: Nonce, + contract_id: &AccountId, + code: Vec, + signer: &Signer, + block_hash: CryptoHash, + deploy_mode: GlobalContractDeployMode, + ) -> SignedTransaction { + let signer_id = contract_id.clone(); + let receiver_id = contract_id.clone(); + SignedTransaction::from_actions( + nonce, + signer_id, + receiver_id, + &signer, + vec![Action::DeployGlobalContract(DeployGlobalContractAction { + code: code.into(), + deploy_mode, + })], + block_hash, + 0, + ) + } + + pub fn use_global_contract( + nonce: Nonce, + contract_id: &AccountId, + signer: &Signer, + block_hash: CryptoHash, + contract_identifier: GlobalContractIdentifier, + ) -> SignedTransaction { + let signer_id = contract_id.clone(); + let receiver_id = contract_id.clone(); + SignedTransaction::from_actions( + nonce, + signer_id, + receiver_id, + &signer, + vec![Action::UseGlobalContract(Box::new(UseGlobalContractAction { + contract_identifier, + }))], + block_hash, + 0, + ) + } + pub fn create_contract( nonce: Nonce, originator: AccountId, diff --git a/core/store/src/trie/update.rs b/core/store/src/trie/update.rs index 69ddca2aa10..539dceeeb50 100644 --- a/core/store/src/trie/update.rs +++ b/core/store/src/trie/update.rs @@ -359,9 +359,6 @@ impl TrieUpdate { // Only record the call if trie contains the contract (with the given hash) being called deployed to the given account. // This avoids recording contracts that do not exist or are newly-deployed to the account. // Note that the check below to see if the contract exists has no side effects (not charging gas or recording trie nodes) - if account_contract.is_none() { - return Ok(()); - } let trie_key = match account_contract { AccountContract::None => return Ok(()), AccountContract::Local(_) => TrieKey::ContractCode { account_id }, diff --git a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs index 0c259edf8bc..0de6d67ad95 100644 --- a/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs +++ b/integration-tests/src/test_loop/tests/contract_distribution_cross_shard.rs @@ -4,6 +4,8 @@ use near_chain_configs::test_genesis::{ build_genesis_and_epoch_config_store, GenesisAndEpochConfigParams, ValidatorsSpec, }; use near_o11y::testonly::init_test_logger; +use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; +use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; use near_primitives::types::AccountId; use near_primitives::version::PROTOCOL_VERSION; @@ -19,6 +21,7 @@ use crate::test_loop::utils::get_head_height; use crate::test_loop::utils::transactions::{ call_contract, check_txs, deploy_contract, make_accounts, }; +use crate::test_loop::utils::transactions::{deploy_global_contract, use_global_contract}; const EPOCH_LENGTH: u64 = 10; const GENESIS_HEIGHT: u64 = 1000; @@ -74,97 +77,91 @@ fn test_contract_distribution_cross_shard() { env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } -#[cfg(feature = "nightly_protocol")] -mod global_contracts_tests { - use super::*; - use crate::test_loop::utils::transactions::{deploy_global_contract, use_global_contract}; - use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; - use near_primitives::hash::CryptoHash; - - #[test] - fn test_global_contract_by_hash() { - let (env, accounts, contract, rpc_id) = setup_global_contract_test(); - let deploy_mode = GlobalContractDeployMode::CodeHash; - test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); - } +#[cfg_attr(not(feature = "nightly_protocol"), ignore)] +#[test] +fn test_global_contract_by_hash() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::CodeHash; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); +} - #[test] - fn test_global_contract_by_account_id() { - let (env, accounts, contract, rpc_id) = setup_global_contract_test(); - let deploy_mode = GlobalContractDeployMode::AccountId; - test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); - } +#[cfg_attr(not(feature = "nightly_protocol"), ignore)] +#[test] +fn test_global_contract_by_account_id() { + let (env, accounts, contract, rpc_id) = setup_global_contract_test(); + let deploy_mode = GlobalContractDeployMode::AccountId; + test_global_contract(env, deploy_mode, accounts.as_slice(), &contract, rpc_id); +} - fn setup_global_contract_test() -> (TestLoopEnv, Vec, ContractCode, AccountId) { - init_test_logger(); - let accounts = make_accounts(NUM_ACCOUNTS); +fn setup_global_contract_test() -> (TestLoopEnv, Vec, ContractCode, AccountId) { + init_test_logger(); + let accounts = make_accounts(NUM_ACCOUNTS); - let (env, rpc_id) = setup(&accounts); + let (env, rpc_id) = setup(&accounts); - let rpc_index = 8; - assert_eq!(accounts[rpc_index], rpc_id); + let rpc_index = 8; + assert_eq!(accounts[rpc_index], rpc_id); - let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); - (env, accounts, contract, rpc_id) - } + let contract = ContractCode::new(near_test_contracts::rs_contract().to_vec(), None); + (env, accounts, contract, rpc_id) +} - fn test_global_contract( - mut env: TestLoopEnv, - deploy_mode: GlobalContractDeployMode, - accounts: &[AccountId], - contract: &ContractCode, - rpc_id: AccountId, - ) { - let mut nonce = 1; - let deploy_tx = deploy_global_contract( +fn test_global_contract( + mut env: TestLoopEnv, + deploy_mode: GlobalContractDeployMode, + accounts: &[AccountId], + contract: &ContractCode, + rpc_id: AccountId, +) { + let mut nonce = 1; + let deploy_tx = deploy_global_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + &accounts[0], + contract.code().into(), + deploy_mode.clone(), + nonce, + ); + nonce += 1; + env.test_loop.run_for(Duration::seconds(3)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); + let identifier = match deploy_mode { + GlobalContractDeployMode::CodeHash => { + let code_hash = CryptoHash::hash_bytes(contract.code()); + GlobalContractIdentifier::CodeHash(code_hash) + } + GlobalContractDeployMode::AccountId => { + GlobalContractIdentifier::AccountId(accounts[0].clone()) + } + }; + // test on accounts from different shards + for account in [&accounts[1], &accounts[6]] { + let use_tx = use_global_contract( &mut env.test_loop, &env.datas, &rpc_id, - &accounts[0], - contract.code().into(), - deploy_mode.clone(), + account, + identifier.clone(), nonce, ); nonce += 1; env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[deploy_tx]); - let identifier = match deploy_mode { - GlobalContractDeployMode::CodeHash => { - let code_hash = CryptoHash::hash_bytes(contract.code()); - GlobalContractIdentifier::CodeHash(code_hash) - } - GlobalContractDeployMode::AccountId => { - GlobalContractIdentifier::AccountId(accounts[0].clone()) - } - }; - // test on accounts from different shards - for account in [&accounts[1], &accounts[6]] { - let use_tx = use_global_contract( - &mut env.test_loop, - &env.datas, - &rpc_id, - account, - identifier.clone(), - nonce, - ); - nonce += 1; - env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); - let call_tx = call_contract( - &mut env.test_loop, - &env.datas, - &rpc_id, - account, - account, - "log_something".to_owned(), - vec![], - nonce, - ); - env.test_loop.run_for(Duration::seconds(3)); - check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); - } - env.shutdown_and_drain_remaining_events(Duration::seconds(20)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[use_tx]); + let call_tx = call_contract( + &mut env.test_loop, + &env.datas, + &rpc_id, + account, + account, + "log_something".to_owned(), + vec![], + nonce, + ); + env.test_loop.run_for(Duration::seconds(3)); + check_txs(&env.test_loop.data, &env.datas, &rpc_id, &[call_tx]); } + env.shutdown_and_drain_remaining_events(Duration::seconds(20)); } fn setup(accounts: &Vec) -> (TestLoopEnv, AccountId) { diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 1289e26373f..5e7a1852d6e 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -13,11 +13,7 @@ use near_client::test_utils::test_loop::ClientQueries; use near_client::{Client, ProcessTxResponse}; use near_crypto::Signer; use near_network::client::ProcessTxRequest; -#[cfg(feature = "nightly_protocol")] -use near_primitives::action::{ - Action, DeployGlobalContractAction, GlobalContractDeployMode, GlobalContractIdentifier, - UseGlobalContractAction, -}; +use near_primitives::action::{GlobalContractDeployMode, GlobalContractIdentifier}; use near_primitives::block::Tip; use near_primitives::errors::InvalidTxError; use near_primitives::hash::CryptoHash; @@ -308,7 +304,6 @@ pub fn deploy_contract( tx_hash } -#[cfg(feature = "nightly_protocol")] pub fn deploy_global_contract( test_loop: &mut TestLoopV2, node_datas: &[TestData], @@ -321,17 +316,13 @@ pub fn deploy_global_contract( let block_hash = get_shared_block_hash(node_datas, &test_loop.data); let signer = create_user_test_signer(&account_id); - let tx = SignedTransaction::from_actions( + let tx = SignedTransaction::deploy_global_contract( nonce, - account_id.clone(), - account_id.clone(), + account_id, + code, &signer, - vec![Action::DeployGlobalContract(DeployGlobalContractAction { - code: code.into(), - deploy_mode, - })], block_hash, - 0, + deploy_mode, ); let tx_hash = tx.get_hash(); submit_tx(node_datas, rpc_id, tx); @@ -339,7 +330,6 @@ pub fn deploy_global_contract( tx_hash } -#[cfg(feature = "nightly_protocol")] pub fn use_global_contract( test_loop: &mut TestLoopV2, node_datas: &[TestData], @@ -351,16 +341,12 @@ pub fn use_global_contract( let block_hash = get_shared_block_hash(node_datas, &test_loop.data); let signer = create_user_test_signer(&account_id); - let tx = SignedTransaction::from_actions( + let tx = SignedTransaction::use_global_contract( nonce, - account_id.clone(), - account_id.clone(), + account_id, &signer, - vec![Action::UseGlobalContract(Box::new(UseGlobalContractAction { - contract_identifier: global_contract_identifier, - }))], block_hash, - 0, + global_contract_identifier, ); let tx_hash = tx.get_hash(); submit_tx(node_datas, rpc_id, tx); diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 0d9d7fdd28d..0f4503116b1 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -689,15 +689,14 @@ pub(crate) fn action_use_global_contract( current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); + let key = TrieKey::ContractCode { account_id: account_id.clone() }; + state_update.remove(key); } - match &action.contract_identifier { - GlobalContractIdentifier::CodeHash(code_hash) => { - account.set_contract(AccountContract::Global(*code_hash)); - } - GlobalContractIdentifier::AccountId(id) => { - account.set_contract(AccountContract::GlobalByAccount(id.clone())); - } + let contract = match &action.contract_identifier { + GlobalContractIdentifier::CodeHash(code_hash) => AccountContract::Global(*code_hash), + GlobalContractIdentifier::AccountId(id) => AccountContract::GlobalByAccount(id.clone()), }; + account.set_contract(contract); Ok(()) } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 017ce6eaa01..bda5919e696 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -504,7 +504,7 @@ impl Runtime { let key = TrieKey::GlobalContractCode { identifier: identifier.clone().into() }; let value_ref = state_update - .get_ref_no_side_effects(&key, KeyLookupMode::FlatStorage)? + .get_ref(&key, KeyLookupMode::FlatStorage)? .ok_or(RuntimeError::GlobalContractError( GlobalContractError::IdentifierNotFound(identifier), ))?; From bc20a98a269556040c16f5e7caf9781fc4734ce0 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 14 Feb 2025 12:34:42 +0100 Subject: [PATCH 41/43] remove log --- integration-tests/src/test_loop/utils/transactions.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 5e7a1852d6e..4198343c9dd 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -351,7 +351,6 @@ pub fn use_global_contract( let tx_hash = tx.get_hash(); submit_tx(node_datas, rpc_id, tx); - tracing::debug!(target: "test", ?account_id, ?tx_hash, "used global contract"); tx_hash } From 41239c19974f486b3a1c71adb859613626426305 Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 14 Feb 2025 15:45:21 +0100 Subject: [PATCH 42/43] add new field in BalanceStats for gas burnt by global actions --- core/primitives/src/chunk_apply_stats.rs | 1 + runtime/runtime/src/actions.rs | 5 ++- runtime/runtime/src/balance_checker.rs | 48 ++++++++++-------------- runtime/runtime/src/lib.rs | 3 ++ 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/core/primitives/src/chunk_apply_stats.rs b/core/primitives/src/chunk_apply_stats.rs index 8cad3fc5682..64026535f30 100644 --- a/core/primitives/src/chunk_apply_stats.rs +++ b/core/primitives/src/chunk_apply_stats.rs @@ -206,6 +206,7 @@ pub struct BalanceStats { /// This is a negative amount. This amount was not charged from the account that issued /// the transaction. It's likely due to the delayed queue of the receipts. pub gas_deficit_amount: Balance, + pub global_actions_burnt_amount: Balance, } /// Convert a bandwidth request from the bitmap representation to a list of requested values. diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 0f4503116b1..a175b47b509 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -4,7 +4,7 @@ use crate::config::{ }; use crate::ext::{ExternalError, RuntimeExt}; use crate::receipt_manager::ReceiptManager; -use crate::{metrics, ActionResult, ApplyState}; +use crate::{metrics, ActionResult, ApplyState, ChunkApplyStatsV0}; use near_crypto::PublicKey; use near_parameters::{AccountCreationConfig, ActionCosts, RuntimeConfig, RuntimeFeesConfig}; use near_primitives::account::{AccessKey, AccessKeyPermission, Account, AccountContract}; @@ -632,6 +632,7 @@ pub(crate) fn action_deploy_global_contract( apply_state: &ApplyState, deploy_contract: &DeployGlobalContractAction, result: &mut ActionResult, + stats: &mut ChunkApplyStatsV0, ) { let _span = tracing::debug_span!(target: "runtime", "action_deploy_global_contract").entered(); @@ -649,6 +650,8 @@ pub(crate) fn action_deploy_global_contract( .into()); return; }; + stats.balance.global_actions_burnt_amount = + stats.balance.global_actions_burnt_amount.saturating_add(storage_cost); account.set_amount(updated_balance); let id = match deploy_contract.deploy_mode { diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index b96fd40c4ab..69726590e43 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -43,7 +43,6 @@ fn get_delayed_receipts( fn receipt_cost( config: &RuntimeConfig, receipt: &Receipt, - is_outgoing: bool, ) -> Result { Ok(match receipt.receipt() { ReceiptEnum::Action(action_receipt) | ReceiptEnum::PromiseYield(action_receipt) => { @@ -67,19 +66,9 @@ fn receipt_cost( } total_cost } - ReceiptEnum::GlobalContractDistribution(contract_data) => { - // Cost is only paid when the outgoing receipt is generated - if is_outgoing { - config - .fees - .storage_usage_config - .global_contract_storage_amount_per_byte - .saturating_mul(contract_data.code.len() as u128) - } else { - 0 - } - } - ReceiptEnum::Data(_) | ReceiptEnum::PromiseResume(_) => 0, + ReceiptEnum::GlobalContractDistribution(_) + | ReceiptEnum::Data(_) + | ReceiptEnum::PromiseResume(_) => 0, }) } @@ -87,10 +76,9 @@ fn receipt_cost( fn total_receipts_cost( config: &RuntimeConfig, receipts: &[Receipt], - is_outgoing: bool, ) -> Result { receipts.iter().try_fold(0, |accumulator, receipt| { - let cost = receipt_cost(config, receipt, is_outgoing)?; + let cost = receipt_cost(config, receipt)?; safe_add_balance(accumulator, cost) }) } @@ -128,13 +116,13 @@ fn total_postponed_receipts_cost( PostponedReceiptType::Action => { match get_postponed_receipt(state, account_id, *lookup_id)? { None => return Ok(total), - Some(receipt) => receipt_cost(config, &receipt, false)?, + Some(receipt) => receipt_cost(config, &receipt)?, } } PostponedReceiptType::PromiseYield => { match get_promise_yield_receipt(state, account_id, *lookup_id)? { None => return Ok(total), - Some(receipt) => receipt_cost(config, &receipt, false)?, + Some(receipt) => receipt_cost(config, &receipt)?, } } }; @@ -332,17 +320,16 @@ pub(crate) fn check_balance( validator_accounts_update.as_ref().map(validator_rewards).transpose()?.unwrap_or(0); // Receipts - let receipts_cost = - |receipts: &[Receipt], is_outgoing: bool| -> Result { - total_receipts_cost(config, receipts, is_outgoing) - }; + let receipts_cost = |receipts: &[Receipt]| -> Result { + total_receipts_cost(config, receipts) + }; let incoming_receipts_balance = - receipts_cost(incoming_receipts, false)? + receipts_cost(yield_timeout_receipts, false)?; - let outgoing_receipts_balance = receipts_cost(outgoing_receipts, true)?; - let processed_delayed_receipts_balance = receipts_cost(&processed_delayed_receipts, false)?; - let new_delayed_receipts_balance = receipts_cost(&new_delayed_receipts, false)?; - let forwarded_buffered_receipts_balance = receipts_cost(&forwarded_receipts, false)?; - let new_buffered_receipts_balance = receipts_cost(&new_buffered_receipts, false)?; + receipts_cost(incoming_receipts)? + receipts_cost(yield_timeout_receipts)?; + let outgoing_receipts_balance = receipts_cost(outgoing_receipts)?; + let processed_delayed_receipts_balance = receipts_cost(&processed_delayed_receipts)?; + let new_delayed_receipts_balance = receipts_cost(&new_delayed_receipts)?; + let forwarded_buffered_receipts_balance = receipts_cost(&forwarded_receipts)?; + let new_buffered_receipts_balance = receipts_cost(&new_buffered_receipts)?; // Postponed actions receipts. let all_potential_postponed_receipt_ids = potential_postponed_receipt_ids( @@ -374,7 +361,8 @@ pub(crate) fn check_balance( stats.tx_burnt_amount, stats.slashed_burnt_amount, new_buffered_receipts_balance, - stats.other_burnt_amount + stats.other_burnt_amount, + stats.global_actions_burnt_amount ); if initial_balance != final_balance { Err(BalanceMismatchError { @@ -577,6 +565,7 @@ mod tests { gas_deficit_amount: 0, other_burnt_amount: 0, slashed_burnt_amount: 0, + global_actions_burnt_amount: 0, }, ) .unwrap(); @@ -772,6 +761,7 @@ mod tests { gas_deficit_amount: 0, other_burnt_amount: 0, slashed_burnt_amount: 0, + global_actions_burnt_amount: 0, }, ) .unwrap(); diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index bda5919e696..bc8f2c81913 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -413,6 +413,7 @@ impl Runtime { action_index: usize, actions: &[Action], epoch_info_provider: &dyn EpochInfoProvider, + stats: &mut ChunkApplyStatsV0, ) -> Result { let _span = tracing::debug_span!( target: "runtime", @@ -478,6 +479,7 @@ impl Runtime { apply_state, deploy_global_contract, &mut result, + stats, ); } Action::UseGlobalContract(use_global_contract) => { @@ -688,6 +690,7 @@ impl Runtime { action_index, &action_receipt.actions, epoch_info_provider, + stats, )?; if new_result.result.is_ok() { if let Err(e) = new_result.new_receipts.iter().try_for_each(|receipt| { From 0d07078bedec7776c7caa3d03b3f2a7afef2f67c Mon Sep 17 00:00:00 2001 From: Stefan Neamtu Date: Fri, 14 Feb 2025 18:41:33 +0100 Subject: [PATCH 43/43] comments --- core/primitives/src/action/mod.rs | 15 +++++ core/primitives/src/errors.rs | 63 +++++++++++++++---- core/primitives/src/test_utils.rs | 4 +- .../src/test_loop/utils/transactions.rs | 2 +- runtime/runtime/src/actions.rs | 16 ++--- runtime/runtime/src/lib.rs | 15 +++-- 6 files changed, 86 insertions(+), 29 deletions(-) diff --git a/core/primitives/src/action/mod.rs b/core/primitives/src/action/mod.rs index 85eec2b652c..54389068300 100644 --- a/core/primitives/src/action/mod.rs +++ b/core/primitives/src/action/mod.rs @@ -14,6 +14,8 @@ use serde_with::serde_as; use std::fmt; use std::sync::Arc; +use crate::trie_key::GlobalContractCodeIdentifier; + pub fn base64(s: &[u8]) -> String { use base64::Engine; base64::engine::general_purpose::STANDARD.encode(s) @@ -179,6 +181,19 @@ pub enum GlobalContractIdentifier { AccountId(AccountId), } +impl From for GlobalContractIdentifier { + fn from(identifier: GlobalContractCodeIdentifier) -> Self { + match identifier { + GlobalContractCodeIdentifier::CodeHash(hash) => { + GlobalContractIdentifier::CodeHash(hash) + } + GlobalContractCodeIdentifier::AccountId(account_id) => { + GlobalContractIdentifier::AccountId(account_id) + } + } + } +} + /// Use global contract action #[serde_as] #[derive( diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 3c177ab9a66..de31275fdc5 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -520,9 +520,13 @@ impl std::error::Error for ActionError {} )] pub enum ActionErrorKind { /// Happens when CreateAccount action tries to create an account with account_id which is already exists in the storage - AccountAlreadyExists { account_id: AccountId }, + AccountAlreadyExists { + account_id: AccountId, + }, /// Happens when TX receiver_id doesn't exist (but action is not Action::CreateAccount) - AccountDoesNotExist { account_id: AccountId }, + AccountDoesNotExist { + account_id: AccountId, + }, /// A top-level account ID can only be created by registrar. CreateAccountOnlyByRegistrar { account_id: AccountId, @@ -531,16 +535,30 @@ pub enum ActionErrorKind { }, /// A newly created account must be under a namespace of the creator account - CreateAccountNotAllowed { account_id: AccountId, predecessor_id: AccountId }, + CreateAccountNotAllowed { + account_id: AccountId, + predecessor_id: AccountId, + }, /// Administrative actions like `DeployContract`, `Stake`, `AddKey`, `DeleteKey`. can be proceed only if sender=receiver /// or the first TX action is a `CreateAccount` action - ActorNoPermission { account_id: AccountId, actor_id: AccountId }, + ActorNoPermission { + account_id: AccountId, + actor_id: AccountId, + }, /// Account tries to remove an access key that doesn't exist - DeleteKeyDoesNotExist { account_id: AccountId, public_key: Box }, + DeleteKeyDoesNotExist { + account_id: AccountId, + public_key: Box, + }, /// The public key is already used for an existing access key - AddKeyAlreadyExists { account_id: AccountId, public_key: Box }, + AddKeyAlreadyExists { + account_id: AccountId, + public_key: Box, + }, /// Account is staking and can not be deleted - DeleteAccountStaking { account_id: AccountId }, + DeleteAccountStaking { + account_id: AccountId, + }, /// ActionReceipt can't be completed, because the remaining balance will not be enough to cover storage. LackBalanceForState { /// An account which needs balance @@ -550,7 +568,9 @@ pub enum ActionErrorKind { amount: Balance, }, /// Account is not yet staked, but tries to unstake - TriesToUnstake { account_id: AccountId }, + TriesToUnstake { + account_id: AccountId, + }, /// The account doesn't have enough balance to increase the stake. TriesToStake { account_id: AccountId, @@ -579,21 +599,37 @@ pub enum ActionErrorKind { /// /// TODO(#8598): This error is named very poorly. A better name would be /// `OnlyNamedAccountCreationAllowed`. - OnlyImplicitAccountCreationAllowed { account_id: AccountId }, + OnlyImplicitAccountCreationAllowed { + account_id: AccountId, + }, /// Delete account whose state is large is temporarily banned. - DeleteAccountWithLargeState { account_id: AccountId }, + DeleteAccountWithLargeState { + account_id: AccountId, + }, /// Signature does not match the provided actions and given signer public key. DelegateActionInvalidSignature, /// Receiver of the transaction doesn't match Sender of the delegate action - DelegateActionSenderDoesNotMatchTxReceiver { sender_id: AccountId, receiver_id: AccountId }, + DelegateActionSenderDoesNotMatchTxReceiver { + sender_id: AccountId, + receiver_id: AccountId, + }, /// Delegate action has expired. `max_block_height` is less than actual block height. DelegateActionExpired, /// The given public key doesn't exist for Sender account DelegateActionAccessKeyError(InvalidAccessKeyError), /// DelegateAction nonce must be greater sender[public_key].nonce - DelegateActionInvalidNonce { delegate_nonce: Nonce, ak_nonce: Nonce }, + DelegateActionInvalidNonce { + delegate_nonce: Nonce, + ak_nonce: Nonce, + }, /// DelegateAction nonce is larger than the upper bound given by the block height - DelegateActionNonceTooLarge { delegate_nonce: Nonce, upper_bound: Nonce }, + DelegateActionNonceTooLarge { + delegate_nonce: Nonce, + upper_bound: Nonce, + }, + GlobalContractIdentifierNotFound { + identifier: GlobalContractIdentifier, + }, } impl From for ActionError { @@ -934,6 +970,7 @@ impl Display for ActionErrorKind { ActionErrorKind::DelegateActionAccessKeyError(access_key_error) => Display::fmt(&access_key_error, f), ActionErrorKind::DelegateActionInvalidNonce { delegate_nonce, ak_nonce } => write!(f, "DelegateAction nonce {} must be larger than nonce of the used access key {}", delegate_nonce, ak_nonce), ActionErrorKind::DelegateActionNonceTooLarge { delegate_nonce, upper_bound } => write!(f, "DelegateAction nonce {} must be smaller than the access key nonce upper bound {}", delegate_nonce, upper_bound), + ActionErrorKind::GlobalContractIdentifierNotFound { identifier } => write!(f, "Global contract identifier {:?} not found", identifier), } } } diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs index ae35b47abde..1fe13786621 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -279,14 +279,14 @@ impl SignedTransaction { pub fn deploy_global_contract( nonce: Nonce, - contract_id: &AccountId, + contract_id: AccountId, code: Vec, signer: &Signer, block_hash: CryptoHash, deploy_mode: GlobalContractDeployMode, ) -> SignedTransaction { let signer_id = contract_id.clone(); - let receiver_id = contract_id.clone(); + let receiver_id = contract_id; SignedTransaction::from_actions( nonce, signer_id, diff --git a/integration-tests/src/test_loop/utils/transactions.rs b/integration-tests/src/test_loop/utils/transactions.rs index 4198343c9dd..57b61f04a51 100644 --- a/integration-tests/src/test_loop/utils/transactions.rs +++ b/integration-tests/src/test_loop/utils/transactions.rs @@ -318,7 +318,7 @@ pub fn deploy_global_contract( let tx = SignedTransaction::deploy_global_contract( nonce, - account_id, + account_id.clone(), code, &signer, block_hash, diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index a175b47b509..5d297da949b 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -15,9 +15,7 @@ use near_primitives::action::{ }; use near_primitives::checked_feature; use near_primitives::config::ViewConfig; -use near_primitives::errors::{ - ActionError, ActionErrorKind, GlobalContractError, InvalidAccessKeyError, RuntimeError, -}; +use near_primitives::errors::{ActionError, ActionErrorKind, InvalidAccessKeyError, RuntimeError}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::{ ActionReceipt, DataReceipt, Receipt, ReceiptEnum, ReceiptPriority, ReceiptV0, @@ -676,13 +674,16 @@ pub(crate) fn action_use_global_contract( account: &mut Account, action: &UseGlobalContractAction, current_protocol_version: ProtocolVersion, + result: &mut ActionResult, ) -> Result<(), RuntimeError> { let _span = tracing::debug_span!(target: "runtime", "action_use_global_contract").entered(); let key = TrieKey::GlobalContractCode { identifier: action.contract_identifier.clone().into() }; if !state_update.contains_key(&key)? { - return Err(RuntimeError::GlobalContractError(GlobalContractError::IdentifierNotFound( - action.contract_identifier.clone(), - ))); + result.result = Err(ActionErrorKind::GlobalContractIdentifierNotFound { + identifier: action.contract_identifier.clone(), + } + .into()); + return Ok(()); } if let AccountContract::Local(code_hash) = account.contract().as_ref() { let prev_code_len = get_code_len_or_default( @@ -692,8 +693,7 @@ pub(crate) fn action_use_global_contract( current_protocol_version, )?; account.set_storage_usage(account.storage_usage().saturating_sub(prev_code_len)); - let key = TrieKey::ContractCode { account_id: account_id.clone() }; - state_update.remove(key); + state_update.remove(TrieKey::ContractCode { account_id: account_id.clone() }); } let contract = match &action.contract_identifier { GlobalContractIdentifier::CodeHash(code_hash) => AccountContract::Global(*code_hash), diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index bc8f2c81913..6316598c5c2 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -490,6 +490,7 @@ impl Runtime { account, use_global_contract, apply_state.current_protocol_version, + &mut result, )?; } Action::FunctionCall(function_call) => { @@ -503,13 +504,17 @@ impl Runtime { } AccountContract::GlobalByAccount(account_id) => { let identifier = GlobalContractIdentifier::AccountId(account_id.clone()); - let key = - TrieKey::GlobalContractCode { identifier: identifier.clone().into() }; + let key = TrieKey::GlobalContractCode { identifier: identifier.into() }; let value_ref = state_update .get_ref(&key, KeyLookupMode::FlatStorage)? - .ok_or(RuntimeError::GlobalContractError( - GlobalContractError::IdentifierNotFound(identifier), - ))?; + .ok_or_else(|| { + let TrieKey::GlobalContractCode { identifier } = key else { + unreachable!() + }; + RuntimeError::GlobalContractError( + GlobalContractError::IdentifierNotFound(identifier.into()), + ) + })?; value_ref.value_hash() } };