Revert "Skip invalid txs due to nonce, intrinsic gas and insufficient…

… funds (#115)"

This reverts commit 7aef0f5.

bus-mapping/src/circuit_input_builder.rs

@@ -153,7 +153,6 @@ impl<'a> CircuitInputBuilder {
         &mut self,
         eth_tx: &eth_types::Transaction,
         is_success: bool,
-        is_invalid: bool,
     ) -> Result<Transaction, Error> {
         let call_id = self.block_ctx.rwc.0;
 
@@ -168,14 +167,7 @@ impl<'a> CircuitInputBuilder {
             ),
         );
 
-        Transaction::new(
-            call_id,
-            &self.sdb,
-            &mut self.code_db,
-            eth_tx,
-            is_success,
-            is_invalid,
-        )
+        Transaction::new(call_id, &self.sdb, &mut self.code_db, eth_tx, is_success)
     }
 
     /// Iterate over all generated CallContext RwCounterEndOfReversion
@@ -292,8 +284,9 @@ impl<'a> CircuitInputBuilder {
         is_anchor_tx: bool,
         is_last_tx: bool,
     ) -> Result<(), Error> {
-        let mut tx = self.new_tx(eth_tx, !geth_trace.failed, geth_trace.invalid)?;
+        let mut tx = self.new_tx(eth_tx, !geth_trace.failed)?;
         let mut tx_ctx = TransactionContext::new(eth_tx, geth_trace, is_anchor_tx, is_last_tx)?;
+
         // Generate BeginTx step
         let begin_tx_step = gen_associated_steps(
             &mut self.state_ref(&mut tx, &mut tx_ctx),

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -472,7 +472,6 @@ impl<'a> CircuitInputStateRef<'a> {
         receiver: Address,
         receiver_exists: bool,
         must_create: bool,
-        must_read_caller_balance: bool,
         value: Word,
         fee: Option<Word>,
         is_anchor_tx: bool,
@@ -535,25 +534,21 @@ impl<'a> CircuitInputStateRef<'a> {
                 },
             )?;
         }
-
-        // Read the caller balance when required, skip if value == 0 otherwise
-        if must_read_caller_balance || !value.is_zero() {
-            self.push_op_reversible(
-                step,
-                AccountOp {
-                    address: sender,
-                    field: AccountField::Balance,
-                    value: sender_balance,
-                    value_prev: sender_balance_prev,
-                },
-            )?;
-        }
-
         if value.is_zero() {
             // Skip transfer if value == 0
             return Ok(());
         }
 
+        self.push_op_reversible(
+            step,
+            AccountOp {
+                address: sender,
+                field: AccountField::Balance,
+                value: sender_balance,
+                value_prev: sender_balance_prev,
+            },
+        )?;
+
         let (_found, receiver_account) = self.sdb.get_account(&receiver);
         let receiver_balance_prev = receiver_account.balance;
         let receiver_balance = receiver_account.balance + value;
@@ -586,7 +581,6 @@ impl<'a> CircuitInputStateRef<'a> {
             receiver,
             receiver_exists,
             must_create,
-            false,
             value,
             None,
             false,

bus-mapping/src/circuit_input_builder/tracer_tests.rs

@@ -37,14 +37,13 @@ impl CircuitInputBuilderTx {
         let block = crate::mock::BlockData::new_from_geth_data(geth_data.clone());
         let mut builder = block.new_circuit_input_builder();
         let tx = builder
-            .new_tx(&block.eth_block.transactions[0], true, false)
+            .new_tx(&block.eth_block.transactions[0], true)
             .unwrap();
         let tx_ctx = TransactionContext::new(
             &block.eth_block.transactions[0],
             &GethExecTrace {
                 gas: Gas(0),
                 failed: false,
-                invalid: false,
                 return_value: "".to_owned(),
                 struct_logs: vec![geth_step.clone()],
             },

bus-mapping/src/circuit_input_builder/transaction.rs

@@ -189,10 +189,6 @@ impl TransactionContext {
 pub struct Transaction {
     /// The raw transaction fields
     pub tx: geth_types::Transaction,
-    /// Invalid tx
-    pub invalid_tx: bool,
-    /// AccessListGasCost
-    pub access_list_gas_cost: u64,
     /// Calls made in the transaction
     pub(crate) calls: Vec<Call>,
     /// Execution steps
@@ -207,7 +203,6 @@ impl Transaction {
         code_db: &mut CodeDB,
         eth_tx: &eth_types::Transaction,
         is_success: bool,
-        is_invalid: bool,
     ) -> Result<Self, Error> {
         let (found, _) = sdb.get_account(&eth_tx.from);
         if !found {
@@ -258,8 +253,6 @@ impl Transaction {
 
         Ok(Self {
             tx: eth_tx.into(),
-            invalid_tx: is_invalid,
-            access_list_gas_cost: 0,
             calls: vec![call],
             steps: Vec::new(),
         })

bus-mapping/src/evm/opcodes/begin_end_tx.rs

@@ -48,19 +48,14 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
         state.call_context_write(&mut exec_step, call.call_id, field, value);
     }
 
-    // Increase caller's nonce when the tx is not invalid
+    // Increase caller's nonce
     let caller_address = call.caller_address;
     let nonce_prev = state.sdb.get_account(&caller_address).1.nonce;
-    let nonce = if !state.tx.invalid_tx {
-        nonce_prev + 1
-    } else {
-        nonce_prev
-    };
     state.account_write(
         &mut exec_step,
         caller_address,
         AccountField::Nonce,
-        nonce.into(),
+        (nonce_prev + 1).into(),
         nonce_prev.into(),
     )?;
 
@@ -81,20 +76,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
     } else {
         GasCost::TX.as_u64()
     } + state.tx.tx.call_data_gas_cost();
-
-    // Don't pay any fee or transfer any ETH for invalid transactions
-    let (gas_cost, value, fee) = if state.tx.invalid_tx {
-        (0, Word::zero(), Some(Word::zero()))
-    } else {
-        (
-            intrinsic_gas_cost,
-            call.value,
-            Some(state.tx.tx.gas_price * state.tx.gas()),
-        )
-    };
-
-    // Set the gas cost
-    exec_step.gas_cost = GasCost(gas_cost);
+    exec_step.gas_cost = GasCost(intrinsic_gas_cost);
 
     // Get code_hash of callee
     let (_, callee_account) = state.sdb.get_account(&call.address);
@@ -122,10 +104,9 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
         call.caller_address,
         call.address,
         callee_exists,
-        !state.tx.invalid_tx && call.is_create(),
-        true,
-        value,
-        fee,
+        call.is_create(),
+        call.value,
+        Some(state.tx.tx.gas_price * state.tx.gas()),
         state.tx_ctx.is_anchor_tx(),
     )?;
 
@@ -151,7 +132,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
     match (
         call.is_create(),
         state.is_precompiled(&call.address),
-        is_empty_code_hash || state.tx.invalid_tx,
+        is_empty_code_hash,
     ) {
         // 1. Creation transaction.
         (true, _, _) => {
@@ -200,13 +181,13 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
             evm_unimplemented!("Call to precompiled is left unimplemented");
             Ok(exec_step)
         }
-        (_, _, do_not_run_code) => {
+        (_, _, is_empty_code_hash) => {
             // 3. Call to account with empty code.
-            if do_not_run_code {
+            if is_empty_code_hash {
                 return Ok(exec_step);
             }
 
-            // 4. Call to account with non-empty code/invalid tx.
+            // 4. Call to account with non-empty code.
             for (field, value) in [
                 (CallContextField::Depth, call.depth.into()),
                 (

bus-mapping/src/lib.rs

@@ -141,7 +141,6 @@
 //!     return_value: "".to_string(),
 //!     gas: Gas(block.eth_block.transactions[0].gas.as_u64()),
 //!     failed: false,
-//!     invalid: false,
 //!     struct_logs: geth_steps,
 //! };
 //!

eth-types/src/evm_types.rs

@@ -144,10 +144,6 @@ impl GasCost {
     pub const CALL_WITH_VALUE: Self = Self(9000);
     /// Constant cost for turning empty account into non-empty account
     pub const NEW_ACCOUNT: Self = Self(25000);
-    /// Gas cost of warming up an account with the access list
-    pub const ACCESS_LIST_ADDRESS: Self = Self(2400);
-    /// Gas cost of warming up a storage with the access list
-    pub const ACCESS_LIST_STORAGE: Self = Self(1900);
     /// Cost per byte of deploying a new contract
     pub const CODE_DEPOSIT_BYTE_COST: Self = Self(200);
     /// Denominator of quadratic part of memory expansion gas cost

eth-types/src/lib.rs

@@ -446,9 +446,6 @@ pub struct GethExecTrace {
     pub gas: Gas,
     /// True when the transaction has failed.
     pub failed: bool,
-    /// True when the tx could not execute
-    #[serde(default)]
-    pub invalid: bool,
     /// Return value of execution which is a hex encoded byte array
     #[serde(rename = "returnValue")]
     pub return_value: String,
@@ -562,7 +559,6 @@ mod tests {
         assert_eq!(
             trace,
             GethExecTrace {
-                invalid: false,
                 gas: Gas(26809),
                 failed: false,
                 return_value: "".to_owned(),

external-tracer/src/lib.rs

@@ -71,20 +71,6 @@ pub fn trace(config: &TraceConfig) -> Result<Vec<GethExecTrace>, Error> {
         },
     )?;
 
-    let trace: Vec<GethExecTrace> =
-        serde_json::from_str(&trace_string).map_err(Error::SerdeError)?;
-    // Don't throw only for specific invalid transactions we support.
-    for trace in trace.iter() {
-        if trace.invalid
-            && !(trace.return_value.starts_with("nonce too low")
-                || trace.return_value.starts_with("nonce too high")
-                || trace.return_value.starts_with("intrinsic gas too low")
-                || trace
-                    .return_value
-                    .starts_with("insufficient funds for gas * price + value"))
-        {
-            return Err(Error::TracingError(trace.return_value.clone()));
-        }
-    }
+    let trace = serde_json::from_str(&trace_string).map_err(Error::SerdeError)?;
     Ok(trace)
 }

geth-utils/Cargo.toml

@@ -7,9 +7,4 @@ license = "MIT OR Apache-2.0"
 [build-dependencies]
 gobuild = "0.1.0-alpha.1"
 log = "0.4.14"
-env_logger = "0.9"
-
-[dev-dependencies]
-eth-types = { path = "../eth-types" }
-serde = {version = "1.0.130", features = ["derive"] }
-serde_json = "1.0.66"
+env_logger = "0.9"

geth-utils/gethutil/trace.go

@@ -20,7 +20,6 @@ import (
 // while replaying a transaction in debug mode as well as transaction
 // execution status, the amount of gas used and the return value
 type ExecutionResult struct {
-	Invalid     bool           `json:"invalid"`
 	Gas         uint64         `json:"gas"`
 	Failed      bool           `json:"failed"`
 	ReturnValue string         `json:"returnValue"`
@@ -222,23 +221,15 @@ func Trace(config TraceConfig) ([]*ExecutionResult, error) {
 
 		result, err := core.ApplyMessage(evm, &message, new(core.GasPool).AddGas(message.GasLimit))
 		if err != nil {
-			executionResults[i] = &ExecutionResult{
-				Invalid:     true,
-				Gas:         0,
-				Failed:      false,
-				ReturnValue: fmt.Sprintf("%v", err),
-				StructLogs:  []StructLogRes{},
-			}
-		} else {
-			stateDB.Finalise(true)
-
-			executionResults[i] = &ExecutionResult{
-				Invalid:     false,
-				Gas:         result.UsedGas,
-				Failed:      result.Failed(),
-				ReturnValue: fmt.Sprintf("%x", result.ReturnData),
-				StructLogs:  FormatLogs(tracer.StructLogs()),
-			}
+			return nil, fmt.Errorf("Failed to apply config.Transactions[%d]: %w", i, err)
+		}
+		stateDB.Finalise(true)
+
+		executionResults[i] = &ExecutionResult{
+			Gas:         result.UsedGas,
+			Failed:      result.Failed(),
+			ReturnValue: fmt.Sprintf("%x", result.ReturnData),
+			StructLogs:  FormatLogs(tracer.StructLogs()),
 		}
 	}
 

geth-utils/src/lib.rs

@@ -53,7 +53,6 @@ impl Display for Error {
 #[cfg(test)]
 mod test {
     use crate::trace;
-    use eth_types::{Error, GethExecTrace};
 
     #[test]
     fn valid_tx() {
@@ -103,15 +102,7 @@ mod test {
                 ]
             }"#,
         ] {
-            let trace_result = trace(config);
-            assert!(trace_result.is_ok());
-            let trace_string = trace_result.unwrap();
-            let trace: Vec<GethExecTrace> = serde_json::from_str(&trace_string)
-                .map_err(Error::SerdeError)
-                .unwrap();
-            for trace in trace.iter() {
-                assert!(!trace.invalid);
-            }
+            assert!(trace(config).is_ok());
         }
     }
 
@@ -159,15 +150,7 @@ mod test {
                 ]
             }"#,
         ] {
-            let trace_result = trace(config);
-            assert!(trace_result.is_ok());
-            let trace_string = trace_result.unwrap();
-            let trace: Vec<GethExecTrace> = serde_json::from_str(&trace_string)
-                .map_err(Error::SerdeError)
-                .unwrap();
-            for trace in trace.iter() {
-                assert!(trace.invalid);
-            }
+            assert!(trace(config).is_err())
         }
     }
 }