Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into dori/merge-main-v0.13…
Browse files Browse the repository at this point in the history
….2-into-main-1724654462 (with conflicts)
  • Loading branch information
dorimedini-starkware committed Aug 27, 2024
2 parents d3a7628 + 0d773ce commit 41c1918
Show file tree
Hide file tree
Showing 24 changed files with 247 additions and 149 deletions.
42 changes: 22 additions & 20 deletions crates/blockifier/src/blockifier/block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::num::NonZeroU128;

use log::warn;
use starknet_api::block::{BlockHash, BlockNumber, BlockTimestamp};
use starknet_api::core::ContractAddress;
use starknet_api::state::StorageKey;
Expand All @@ -9,7 +10,7 @@ use crate::abi::constants;
use crate::state::errors::StateError;
use crate::state::state_api::{State, StateResult};
use crate::transaction::objects::FeeType;
use crate::versioned_constants::{VersionedConstants, VersionedConstantsOverrides};
use crate::versioned_constants::VersionedConstants;

#[cfg(test)]
#[path = "block_test.rs"]
Expand Down Expand Up @@ -42,26 +43,27 @@ impl GasPrices {
strk_l1_gas_price: NonZeroU128,
eth_l1_data_gas_price: NonZeroU128,
strk_l1_data_gas_price: NonZeroU128,
eth_l2_gas_price: NonZeroU128,
strk_l2_gas_price: NonZeroU128,
) -> Self {
// TODO(Aner): get gas prices from python.
let eth_l2_gas_price = NonZeroU128::new(
VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
validate_max_n_steps: 0,
max_recursion_depth: 0,
versioned_constants_base_overrides: None,
})
.l1_to_l2_gas_price_conversion(eth_l1_gas_price.into()),
)
.expect("L1 to L2 price conversion error (Rust side).");
let strk_l2_gas_price = NonZeroU128::new(
VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
validate_max_n_steps: 0,
max_recursion_depth: 0,
versioned_constants_base_overrides: None,
})
.l1_to_l2_gas_price_conversion(strk_l1_gas_price.into()),
)
.expect("L1 to L2 price conversion error (Rust side).");
// TODO(Aner): fix backwards compatibility.
let expected_eth_l2_gas_price = VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(eth_l1_gas_price.into());
if u128::from(eth_l2_gas_price) != expected_eth_l2_gas_price {
warn!(
"eth_l2_gas_price does not match expected! eth_l2_gas_price:{eth_l2_gas_price}, \
expected:{expected_eth_l2_gas_price}."
)
}
let expected_strk_l2_gas_price = VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(strk_l1_gas_price.into());
if u128::from(strk_l2_gas_price) != expected_strk_l2_gas_price {
warn!(
"strk_l2_gas_price does not match expected! \
strk_l2_gas_price:{strk_l2_gas_price}, expected:{expected_strk_l2_gas_price}."
)
}

