test: add tests for bundle guarantees (#112)

* test: add tests for bundle guarantees

* fix: bundle driver rejection works more correctly

* refactor: distinguish between sender and orderer

* lint: clippy

Author: prestwich

crates/bundle/src/lib.rs

@@ -51,6 +51,6 @@ pub use call::{SignetBundleDriver, SignetCallBundle, SignetCallBundleResponse};
 
 mod send;
 pub use send::{
-    SignetEthBundle, SignetEthBundleDriver, SignetEthBundleError, SignetEthBundleInsp,
-    SignetEthBundleResponse,
+    BundleInspector, SignetEthBundle, SignetEthBundleDriver, SignetEthBundleError,
+    SignetEthBundleInsp, SignetEthBundleResponse,
 };

crates/bundle/src/send/bundle.rs

@@ -12,7 +12,14 @@ use alloy::{
 use serde::{Deserialize, Serialize};
 use signet_types::{SignedFill, SignedPermitError};
 use signet_zenith::HostOrders::HostOrdersInstance;
-use trevm::{revm::Database, BundleError};
+use trevm::{
+    inspectors::{Layered, TimeLimit},
+    revm::{inspector::NoOpInspector, Database},
+    BundleError,
+};
+
+/// The inspector type required by the Signet bundle driver.
+pub type BundleInspector<I = NoOpInspector> = Layered<TimeLimit, I>;
 
 /// Bundle of transactions for `signet_sendBundle`.
 ///

crates/bundle/src/send/driver.rs

@@ -1,8 +1,10 @@
 use crate::send::SignetEthBundle;
 use alloy::primitives::U256;
-use signet_evm::{DriveBundleResult, EvmNeedsTx, EvmTransacted, SignetInspector, SignetLayered};
+use signet_evm::{
+    DriveBundleResult, EvmErrored, EvmNeedsTx, EvmTransacted, SignetInspector, SignetLayered,
+};
 use signet_types::{AggregateFills, MarketError, SignedPermitError};
-use tracing::debug;
+use tracing::{debug, error};
 use trevm::{
     helpers::Ctx,
     inspectors::{Layered, TimeLimit},
@@ -143,9 +145,7 @@ impl<'a> SignetEthBundleDriver<'a> {
 
         // We check the AggregateFills here, and if it fails, we discard the
         // transaction outcome and push a failure receipt.
-        self.agg_fills.checked_remove_ru_tx_events(&agg_orders, &agg_fills).inspect_err(|err| {
-            debug!(%err, "Discarding transaction outcome due to market error");
-        })
+        self.agg_fills.checked_remove_ru_tx_events(&agg_orders, &agg_fills)
     }
 }
 
@@ -200,6 +200,8 @@ where
         let txs = trevm_try!(self.bundle.decode_and_validate_txs(), trevm);
 
         for tx in txs.into_iter() {
+            let _span = tracing::debug_span!("bundle_tx_loop", tx_hash = %tx.hash()).entered();
+
             // Update the inner deadline.
             let limit = trevm.inner_mut_unchecked().ctx_inspector().1.outer_mut().outer_mut();
             *limit = TimeLimit::new(self.deadline - std::time::Instant::now());
@@ -209,7 +211,10 @@ where
             // Temporary rebinding of trevm within each loop iteration.
             // The type of t is `EvmTransacted`, while the type of trevm is
             // `EvmNeedsTx`.
-            let mut t = trevm.run_tx(&tx).map_err(|err| err.err_into())?;
+            let mut t = trevm
+                .run_tx(&tx)
+                .map_err(EvmErrored::err_into)
+                .inspect_err(|err| error!(err = %err.error(), "error while running transaction"))?;
 
             // Check the result of the transaction.
             let result = t.result();
@@ -221,10 +226,14 @@ where
             // not, and the tx is not marked as revertible by the bundle, we
             // error our simulation.
             if result.is_success() {
-                if self.check_fills(&mut t).is_err()
-                    && !self.bundle.reverting_tx_hashes().contains(tx_hash)
-                {
-                    return Err(t.errored(BundleError::BundleReverted.into()));
+                if self.check_fills(&mut t).is_err() {
+                    debug!("transaction dropped due to insufficient fills");
+                    if self.bundle.reverting_tx_hashes().contains(tx_hash) {
+                        trevm = t.reject();
+                        continue;
+                    } else {
+                        return Err(t.errored(BundleError::BundleReverted.into()));
+                    }
                 }
 
                 self.total_gas_used = self.total_gas_used.saturating_add(gas_used);
@@ -233,13 +242,14 @@ where
                 // not marked as revertible by the bundle, we error our
                 // simulation.
                 if !self.bundle.reverting_tx_hashes().contains(tx_hash) {
+                    debug!("transaction reverted, not marked as revertible");
                     return Err(t.errored(BundleError::BundleReverted.into()));
                 }
                 self.total_gas_used = self.total_gas_used.saturating_add(gas_used);
             }
 
-            // If we did not shortcut return, we accept the state changes
-            // from this transaction.
+            // If we did not shortcut return/continue, we accept the state
+            // changes from this transaction.
             trevm = t.accept_state()
         }
 

crates/bundle/src/send/mod.rs

@@ -1,5 +1,5 @@
 mod bundle;
-pub use bundle::{SignetEthBundle, SignetEthBundleResponse};
+pub use bundle::{BundleInspector, SignetEthBundle, SignetEthBundleResponse};
 
 mod driver;
 pub use driver::{SignetEthBundleDriver, SignetEthBundleError, SignetEthBundleInsp};

crates/test-utils/src/contracts/counter.rs

@@ -1,5 +1,5 @@
 use alloy::{
-    primitives::{b256, bytes, Address, Bytes, B256},
+    primitives::{b256, bytes, Address, Bytes, B256, U256},
     providers::Provider,
 };
 
@@ -14,6 +14,9 @@ alloy::sol! {
    }
 }
 
+/// The storage slot where the counter value is stored in the Counter contract.
+pub const COUNTER_SLOT: U256 = U256::ZERO;
+
 /// A test address for the Counter.sol contract, which will be pre-deployed in
 /// test EVMs.
 pub const COUNTER_TEST_ADDRESS: Address = Address::repeat_byte(0x49);

crates/test-utils/src/contracts/mod.rs

@@ -1,5 +1,11 @@
+/// A Counter contract that can be incremented.
 pub mod counter;
 
+/// Orders, Passage, and Token system contracts.
 pub mod system;
 
+/// A token contract, deployed for Wbtc and Weth.
 pub mod token;
+
+/// A contract that always reverts.
+pub mod reverts;

crates/test-utils/src/contracts/reverts.rs

@@ -0,0 +1,21 @@
+use crate::contracts::counter::Counter;
+use alloy::{
+    primitives::{bytes, Address, Bytes},
+    providers::Provider,
+};
+
+/// A test address for the Revert contract, which will be pre-deployed in
+/// test EVMs.
+pub const REVERT_TEST_ADDRESS: Address = Address::repeat_byte(0x50);
+
+/// Revert deploycode for testing purposes.
+pub const REVERT_DEPLOY_CODE: Bytes = bytes!("0x608060405234601957600e601d565b60c76028823960c790f35b6023565b60405190565b5f80fdfe608060405234156073576013565b60405190565b5f80fd5b60209181520190565b5f7f5265766572742063616c6c656400000000000000000000000000000000000000910152565b6052600d6020926017565b6059816020565b0190565b60709060208101905f8183039101526047565b90565b6079600d565b62461bcd60e51b815280608d60048201605d565b0390fdfea2646970667358221220ca26ecb1412cc6e87fe99712d1085a242349863e03e3e43ef6ee7dde7a3478cc64736f6c634300081a0033");
+
+/// Revert bytecode for testing purposes.
+pub const REVERT_BYTECODE: Bytes = bytes!(
+  "0x608060405234156073576013565b60405190565b5f80fd5b60209181520190565b5f7f5265766572742063616c6c656400000000000000000000000000000000000000910152565b6052600d6020926017565b6059816020565b0190565b60709060208101905f8183039101526047565b90565b6079600d565b62461bcd60e51b815280608d60048201605d565b0390fdfea2646970667358221220ca26ecb1412cc6e87fe99712d1085a242349863e03e3e43ef6ee7dde7a3478cc64736f6c634300081a0033");
+
+/// Get an instance of the pre-deployed Counter contract.
+pub fn revert<P: Provider>(p: P) -> Counter::CounterInstance<P> {
+    Counter::CounterInstance::new(REVERT_TEST_ADDRESS, p)
+}

crates/test-utils/src/evm.rs

@@ -1,22 +1,57 @@
-use signet_constants::test_utils::*;
-use trevm::revm::{
-    context::CfgEnv, database::in_memory_db::InMemoryDB, primitives::hardfork::SpecId,
-    state::Bytecode,
+use crate::{
+    contracts::{
+        counter::{COUNTER_BYTECODE, COUNTER_TEST_ADDRESS},
+        reverts::{REVERT_BYTECODE, REVERT_TEST_ADDRESS},
+        system::{RU_ORDERS_BYTECODE, RU_PASSAGE_BYTECODE},
+        token::{
+            MINTER, MINTER_SLOT, NAME_SLOT, SYMBOL_SLOT, TOKEN_BYTECODE, WBTC_NAME, WBTC_SYMBOL,
+            WETH_NAME, WETH_SYMBOL,
+        },
+    },
+    users::TEST_USERS,
 };
-
-use crate::contracts::{
-    counter::{COUNTER_BYTECODE, COUNTER_TEST_ADDRESS},
-    system::{RU_ORDERS_BYTECODE, RU_PASSAGE_BYTECODE},
-    token::{
-        MINTER, MINTER_SLOT, NAME_SLOT, SYMBOL_SLOT, TOKEN_BYTECODE, WBTC_NAME, WBTC_SYMBOL,
-        WETH_NAME, WETH_SYMBOL,
+use alloy::{consensus::constants::ETH_TO_WEI, primitives::U256};
+use signet_constants::test_utils::*;
+use trevm::{
+    helpers::Ctx,
+    revm::{
+        context::CfgEnv, database::in_memory_db::InMemoryDB, inspector::NoOpInspector,
+        primitives::hardfork::SpecId, state::Bytecode, Inspector,
     },
 };
 
-/// Create a new Signet EVM with an in-memory database for testing. Deploy
-/// system contracts and pre-deployed tokens.
+/// Create a new Signet EVM with an in-memory database for testing.
+///
+/// Performs initial setup to
+/// - Deploy [`RU_ORDERS`] and and [`RU_PASSAGE`] system contracts
+/// - Deploy a [`COUNTER`] contract for testing at [`COUNTER_TEST_ADDRESS`].
+/// - Deploy Token contracts for WBTC and WETH with their respective bytecodes
+///   and storage.
+/// - Deploy a `Revert` contract for testing at [`REVERT_TEST_ADDRESS`].
+/// - Fund the [`TEST_USERS`] with 1000 ETH each.
+///
+/// [`COUNTER`]: crate::contracts::counter::Counter
 pub fn test_signet_evm() -> signet_evm::EvmNeedsBlock<InMemoryDB> {
-    let mut evm = signet_evm::signet_evm(InMemoryDB::default(), TEST_SYS).fill_cfg(&TestCfg);
+    test_signet_evm_with_inspector(NoOpInspector)
+}
+
+/// Create a new Signet EVM with an in-memory database for testing.
+///
+/// Performs initial setup to
+/// - Deploy [`RU_ORDERS`] and and [`RU_PASSAGE`] system contracts
+/// - Deploy a [`COUNTER`] contract for testing at [`COUNTER_TEST_ADDRESS`].
+/// - Deploy Token contracts for WBTC and WETH with their respective bytecodes
+///   and storage.
+/// - Deploy a `Revert` contract for testing at [`REVERT_TEST_ADDRESS`].
+/// - Fund the [`TEST_USERS`] with 1000 ETH each.
+///
+/// [`COUNTER`]: crate::contracts::counter::Counter
+pub fn test_signet_evm_with_inspector<I>(inspector: I) -> signet_evm::EvmNeedsBlock<InMemoryDB, I>
+where
+    I: Inspector<Ctx<InMemoryDB>>,
+{
+    let mut evm = signet_evm::signet_evm_with_inspector(InMemoryDB::default(), inspector, TEST_SYS)
+        .fill_cfg(&TestCfg);
 
     // Set the bytecode for system contracts
     evm.set_bytecode_unchecked(TEST_SYS.ru_orders(), Bytecode::new_legacy(RU_ORDERS_BYTECODE));
@@ -37,6 +72,14 @@ pub fn test_signet_evm() -> signet_evm::EvmNeedsBlock<InMemoryDB> {
     // Set the bytecode for the Counter contract
     evm.set_bytecode_unchecked(COUNTER_TEST_ADDRESS, Bytecode::new_legacy(COUNTER_BYTECODE));
 
+    // Set the bytecode for the Revert contract
+    evm.set_bytecode_unchecked(REVERT_TEST_ADDRESS, Bytecode::new_legacy(REVERT_BYTECODE));
+
+    // increment the balance for each test signer
+    TEST_USERS.iter().copied().for_each(|user| {
+        evm.set_balance_unchecked(user, U256::from(1000 * ETH_TO_WEI));
+    });
+
     evm
 }
 

crates/test-utils/src/specs/mod.rs

@@ -8,16 +8,22 @@ mod ru_spec;
 pub use ru_spec::RuBlockSpec;
 
 use alloy::{
-    consensus::{constants::GWEI_TO_WEI, SignableTransaction, TxEip1559},
+    consensus::{
+        constants::GWEI_TO_WEI, SignableTransaction, TxEip1559, TxEnvelope, TypedTransaction,
+    },
+    eips::Encodable2718,
     primitives::{Address, TxKind, B256, U256},
+    rpc::types::mev::EthSendBundle,
     signers::{local::PrivateKeySigner, SignerSync},
+    sol_types::SolCall,
 };
-use signet_types::primitives::{Transaction, TransactionSigned};
+use signet_bundle::SignetEthBundle;
+use signet_types::SignedFill;
 
 /// Sign a transaction with a wallet.
-pub fn sign_tx_with_key_pair(wallet: &PrivateKeySigner, tx: Transaction) -> TransactionSigned {
+pub fn sign_tx_with_key_pair(wallet: &PrivateKeySigner, tx: TypedTransaction) -> TxEnvelope {
     let signature = wallet.sign_hash_sync(&tx.signature_hash()).unwrap();
-    TransactionSigned::new_unhashed(tx, signature)
+    TxEnvelope::new_unhashed(tx, signature)
 }
 
 /// Make a wallet with a deterministic keypair.
@@ -26,7 +32,7 @@ pub fn make_wallet(i: u8) -> PrivateKeySigner {
 }
 
 /// Make a simple send transaction.
-pub fn simple_send(to: Address, amount: U256, nonce: u64, ru_chain_id: u64) -> Transaction {
+pub fn simple_send(to: Address, amount: U256, nonce: u64, ru_chain_id: u64) -> TypedTransaction {
     TxEip1559 {
         nonce,
         gas_limit: 21_000,
@@ -39,3 +45,54 @@ pub fn simple_send(to: Address, amount: U256, nonce: u64, ru_chain_id: u64) -> T
     }
     .into()
 }
+
+/// Make a simple contract call transaction.
+pub fn simple_call<T>(
+    to: Address,
+    input: &T,
+    value: U256,
+    nonce: u64,
+    ru_chain_id: u64,
+) -> TypedTransaction
+where
+    T: SolCall,
+{
+    TxEip1559 {
+        nonce,
+        gas_limit: 2_100_000,
+        to: TxKind::Call(to),
+        value,
+        chain_id: ru_chain_id,
+        max_fee_per_gas: GWEI_TO_WEI as u128 * 100,
+        max_priority_fee_per_gas: GWEI_TO_WEI as u128,
+        input: input.abi_encode().into(),
+        ..Default::default()
+    }
+    .into()
+}
+
+/// Create a simple bundle from a list of transactions.
+pub fn simple_bundle<'a>(
+    txs: impl IntoIterator<Item = &'a TxEnvelope>,
+    host_fills: Option<SignedFill>,
+    block_number: u64,
+) -> SignetEthBundle {
+    let txs = txs.into_iter().map(|tx| tx.encoded_2718().into()).collect();
+
+    SignetEthBundle {
+        bundle: EthSendBundle {
+            txs,
+            block_number,
+            min_timestamp: None,
+            max_timestamp: None,
+            reverting_tx_hashes: vec![],
+            replacement_uuid: None,
+            dropping_tx_hashes: vec![],
+            refund_percent: None,
+            refund_recipient: None,
+            refund_tx_hashes: vec![],
+            extra_fields: Default::default(),
+        },
+        host_fills,
+    }
+}

crates/test-utils/src/specs/ru_spec.rs

@@ -8,10 +8,7 @@ use alloy::{
 };
 use signet_constants::test_utils::*;
 use signet_extract::{Extractable, Extracts};
-use signet_types::{
-    constants::{KnownChains, ParseChainError, SignetSystemConstants},
-    primitives::TransactionSigned,
-};
+use signet_types::constants::{KnownChains, ParseChainError, SignetSystemConstants};
 use signet_zenith::Zenith::{self};
 use std::str::FromStr;
 
@@ -66,7 +63,7 @@ impl RuBlockSpec {
     }
 
     /// Add a transaction to the block.
-    pub fn add_tx(&mut self, tx: &TransactionSigned) {
+    pub fn add_tx(&mut self, tx: &TxEnvelope) {
         self.tx.push(tx.encoded_2718());
     }
 
@@ -81,7 +78,7 @@ impl RuBlockSpec {
     }
 
     /// Add a transaction to the block, returning self.
-    pub fn tx(mut self, tx: &TransactionSigned) -> Self {
+    pub fn tx(mut self, tx: &TxEnvelope) -> Self {
         self.add_tx(tx);
         self
     }
@@ -99,7 +96,7 @@ impl RuBlockSpec {
         to: Address,
         amount: U256,
         nonce: u64,
-    ) -> TransactionSigned {
+    ) -> TxEnvelope {
         let tx = sign_tx_with_key_pair(
             wallet,
             simple_send(to, amount, nonce, self.constants.ru_chain_id()),