Skip to content

Comments

chore: refactor validate_against_state_and_deduct_caller#2991

Closed
rakita wants to merge 9 commits intomainfrom
rakita/refactor-deduct-caller
Closed

chore: refactor validate_against_state_and_deduct_caller#2991
rakita wants to merge 9 commits intomainfrom
rakita/refactor-deduct-caller

Conversation

@rakita
Copy link
Member

@rakita rakita commented Sep 17, 2025

closes #2983

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 17, 2025

CodSpeed Performance Report

Merging #2991 will degrade performances by 3.18%

Comparing rakita/refactor-deduct-caller (ed1ad3b) with main (0e05a30)

Summary

❌ 1 regression
✅ 172 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
SELFBALANCE_50 19.4 µs 20 µs -3.18%

@rakita rakita marked this pull request as ready for review September 19, 2025 10:48

// journal the change
journal.caller_accounting_journal_entry(tx.caller(), old_balance, tx.kind().is_call());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@klkvr @mattsse can you take a look? This function is a lot simpler than before as few things got abstracted with functions


// Touch account so we know it is changed.
caller_account.mark_touch();
let new_balance = deduct_caller_balance_with_components(account_balance, tx, block, cfg)?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All max and effective balance deduction and checks are wrapped inside this function


if tx.kind().is_call() {
caller_account.info.nonce = caller_account.info.nonce.saturating_add(1);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Touching of account and nonce bumps are now inside caller_touch_and_change

let is_nonce_check_disabled = context.cfg().is_nonce_check_disabled();
let caller = context.tx().caller();
let value = context.tx().value();
let (tx, block, cfg, journal) = evm.ctx_mut().tx_block_cfg_journal_mut();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for temp fields, and we can use tx_block_cfg_journal_mut to get all needed context objects.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can do this in smaller chunks, e.g. especially the balance functions can be introduced as standalone prs

this includes a lot of op changes as well

Comment on lines -190 to -194
// If the transaction is not a deposit transaction, subtract the L1 data fee from the
// caller's balance directly after minting the requested amount of ETH.
// Additionally deduct the operator fee from the caller's account.
//
// In case of deposit additional cost will be zero.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is useful context, that we now no longer have?

Copy link
Member Author

@rakita rakita Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment became obsolete, as flows for deposit and ordinary transactions are now split for clarity.

And it is not fully correct, mint is only available for deposit transactions. And a deposit transaction does not have additional fee (l1 cost and operator fee)

Would assume the intention for this is to mint first, then to deduct the effective gas.

@rakita
Copy link
Member Author

rakita commented Oct 17, 2025

last PR to chunk all changes is here #3069

All of rest are merged

@rakita rakita closed this Oct 17, 2025
@rakita rakita deleted the rakita/refactor-deduct-caller branch February 16, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modularize the validate_against_state_and_deduct_caller function

2 participants