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

feat(blockifier): disallow contract addresses less than 16 #2252

Closed
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 @@ -1027,7 +1027,7 @@
"0x43616c6c436f6e7472616374",
"0x400280007ffd7fff",
"0x480680017fff8000",
"0x0",
"0x19",
"0x400280017ffd7fff",
"0x480680017fff8000",
"0x0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func test_bad_call_address{syscall_ptr: felt*}() {
func test_bad_syscall_request_arg_type{syscall_ptr: felt*}() {
assert syscall_ptr[0] = CALL_CONTRACT_SELECTOR;
// Contract address.
assert syscall_ptr[1] = 0;
assert syscall_ptr[1] = 25;
// Function selector.
assert syscall_ptr[2] = 0;
// Calldata size.
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/resources/versioned_constants_0_13_4.json
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,10 @@
"n_memory_holes": 0
},
"Deploy": {
"n_steps": 1132,
"n_steps": 1158,
"builtin_instance_counter": {
"pedersen_builtin": 7,
"range_check_builtin": 18
"range_check_builtin": 22
},
"n_memory_holes": 0
},
Expand Down
3 changes: 1 addition & 2 deletions crates/blockifier/src/execution/syscalls/syscall_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ pub fn get_block_hash_base(
}

let key = StorageKey::try_from(Felt::from(requested_block_number))?;
let block_hash_contract_address =
ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))?;
let block_hash_contract_address = ContractAddress::from(constants::BLOCK_HASH_CONTRACT_ADDRESS);
Ok(state.get_storage_at(block_hash_contract_address, key)?)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use starknet_api::{calldata, felt};
use starknet_types_core::felt::Felt;
use test_case::test_case;

use crate::abi::constants;
use crate::abi::constants::{self, BLOCK_HASH_CONTRACT_ADDRESS};
use crate::context::ChainInfo;
use crate::execution::call_info::CallExecution;
use crate::execution::entry_point::CallEntryPoint;
Expand All @@ -33,8 +33,7 @@ fn initialize_state(test_contract: FeatureContract) -> (CachedState<DictStateRea
let block_number = felt!(upper_bound_block_number);
let block_hash = felt!(66_u64);
let key = StorageKey::try_from(block_number).unwrap();
let block_hash_contract_address =
ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS)).unwrap();
let block_hash_contract_address = ContractAddress::from(BLOCK_HASH_CONTRACT_ADDRESS);
state.set_storage_at(block_hash_contract_address, key, block_hash).unwrap();

(state, block_number, block_hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use starknet_api::executable_transaction::{
DeclareTransaction as ApiExecutableDeclareTransaction,
};
use starknet_api::execution_resources::GasAmount;
use starknet_api::hash::StarkHash;
use starknet_api::state::StorageKey;
use starknet_api::test_utils::invoke::InvokeTxArgs;
use starknet_api::test_utils::NonceManager;
Expand Down Expand Up @@ -338,7 +337,7 @@ fn test_invoke_tx_from_non_deployed_account(
// Invoke a function from the newly deployed contract.
let entry_point_selector = selector_from_name("return_result");

let non_deployed_contract_address = StarkHash::TWO;
let non_deployed_contract_address = Felt::from(22);

let tx_result = run_invoke_tx(
&mut state,
Expand Down
18 changes: 8 additions & 10 deletions crates/native_blockifier/src/py_block_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,14 @@ impl PyOsConfig {
pub fn into_chain_info(self) -> ChainInfo {
ChainInfo::try_from(self).expect("Failed to convert chain info.")
}

pub fn create_for_testing() -> Self {
Self {
chain_id: ChainId::Other("".to_string()),
deprecated_fee_token_address: PyFelt::from(19_u8),
fee_token_address: PyFelt::from(19_u8),
}
}
}

impl TryFrom<PyOsConfig> for ChainInfo {
Expand All @@ -474,16 +482,6 @@ impl TryFrom<PyOsConfig> for ChainInfo {
}
}

impl Default for PyOsConfig {
fn default() -> Self {
Self {
chain_id: ChainId::Other("".to_string()),
deprecated_fee_token_address: Default::default(),
fee_token_address: Default::default(),
}
}
}

fn serialize_failure_reason(error: TransactionExecutorError) -> RawTransactionExecutionResult {
// TODO(Yoni, 1/7/2024): re-consider this serialization.
serde_json::to_vec(&format!("{}", error)).expect(RESULT_SERIALIZE_ERR)
Expand Down
10 changes: 7 additions & 3 deletions crates/native_blockifier/src/py_block_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn global_contract_cache_update() {
let temp_storage_path = tempfile::tempdir().unwrap().into_path();
let mut block_executor = PyBlockExecutor::create_for_testing(
PyConcurrencyConfig::default(),
PyOsConfig::default(),
PyOsConfig::create_for_testing(),
temp_storage_path,
4000,
);
Expand All @@ -63,7 +63,11 @@ fn global_contract_cache_update() {
let sentinel_block_number_and_hash = None; // Information does not exist for block 0.
block_executor
.setup_block_execution(
PyBlockInfo { block_number: 1, ..PyBlockInfo::default() },
PyBlockInfo {
block_number: 1,
sequencer_address: PyFelt::from(19_u8),
..PyBlockInfo::default()
},
sentinel_block_number_and_hash,
)
.unwrap();
Expand Down Expand Up @@ -119,7 +123,7 @@ fn global_contract_cache_update_large_contract() {
let temp_storage_path = tempfile::tempdir().unwrap().into_path();
let mut block_executor = PyBlockExecutor::native_create_for_testing(
Default::default(),
Default::default(),
PyOsConfig::create_for_testing(),
temp_storage_path,
4000,
);
Expand Down
5 changes: 5 additions & 0 deletions crates/starknet_api/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ impl From<u128> for ContractAddress {

impl_from_through_intermediate!(u128, ContractAddress, u8, u16, u32, u64);

/// The maximal contract address of reserved contracts.
pub const MAX_RESERVED_CONTRACT_ADDRESS: u128 = 15;
/// The maximal size of storage var.
pub const MAX_STORAGE_ITEM_SIZE: u16 = 256;
/// The prefix used in the calculation of a contract address.
Expand All @@ -152,6 +154,9 @@ pub static L2_ADDRESS_UPPER_BOUND: LazyLock<NonZeroFelt> = LazyLock::new(|| {
impl TryFrom<StarkHash> for ContractAddress {
type Error = StarknetApiError;
fn try_from(hash: StarkHash) -> Result<Self, Self::Error> {
if hash <= MAX_RESERVED_CONTRACT_ADDRESS.into() {
return Err(Self::Error::ReservedContractAddress(hash));
}
Ok(Self(PatriciaKey::try_from(hash)?))
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/starknet_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub mod type_utils;

use std::num::ParseIntError;

use hash::StarkHash;
use serde_utils::InnerDeserializationError;

use crate::transaction::TransactionVersion;
Expand Down Expand Up @@ -65,6 +66,8 @@ pub enum StarknetApiError {
version {cairo_version:?}.", **declare_version
)]
ContractClassVersionMismatch { declare_version: TransactionVersion, cairo_version: u64 },
#[error("Invalid contract address, must be < 16, as these addresses are reserved. Got {0}")]
ReservedContractAddress(StarkHash),
}

pub type StarknetApiResult<T> = Result<T, StarknetApiError>;
Loading