Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Commit 5448721

Browse files
matias-gonzjuanbonoMegaRedHand
authored
Update charge fee and add n_steps for reverted transactions (#787)
* Update charge fee * Fix fee test * Update n_reverted_steps usage * Add docs * apply doc suggestion to src/transaction/fee.rs Co-authored-by: Tomás <[email protected]> * Test test_reverted_transaction_wrong_entry_point * Update docs --------- Co-authored-by: Juan Bono <[email protected]> Co-authored-by: Tomás <[email protected]>
1 parent 8809ba0 commit 5448721

File tree

13 files changed

+268
-223
lines changed

13 files changed

+268
-223
lines changed

‎src/definitions/constants.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ lazy_static! {
2222
pub(crate) const LOG_MSG_TO_L1_ENCODED_DATA_SIZE: usize =
2323
(L2_TO_L1_MSG_HEADER_SIZE + 1) - LOG_MSG_TO_L1_N_TOPICS;
2424

25+
/// Fee factor used for the final fee calculation:
26+
/// actual_fee = min(max_fee, consumed_resources) * FEE_FACTOR
27+
pub(crate) const FEE_FACTOR: u128 = 1;
28+
2529
/// The (empirical) L1 gas cost of each Cairo step.
2630
pub(crate) const N_STEPS_FEE_WEIGHT: f64 = 0.01;
2731

‎src/lib.rs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ mod test {
287287
let transaction = Transaction::InvokeFunction(invoke_function);
288288

289289
let estimated_fee = estimate_fee(&[transaction], state, &block_context).unwrap();
290-
assert_eq!(estimated_fee[0], (12, 0));
290+
assert_eq!(estimated_fee[0], (30, 0));
291291
}
292292

293293
#[test]
@@ -375,15 +375,10 @@ mod test {
375375
state.set_contract_classes(contract_classes).unwrap();
376376

377377
let mut block_context = BlockContext::default();
378-
block_context.cairo_resource_fee_weights = HashMap::from([
379-
(String::from("l1_gas_usage"), 0.into()),
380-
(String::from("pedersen_builtin"), 16.into()),
381-
(String::from("range_check_builtin"), 70.into()),
382-
]);
383378
block_context.starknet_os_config.gas_price = 1;
384379

385380
let estimated_fee = estimate_message_fee(&l1_handler, state, &block_context).unwrap();
386-
assert_eq!(estimated_fee, (20081, 18471));
381+
assert_eq!(estimated_fee, (18484, 18471));
387382
}
388383

389384
#[test]
@@ -921,11 +916,6 @@ mod test {
921916
.unwrap();
922917

923918
let mut block_context = BlockContext::default();
924-
block_context.cairo_resource_fee_weights = HashMap::from([
925-
(String::from("l1_gas_usage"), 0.into()),
926-
(String::from("pedersen_builtin"), 16.into()),
927-
(String::from("range_check_builtin"), 70.into()),
928-
]);
929919
block_context.starknet_os_config.gas_price = 1;
930920

931921
simulate_transaction(

‎src/testing/state.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ mod tests {
358358

359359
let mut actual_resources = HashMap::new();
360360
actual_resources.insert("l1_gas_usage".to_string(), 1224);
361+
actual_resources.insert("n_steps".to_string(), 0);
361362

362363
let transaction_exec_info = TransactionExecutionInfo {
363364
validate_info: None,
@@ -572,6 +573,7 @@ mod tests {
572573
)
573574
.unwrap();
574575
let actual_resources = HashMap::from([
576+
("n_steps".to_string(), 2933),
575577
("l1_gas_usage".to_string(), 0),
576578
("range_check_builtin".to_string(), 70),
577579
("pedersen_builtin".to_string(), 16),

‎src/transaction/declare.rs‎

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,16 @@ use crate::{
1818
services::api::contract_classes::deprecated_contract_class::ContractClass,
1919
state::state_api::{State, StateReader},
2020
state::ExecutionResourcesManager,
21-
transaction::{
22-
error::TransactionError,
23-
fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo},
24-
},
21+
transaction::error::TransactionError,
2522
utils::{
2623
calculate_tx_resources, felt_to_hash, verify_no_calls_to_other_contracts, Address,
2724
ClassHash,
2825
},
2926
};
3027
use cairo_vm::felt::Felt252;
3128
use num_traits::Zero;
32-
use std::collections::HashMap;
3329

30+
use super::fee::charge_fee;
3431
use super::{verify_version, Transaction};
3532

3633
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -176,6 +173,7 @@ impl Declare {
176173
TransactionType::Declare,
177174
changes,
178175
None,
176+
0,
179177
)
180178
.map_err(|_| TransactionError::ResourcesCalculation)?;
181179

@@ -243,38 +241,6 @@ impl Declare {
243241
Ok(Some(call_info))
244242
}
245243

246-
/// Calculates and charges the actual fee.
247-
pub fn charge_fee<S: StateReader>(
248-
&self,
249-
state: &mut CachedState<S>,
250-
resources: &HashMap<String, usize>,
251-
block_context: &BlockContext,
252-
) -> Result<FeeInfo, TransactionError> {
253-
if self.max_fee.is_zero() {
254-
return Ok((None, 0));
255-
}
256-
257-
let actual_fee = calculate_tx_fee(
258-
resources,
259-
block_context.starknet_os_config.gas_price,
260-
block_context,
261-
)?;
262-
263-
let mut tx_execution_context =
264-
self.get_execution_context(block_context.invoke_tx_max_n_steps);
265-
let fee_transfer_info = if self.skip_fee_transfer {
266-
None
267-
} else {
268-
Some(execute_fee_transfer(
269-
state,
270-
block_context,
271-
&mut tx_execution_context,
272-
actual_fee,
273-
)?)
274-
};
275-
Ok((fee_transfer_info, actual_fee))
276-
}
277-
278244
fn handle_nonce<S: State + StateReader>(&self, state: &mut S) -> Result<(), TransactionError> {
279245
if self.version.is_zero() || self.version == *QUERY_VERSION_BASE {
280246
return Ok(());
@@ -304,8 +270,16 @@ impl Declare {
304270
let mut tx_exec_info = self.apply(state, block_context)?;
305271
self.handle_nonce(state)?;
306272

307-
let (fee_transfer_info, actual_fee) =
308-
self.charge_fee(state, &tx_exec_info.actual_resources, block_context)?;
273+
let mut tx_execution_context =
274+
self.get_execution_context(block_context.invoke_tx_max_n_steps);
275+
let (fee_transfer_info, actual_fee) = charge_fee(
276+
state,
277+
&tx_exec_info.actual_resources,
278+
block_context,
279+
self.max_fee,
280+
&mut tx_execution_context,
281+
self.skip_fee_transfer,
282+
)?;
309283

310284
state.set_contract_class(&self.class_hash, &self.contract_class)?;
311285

@@ -444,6 +418,7 @@ mod tests {
444418
});
445419

446420
let actual_resources = HashMap::from([
421+
("n_steps".to_string(), 2348),
447422
("l1_gas_usage".to_string(), 0),
448423
("range_check_builtin".to_string(), 57),
449424
("pedersen_builtin".to_string(), 15),

‎src/transaction/declare_v2.rs‎

Lines changed: 28 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use super::fee::charge_fee;
12
use super::{verify_version, Transaction};
23
use crate::core::contract_address::{compute_casm_class_hash, compute_sierra_class_hash};
34
use crate::definitions::constants::QUERY_VERSION_BASE;
@@ -13,23 +14,18 @@ use crate::{
1314
transaction_type::TransactionType,
1415
},
1516
execution::{
16-
execution_entry_point::ExecutionEntryPoint, CallInfo, CallType,
17-
TransactionExecutionContext, TransactionExecutionInfo,
17+
execution_entry_point::ExecutionEntryPoint, CallType, TransactionExecutionContext,
18+
TransactionExecutionInfo,
1819
},
1920
state::state_api::{State, StateReader},
2021
state::ExecutionResourcesManager,
21-
transaction::{
22-
error::TransactionError,
23-
fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo},
24-
invoke_function::verify_no_calls_to_other_contracts,
25-
},
22+
transaction::{error::TransactionError, invoke_function::verify_no_calls_to_other_contracts},
2623
utils::{calculate_tx_resources, Address},
2724
};
2825
use cairo_lang_starknet::casm_contract_class::CasmContractClass;
2926
use cairo_lang_starknet::contract_class::ContractClass as SierraContractClass;
3027
use cairo_vm::felt::Felt252;
3128
use num_traits::Zero;
32-
use std::collections::HashMap;
3329

3430
/// Represents a declare transaction in the starknet network.
3531
/// Declare creates a blueprint of a contract class that is used to deploy instances of the contract
@@ -274,43 +270,6 @@ impl DeclareV2 {
274270
Vec::from([bytes])
275271
}
276272

277-
/// Calculates and charges the actual fee.
278-
/// ## Parameter:
279-
/// - state: An state that implements the State and StateReader traits.
280-
/// - resources: the resources that are in use by the contract
281-
/// - block_context: The block that contains the execution context
282-
pub fn charge_fee<S: StateReader>(
283-
&self,
284-
state: &mut CachedState<S>,
285-
resources: &HashMap<String, usize>,
286-
block_context: &BlockContext,
287-
) -> Result<FeeInfo, TransactionError> {
288-
if self.max_fee.is_zero() {
289-
return Ok((None, 0));
290-
}
291-
292-
let actual_fee = calculate_tx_fee(
293-
resources,
294-
block_context.starknet_os_config.gas_price,
295-
block_context,
296-
)?;
297-
298-
let mut tx_execution_context =
299-
self.get_execution_context(block_context.invoke_tx_max_n_steps);
300-
let fee_transfer_info = if self.skip_fee_transfer {
301-
None
302-
} else {
303-
Some(execute_fee_transfer(
304-
state,
305-
block_context,
306-
&mut tx_execution_context,
307-
actual_fee,
308-
)?)
309-
};
310-
311-
Ok((fee_transfer_info, actual_fee))
312-
}
313-
314273
// TODO: delete once used
315274
#[allow(dead_code)]
316275
fn handle_nonce<S: State + StateReader>(&self, state: &mut S) -> Result<(), TransactionError> {
@@ -347,33 +306,42 @@ impl DeclareV2 {
347306

348307
let mut resources_manager = ExecutionResourcesManager::default();
349308

350-
let (validate_info, _remaining_gas) = if self.skip_validate {
351-
(None, 0)
309+
let (execution_result, _remaining_gas) = if self.skip_validate {
310+
(ExecutionResult::default(), 0)
352311
} else {
353312
let (info, gas) = self.run_validate_entrypoint(
354313
initial_gas,
355314
state,
356315
&mut resources_manager,
357316
block_context,
358317
)?;
359-
(Some(info), gas)
318+
(info, gas)
360319
};
361320

362321
let storage_changes = state.count_actual_storage_changes();
363322
let actual_resources = calculate_tx_resources(
364323
resources_manager,
365-
&[validate_info.clone()],
324+
&[execution_result.call_info.clone()],
366325
self.tx_type,
367326
storage_changes,
368327
None,
328+
execution_result.n_reverted_steps,
369329
)?;
370330

371-
let (fee_transfer_info, actual_fee) =
372-
self.charge_fee(state, &actual_resources, block_context)?;
331+
let mut tx_execution_context =
332+
self.get_execution_context(block_context.invoke_tx_max_n_steps);
333+
let (fee_transfer_info, actual_fee) = charge_fee(
334+
state,
335+
&actual_resources,
336+
block_context,
337+
self.max_fee,
338+
&mut tx_execution_context,
339+
self.skip_fee_transfer,
340+
)?;
373341
self.compile_and_store_casm_class(state)?;
374342

375343
let mut tx_exec_info = TransactionExecutionInfo::new_without_fee_info(
376-
validate_info,
344+
execution_result.call_info,
377345
None,
378346
None,
379347
actual_resources,
@@ -415,7 +383,7 @@ impl DeclareV2 {
415383
state: &mut CachedState<S>,
416384
resources_manager: &mut ExecutionResourcesManager,
417385
block_context: &BlockContext,
418-
) -> Result<(CallInfo, u128), TransactionError> {
386+
) -> Result<(ExecutionResult, u128), TransactionError> {
419387
let calldata = [self.compiled_class_hash.clone()].to_vec();
420388

421389
let entry_point = ExecutionEntryPoint {
@@ -433,7 +401,7 @@ impl DeclareV2 {
433401
let mut tx_execution_context =
434402
self.get_execution_context(block_context.validate_max_n_steps);
435403

436-
let ExecutionResult { call_info, .. } = if self.skip_execute {
404+
let execution_result = if self.skip_execute {
437405
ExecutionResult::default()
438406
} else {
439407
entry_point.execute(
@@ -445,10 +413,13 @@ impl DeclareV2 {
445413
block_context.validate_max_n_steps,
446414
)?
447415
};
448-
let call_info = verify_no_calls_to_other_contracts(&call_info)?;
449-
remaining_gas -= call_info.gas_consumed;
450416

451-
Ok((call_info, remaining_gas))
417+
if execution_result.call_info.is_some() {
418+
verify_no_calls_to_other_contracts(&execution_result.call_info)?;
419+
remaining_gas -= execution_result.call_info.clone().unwrap().gas_consumed;
420+
}
421+
422+
Ok((execution_result, remaining_gas))
452423
}
453424

454425
// ---------------

‎src/transaction/deploy.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ impl Deploy {
185185
self.tx_type,
186186
changes,
187187
None,
188+
0,
188189
)?;
189190

190191
Ok(TransactionExecutionInfo::new_without_fee_info(
@@ -230,7 +231,7 @@ impl Deploy {
230231
let ExecutionResult {
231232
call_info,
232233
revert_error,
233-
..
234+
n_reverted_steps,
234235
} = call.execute(
235236
state,
236237
block_context,
@@ -247,6 +248,7 @@ impl Deploy {
247248
self.tx_type,
248249
changes,
249250
None,
251+
n_reverted_steps,
250252
)?;
251253

252254
Ok(TransactionExecutionInfo::new_without_fee_info(

0 commit comments

Comments
 (0)