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(executor/estimate): fee estimation should fail for reverts #2597

Merged
merged 4 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions crates/executor/src/error_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use pathfinder_common::{ClassHash, ContractAddress, EntryPoint};

use crate::IntoFelt;

#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug, Default, PartialEq)]
pub struct ErrorStack(pub Vec<Frame>);

impl From<BlockifierErrorStack> for ErrorStack {
Expand Down Expand Up @@ -53,7 +53,7 @@ impl From<Cairo1RevertSummary> for ErrorStack {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub enum Frame {
CallFrame(CallFrame),
StringFrame(String),
Expand Down Expand Up @@ -87,7 +87,7 @@ impl From<Cairo1RevertFrame> for Frame {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct CallFrame {
pub storage_address: ContractAddress,
pub class_hash: ClassHash,
Expand Down
23 changes: 17 additions & 6 deletions crates/executor/src/estimate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,23 @@ pub fn estimate(
"Transaction estimation finished"
);

Ok(FeeEstimate::from_tx_and_tx_info(
&tx,
&tx_info,
&gas_vector_computation_mode,
&block_context,
))
if let Some(revert_error) = tx_info.revert_error {
let revert_string = revert_error.to_string();
tracing::debug!(revert_error=%revert_string, "Transaction reverted");

Err(TransactionExecutionError::ExecutionError {
transaction_index: tx_index,
error: revert_string,
error_stack: revert_error.into(),
})
} else {
Ok(FeeEstimate::from_tx_and_tx_info(
&tx,
&tx_info,
&gas_vector_computation_mode,
&block_context,
))
}
})
.collect()
}
Expand Down
108 changes: 108 additions & 0 deletions crates/rpc/src/method/estimate_fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,114 @@ mod tests {
);
}

/// Invokes the test contract with an invalid entry point so that
/// the transaction is expected to be reverted.
fn invoke_v3_transaction_with_invalid_entry_point(
sender_address: ContractAddress,
nonce: TransactionNonce,
depth: CallParam,
) -> BroadcastedTransaction {
BroadcastedTransaction::Invoke(BroadcastedInvokeTransaction::V3(
BroadcastedInvokeTransactionV3 {
version: TransactionVersion::THREE,
signature: vec![],
sender_address,
calldata: vec![
// Number of calls
call_param!("0x1"),
// Address of the deployed test contract
CallParam(felt!(
"0x17c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b"
)),
// Entry point selector for the called contract, i.e.
CallParam(EntryPoint::hashed(b"bogus").0),
// Length of the call data for the called contract, i.e.
call_param!("1"),
depth,
],
nonce,
resource_bounds: ResourceBounds {
l1_gas: ResourceBound {
max_amount: ResourceAmount(50),
max_price_per_unit: ResourcePricePerUnit(1000),
},
l1_data_gas: Some(ResourceBound {
max_amount: ResourceAmount(100),
max_price_per_unit: ResourcePricePerUnit(1000),
}),
l2_gas: ResourceBound {
max_amount: ResourceAmount(800_000),
max_price_per_unit: ResourcePricePerUnit(1000),
},
},
tip: Tip(0),
paymaster_data: vec![],
account_deployment_data: vec![],
nonce_data_availability_mode: DataAvailabilityMode::L2,
fee_data_availability_mode: DataAvailabilityMode::L2,
},
))
}

#[tokio::test]
async fn declare_deploy_and_invoke_sierra_class_reverts_on_starknet_0_13_4() {
let (context, last_block_header, account_contract_address, universal_deployer_address) =
crate::test_setup::test_context_with_starknet_version(StarknetVersion::new(
0, 13, 4, 0,
))
.await;

// declare test class
let declare_transaction = declare_v3_transaction(account_contract_address);
// deploy with universal deployer contract
let deploy_transaction =
deploy_v3_transaction(account_contract_address, universal_deployer_address);
// invoke deployed contract
let invoke_transaction = invoke_v3_transaction_with_invalid_entry_point(
account_contract_address,
transaction_nonce!("0x2"),
call_param!("7"),
);

let input = Input {
request: vec![declare_transaction, deploy_transaction, invoke_transaction],
simulation_flags: vec![SimulationFlag::SkipValidate],
block_id: BlockId::Number(last_block_header.number),
};
let error = super::estimate_fee(context, input).await.unwrap_err();

assert_matches::assert_matches!(error, EstimateFeeError::TransactionExecutionError { transaction_index, error, error_stack } => {
assert_eq!(transaction_index, 2);
assert_eq!(error, "Transaction execution has failed:\n\
0: Error in the called contract (contract address: 0x0000000000000000000000000000000000000000000000000000000000000c01, class hash: 0x019cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):\n\
Execution failed. Failure reason:\n\
Error in contract (contract address: 0x0000000000000000000000000000000000000000000000000000000000000c01, class hash: 0x019cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):\n\
Error in contract (contract address: 0x017c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b, class hash: 0x01a48fd3f75d0a7c2288ac23fb6aba26cd375607ba63e4a3b3ed47fc8e99dc21, selector: 0x02a1f595e2db7bf53e1a4bc9834eef6b86d3cd66ec9c8b3588c09253d0affc51):\n\
0x454e545259504f494e545f4e4f545f464f554e44 ('ENTRYPOINT_NOT_FOUND').\n");
assert_eq!(error_stack, pathfinder_executor::ErrorStack(vec![
pathfinder_executor::Frame::CallFrame(pathfinder_executor::CallFrame {
storage_address: account_contract_address,
class_hash: crate::test_setup::OPENZEPPELIN_ACCOUNT_CLASS_HASH,
selector: Some(EntryPoint::hashed(b"__execute__")),
}),
pathfinder_executor::Frame::StringFrame(
"Cairo1RevertSummary { header: Execution, stack: \
[Cairo1RevertFrame { \
contract_address: ContractAddress(PatriciaKey(0xc01)), \
class_hash: Some(ClassHash(0x19cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978)), \
selector: EntryPointSelector(0x15d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad) \
}, \
Cairo1RevertFrame { \
contract_address: ContractAddress(PatriciaKey(0x17c54b787c2eccfb057cf6aa2f941d612249549fff74140adc20bb949eab74b)), \
class_hash: Some(ClassHash(0x1a48fd3f75d0a7c2288ac23fb6aba26cd375607ba63e4a3b3ed47fc8e99dc21)), \
selector: EntryPointSelector(0x2a1f595e2db7bf53e1a4bc9834eef6b86d3cd66ec9c8b3588c09253d0affc51) \
}], \
last_retdata: Retdata([0x454e545259504f494e545f4e4f545f464f554e44]) }".to_owned()
)
]));
});
}

#[tokio::test]
async fn starknet_0_13_4_max_gas_exceeded() {
let (context, last_block_header, account_contract_address, universal_deployer_address) =
Expand Down