fix or add some context for special cases

Author: randygrok

Cargo.lock

@@ -227,6 +227,10 @@ where
         let mut total_refunded: i64 = 0;
         let mut last_result: Option<FrameResult> = None;
 
+        // Execute each call in the batch sequentially.
+        // set_batch_call only modifies (kind, value, data) - the nonce is intentionally
+        // shared since a batch is a single atomic transaction with one nonce.
+        // Note: only the first call may be CREATE (enforced in validate_initial_tx_gas).
         for call in &calls {
             let mut call_tx = base_tx.clone();
             call_tx.set_batch_call(call);
@@ -239,6 +243,10 @@ where
 
             if !instruction_result.is_ok() {
                 evm.ctx_mut().journal_mut().checkpoint_revert(checkpoint);
+                // For CREATE batches: the checkpoint revert undoes the nonce increment that
+                // happened during CREATE execution. We must manually re-increment it here
+                // to match Ethereum's behavior where nonce always increments even on failure.
+                // For CALL batches: nonce was incremented before checkpoint, so revert preserves it.
                 if calls
                     .first()
                     .map(|call| call.to.is_create())
@@ -581,6 +589,10 @@ where
             );
         }
 
+        // Nonce handling for batches:
+        // - CALL batches: increment nonce here (standard pre-execution behavior)
+        // - CREATE batches: nonce is incremented during CREATE frame execution,
+        //   which also uses it for contract address derivation
         if is_call {
             caller.info.set_nonce(caller.info.nonce.saturating_add(1));
         }
@@ -600,6 +612,10 @@ where
         );
     }
 
+    // Note: We deduct effective_gas_price (not max_fee_per_gas) upfront.
+    // This is safe because effective_gas_price <= max_fee_per_gas by construction,
+    // and the check above ensures sponsor can cover the worst case (max_gas_cost).
+    // This approach is more gas-efficient than deducting max upfront and reimbursing.
     let effective_gas_price = tx.effective_gas_price(basefee);
     let gas_cost = U256::from(tx.gas_limit()).saturating_mul(U256::from(effective_gas_price));
     let mut new_sponsor_balance = sponsor_balance.saturating_sub(gas_cost);
@@ -1196,6 +1212,83 @@ mod tests {
         );
     }
 
