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

Commit 4c39a34

Browse files
fmolettafmoletta
authored
Fix storage changes count for transactions with fee transfers (#1015)
* Push clean changes * fmt * Fix test * Fix test * Fix test * Fix test * Fix tests * Fix tests * Fix tests
1 parent 7f7fd62 commit 4c39a34

File tree

9 files changed

+44
-56
lines changed

9 files changed

+44
-56
lines changed

‎src/definitions/constants.rs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,6 @@ pub(crate) const N_DEFAULT_TOPICS: usize = 1; // Events have one default topic.
1414
pub(crate) const CONSUMED_MSG_TO_L2_ENCODED_DATA_SIZE: usize =
1515
(L1_TO_L2_MSG_HEADER_SIZE + 1) - CONSUMED_MSG_TO_L2_N_TOPICS;
1616

17-
/// Sender and sequencer balance updates.
18-
pub(crate) const FEE_TRANSFER_N_STORAGE_CHANGES: usize = 2;
19-
20-
/// Exclude the sequencer balance update, since it's charged once throught the batch.
21-
pub(crate) const FEE_TRANSFER_N_STORAGE_CHANGES_TO_CHARGE: usize =
22-
FEE_TRANSFER_N_STORAGE_CHANGES - 1;
23-
2417
lazy_static! {
2518
pub(crate) static ref QUERY_VERSION_BASE: Felt252 =
2619
felt_str!("340282366920938463463374607431768211456");

‎src/lib.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ mod test {
298298
let transaction = Transaction::InvokeFunction(invoke_function);
299299

300300
let estimated_fee = estimate_fee(&[transaction], state, &block_context).unwrap();
301-
assert_eq!(estimated_fee[0], (3707, 3672));
301+
assert_eq!(estimated_fee[0], (2483, 2448));
302302
}
303303

304304
#[test]
@@ -392,7 +392,7 @@ mod test {
392392
block_context.starknet_os_config.gas_price = 1;
393393

394394
let estimated_fee = estimate_message_fee(&l1_handler, state, &block_context).unwrap();
395-
assert_eq!(estimated_fee, (19709, 19695));
395+
assert_eq!(estimated_fee, (18485, 18471));
396396
}
397397

398398
#[test]
@@ -1035,7 +1035,7 @@ mod test {
10351035

10361036
assert_eq!(
10371037
estimate_fee(&[deploy, invoke_tx], state, block_context,).unwrap(),
1038-
[(0, 3672), (0, 3672)]
1038+
[(0, 2448), (0, 2448)]
10391039
);
10401040
}
10411041

‎src/state/cached_state.rs‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -253,32 +253,28 @@ impl<T: StateReader> State for CachedState<T> {
253253
self.cache.storage_initial_values.clone(),
254254
);
255255

256-
let n_modified_contracts = {
257-
let storage_unique_updates = storage_updates.keys().map(|k| k.0.clone());
258-
259-
let class_hash_updates: Vec<_> = subtract_mappings(
260-
self.cache.class_hash_writes.clone(),
261-
self.cache.class_hash_initial_values.clone(),
262-
)
263-
.keys()
264-
.cloned()
265-
.collect();
256+
let storage_unique_updates = storage_updates.keys().map(|k| k.0.clone());
266257

267-
let nonce_updates: Vec<_> = subtract_mappings(
268-
self.cache.nonce_writes.clone(),
269-
self.cache.nonce_initial_values.clone(),
270-
)
271-
.keys()
272-
.cloned()
273-
.collect();
258+
let class_hash_updates: Vec<_> = subtract_mappings(
259+
self.cache.class_hash_writes.clone(),
260+
self.cache.class_hash_initial_values.clone(),
261+
)
262+
.keys()
263+
.cloned()
264+
.collect();
274265

275-
let mut modified_contracts: HashSet<Address> = HashSet::new();
276-
modified_contracts.extend(storage_unique_updates);
277-
modified_contracts.extend(class_hash_updates);
278-
modified_contracts.extend(nonce_updates);
266+
let nonce_updates: Vec<_> = subtract_mappings(
267+
self.cache.nonce_writes.clone(),
268+
self.cache.nonce_initial_values.clone(),
269+
)
270+
.keys()
271+
.cloned()
272+
.collect();
279273

280-
modified_contracts.len()
281-
};
274+
let mut modified_contracts: HashSet<Address> = HashSet::new();
275+
modified_contracts.extend(storage_unique_updates);
276+
modified_contracts.extend(class_hash_updates);
277+
modified_contracts.extend(nonce_updates);
282278

283279
// Add fee transfer storage update before actually charging it, as it needs to be included in the
284280
// calculation of the final fee.
@@ -288,9 +284,10 @@ impl<T: StateReader> State for CachedState<T> {
288284
(fee_token_address.clone(), sender_low_key),
289285
Felt252::default(),
290286
);
287+
modified_contracts.remove(fee_token_address);
291288
}
292289

293-
Ok((n_modified_contracts, storage_updates.len()))
290+
Ok((modified_contracts.len(), storage_updates.len()))
294291
}
295292

