Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Always use latest ETH wallet contract #12038

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn test_eth_implicit_account_creation() {
env.produce_block(0, i);
}

let magic_bytes = wallet_contract_magic_bytes(chain_id, PROTOCOL_VERSION);
let magic_bytes = wallet_contract_magic_bytes(chain_id);

// Verify the ETH-implicit account has zero balance and appropriate code hash.
// Check that the account storage fits within zero balance account limit.
Expand Down Expand Up @@ -219,7 +219,7 @@ fn test_transaction_from_eth_implicit_account_fail() {
assert_eq!(response, expected_tx_error);

// Try to deploy the Wallet Contract again to the ETH-implicit account. Should fail because there is no access key.
let wallet_contract_code = wallet_contract(chain_id, PROTOCOL_VERSION).code().to_vec();
let wallet_contract_code = wallet_contract(chain_id).code().to_vec();
let add_access_key_to_eth_implicit_account_tx = SignedTransaction::from_actions(
nonce,
eth_implicit_account_id.clone(),
Expand Down
127 changes: 11 additions & 116 deletions runtime/near-wallet-contract/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#![doc = include_str!("../README.md")]
use near_primitives_core::{
chains, hash::CryptoHash, types::ProtocolVersion, version::ProtocolFeature,
};
use near_primitives_core::chains;
use near_vm_runner::ContractCode;
use std::sync::{Arc, OnceLock};

Expand All @@ -11,78 +9,27 @@ static MAINNET: WalletContract =
static TESTNET: WalletContract =
WalletContract::new(include_bytes!("../res/wallet_contract_testnet.wasm"));

/// Initial version of WalletContract. It was released to testnet, but not mainnet.
/// We still use this one on testnet protocol version 70 for consistency.
/// Example account:
/// https://testnet.nearblocks.io/address/0xcc5a584f545b2ca3ebacc1346556d1f5b82b8fc6
static OLD_TESTNET: WalletContract =
WalletContract::new(include_bytes!("../res/wallet_contract_testnet_pv70.wasm"));

/// The protocol version on testnet where it is safe to start using the new wallet contract.
const NEW_WALLET_CONTRACT_VERSION: ProtocolVersion =
ProtocolFeature::FixMinStakeRatio.protocol_version();

static LOCALNET: WalletContract =
WalletContract::new(include_bytes!("../res/wallet_contract_localnet.wasm"));

/// Get wallet contract code for different Near chains.
pub fn wallet_contract(chain_id: &str, protocol_version: ProtocolVersion) -> Arc<ContractCode> {
pub fn wallet_contract(chain_id: &str) -> Arc<ContractCode> {
match chain_id {
chains::MAINNET => MAINNET.read_contract(),
chains::TESTNET => {
if protocol_version < NEW_WALLET_CONTRACT_VERSION {
OLD_TESTNET.read_contract()
} else {
TESTNET.read_contract()
}
}
chains::TESTNET => TESTNET.read_contract(),
_ => LOCALNET.read_contract(),
}
}

/// near[wallet contract hash]
pub fn wallet_contract_magic_bytes(
chain_id: &str,
protocol_version: ProtocolVersion,
) -> Arc<ContractCode> {
pub fn wallet_contract_magic_bytes(chain_id: &str) -> Arc<ContractCode> {
match chain_id {
chains::MAINNET => MAINNET.magic_bytes(),
chains::TESTNET => {
if protocol_version < NEW_WALLET_CONTRACT_VERSION {
OLD_TESTNET.magic_bytes()
} else {
TESTNET.magic_bytes()
}
}
chains::TESTNET => TESTNET.magic_bytes(),
_ => LOCALNET.magic_bytes(),
}
}

/// Checks if the given code hash corresponds to the wallet contract (signalling
/// the runtime should treat the wallet contract as the code for the account).
pub fn code_hash_matches_wallet_contract(
chain_id: &str,
code_hash: &CryptoHash,
protocol_version: ProtocolVersion,
) -> bool {
let magic_bytes = wallet_contract_magic_bytes(&chain_id, protocol_version);

if code_hash == magic_bytes.hash() {
return true;
}

// Extra check needed for an old version of the wallet contract
// that was on testnet. Accounts with that hash are still intentionally
// made to run the current version of the wallet contract because
// the previous version had a bug in its implementation.
if chain_id == chains::TESTNET {
let alt_testnet_code = OLD_TESTNET.magic_bytes();
return code_hash == alt_testnet_code.hash();
}

false
}

struct WalletContract {
contract: OnceLock<Arc<ContractCode>>,
magic_bytes: OnceLock<Arc<ContractCode>>,
Expand Down Expand Up @@ -111,44 +58,13 @@ impl WalletContract {

#[cfg(test)]
mod tests {
use crate::{
code_hash_matches_wallet_contract, wallet_contract, wallet_contract_magic_bytes,
OLD_TESTNET,
};
use crate::{wallet_contract, wallet_contract_magic_bytes};
use near_primitives_core::{
chains::{MAINNET, TESTNET},
hash::CryptoHash,
version::{ProtocolFeature, PROTOCOL_VERSION},
};
use std::str::FromStr;

#[test]
fn test_code_hash_matches_wallet_contract() {
let chain_ids = [MAINNET, TESTNET, "localnet"];
let testnet_code_v70 = OLD_TESTNET.magic_bytes();
let other_code_hash =
CryptoHash::from_str("9rmLr4dmrg5M6Ts6tbJyPpbCrNtbL9FCdNv24FcuWP5a").unwrap();
for id in chain_ids {
assert!(
code_hash_matches_wallet_contract(
id,
wallet_contract_magic_bytes(id, PROTOCOL_VERSION).hash(),
PROTOCOL_VERSION
),
"Wallet contract magic bytes matches wallet contract"
);
assert_eq!(
code_hash_matches_wallet_contract(id, testnet_code_v70.hash(), PROTOCOL_VERSION),
id == TESTNET,
"Special case only matches on testnet"
);
assert!(
!code_hash_matches_wallet_contract(id, &other_code_hash, PROTOCOL_VERSION),
"Other code hashes do not match wallet contract"
);
}
}

#[test]
fn check_mainnet_wallet_contract() {
const WALLET_CONTRACT_HASH: &'static str = "5j8XPMMKMn5cojVs4qQ65dViGtgMHgrfNtJgrC18X8Qw";
Expand All @@ -165,24 +81,6 @@ mod tests {
check_wallet_contract_magic_bytes(TESTNET, WALLET_CONTRACT_HASH, MAGIC_BYTES_HASH);
}

#[test]
fn check_old_testnet_wallet_contract() {
// Make sure the old contract is returned on v70 on testnet.
const WALLET_CONTRACT_HASH: &'static str = "3Za8tfLX6nKa2k4u2Aq5CRrM7EmTVSL9EERxymfnSFKd";
let protocol_version = ProtocolFeature::EthImplicitAccounts.protocol_version();
let contract = wallet_contract(TESTNET, protocol_version);

assert!(!contract.code().is_empty());
let expected_hash = CryptoHash::from_str(WALLET_CONTRACT_HASH).unwrap();
assert_eq!(*contract.hash(), expected_hash, "wallet contract hash mismatch");

const MAGIC_BYTES_HASH: &'static str = "4reLvkAWfqk5fsqio1KLudk46cqRz9erQdaHkWZKMJDZ";
let magic_bytes = wallet_contract_magic_bytes(TESTNET, protocol_version);
assert!(!magic_bytes.code().is_empty());
let expected_hash = CryptoHash::from_str(MAGIC_BYTES_HASH).unwrap();
assert_eq!(magic_bytes.hash(), &expected_hash, "magic bytes hash mismatch");
}

#[test]
fn check_localnet_wallet_contract() {
const WALLET_CONTRACT_HASH: &'static str = "FAq9tQRbwJPTV3PQLn2F7AUD3FW2Fw1V8ZeZuazfeu1v";
Expand All @@ -193,11 +91,11 @@ mod tests {
}

fn check_wallet_contract(chain_id: &str, expected_hash: &str) {
assert!(!wallet_contract(chain_id, PROTOCOL_VERSION).code().is_empty());
assert!(!wallet_contract(chain_id).code().is_empty());
let expected_hash =
CryptoHash::from_str(expected_hash).expect("Failed to parse hash from string");
assert_eq!(
*wallet_contract(chain_id, PROTOCOL_VERSION).hash(),
*wallet_contract(chain_id).hash(),
expected_hash,
"wallet contract hash mismatch"
);
Expand All @@ -208,19 +106,16 @@ mod tests {
expected_code_hash: &str,
expected_magic_hash: &str,
) {
assert!(!wallet_contract_magic_bytes(chain_id, PROTOCOL_VERSION).code().is_empty());
assert!(!wallet_contract_magic_bytes(chain_id).code().is_empty());
let expected_hash =
CryptoHash::from_str(expected_magic_hash).expect("Failed to parse hash from string");
assert_eq!(
*wallet_contract_magic_bytes(chain_id, PROTOCOL_VERSION).hash(),
*wallet_contract_magic_bytes(chain_id).hash(),
expected_hash,
"magic bytes hash mismatch"
);

let expected_code = format!("near{}", expected_code_hash);
assert_eq!(
wallet_contract_magic_bytes(chain_id, PROTOCOL_VERSION).code(),
expected_code.as_bytes()
);
assert_eq!(wallet_contract_magic_bytes(chain_id).code(), expected_code.as_bytes());
}
}
4 changes: 2 additions & 2 deletions runtime/runtime/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ pub(crate) fn action_implicit_account_creation_transfer(
// We deploy "near[wallet contract hash]" magic bytes as the contract code,
// to mark that this is a neard-defined contract. It will not be used on a function call.
// Instead, neard-defined Wallet Contract implementation will be used.
let magic_bytes = wallet_contract_magic_bytes(&chain_id, current_protocol_version);
let magic_bytes = wallet_contract_magic_bytes(&chain_id);

let storage_usage = fee_config.storage_usage_config.num_bytes_account
+ magic_bytes.code().len() as u64
Expand All @@ -622,7 +622,7 @@ pub(crate) fn action_implicit_account_creation_transfer(
// Note this contract is shared among ETH-implicit accounts and `precompile_contract`
// is a no-op if the contract was already compiled.
precompile_contract(
&wallet_contract(&chain_id, current_protocol_version),
&wallet_contract(&chain_id),
Arc::clone(&apply_state.config.wasm_config),
apply_state.cache.as_deref(),
)
Expand Down
7 changes: 4 additions & 3 deletions runtime/runtime/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use near_vm_runner::logic::errors::{AnyError, VMLogicError};
use near_vm_runner::logic::types::ReceiptIndex;
use near_vm_runner::logic::{External, StorageGetMode, ValuePtr};
use near_vm_runner::{Contract, ContractCode};
use near_wallet_contract::{code_hash_matches_wallet_contract, wallet_contract};
use near_wallet_contract::wallet_contract;
use std::sync::Arc;

pub struct RuntimeExt<'a> {
Expand Down Expand Up @@ -379,11 +379,12 @@ impl<'a> Contract for RuntimeContractExt<'a> {
let code_hash = self.hash();
let version = self.current_protocol_version;
let chain_id = self.chain_id;
// We are removing the check to match code_hash_matches_wallet_contract
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. once you remove the code below, this comment will not refer to any check after merging.

// If the account type is EthImplicitAccount, we always load the latest (PV 71) wallet contract.
if checked_feature!("stable", EthImplicitAccounts, version)
&& account_id.get_account_type() == AccountType::EthImplicitAccount
&& code_hash_matches_wallet_contract(chain_id, &code_hash, version)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: Verify if it is fine to remove this check

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually cannot remove this check because not all accounts that look like eth-implicit have the wallet contract magic bytes (see details in this PR description).

{
return Some(wallet_contract(&chain_id, version));
return Some(wallet_contract(&chain_id));
}
let mode = match checked_feature!("stable", ChunkNodesCache, version) {
true => Some(TrieCacheMode::CachingShard),
Expand Down
Loading