Skip to content

Commit

Permalink
Addressed first call precompile gas calculation
Browse files Browse the repository at this point in the history
Added a way to mark if an account is a precompile that has been or not
been called to allow for more accurate gas calculations
  • Loading branch information
Parker Thompson committed Aug 10, 2022
1 parent b1768d7 commit 9f9a7a2
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 8 deletions.
4 changes: 3 additions & 1 deletion crates/revm/src/db/in_memory_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use hashbrown::{hash_map::Entry, HashMap as Map};

use primitive_types::{H160, H256, U256};

use crate::{Account, AccountInfo, Log};
use crate::{Account, AccountInfo, Log, PrecompState};
use bytes::Bytes;
use sha3::{Digest, Keccak256};

Expand Down Expand Up @@ -129,6 +129,7 @@ impl<ExtDB: DatabaseRef> DatabaseCommit for CacheDB<ExtDB> {
btree_map::Entry::Occupied(mut entry) => {
let db_acc = entry.get_mut();
db_acc.info = acc.info;

if matches!(acc.filth, Filth::NewlyCreated) {
db_acc.account_state = AccountState::EVMStorageCleared;
db_acc.storage = acc.storage.into_iter().collect();
Expand Down Expand Up @@ -297,6 +298,7 @@ impl Database for BenchmarkDB {
balance: U256::from(10000000),
code: Some(self.0.clone()),
code_hash: KECCAK_EMPTY,
precomp_state: PrecompState::None,
};
}
AccountInfo::default()
Expand Down
4 changes: 2 additions & 2 deletions crates/revm/src/evm_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,13 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB,
for (add, _) in precompiles.as_slice() {
precompile_acc.push(*add);
}
subroutine.load_precompiles_default(&precompile_acc);
subroutine.load_precompiles_default(&precompile_acc, env.cfg.precompiles_called);
} else {
let mut precompile_acc = Map::new();
for (add, _) in precompiles.as_slice() {
precompile_acc.insert(*add, db.basic(*add));
}
subroutine.load_precompiles(precompile_acc);
subroutine.load_precompiles(precompile_acc, env.cfg.precompiles_called);
}
Self {
data: EVMData {
Expand Down
1 change: 1 addition & 0 deletions crates/revm/src/instructions/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ pub fn call<H: Host, SPEC: Spec>(
// load account and calculate gas cost.
let (is_cold, exist) = host.load_account(to);
let is_new = !exist;

//let is_cold = false;
gas!(
interp,
Expand Down
16 changes: 16 additions & 0 deletions crates/revm/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ pub const KECCAK_EMPTY: H256 = H256([
0xe5, 0x00, 0xb6, 0x53, 0xca, 0x82, 0x27, 0x3b, 0x7b, 0xfa, 0xd8, 0x04, 0x5d, 0x85, 0xa4, 0x70,
]);

/// State of a precompile
#[derive(Clone, Eq, PartialEq, Debug)]
#[cfg_attr(feature = "with-serde", derive(serde::Serialize, serde::Deserialize))]
pub enum PrecompState {
None,
Pending,
Called,
}

/// AccountInfo account information.
#[derive(Clone, Eq, PartialEq, Debug)]
#[cfg_attr(feature = "with-serde", derive(serde::Serialize, serde::Deserialize))]
Expand All @@ -24,6 +33,8 @@ pub struct AccountInfo {
/// inside of revm.
#[cfg_attr(feature = "with-serde", serde(with = "serde_hex_bytes_opt"))]
pub code: Option<Bytes>,

pub precomp_state: PrecompState,
}

impl Default for AccountInfo {
Expand All @@ -33,6 +44,7 @@ impl Default for AccountInfo {
code_hash: KECCAK_EMPTY,
code: Some(Bytes::new()),
nonce: 0,
precomp_state: PrecompState::None,
}
}
}
Expand All @@ -49,6 +61,7 @@ impl AccountInfo {
nonce,
code: Some(code),
code_hash,
precomp_state: PrecompState::None,
}
}

Expand Down Expand Up @@ -219,6 +232,8 @@ pub struct TxEnv {
pub struct CfgEnv {
pub chain_id: U256,
pub spec_id: SpecId,
/// should precompiles be marked as "called" or not, for gas calculation.
pub precompiles_called: bool,
/// If all precompiles have some balance we can skip initially fetching them from the database.
/// This is is not really needed on mainnet, and defaults to false, but in most cases it is
/// safe to be set to `true`, depending on the chain.
Expand All @@ -237,6 +252,7 @@ impl Default for CfgEnv {
CfgEnv {
chain_id: 1.into(),
spec_id: SpecId::LATEST,
precompiles_called: true,
perf_all_precompiles_have_balance: false,
#[cfg(feature = "memory_limit")]
memory_limit: 2u64.pow(32) - 1,
Expand Down
34 changes: 29 additions & 5 deletions crates/revm/src/subroutine.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{models::SelfDestructResult, Return, KECCAK_EMPTY};
use crate::{models::SelfDestructResult, PrecompState, Return, KECCAK_EMPTY};
use alloc::{vec, vec::Vec};
use core::mem::{self};
use hashbrown::{hash_map::Entry, HashMap as Map};
Expand Down Expand Up @@ -90,24 +90,30 @@ impl SubRoutine {
&mut self.state
}

pub fn load_precompiles(&mut self, precompiles: Map<H160, AccountInfo>) {
pub fn load_precompiles(&mut self, precompiles: Map<H160, AccountInfo>, called: bool) {
let state: Map<H160, Account> = precompiles
.into_iter()
.map(|(k, value)| {
let mut acc = Account::from(value);
acc.filth = Filth::Precompile(false);
if called {
acc.info.precomp_state = PrecompState::Called;
}
(k, acc)
})
.collect();
self.state.extend(state);
}

pub fn load_precompiles_default(&mut self, precompiles: &[H160]) {
pub fn load_precompiles_default(&mut self, precompiles: &[H160], called: bool) {
let state: State = precompiles
.iter()
.map(|&k| {
let mut acc = Account::from(AccountInfo::default());
acc.filth = Filth::Precompile(false);
if called {
acc.info.precomp_state = PrecompState::Called;
}
(k, acc)
})
.collect();
Expand All @@ -127,6 +133,13 @@ impl SubRoutine {
match acc.filth {
// acc was destroyed or if it is changed precompile, just add it to output.
Filth::Destroyed | Filth::Precompile(true) => {
if acc.filth.is_precompile() {
if acc.info.precomp_state == PrecompState::Pending {
acc.info.precomp_state = PrecompState::Called;
} else {
continue;
}
}
acc.info.code = None;
acc.info.code_hash = KECCAK_EMPTY;
out.insert(add, acc);
Expand Down Expand Up @@ -246,6 +259,7 @@ impl SubRoutine {
let to_is_cold = self.load_account(to, db);
// check from balance and substract value
let from = self.log_dirty(from, |_| {});

if from.info.balance < value {
return Err(Return::OutOfFund);
}
Expand Down Expand Up @@ -505,7 +519,17 @@ impl SubRoutine {
pub fn load_account_exist<DB: Database>(&mut self, address: H160, db: &mut DB) -> (bool, bool) {
let (acc, is_cold) = self.load_code(address, db);
let info = acc.info.clone();
let is_empty = info.is_empty() && !acc.filth.is_precompile();
let is_empty = if acc.filth.is_precompile() {
if acc.info.precomp_state == PrecompState::Called {
false
} else {
acc.filth.make_dirty();
acc.info.precomp_state = PrecompState::Pending;
true
}
} else {
info.is_empty()
};
(is_cold, !is_empty)
}

Expand Down Expand Up @@ -630,7 +654,7 @@ pub struct Account {

impl Account {
pub fn is_empty(&self) -> bool {
self.info.is_empty()
self.info.is_empty() && !self.filth.is_precompile()
}
}

Expand Down

0 comments on commit 9f9a7a2

Please sign in to comment.