Skip to content

Commit

Permalink
Revert "resolve compilation errors"
Browse files Browse the repository at this point in the history
This reverts commit 17ff9f2.
  • Loading branch information
marioiordanov committed Oct 23, 2024
1 parent 17ff9f2 commit cf2eb6c
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 83 deletions.
14 changes: 10 additions & 4 deletions crates/starknet-devnet-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub enum Error {
#[error("{0:?}")]
ContractExecutionError(ErrorStack),
#[error("Execution error in simulating transaction no. {failure_index}: {error_stack:?}")]
ContractExecutionErrorInSimulation { failure_index: usize, error_stack: ErrorStack },
ContractExecutionErrorInSimulation { failure_index: u64, error_stack: ErrorStack },
#[error("Types error: {0}")]
TypesError(#[from] starknet_types::error::Error),
#[error("I/O error: {0}")]
Expand Down Expand Up @@ -78,6 +78,12 @@ impl From<starknet_types_core::felt::FromStrError> for Error {
}
}

impl From<blockifier::transaction::errors::TransactionExecutionError> for Error {
fn from(e: blockifier::transaction::errors::TransactionExecutionError) -> Self {
Self::ContractExecutionError(gen_tx_execution_error_trace(&e))
}
}

#[derive(Debug, Error)]
pub enum StateError {
#[error("No class hash {0:x} found")]
Expand Down Expand Up @@ -118,7 +124,7 @@ impl From<TransactionExecutionError> for Error {
TransactionExecutionError::ValidateTransactionError { .. } => {
TransactionValidationError::ValidationFailure { reason: value.to_string() }.into()
}
other => Self::ContractExecutionError(gen_tx_execution_error_trace(&other)),
other => Self::BlockifierTransactionError(other),
}
}
}
Expand All @@ -127,7 +133,7 @@ impl From<FeeCheckError> for Error {
fn from(value: FeeCheckError) -> Self {
match value {
FeeCheckError::MaxL1GasAmountExceeded { .. } | FeeCheckError::MaxFeeExceeded { .. } => {
TransactionValidationError::InsufficientResourcesForValidate.into()
TransactionValidationError::InsufficientMaxFee.into()
}
FeeCheckError::InsufficientFeeTokenBalance { .. } => {
TransactionValidationError::InsufficientAccountBalance.into()
Expand All @@ -143,7 +149,7 @@ impl From<TransactionFeeError> for Error {
| TransactionFeeError::MaxFeeTooLow { .. }
| TransactionFeeError::MaxL1GasPriceTooLow { .. }
| TransactionFeeError::MaxL1GasAmountTooLow { .. } => {
TransactionValidationError::InsufficientResourcesForValidate.into()
TransactionValidationError::InsufficientMaxFee.into()
}
TransactionFeeError::MaxFeeExceedsBalance { .. }
| TransactionFeeError::L1GasBoundsExceedBalance { .. } => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn add_declare_transaction(
broadcasted_declare_transaction: BroadcastedDeclareTransaction,
) -> DevnetResult<(TransactionHash, ClassHash)> {
if broadcasted_declare_transaction.is_max_fee_zero_value() {
return Err(TransactionValidationError::InsufficientResourcesForValidate.into());
return Err(TransactionValidationError::InsufficientMaxFee.into());
}

if broadcasted_declare_transaction.is_only_query() {
Expand Down Expand Up @@ -219,9 +219,7 @@ mod tests {

assert!(result.is_err());
match result.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand All @@ -244,9 +242,7 @@ mod tests {

assert!(result.is_err());
match result.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand Down Expand Up @@ -387,9 +383,7 @@ mod tests {

assert!(result.is_err());
match result.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub fn add_deploy_account_transaction(
broadcasted_deploy_account_transaction: BroadcastedDeployAccountTransaction,
) -> DevnetResult<(TransactionHash, ContractAddress)> {
if broadcasted_deploy_account_transaction.is_max_fee_zero_value() {
return Err(TransactionValidationError::InsufficientResourcesForValidate.into());
return Err(TransactionValidationError::InsufficientMaxFee.into());
}

if broadcasted_deploy_account_transaction.is_only_query() {
Expand Down Expand Up @@ -155,9 +155,7 @@ mod tests {

assert!(result.is_err());
match result.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand All @@ -174,9 +172,7 @@ mod tests {
))
.unwrap_err();
match txn_err {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub fn add_invoke_transaction(
broadcasted_invoke_transaction: BroadcastedInvokeTransaction,
) -> DevnetResult<TransactionHash> {
if broadcasted_invoke_transaction.is_max_fee_zero_value() {
return Err(TransactionValidationError::InsufficientResourcesForValidate.into());
return Err(TransactionValidationError::InsufficientMaxFee.into());
}

if broadcasted_invoke_transaction.is_only_query() {
Expand Down Expand Up @@ -207,9 +207,7 @@ mod tests {
.expect_err("Expected MaxFeeZeroError");

match invoke_v3_txn_error {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand Down Expand Up @@ -279,7 +277,7 @@ mod tests {
assert!(transaction.is_err());
match transaction.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
TransactionValidationError::InsufficientMaxFee,
) => {}
_ => {
panic!("Wrong error type")
Expand Down Expand Up @@ -381,9 +379,7 @@ mod tests {

assert!(result.is_err());
match result.err().unwrap() {
Error::TransactionValidationError(
TransactionValidationError::InsufficientResourcesForValidate,
) => {}
Error::TransactionValidationError(TransactionValidationError::InsufficientMaxFee) => {}
_ => panic!("Wrong error type"),
}
}
Expand Down
6 changes: 4 additions & 2 deletions crates/starknet-devnet-core/src/starknet/estimations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ pub fn estimate_fee(
blockifier::transaction::transaction_execution::Transaction::AccountTransaction(tx),
charge_fee,
validate,
return_error_on_reverted_execution,
)
.map_err(|e| match e {
Error::ContractExecutionError(error_stack) => {
Error::ContractExecutionErrorInSimulation { failure_index: tx_i, error_stack }
Error::ContractExecutionErrorInSimulation {
failure_index: tx_i as u64,
error_stack,
}
}
other => other,
})?;
Expand Down
34 changes: 17 additions & 17 deletions crates/starknet-devnet-core/src/starknet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use blockifier::execution::entry_point::CallEntryPoint;
use blockifier::state::cached_state::CachedState;
use blockifier::state::state_api::StateReader;
use blockifier::transaction::account_transaction::AccountTransaction;
use blockifier::transaction::errors::TransactionExecutionError;
use blockifier::transaction::errors::{
TransactionExecutionError, TransactionFeeError, TransactionPreValidationError,
};
use blockifier::transaction::objects::TransactionExecutionInfo;
use blockifier::transaction::transactions::ExecutableTransaction;
use ethers::types::H256;
Expand Down Expand Up @@ -71,7 +73,7 @@ use crate::error::{DevnetResult, Error, TransactionValidationError};
use crate::messaging::MessagingBroker;
use crate::predeployed_accounts::PredeployedAccounts;
use crate::raw_execution::RawExecutionV1;
use crate::stack_trace::{gen_tx_execution_error_trace, ErrorStack};
use crate::stack_trace::gen_tx_execution_error_trace;
use crate::state::state_diff::StateDiff;
use crate::state::{CommittedClassStorage, CustomState, CustomStateReader, StarknetState};
use crate::traits::{AccountGenerator, Deployed, HashIdentified, HashIdentifiedMut};
Expand Down Expand Up @@ -445,12 +447,12 @@ impl Starknet {
let chain_info = ChainInfo {
chain_id: chain_id.into(),
fee_token_addresses: blockifier::context::FeeTokenAddresses {
eth_fee_token_address: contract_address!(
eth_fee_token_address.to_hex_string().as_str()
),
strk_fee_token_address: contract_address!(
strk_fee_token_address.to_hex_string().as_str()
),
eth_fee_token_address: contract_address!(eth_fee_token_address
.to_hex_string()
.as_str()),
strk_fee_token_address: contract_address!(strk_fee_token_address
.to_hex_string()
.as_str()),
},
};

Expand Down Expand Up @@ -1119,18 +1121,16 @@ impl Starknet {
transactions
.iter()
.enumerate()
.map(|(tx_idx, txn)| {
.map(|(idx, txn)| {
// According to this conversation https://spaceshard.slack.com/archives/C03HL8DH52N/p1710683496750409, simulating a transaction will:
// fail if the fee provided is 0
// succeed if the fee provided is 0 and SKIP_FEE_CHARGE is set
// succeed if the fee provided is > 0
if txn.is_max_fee_zero_value() && !skip_fee_charge {
return Err(Error::ContractExecutionErrorInSimulation {
failure_index: tx_idx,
error_stack: ErrorStack::from_str_err(
&TransactionValidationError::InsufficientResourcesForValidate
.to_string(),
),
return Err(Error::ExecutionError {
execution_error: TransactionValidationError::InsufficientMaxFee
.to_string(),
index: idx,
});
}

Expand All @@ -1150,7 +1150,7 @@ impl Starknet {
let mut transactional_state =
CachedState::new(CachedState::create_transactional(&mut state.state));

for (tx_idx, (blockifier_tx, transaction_type, skip_validate_due_to_impersonation)) in
for (tx_i, (blockifier_tx, transaction_type, skip_validate_due_to_impersonation)) in
blockifier_transactions.into_iter().enumerate()
{
let tx_execution_info = blockifier_tx
Expand All @@ -1161,7 +1161,7 @@ impl Starknet {
!(skip_validate || skip_validate_due_to_impersonation),
)
.map_err(|e| Error::ContractExecutionErrorInSimulation {
failure_index: tx_idx,
failure_index: tx_i as u64,
error_stack: gen_tx_execution_error_trace(&e),
})?;

Expand Down
12 changes: 11 additions & 1 deletion crates/starknet-devnet-server/src/api/json_rpc/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub enum ApiError {
#[error("Contract error")]
ContractError { error_stack: ErrorStack },
#[error("Transaction execution error")]
TransactionExecutionError { failure_index: usize, error_stack: ErrorStack },
TransactionExecutionError { failure_index: u64, error_stack: ErrorStack },
#[error("There are no blocks")]
NoBlocks,
#[error("Requested page size is too big")]
Expand Down Expand Up @@ -62,6 +62,8 @@ pub enum ApiError {
HttpApiError(#[from] HttpApiError),
#[error("the compiled class hash did not match the one supplied in the transaction")]
CompiledClassHashMismatch,
#[error("Transaction execution error")]
ExecutionError { execution_error: String, index: usize },
}

impl ApiError {
Expand Down Expand Up @@ -204,6 +206,14 @@ impl ApiError {
message: error_message.into(),
data: None,
},
ApiError::ExecutionError { execution_error, index } => RpcError {
code: crate::rpc_core::error::ErrorCode::ServerError(41),
message: error_message.into(),
data: Some(json!({
"transaction_index": index,
"execution_error": execution_error
})),
},
ApiError::HttpApiError(http_api_error) => http_api_error.http_api_error_to_rpc_error(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet-devnet/tests/common/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl Drop for UniqueAutoDeletableFile {
/// Deploys an instance of the class whose sierra hash is provided as `class_hash`. Uses a v1 invoke
/// transaction. Returns the address of the newly deployed contract.
pub async fn deploy_v1(
account: &SingleOwnerAccount<&JsonRpcClient<HttpTransport>, LocalWallet>,
account: Arc<SingleOwnerAccount<JsonRpcClient<HttpTransport>, LocalWallet>>,
class_hash: Felt,
ctor_args: &[Felt],
) -> Result<Felt, anyhow::Error> {
Expand Down
13 changes: 7 additions & 6 deletions crates/starknet-devnet/tests/test_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pub mod common;

mod call {
use std::sync::Arc;

use starknet_core::constants::{CAIRO_1_ERC20_CONTRACT_CLASS_HASH, ETH_ERC20_CONTRACT_ADDRESS};
use starknet_rs_accounts::SingleOwnerAccount;
Expand All @@ -16,7 +17,7 @@ mod call {
CAIRO_1_PANICKING_CONTRACT_SIERRA_PATH, PREDEPLOYED_ACCOUNT_ADDRESS,
};
use crate::common::utils::{
assert_json_rpc_errors_equal, declare_v3_deploy_v3, deploy_v1, extract_json_rpc_error,
assert_json_rpc_errors_equal, declare_deploy_v1, deploy_v1, extract_json_rpc_error,
get_flattened_sierra_contract_and_casm_hash,
};

Expand Down Expand Up @@ -87,20 +88,20 @@ mod call {
let devnet = BackgroundDevnet::spawn().await.unwrap();

let (signer, account_address) = devnet.get_first_predeployed_account().await;
let account = SingleOwnerAccount::new(
&devnet.json_rpc_client,
let account = Arc::new(SingleOwnerAccount::new(
devnet.clone_provider(),
signer,
account_address,
devnet.json_rpc_client.chain_id().await.unwrap(),
starknet_rs_accounts::ExecutionEncoding::New,
);
));

let (contract_class, casm_hash) =
get_flattened_sierra_contract_and_casm_hash(CAIRO_1_PANICKING_CONTRACT_SIERRA_PATH);

let (class_hash, contract_address) =
declare_v3_deploy_v3(&account, contract_class, casm_hash, &[]).await.unwrap();
let other_contract_address = deploy_v1(&account, class_hash, &[]).await.unwrap();
declare_deploy_v1(account.clone(), contract_class, casm_hash, &[]).await.unwrap();
let other_contract_address = deploy_v1(account, class_hash, &[]).await.unwrap();

let top_selector = get_selector_from_name("create_panic_in_another_contract").unwrap();
let panic_message = cairo_short_string_to_felt("funny_text").unwrap();
Expand Down
8 changes: 4 additions & 4 deletions crates/starknet-devnet/tests/test_estimate_fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,8 +680,8 @@ mod estimate_fee_tests {
}

#[tokio::test]
async fn estimate_fee_of_declare_and_deploy_via_udc_returns_index_of_second_transaction_when_executed_with_non_existing_method()
{
async fn estimate_fee_of_declare_and_deploy_via_udc_returns_index_of_second_transaction_when_executed_with_non_existing_method(
) {
let devnet = BackgroundDevnet::spawn().await.expect("Could not start devnet");

// get account
Expand Down Expand Up @@ -779,8 +779,8 @@ mod estimate_fee_tests {
}

#[tokio::test]
async fn estimate_fee_of_multiple_failing_txs_should_return_index_of_the_first_failing_transaction()
{
async fn estimate_fee_of_multiple_failing_txs_should_return_index_of_the_first_failing_transaction(
) {
let devnet = BackgroundDevnet::spawn().await.expect("Could not start devnet");

// get account
Expand Down
4 changes: 2 additions & 2 deletions crates/starknet-devnet/tests/test_old_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ mod old_state {
// estimate fee of invoke transaction that reverts must fail, but simulating the same invoke
// transaction have to produce trace of a reverted transaction
#[tokio::test]
async fn estimate_fee_and_simulate_transaction_for_contract_deployment_in_an_old_block_should_not_produce_the_same_error()
{
async fn estimate_fee_and_simulate_transaction_for_contract_deployment_in_an_old_block_should_not_produce_the_same_error(
) {
let devnet =
BackgroundDevnet::spawn_with_additional_args(&["--state-archive-capacity", "full"])
.await
Expand Down
Loading

0 comments on commit cf2eb6c

Please sign in to comment.