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

Split gas info into and fill in correct ether address value #142

Merged
merged 4 commits into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 1 addition & 27 deletions bus-mapping/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ pub(crate) mod opcodes;
pub mod stack;
pub mod storage;

use crate::{
error::{EthAddressParsingError, EvmWordParsingError},
Gas,
};
use crate::error::{EthAddressParsingError, EvmWordParsingError};
use core::str::FromStr;
use serde::{Deserialize, Serialize};
use std::fmt;
Expand Down Expand Up @@ -263,29 +260,6 @@ impl EthAddress {
/// Defines the gas consumed by an
/// [`ExecutionStep`](crate::exec_trace::ExecutionStep) as well as the gas left
/// to operate.
#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord)]
pub struct GasInfo {
pub(crate) gas: Gas,
pub(crate) gas_cost: GasCost,
}

impl GasInfo {
/// Generates a new `GasInfo` instance from it's fields.
pub fn new(gas: Gas, gas_cost: GasCost) -> GasInfo {
GasInfo { gas, gas_cost }
}

/// Returns the gas left marked by a GasInfo instance.
pub fn gas(&self) -> Gas {
self.gas
}

/// Returns the gas consumed by an [`OpcodeId`] execution.
pub fn gas_cost(&self) -> GasCost {
self.gas_cost
}
}

/// Gas Cost structure which is integrated inside [`GasInfo`].
#[derive(
Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize,
Expand Down
11 changes: 5 additions & 6 deletions bus-mapping/src/evm/opcodes/mload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ mod mload_tests {
use super::*;
use crate::{
bytecode,
evm::{
EvmWord, GasCost, GasInfo, OpcodeId, Stack, StackAddress, Storage,
},
evm::{EvmWord, GasCost, OpcodeId, Stack, StackAddress, Storage},
external_tracer, BlockConstants, ExecutionTrace,
};
use pasta_curves::pallas::Scalar;
Expand Down Expand Up @@ -105,7 +103,6 @@ mod mload_tests {

// Start from the same pc and gas limit
let mut pc = obtained_steps[0].pc();
let mut gas = obtained_steps[0].gas_info().gas;

// The memory is the same in both steps as none of them edits the
// memory of the EVM.
Expand All @@ -117,7 +114,8 @@ mod mload_tests {
stack: Stack(vec![EvmWord::from(0x40u8)]),
storage: Storage::empty(),
instruction: OpcodeId::MLOAD,
gas_info: gas_info!(gas, FASTEST),
gas: obtained_steps[0].gas(),
gas_cost: GasCost::FASTEST,
depth: 1u8,
pc: pc.inc_pre(),
gc: ctx.gc,
Expand Down Expand Up @@ -163,7 +161,8 @@ mod mload_tests {
stack: Stack(vec![EvmWord::from(0x80u8)]),
storage: Storage::empty(),
instruction: OpcodeId::STOP,
gas_info: gas_info!(gas, ZERO),
gas: obtained_steps[1].gas(),
gas_cost: GasCost::ZERO,
depth: 1u8,
pc: pc.inc_pre(),
gc: ctx.gc,
Expand Down
9 changes: 4 additions & 5 deletions bus-mapping/src/evm/opcodes/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ mod push_tests {
use super::*;
use crate::{
bytecode,
evm::{
EvmWord, GasCost, GasInfo, OpcodeId, Stack, StackAddress, Storage,
},
evm::{EvmWord, GasCost, OpcodeId, Stack, StackAddress, Storage},
external_tracer, BlockConstants, ExecutionTrace,
};
use pasta_curves::pallas::Scalar;
Expand Down Expand Up @@ -78,7 +76,7 @@ mod push_tests {

// Start from the same pc and gas limit
let mut pc = obtained_steps[0].pc();
let mut gas = obtained_steps[0].gas_info().gas;
let gas = obtained_steps[0].gas();

// The memory is the same in both steps as none of them edits the
// memory of the EVM.
Expand All @@ -90,7 +88,8 @@ mod push_tests {
stack: Stack::empty(),
storage: Storage::empty(),
instruction: OpcodeId::PUSH1,
gas_info: gas_info!(gas, FASTEST),
gas,
gas_cost: GasCost::FASTEST,
depth: 1u8,
pc: pc.inc_pre(),
gc: ctx.gc,
Expand Down
18 changes: 12 additions & 6 deletions bus-mapping/src/evm/opcodes/sload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ impl Opcode for Sload {
exec_step,
StorageOp::new(
RW::READ,
EthAddress([0u8; 20]), // TODO: Fill with the correct value
EthAddress([
154, 12, 99, 235, 183, 139, 53, 215, 194, 9, 175, 189, 41,
155, 5, 96, 152, 181, 67, 155,
]),
Copy link
Member

Choose a reason for hiding this comment

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

Where does this address come from exactly? Not sure about the origin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I referred the address from test in evm.

Copy link
Member

Choose a reason for hiding this comment

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

kk. Makes sense.
Don't know what @ed255 will want to do with that neither. Since this actually depends on the exact Geth instance that you set up.

So maybe we just want to have al 0's since this addr kinda comes from nowhere. Let's wait on the input and see :)

Copy link
Member

Choose a reason for hiding this comment

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

This field is supposed to contain the Ethereum Address for which the storage access is being made. We don't have yet that information in the current implementation (it will be introduced in the TraceContext). So for now, this field should be left with a 0 value, and with the TODO note. @noctrlz could you revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review.
I reverted that Ethererum address commit.

stack_value_read,
storage_value_read,
storage_value_read,
Expand All @@ -64,8 +67,7 @@ mod sload_tests {
use crate::{
bytecode,
evm::{
EvmWord, GasCost, GasInfo, Memory, OpcodeId, Stack, StackAddress,
Storage,
EvmWord, GasCost, Memory, OpcodeId, Stack, StackAddress, Storage,
},
external_tracer, BlockConstants, ExecutionTrace,
};
Expand Down Expand Up @@ -104,7 +106,7 @@ mod sload_tests {

// Start from the same pc and gas limit
let mut pc = obtained_steps[0].pc();
let mut gas = obtained_steps[0].gas_info().gas;
let gas = obtained_steps[0].gas();

// Generate Step1 corresponding to SLOAD
let mut step_1 = ExecutionStep {
Expand All @@ -115,7 +117,8 @@ mod sload_tests {
EvmWord::from(0x6fu32),
)])),
instruction: OpcodeId::SLOAD,
gas_info: gas_info!(gas, WARM_STORAGE_READ_COST),
gas,
gas_cost: GasCost::WARM_STORAGE_READ_COST,
depth: 1u8,
pc: pc.inc_pre(),
gc: ctx.gc,
Expand All @@ -136,7 +139,10 @@ mod sload_tests {
&mut step_1,
StorageOp::new(
RW::READ,
EthAddress([0u8; 20]), // TODO: Fill with the correct value
EthAddress([
154, 12, 99, 235, 183, 139, 53, 215, 194, 9, 175, 189, 41,
155, 5, 96, 152, 181, 67, 155,
]),
EvmWord::from(0x0u32),
EvmWord::from(0x6fu32),
EvmWord::from(0x6fu32),
Expand Down
24 changes: 16 additions & 8 deletions bus-mapping/src/exec_trace/exec_step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

use super::OperationRef;
use crate::evm::{
EvmWord, GasInfo, GlobalCounter, Memory, ProgramCounter, Stack, Storage,
EvmWord, GasCost, GlobalCounter, Memory, ProgramCounter, Stack, Storage,
};
use crate::Gas;
use crate::{
error::Error,
evm::{opcodes::Opcode, OpcodeId},
Expand All @@ -26,8 +27,8 @@ pub struct ExecutionStep {
pub(crate) stack: Stack,
pub(crate) storage: Storage,
pub(crate) instruction: OpcodeId,
// TODO: Split into gas, gas_cost
pub(crate) gas_info: GasInfo,
pub(crate) gas: Gas,
pub(crate) gas_cost: GasCost,
pub(crate) depth: u8,
pub(crate) pc: ProgramCounter,
pub(crate) gc: GlobalCounter,
Expand All @@ -44,7 +45,8 @@ impl ExecutionStep {
stack: Vec<EvmWord>,
storage: HashMap<EvmWord, EvmWord>,
instruction: OpcodeId,
gas_info: GasInfo,
gas: Gas,
gas_cost: GasCost,
depth: u8,
pc: ProgramCounter,
gc: GlobalCounter,
Expand All @@ -54,7 +56,8 @@ impl ExecutionStep {
stack: Stack::from_vec(stack),
storage: Storage::from(storage),
instruction,
gas_info,
gas,
gas_cost,
depth,
pc,
gc,
Expand Down Expand Up @@ -82,9 +85,14 @@ impl ExecutionStep {
&self.instruction
}

/// Returns the [`GasInfo`] of this step.
pub const fn gas_info(&self) -> &GasInfo {
&self.gas_info
/// Returns the [`Gas`] of this step.
pub const fn gas(&self) -> Gas {
self.gas
}

/// Returns the [`GasCost`] of this step.
pub const fn gas_cost(&self) -> GasCost {
self.gas_cost
}

/// Returns the call-depth we're operating at this step.
Expand Down
8 changes: 5 additions & 3 deletions bus-mapping/src/exec_trace/parsing.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Doc this

use crate::evm::{EvmWord, GasCost, GasInfo, ProgramCounter};
use crate::evm::{EvmWord, GasCost, ProgramCounter};
use crate::ExecutionStep;
use crate::Gas;
use crate::{
Expand Down Expand Up @@ -54,7 +54,8 @@ impl<'a> TryFrom<&ParsedExecutionStep<'a>> for ExecutionStep {
storage,
// Avoid setting values now. This will be done at the end.
OpcodeId::from_str(parsed_step.op)?,
GasInfo::new(parsed_step.gas, parsed_step.gas_cost),
parsed_step.gas,
parsed_step.gas_cost,
parsed_step.depth,
parsed_step.pc,
0.into(),
Expand Down Expand Up @@ -127,7 +128,8 @@ mod tests {
stack: Stack(vec![EvmWord::from(0x40u8)]),
storage: Storage::empty(),
instruction: OpcodeId::JUMPDEST,
gas_info: GasInfo::new(82, GasCost::from(3u8)),
gas: 82,
gas_cost: GasCost::from(3u8),
depth: 1,
pc: ProgramCounter(5),
gc: GlobalCounter(0),
Expand Down
3 changes: 0 additions & 3 deletions bus-mapping/src/macros.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
//! Collection of utility macros used within this crate.

#[macro_use]
pub(crate) mod evm;

macro_rules! impl_from_evm_word_wrappers {
($($implementor:ty),*) => {
$(impl From<$implementor> for EvmWord {
Expand Down
18 changes: 0 additions & 18 deletions bus-mapping/src/macros/evm.rs

This file was deleted.