GasPrices {
eth_l1_gas_price,
strk_l1_gas_price,
Expand Down
47 changes: 36 additions & 11 deletions crates/blockifier/src/execution/syscalls/syscall_tests/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use crate::test_utils::contracts::FeatureContract;
use crate::test_utils::initial_test_state::test_state;
use crate::test_utils::{calldata_for_deploy_test, trivial_external_entry_point_new, CairoVersion};

#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1);"VM")]
fn no_constructor(deployer_contract: FeatureContract) {
#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 204260;"VM")]
fn no_constructor(deployer_contract: FeatureContract, expected_gas: u64) {
// TODO(Yoni): share the init code of the tests in this file.

let empty_contract = FeatureContract::Empty(CairoVersion::Cairo1);
Expand All @@ -33,6 +33,17 @@ fn no_constructor(deployer_contract: FeatureContract) {
calldata,
..trivial_external_entry_point_new(deployer_contract)
};

let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap();
assert_eq!(
deploy_call.execution,
CallExecution {
retdata: retdata![],
gas_consumed: expected_gas,
..CallExecution::default()
}
);

let deployed_contract_address = calculate_contract_address(
ContractAddressSalt::default(),
class_hash,
Expand All @@ -41,11 +52,11 @@ fn no_constructor(deployer_contract: FeatureContract) {
)
.unwrap();

let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap().inner_calls[0];
let constructor_call = &deploy_call.inner_calls[0];

assert_eq!(deploy_call.call.storage_address, deployed_contract_address);
assert_eq!(constructor_call.call.storage_address, deployed_contract_address);
assert_eq!(
deploy_call.execution,
constructor_call.execution,
CallExecution { retdata: retdata![], gas_consumed: 0, ..CallExecution::default() }
);
assert_eq!(state.get_class_hash_at(deployed_contract_address).unwrap(), class_hash);
Expand Down Expand Up @@ -77,8 +88,12 @@ fn no_constructor_nonempty_calldata(deployer_contract: FeatureContract) {
));
}

#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1), 5210;"VM")]
fn with_constructor(deployer_contract: FeatureContract, expected_gas: u64) {
#[test_case(FeatureContract::TestContract(CairoVersion::Cairo1),214210, 5210;"VM")]
fn with_constructor(
deployer_contract: FeatureContract,
expected_gas: u64,
expected_constructor_gas: u64,
) {
let empty_contract = FeatureContract::Empty(CairoVersion::Cairo1);
let mut state = test_state(
&ChainInfo::create_for_testing(),
Expand Down Expand Up @@ -108,17 +123,27 @@ fn with_constructor(deployer_contract: FeatureContract, expected_gas: u64) {
deployer_contract.get_instance_address(0),
)
.unwrap();
// Note, this is the call info of the constructor call (inner call).
let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap().inner_calls[0];

assert_eq!(deploy_call.call.storage_address, contract_address);
let deploy_call = &entry_point_call.execute_directly(&mut state).unwrap();
assert_eq!(
deploy_call.execution,
CallExecution {
retdata: retdata![],
gas_consumed: expected_gas,
..CallExecution::default()
}
);

let constructor_call = &deploy_call.inner_calls[0];

assert_eq!(constructor_call.call.storage_address, contract_address);
assert_eq!(
constructor_call.execution,
CallExecution {
// The test contract constructor returns its first argument.
retdata: retdata![constructor_calldata[0]],
// This reflects the gas cost of storage write syscall.
gas_consumed: expected_gas,
gas_consumed: expected_constructor_gas,
..CallExecution::default()
}
);
Expand Down
8 changes: 8 additions & 0 deletions crates/blockifier/src/fee/fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ fn test_discounted_gas_overdraft(
gas_price.try_into().unwrap(),
DEFAULT_ETH_L1_DATA_GAS_PRICE.try_into().unwrap(),
data_gas_price.try_into().unwrap(),
VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(DEFAULT_ETH_L1_GAS_PRICE)
.try_into()
.unwrap(),
VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(gas_price)
.try_into()
.unwrap(),
);

let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0);
Expand Down
8 changes: 8 additions & 0 deletions crates/blockifier/src/test_utils/struct_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,14 @@ impl BlockInfo {
DEFAULT_STRK_L1_GAS_PRICE.try_into().unwrap(),
DEFAULT_ETH_L1_DATA_GAS_PRICE.try_into().unwrap(),
DEFAULT_STRK_L1_DATA_GAS_PRICE.try_into().unwrap(),
VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(DEFAULT_ETH_L1_GAS_PRICE)
.try_into()
.unwrap(),
VersionedConstants::latest_constants()
.l1_to_l2_gas_price_conversion(DEFAULT_STRK_L1_GAS_PRICE)
.try_into()
.unwrap(),
),
use_kzg_da: false,
}
Expand Down
5 changes: 2 additions & 3 deletions crates/gateway/src/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ impl GatewayCompiler {
) -> GatewayResult<CasmContractClass> {
match self.sierra_to_casm_compiler.compile(cairo_lang_contract_class) {
Ok(casm_contract_class) => Ok(casm_contract_class),
Err(starknet_sierra_compile::errors::CompilationUtilError::CompilationPanic) => {
// TODO(Arni): Log the panic.
error!("Compilation panicked.");
Err(starknet_sierra_compile::errors::CompilationUtilError::UnexpectedError(error)) => {
error!("Compilation panicked. Error: {:?}", error);
Err(GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() })
}
Err(e) => {
Expand Down
12 changes: 3 additions & 9 deletions crates/gateway/src/compilation_test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use assert_matches::assert_matches;
use cairo_lang_sierra_to_casm::compiler::CompilationError;
use cairo_lang_starknet_classes::allowed_libfuncs::AllowedLibfuncsError;
use cairo_lang_starknet_classes::casm_contract_class::StarknetSierraCompilationError;
use mempool_test_utils::starknet_api_test_utils::{
compiled_class_hash as test_contract_compiled_class_hash,
declare_tx as rpc_declare_tx,
Expand Down Expand Up @@ -67,11 +64,8 @@ fn test_compile_contract_class_bytecode_size_validation(declare_tx_v3: RpcDeclar

let result = gateway_compiler.process_declare_tx(&RpcDeclareTransaction::V3(declare_tx_v3));
assert_matches!(result.unwrap_err(), GatewaySpecError::CompilationFailed);
let expected_compilation_error = CompilationUtilError::StarknetSierraCompilationError(
StarknetSierraCompilationError::CompilationError(Box::new(
CompilationError::CodeSizeLimitExceeded,
)),
);
let expected_compilation_error =
CompilationUtilError::CompilationError("Code size limit exceeded.".to_owned());
assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str()));
}

Expand All @@ -90,7 +84,7 @@ fn test_compile_contract_class_bad_sierra(
assert_eq!(err, GatewaySpecError::CompilationFailed);

let expected_compilation_error =
CompilationUtilError::AllowedLibfuncsError(AllowedLibfuncsError::SierraProgramError);
CompilationUtilError::CompilationError("Invalid Sierra program.".to_owned());
assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str()));
}

Expand Down
10 changes: 5 additions & 5 deletions crates/gateway/src/gateway_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::gateway::{add_tx, AppState, SharedMempoolClient};
use crate::state_reader_test_utils::{local_test_state_reader_factory, TestStateReaderFactory};
use crate::stateful_transaction_validator::StatefulTransactionValidator;
use crate::stateless_transaction_validator::StatelessTransactionValidator;
use crate::utils::external_tx_to_account_tx;
use crate::utils::rpc_tx_to_account_tx;

pub fn app_state(
mempool_client: SharedMempoolClient,
Expand Down Expand Up @@ -93,16 +93,16 @@ async fn to_bytes(res: Response) -> Bytes {
res.into_body().collect().await.unwrap().to_bytes()
}

fn calculate_hash(external_tx: &RpcTransaction) -> TransactionHash {
let optional_class_info = match &external_tx {
fn calculate_hash(rpc_tx: &RpcTransaction) -> TransactionHash {
let optional_class_info = match &rpc_tx {
RpcTransaction::Declare(_declare_tx) => {
panic!("Declare transactions are not supported in this test")
}
_ => None,
};

let account_tx = external_tx_to_account_tx(
external_tx,
let account_tx = rpc_tx_to_account_tx(
rpc_tx,
optional_class_info,
&ChainInfo::create_for_testing().chain_id,
)
Expand Down
3 changes: 3 additions & 0 deletions crates/gateway/src/rpc_objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ impl TryInto<BlockInfo> for BlockHeader {
parse_gas_price(self.l1_gas_price.price_in_fri)?,
parse_gas_price(self.l1_data_gas_price.price_in_wei)?,
parse_gas_price(self.l1_data_gas_price.price_in_fri)?,
// TODO(Aner): add to BlockHeader and take the value from it.
NonZeroU128::MIN,
NonZeroU128::MIN,
),
use_kzg_da: matches!(self.l1_da_mode, L1DataAvailabilityMode::Blob),
})
Expand Down
13 changes: 5 additions & 8 deletions crates/gateway/src/stateful_transaction_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tracing::error;
use crate::config::StatefulTransactionValidatorConfig;
use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult};
use crate::state_reader::{MempoolStateReader, StateReaderFactory};
use crate::utils::{external_tx_to_account_tx, get_sender_address};
use crate::utils::{get_sender_address, rpc_tx_to_account_tx};

#[cfg(test)]
#[path = "stateful_transaction_validator_test.rs"]
Expand Down Expand Up @@ -69,22 +69,19 @@ impl StatefulTransactionValidator {
// conversion is also relevant for the Mempool.
pub fn run_validate<V: StatefulTransactionValidatorTrait>(
&self,
external_tx: &RpcTransaction,
rpc_tx: &RpcTransaction,
optional_class_info: Option<ClassInfo>,
mut validator: V,
) -> StatefulTransactionValidatorResult<ValidateInfo> {
let account_tx = external_tx_to_account_tx(
external_tx,
optional_class_info,
&self.config.chain_info.chain_id,
)?;
let account_tx =
rpc_tx_to_account_tx(rpc_tx, optional_class_info, &self.config.chain_info.chain_id)?;
let tx_hash = account_tx.tx_hash();
let sender_address = get_sender_address(&account_tx);
let account_nonce = validator.get_nonce(sender_address).map_err(|e| {
error!("Failed to get nonce for sender address {}: {}", sender_address, e);
GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() }
})?;
let skip_validate = skip_stateful_validations(external_tx, account_nonce);
let skip_validate = skip_stateful_validations(rpc_tx, account_nonce);
validator
.validate(account_tx, skip_validate)
.map_err(|err| GatewaySpecError::ValidationFailure { data: err.to_string() })?;
Expand Down
12 changes: 6 additions & 6 deletions crates/gateway/src/stateful_transaction_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ fn stateful_validator(block_context: BlockContext) -> StatefulTransactionValidat
)]
#[case::invalid_tx(invoke_tx(CairoVersion::Cairo1), Err(STATEFUL_VALIDATOR_FEE_ERROR))]
fn test_stateful_tx_validator(
#[case] external_tx: RpcTransaction,
#[case] rpc_tx: RpcTransaction,
#[case] expected_result: BlockifierStatefulValidatorResult<ValidateInfo>,
stateful_validator: StatefulTransactionValidator,
) {
let optional_class_info = match &external_tx {
let optional_class_info = match &rpc_tx {
RpcTransaction::Declare(declare_tx) => Some(
ClassInfo::try_from(
GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default())
Expand All @@ -103,7 +103,7 @@ fn test_stateful_tx_validator(
mock_validator.expect_validate().return_once(|_, _| expected_result.map(|_| ()));
mock_validator.expect_get_nonce().returning(|_| Ok(Nonce(Felt::ZERO)));

let result = stateful_validator.run_validate(&external_tx, optional_class_info, mock_validator);
let result = stateful_validator.run_validate(&rpc_tx, optional_class_info, mock_validator);
assert_eq!(result, expected_result_as_stateful_transaction_result);
}

Expand Down Expand Up @@ -159,12 +159,12 @@ fn test_instantiate_validator() {
false
)]
fn test_skip_stateful_validation(
#[case] external_tx: RpcTransaction,
#[case] rpc_tx: RpcTransaction,
#[case] sender_nonce: Nonce,
#[case] should_skip_validate: bool,
stateful_validator: StatefulTransactionValidator,
) {
let sender_address = external_tx.calculate_sender_address().unwrap();
let sender_address = rpc_tx.calculate_sender_address().unwrap();
let mut mock_validator = MockStatefulTransactionValidatorTrait::new();
mock_validator
.expect_get_nonce()
Expand All @@ -174,5 +174,5 @@ fn test_skip_stateful_validation(
.expect_validate()
.withf(move |_, skip_validate| *skip_validate == should_skip_validate)
.returning(|_, _| Ok(()));
let _ = stateful_validator.run_validate(&external_tx, None, mock_validator);
let _ = stateful_validator.run_validate(&rpc_tx, None, mock_validator);
}
16 changes: 6 additions & 10 deletions crates/gateway/src/stateless_transaction_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use mempool_test_utils::declare_tx_args;
use mempool_test_utils::starknet_api_test_utils::{
create_resource_bounds_mapping,
external_declare_tx,
external_tx_for_testing,
rpc_tx_for_testing,
zero_resource_bounds_mapping,
TransactionType,
NON_EMPTY_RESOURCE_BOUNDS,
Expand Down Expand Up @@ -118,7 +118,7 @@ fn test_positive_flow(
tx_type: TransactionType,
) {
let tx_validator = StatelessTransactionValidator { config };
let tx = external_tx_for_testing(tx_type, resource_bounds, tx_calldata, signature);
let tx = rpc_tx_for_testing(tx_type, resource_bounds, tx_calldata, signature);

assert_matches!(tx_validator.validate(&tx), Ok(()));
}
Expand Down Expand Up @@ -154,12 +154,8 @@ fn test_invalid_resource_bounds(
tx_type: TransactionType,
) {
let tx_validator = StatelessTransactionValidator { config };
let tx = external_tx_for_testing(
tx_type,
resource_bounds,
calldata![],
TransactionSignature::default(),
);
let tx =
rpc_tx_for_testing(tx_type, resource_bounds, calldata![], TransactionSignature::default());

assert_eq!(tx_validator.validate(&tx).unwrap_err(), expected_error);
}
Expand All @@ -170,7 +166,7 @@ fn test_calldata_too_long(
) {
let tx_validator =
StatelessTransactionValidator { config: default_validator_config_for_testing().clone() };
let tx = external_tx_for_testing(
let tx = rpc_tx_for_testing(
tx_type,
zero_resource_bounds_mapping(),
calldata![Felt::ONE, Felt::TWO],
Expand All @@ -193,7 +189,7 @@ fn test_signature_too_long(
) {
let tx_validator =
StatelessTransactionValidator { config: default_validator_config_for_testing().clone() };
let tx = external_tx_for_testing(
let tx = rpc_tx_for_testing(
tx_type,
zero_resource_bounds_mapping(),
calldata![],
Expand Down
Loading

0 comments on commit 41c1918

Please sign in to comment.