Skip to content

Commit

Permalink
fix: origin nonce used in deploy tx & available balance in tx (#957)
Browse files Browse the repository at this point in the history
* fix: origin nonce used in deploy tx

* fix: withdraw max_fee balance prior to tx

* remove prints
  • Loading branch information
enitrat committed Sep 18, 2024
1 parent 2c2d12a commit 2b1cd7f
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 37 deletions.
2 changes: 1 addition & 1 deletion crates/evm/src/backend/starknet_backend.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub fn get_bytecode(evm_address: EthAddress) -> Span<u8> {
}

/// Populate an Environment with Starknet syscalls.
pub fn get_env(origin: EthAddress, gas_price: u128) -> Environment {
pub fn get_env(origin: Address, gas_price: u128) -> Environment {
let kakarot_state = KakarotCore::unsafe_new_contract_state();
let kakarot_storage = kakarot_state.snapshot_deref();
let block_info = get_block_info().unbox();
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/src/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
/// # Specification: https://www.evm.codes/#32?fork=shanghai
fn exec_origin(ref self: VM) -> Result<(), EVMError> {
self.charge_gas(gas::BASE)?;
self.stack.push(self.env.origin.into())
self.stack.push(self.env.origin.evm.into())
}

/// 0x33 - CALLER
Expand Down Expand Up @@ -448,7 +448,7 @@ mod tests {

// Then
assert_eq!(vm.stack.len(), 1);
assert_eq!(vm.stack.peek().unwrap(), origin().into());
assert_eq!(vm.stack.peek().unwrap(), vm.env.origin.evm.into());
}


Expand Down
177 changes: 154 additions & 23 deletions crates/evm/src/interpreter.cairo
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use contracts::account_contract::{IAccountDispatcher, IAccountDispatcherTrait};
use contracts::kakarot_core::KakarotCore;
use contracts::kakarot_core::interface::IKakarotCore;
use core::num::traits::Zero;
Expand All @@ -16,7 +17,7 @@ use evm::instructions::{
MemoryOperationTrait, Sha3Trait
};

use evm::model::account::{AccountTrait};
use evm::model::account::{Account, AccountTrait};
use evm::model::vm::{VM, VMTrait};
use evm::model::{
Message, Environment, Transfer, ExecutionSummary, ExecutionSummaryTrait, ExecutionResult,
Expand All @@ -35,30 +36,22 @@ use utils::set::{Set, SetTrait};

#[generate_trait]
pub impl EVMImpl of EVMTrait {
fn process_transaction(
ref self: KakarotCore::ContractState, origin: Address, tx: Transaction, intrinsic_gas: u64
) -> TransactionResult {
// Charge the cost of intrinsic gas - which has been verified to be <= gas_limit.
let block_base_fee = self.snapshot_deref().Kakarot_base_fee.read();
let gas_price = tx.effective_gas_price(Option::Some(block_base_fee.into()));
let gas_left = tx.gas_limit() - intrinsic_gas;
let mut env = starknet_backend::get_env(origin.evm, gas_price);

let mut sender_account = env.state.get_account(origin.evm);
sender_account.set_nonce(sender_account.nonce() + 1);
env.state.set_account(sender_account);

// Handle deploy/non-deploy transaction cases
fn prepare_message(
self: @KakarotCore::ContractState,
tx: @Transaction,
sender_account: @Account,
ref env: Environment,
gas_left: u64
) -> (Message, bool) {
let (to, is_deploy_tx, code, code_address, calldata) = match tx.kind() {
TxKind::Create => {
// Deploy tx case.
let mut origin_nonce: u64 = get_tx_info().unbox().nonce.try_into().unwrap();
let to_evm_address = compute_contract_address(origin.evm, origin_nonce);
let origin_nonce: u64 = sender_account.nonce();
let to_evm_address = compute_contract_address(
sender_account.address().evm, origin_nonce
);
let to_starknet_address = self.compute_starknet_address(to_evm_address);
let to = Address { evm: to_evm_address, starknet: to_starknet_address };
let code = tx.input();
let calldata = [].span();
(to, true, code, Zero::zero(), calldata)
(to, true, tx.input(), Zero::zero(), [].span())
},
TxKind::Call(to) => {
let target_starknet_address = self.compute_starknet_address(to);
Expand All @@ -71,7 +64,7 @@ pub impl EVMImpl of EVMTrait {
let mut accessed_addresses: Set<EthAddress> = Default::default();
accessed_addresses.add(env.coinbase);
accessed_addresses.add(to.evm);
accessed_addresses.add(origin.evm);
accessed_addresses.add(env.origin.evm);
accessed_addresses.extend(eth_precompile_addresses().spanset());

let mut accessed_storage_keys: Set<(EthAddress, u256)> = Default::default();
Expand All @@ -86,7 +79,7 @@ pub impl EVMImpl of EVMTrait {
};

let message = Message {
caller: origin,
caller: env.origin,
target: to,
gas_limit: gas_left,
data: calldata,
Expand All @@ -99,8 +92,44 @@ pub impl EVMImpl of EVMTrait {
accessed_addresses: accessed_addresses.spanset(),
accessed_storage_keys: accessed_storage_keys.spanset(),
};

(message, is_deploy_tx)
}

fn process_transaction(
ref self: KakarotCore::ContractState, origin: Address, tx: Transaction, intrinsic_gas: u64
) -> TransactionResult {
// Charge the cost of intrinsic gas - which has been verified to be <= gas_limit.
let block_base_fee = self.snapshot_deref().Kakarot_base_fee.read();
let gas_price = tx.effective_gas_price(Option::Some(block_base_fee.into()));
let gas_left = tx.gas_limit() - intrinsic_gas;
let max_fee = tx.gas_limit().into() * gas_price;
let mut env = starknet_backend::get_env(origin, gas_price);

let (message, is_deploy_tx) = {
let mut sender_account = env.state.get_account(origin.evm);
// Charge the intrinsic gas to the sender so that it's not available for the execution
// of the transaction but don't trigger any actual transfer, as only the actual consumde
// gas is charged at the end of the transaction
sender_account.set_balance(sender_account.balance() - max_fee.into());

let (message, is_deploy_tx) = self
.prepare_message(@tx, @sender_account, ref env, gas_left);

// Increment nonce of sender AFTER computing eventual created address
sender_account.set_nonce(sender_account.nonce() + 1);

env.state.set_account(sender_account);
(message, is_deploy_tx)
};

let mut summary = Self::process_message_call(message, env, is_deploy_tx);

// Cancel the max_fee that was taken from the sender to prevent double charging
let mut sender_account = summary.state.get_account(origin.evm);
sender_account.set_balance(sender_account.balance() + max_fee.into());
summary.state.set_account(sender_account);

// Gas refunds
let gas_used = tx.gas_limit() - summary.gas_left;
let gas_refund = core::cmp::min(gas_used / 5, summary.gas_refund);
Expand Down Expand Up @@ -978,3 +1007,105 @@ pub impl EVMImpl of EVMTrait {
return Result::Err(EVMError::InvalidOpcode(opcode));
}
}

#[cfg(test)]
mod tests {
use contracts::kakarot_core::KakarotCore;
use core::num::traits::Zero;
use evm::model::{Account, Environment, Message};
use evm::state::StateTrait;
use evm::test_utils::{dual_origin, test_dual_address};
use super::EVMTrait;
use utils::constants::EMPTY_KECCAK;
use utils::eth_transaction::common::TxKind;
use utils::eth_transaction::legacy::TxLegacy;
use utils::eth_transaction::transaction::{Transaction, TransactionTrait};

fn setup() -> (KakarotCore::ContractState, Account, Environment) {
let state = KakarotCore::contract_state_for_testing();
let sender_account = Account {
address: test_dual_address(),
nonce: 5,
balance: 1000000000000000000_u256, // 1 ETH
code: array![].span(),
code_hash: EMPTY_KECCAK,
selfdestruct: false,
is_created: true,
};
let mut env = Environment {
origin: dual_origin(),
gas_price: 20000000000_u128, // 20 Gwei
chain_id: 1_u64,
prevrandao: 0_u256,
block_number: 12345_u64,
block_gas_limit: 30000000_u64,
block_timestamp: 1634567890_u64,
coinbase: 0x0000000000000000000000000000000000000000.try_into().unwrap(),
base_fee: 0_u64,
state: Default::default(),
};
env.state.set_account(sender_account);
(state, sender_account, env)
}

#[test]
fn test_prepare_message_create() {
let (mut state, sender_account, mut env) = setup();
let tx = Transaction::Legacy(
TxLegacy {
chain_id: Option::Some(1),
nonce: 5,
gas_price: 20000000000_u128, // 20 Gwei
gas_limit: 1000000_u64,
to: TxKind::Create,
value: 0_u256,
input: array![0x60, 0x80, 0x60, 0x40, 0x52].span(), // Simple contract bytecode
}
);

let (message, is_deploy_tx) = state
.prepare_message(@tx, @sender_account, ref env, tx.gas_limit());

assert_eq!(is_deploy_tx, true);
assert_eq!(message.code, tx.input());
assert_eq!(
message.target.evm, 0xf50541960eec6df5caa295adee1a1a95c3c3241c.try_into().unwrap()
); // compute_contract_address('evm_address', 5);
assert_eq!(message.code_address, Zero::zero());
assert_eq!(message.data, [].span());
assert_eq!(message.gas_limit, tx.gas_limit());
assert_eq!(message.depth, 0);
assert_eq!(message.should_transfer_value, true);
assert_eq!(message.value, 0_u256);
}

#[test]
fn test_prepare_message_call() {
let (mut state, sender_account, mut env) = setup();
let target_address = sender_account.address;
let tx = Transaction::Legacy(
TxLegacy {
chain_id: Option::Some(1),
nonce: 5,
gas_price: 20000000000_u128, // 20 Gwei
gas_limit: 1000000_u64,
to: TxKind::Call(target_address.evm),
value: 1000000000000000000_u256, // 1 ETH
input: array![0x12, 0x34, 0x56, 0x78].span(), // Some calldata
}
);

let (message, is_deploy_tx) = state
.prepare_message(@tx, @sender_account, ref env, tx.gas_limit());

assert_eq!(is_deploy_tx, false);
assert_eq!(message.target.evm, target_address.evm);
assert_eq!(message.code_address.evm, target_address.evm);
assert_eq!(message.code, sender_account.code);
assert_eq!(message.data, tx.input());
assert_eq!(message.gas_limit, tx.gas_limit());
assert_eq!(message.depth, 0);
assert_eq!(message.should_transfer_value, true);
assert_eq!(message.value, 1000000000000000000_u256);
}
}
2 changes: 1 addition & 1 deletion crates/evm/src/model.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use utils::traits::{EthAddressDefault, ContractAddressDefault, SpanDefault};

#[derive(Destruct, Default)]
pub struct Environment {
pub origin: EthAddress,
pub origin: Address,
pub gas_price: u128,
pub chain_id: u64,
pub prevrandao: u256,
Expand Down
13 changes: 5 additions & 8 deletions crates/evm/src/model/account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ pub impl AccountImpl of AccountTrait {
*self.balance
}

#[inline(always)]
fn set_balance(ref self: Account, value: u256) {
self.balance = value;
}

#[inline(always)]
fn address(self: @Account) -> Address {
*self.address
Expand Down Expand Up @@ -268,14 +273,6 @@ pub impl AccountImpl of AccountTrait {
}
}

#[generate_trait]
pub(crate) impl AccountInternals of AccountInternalTrait {
#[inline(always)]
fn set_balance(ref self: Account, value: u256) {
self.balance = value;
}
}

#[cfg(test)]
mod tests {
mod test_has_code_or_nonce {
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/state.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use core::starknet::{EthAddress};
use evm::backend::starknet_backend::fetch_original_storage;

use evm::errors::{ensure, EVMError, BALANCE_OVERFLOW};
use evm::model::account::{AccountTrait, AccountInternalTrait};
use evm::model::account::{AccountTrait};
use evm::model::{Event, Transfer, Account};
use utils::set::{Set, SetTrait};

Expand Down
11 changes: 10 additions & 1 deletion crates/evm/src/test_utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ pub fn origin() -> EthAddress {
'origin'.try_into().unwrap()
}

pub fn dual_origin() -> Address {
let origin_evm = origin();
let origin_starknet = utils::helpers::compute_starknet_address(
test_address(), origin_evm, uninitialized_account()
);
Address { evm: origin_evm, starknet: origin_starknet }
}

pub fn caller() -> EthAddress {
'caller'.try_into().unwrap()
}
Expand Down Expand Up @@ -253,8 +261,9 @@ pub fn preset_message() -> Message {

pub fn preset_environment() -> Environment {
let block_info = starknet::get_block_info().unbox();

Environment {
origin: origin(),
origin: dual_origin(),
gas_price: gas_price(),
chain_id: chain_id(),
prevrandao: 0,
Expand Down

0 comments on commit 2b1cd7f

Please sign in to comment.