+    /// Tests that sponsored transactions with max_fee_per_gas < base_fee are rejected.
+    ///
+    /// This validation happens in revm's `validate_env` (delegated via inner handler)
+    /// BEFORE our custom `validate_and_deduct_sponsored_tx` runs. This test serves
+    /// as a regression test to ensure EIP-1559 fee validation is not bypassed.
+    #[test]
+    fn sponsored_tx_rejects_when_max_fee_below_basefee() {
+        let caller = address!("0x0000000000000000000000000000000000000aaa");
+        let sponsor = address!("0x0000000000000000000000000000000000000bbb");
+
+        let mut state = State::builder()
+            .with_database(CacheDB::<EmptyDB>::default())
+            .with_bundle_update()
+            .build();
+
+        state.insert_account(
+            caller,
+            AccountInfo {
+                balance: U256::from(10_000_000_000u64),
+                nonce: 0,
+                code_hash: KECCAK_EMPTY,
+                code: None,
+            },
+        );
+
+        // Sponsor has plenty of balance - the rejection should be due to fee, not balance
+        state.insert_account(
+            sponsor,
+            AccountInfo {
+                balance: U256::from(100_000_000u64),
+                nonce: 0,
+                code_hash: KECCAK_EMPTY,
+                code: None,
+            },
+        );
+
+        let mut evm_env: EvmEnv<SpecId> = EvmEnv::default();
+        evm_env.cfg_env.chain_id = 1;
+        evm_env.cfg_env.spec = SpecId::CANCUN;
+        evm_env.block_env.basefee = 100; // basefee = 100
+        evm_env.block_env.gas_limit = 30_000_000;
+        evm_env.block_env.number = U256::from(1);
+
+        let mut evm = EvTxEvmFactory::default().create_evm(state, evm_env);
+
+        let calls = vec![Call {
+            to: TxKind::Call(address!("0x0000000000000000000000000000000000000ccc")),
+            value: U256::ZERO,
+            input: Bytes::new(),
+        }];
+
+        // max_fee_per_gas (50) < basefee (100)
+        let tx_env = TxEnv {
+            caller,
+            gas_limit: 100_000,
+            gas_price: 50,
+            gas_priority_fee: Some(1),
+            chain_id: Some(1),
+            tx_type: TransactionType::Eip1559.into(),
+            ..Default::default()
+        };
+
+        let tx = EvTxEnv::with_calls_and_sponsor(tx_env, calls, sponsor);
+        let result = evm.transact_raw(tx);
+
+        assert!(
+            result.is_err(),
+            "Transaction with max_fee < basefee should be rejected"
+        );
+
+        let err = result.unwrap_err();
+        assert!(
+            matches!(err, EVMError::Transaction(InvalidTransaction::GasPriceLessThanBasefee)),
+            "Expected GasPriceLessThanBasefee error, got: {err:?}"
+        );
+    }
+
     #[test]
     fn reject_deploy_for_non_allowlisted_caller() {
         let allowlisted = address!("0x00000000000000000000000000000000000000aa");

crates/ev-revm/src/handler.rs

@@ -89,6 +89,7 @@ reth-tasks.workspace = true
 reth-tracing.workspace = true
 tempfile.workspace = true
 hex = "0.4"
+alloy-rlp.workspace = true
 
 [lints]
 workspace = true

crates/node/Cargo.toml

@@ -398,13 +398,22 @@ impl<Client> EvTransactionValidator<Client> {
         self.validate_evnode_calls(tx)?;
 
         if let Some(signature) = tx.fee_payer_signature.as_ref() {
+            // Sponsored transaction: validate sponsor balance
             let executor = pooled.transaction().signer();
             let sponsor = tx.recover_sponsor(executor, signature).map_err(|_| {
                 InvalidPoolTransactionError::other(EvTxPoolError::InvalidSponsorSignature)
             })?;
 
             let gas_cost = U256::from(tx.max_fee_per_gas).saturating_mul(U256::from(tx.gas_limit));
             self.validate_sponsor_balance(state, sponsor, gas_cost)?;
+        } else {
+            // Non-sponsored EvNode transaction: executor pays gas, validate their balance
+            if sender_balance < *pooled.cost() {
+                return Err(InvalidPoolTransactionError::Overdraft {
+                    cost: *pooled.cost(),
+                    balance: sender_balance,
+                });
+            }
         }
 
         Ok(())
@@ -502,6 +511,10 @@ where
             .set_tx_fee_cap(ctx.config().rpc.rpc_tx_fee_cap)
             .with_max_tx_gas_limit(ctx.config().txpool.max_tx_gas_limit)
             .with_minimum_priority_fee(ctx.config().txpool.minimum_priority_fee)
+            // Disable the standard caller balance check - we handle balance validation
+            // in EvTransactionValidator::validate_evnode which checks:
+            // - Sponsor balance for sponsored EvNode transactions
+            // - Sender balance for non-sponsored EvNode and standard Ethereum transactions
             .disable_balance_check()
             .with_additional_tasks(ctx.config().txpool.additional_validation_tasks)
             .build_with_tasks::<EvPooledTransaction, _, _>(
@@ -528,3 +541,124 @@ where
         Ok(transaction_pool)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use alloy_consensus::Signed;
+    use alloy_eips::eip2930::AccessList;
+    use alloy_primitives::{Bytes, Signature, TxKind};
+    use ev_primitives::{Call, EvNodeSignedTx, EvNodeTransaction};
+    use reth_provider::test_utils::MockEthProvider;
+
+    fn sample_signature() -> Signature {
+        let mut bytes = [0u8; 65];
+        bytes[64] = 27;
+        Signature::from_raw_array(&bytes).expect("valid test signature")
+    }
+
+    /// Creates a non-sponsored EvNode transaction (fee_payer_signature = None)
+    fn create_non_sponsored_evnode_tx(gas_limit: u64, max_fee_per_gas: u128) -> EvNodeSignedTx {
+        let tx = EvNodeTransaction {
+            chain_id: 1,
+            nonce: 0,
+            max_priority_fee_per_gas: 1,
+            max_fee_per_gas,
+            gas_limit,
+            calls: vec![Call {
+                to: TxKind::Call(Address::ZERO),
+                value: U256::ZERO,
+                input: Bytes::new(),
+            }],
+            access_list: AccessList::default(),
+            fee_payer_signature: None, // Non-sponsored
+        };
+        Signed::new_unhashed(tx, sample_signature())
+    }
+
+    fn create_pooled_tx(signed_tx: EvNodeSignedTx, signer: Address) -> EvPooledTransaction {
+        let envelope = EvTxEnvelope::EvNode(signed_tx);
+        let recovered = alloy_consensus::transaction::Recovered::new_unchecked(envelope, signer);
+        let encoded_length = 200; // Approximate length for test
+        EvPooledTransaction::new(recovered, encoded_length)
+    }
+
+    fn create_test_validator() -> EvTransactionValidator<MockEthProvider> {
+        use reth_transaction_pool::{
+            blobstore::InMemoryBlobStore, validate::EthTransactionValidatorBuilder,
+        };
+
+        let provider = MockEthProvider::default();
+        let blob_store = InMemoryBlobStore::default();
+        let inner = EthTransactionValidatorBuilder::new(provider)
+            .no_shanghai()
+            .no_cancun()
+            .build(blob_store);
+        EvTransactionValidator::new(inner)
+    }
+
+    /// Tests that non-sponsored EvNode transactions with insufficient sender balance
+    /// are rejected with an Overdraft error.
+    ///
+    /// BUG: Currently this test FAILS because validate_evnode does not check
+    /// sender balance for non-sponsored EvNode transactions.
+    #[test]
+    fn non_sponsored_evnode_rejects_insufficient_balance() {
+        let validator = create_test_validator();
+
+        // Create a non-sponsored EvNode transaction
+        let gas_limit = 21_000u64;
+        let max_fee_per_gas = 1_000_000_000u128; // 1 gwei
+        let signed_tx = create_non_sponsored_evnode_tx(gas_limit, max_fee_per_gas);
+
+        let signer = Address::random();
+        let pooled = create_pooled_tx(signed_tx, signer);
+
+        // Sender has ZERO balance - clearly insufficient
+        let sender_balance = U256::ZERO;
+        let mut state: Option<Box<dyn AccountInfoReader>> = None;
+
+        // Call validate_evnode - should return Overdraft error
+        let result = validator.validate_evnode(&pooled, sender_balance, &mut state);
+
+        assert!(
+            result.is_err(),
+            "Non-sponsored EvNode with zero balance should be rejected, but got Ok(())"
+        );
+
+        if let Err(err) = result {
+            assert!(
+                matches!(err, InvalidPoolTransactionError::Overdraft { .. }),
+                "Expected Overdraft error, got: {:?}",
+                err
+            );
+        }
+    }
+
+    /// Tests that non-sponsored EvNode transactions with sufficient balance are accepted.
+    #[test]
+    fn non_sponsored_evnode_accepts_sufficient_balance() {
+        let validator = create_test_validator();
+
+        let gas_limit = 21_000u64;
+        let max_fee_per_gas = 1_000_000_000u128;
+        let signed_tx = create_non_sponsored_evnode_tx(gas_limit, max_fee_per_gas);
+
+        let signer = Address::random();
+        let pooled = create_pooled_tx(signed_tx, signer);
+
+        let tx_cost = *pooled.cost();
+
+        // Sender has MORE than enough balance
+        let sender_balance = tx_cost + U256::from(1);
+        let mut state: Option<Box<dyn AccountInfoReader>> = None;
+
+        let result = validator.validate_evnode(&pooled, sender_balance, &mut state);
+
+        assert!(
+            result.is_ok(),
+            "Non-sponsored EvNode with sufficient balance should be accepted, got: {:?}",
+            result
+        );
+    }
+}