296293
/// Returns the class hash for a given contract address.

‎src/testing/state.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ mod tests {
361361
.unwrap();
362362

363363
let mut actual_resources = HashMap::new();
364-
actual_resources.insert("l1_gas_usage".to_string(), 3672);
364+
actual_resources.insert("l1_gas_usage".to_string(), 2448);
365365
actual_resources.insert("n_steps".to_string(), 0);
366366

367367
let transaction_exec_info = TransactionExecutionInfo {
@@ -584,7 +584,7 @@ mod tests {
584584
.unwrap();
585585
let actual_resources = HashMap::from([
586586
("n_steps".to_string(), 3457),
587-
("l1_gas_usage".to_string(), 3672),
587+
("l1_gas_usage".to_string(), 2448),
588588
("range_check_builtin".to_string(), 80),
589589
("pedersen_builtin".to_string(), 16),
590590
]);

‎src/transaction/declare.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ mod tests {
438438

439439
let actual_resources = HashMap::from([
440440
("n_steps".to_string(), 2715),
441-
("l1_gas_usage".to_string(), 2448),
441+
("l1_gas_usage".to_string(), 1224),
442442
("range_check_builtin".to_string(), 63),
443443
("pedersen_builtin".to_string(), 15),
444444
]);

‎src/transaction/l1_handler.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ mod test {
339339
("n_steps".to_string(), 1319),
340340
("pedersen_builtin".to_string(), 13),
341341
("range_check_builtin".to_string(), 23),
342-
("l1_gas_usage".to_string(), 19695),
342+
("l1_gas_usage".to_string(), 18471),
343343
]),
344344
tx_type: Some(TransactionType::L1Handler),
345345
}

‎src/utils.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::core::errors::hash_errors::HashError;
2-
use crate::definitions::constants::FEE_TRANSFER_N_STORAGE_CHANGES_TO_CHARGE;
32
use crate::services::api::contract_classes::deprecated_contract_class::EntryPointType;
43
use crate::state::state_api::State;
54
use crate::{
@@ -188,7 +187,7 @@ pub fn calculate_tx_resources(
188187
let l1_gas_usage = calculate_tx_gas_usage(
189188
l2_to_l1_messages,
190189
n_modified_contracts,
191-
n_storage_changes + FEE_TRANSFER_N_STORAGE_CHANGES_TO_CHARGE,
190+
n_storage_changes,
192191
l1_handler_payload_size,
193192
n_deployments,
194193
);

‎tests/deploy_account.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ fn internal_deploy_account() {
104104
("n_steps", 3612),
105105
("pedersen_builtin", 23),
106106
("range_check_builtin", 83),
107-
("l1_gas_usage", 4896)
107+
("l1_gas_usage", 3672)
108108
]
109109
.into_iter()
110110
.map(|(k, v)| (k.to_string(), v))
@@ -264,7 +264,7 @@ fn internal_deploy_account_cairo1() {
264264
("n_steps", n_steps),
265265
("pedersen_builtin", 23),
266266
("range_check_builtin", 87),
267-
("l1_gas_usage", 6120)
267+
("l1_gas_usage", 4896)
268268
]
269269
.into_iter()
270270
.map(|(k, v)| (k.to_string(), v))

‎tests/internals.rs‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -704,13 +704,13 @@ fn expected_fib_fee_transfer_info(fee: u128) -> CallInfo {
704704
],
705705
}],
706706
storage_read_values: vec![
707-
INITIAL_BALANCE.clone() - Felt252::from(2476),
707+
INITIAL_BALANCE.clone() - Felt252::from(1252),
708708
Felt252::zero(),
709-
INITIAL_BALANCE.clone() - Felt252::from(2476),
709+
INITIAL_BALANCE.clone() - Felt252::from(1252),
710710
Felt252::zero(),
711-
Felt252::from(2476),
711+
Felt252::from(1252),
712712
Felt252::zero(),
713-
Felt252::from(2476),
713+
Felt252::from(1252),
714714
Felt252::zero(),
715715
],
716716
accessed_storage_keys: HashSet::from([
@@ -937,7 +937,7 @@ fn test_declare_tx() {
937937
("n_steps".to_string(), 2715),
938938
("range_check_builtin".to_string(), 63),
939939
("pedersen_builtin".to_string(), 15),
940-
("l1_gas_usage".to_string(), 3672),
940+
("l1_gas_usage".to_string(), 2448),
941941
]);
942942
let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap();
943943

@@ -1025,7 +1025,7 @@ fn test_declarev2_tx() {
10251025
("n_steps".to_string(), 2715),
10261026
("range_check_builtin".to_string(), 63),
10271027
("pedersen_builtin".to_string(), 15),
1028-
("l1_gas_usage".to_string(), 2448),
1028+
("l1_gas_usage".to_string(), 1224),
10291029
]);
10301030
let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap();
10311031

@@ -1243,7 +1243,7 @@ fn expected_transaction_execution_info(block_context: &BlockContext) -> Transact
12431243
let resources = HashMap::from([
12441244
("n_steps".to_string(), 4135),
12451245
("pedersen_builtin".to_string(), 16),
1246-
("l1_gas_usage".to_string(), 3672),
1246+
("l1_gas_usage".to_string(), 2448),
12471247
("range_check_builtin".to_string(), 101),
12481248
]);
12491249
let fee = calculate_tx_fee(&resources, *GAS_PRICE, block_context).unwrap();
@@ -1272,7 +1272,7 @@ fn expected_fib_transaction_execution_info(
12721272
}
12731273
let resources = HashMap::from([
12741274
("n_steps".to_string(), n_steps),
1275-
("l1_gas_usage".to_string(), 7344),
1275+
("l1_gas_usage".to_string(), 4896),
12761276
("pedersen_builtin".to_string(), 16),
12771277
("range_check_builtin".to_string(), 104),
12781278
]);
@@ -1455,7 +1455,7 @@ fn test_invoke_with_declarev2_tx() {
14551455
fn test_deploy_account() {
14561456
let (block_context, mut state) = create_account_tx_test_state().unwrap();
14571457

1458-
let expected_fee = 6157;
1458+
let expected_fee = 3709;
14591459

14601460
let deploy_account_tx = DeployAccount::new(
14611461
felt_to_hash(&TEST_ACCOUNT_CONTRACT_CLASS_HASH),
@@ -1525,12 +1525,12 @@ fn test_deploy_account() {
15251525
("n_steps".to_string(), 3625),
15261526
("range_check_builtin".to_string(), 83),
15271527
("pedersen_builtin".to_string(), 23),
1528-
("l1_gas_usage".to_string(), 6120),
1528+
("l1_gas_usage".to_string(), 3672),
15291529
]);
15301530

15311531
let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap();
15321532

1533-
assert_eq!(fee, 6157);
1533+
assert_eq!(fee, expected_fee);
15341534

15351535
let expected_execution_info = TransactionExecutionInfo::new(
15361536
expected_validate_call_info.into(),
@@ -1564,11 +1564,10 @@ fn expected_deploy_account_states() -> (
15641564
CachedState<InMemoryStateReader>,
15651565
CachedState<InMemoryStateReader>,
15661566
) {
1567-
let fee = Felt252::from(6157);
1567+
let fee = Felt252::from(3709);
15681568
let mut state_before = CachedState::new(
15691569
Arc::new(InMemoryStateReader::new(
15701570
HashMap::from([
1571-
(Address(0x101.into()), felt_to_hash(&0x111.into())),
15721571
(Address(0x100.into()), felt_to_hash(&0x110.into())),
15731572
(Address(0x1001.into()), felt_to_hash(&0x1010.into())),
15741573
]),
@@ -1805,7 +1804,7 @@ fn test_state_for_declare_tx() {
18051804
// ])
18061805
// );
18071806

1808-
let fee = Felt252::from(3700);
1807+
let fee = Felt252::from(2476);
18091808

18101809
// Check state.cache
18111810
assert_eq!(

0 commit comments

Comments
 (0)