Merge pull request #45 from worldcoin/fix/tests

test: cleanup & clippy

world-chain-builder/Justfile

@@ -2,7 +2,7 @@ set positional-arguments
 
 # Tests the world-chain-builder
 test:
-    cargo test
+    cargo test -- --test-threads=1
 
 # Formats the world-chain-builder
 fmt: fmt-fix fmt-check

world-chain-builder/benches/validate_transaction.rs

@@ -37,7 +37,7 @@ where
 fn validator_setup() -> WorldChainTransactionValidator<MockEthProvider, WorldChainPooledTransaction>
 {
     let validator = world_chain_validator();
-    let transaction = get_pbh_transaction();
+    let transaction = get_pbh_transaction(0);
     validator.inner().client().add_account(
         transaction.sender(),
         ExtendedAccount::new(transaction.nonce(), alloy_primitives::U256::MAX),
@@ -82,14 +82,14 @@ fn non_pbh_setup() -> Setup {
 
 fn pbh_setup() -> Setup {
     let pool = pool_setup();
-    let transaction = get_pbh_transaction();
+    let transaction = get_pbh_transaction(0);
 
     Setup { pool, transaction }
 }
 
 fn spoofed_nullifier_setup() -> Setup {
     let pool = pool_setup();
-    let mut transaction = get_pbh_transaction();
+    let mut transaction = get_pbh_transaction(0);
     let pbh_payload = transaction.pbh_payload.as_mut().unwrap();
     pbh_payload.nullifier_hash = Field::default();
     Setup { pool, transaction }

world-chain-builder/src/payload/builder.rs

@@ -1,4 +1,3 @@
-use reth_db::transaction::DbTx;
 use reth_evm::ConfigureEvm;
 use std::sync::Arc;
 
@@ -18,7 +17,7 @@ use reth_basic_payload_builder::{
     PayloadBuilder, PayloadConfig, WithdrawalsOutcome,
 };
 use reth_chain_state::ExecutedBlock;
-use reth_db::{Database, DatabaseEnv, DatabaseError, DatabaseWriteOperation};
+use reth_db::DatabaseEnv;
 use reth_evm::system_calls::SystemCaller;
 use reth_optimism_chainspec::OpChainSpec;
 use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism;
@@ -43,7 +42,6 @@ use revm_primitives::{
 };
 use tracing::{debug, trace, warn};
 
-use crate::pbh::db::set_pbh_nullifier;
 use crate::pool::noop::NoopWorldChainTransactionPool;
 use crate::pool::tx::WorldChainPoolTransaction;
 
@@ -53,25 +51,19 @@ pub struct WorldChainPayloadBuilder<EvmConfig> {
     inner: OptimismPayloadBuilder<EvmConfig>,
     /// The percentage of the blockspace that should be reserved for verified transactions
     verified_blockspace_capacity: u8,
-    pbh_db: Arc<DatabaseEnv>,
 }
 
 impl<EvmConfig> WorldChainPayloadBuilder<EvmConfig>
 where
     EvmConfig: ConfigureEvm<Header = Header>,
 {
     /// `OptimismPayloadBuilder` constructor.
-    pub const fn new(
-        evm_config: EvmConfig,
-        verified_blockspace_capacity: u8,
-        pbh_db: Arc<DatabaseEnv>,
-    ) -> Self {
+    pub const fn new(evm_config: EvmConfig, verified_blockspace_capacity: u8) -> Self {
         let inner = OptimismPayloadBuilder::new(evm_config);
 
         Self {
             inner,
             verified_blockspace_capacity,
-            pbh_db,
         }
     }
 }
@@ -96,7 +88,6 @@ where
 
         worldchain_payload(
             self.inner.evm_config.clone(),
-            self.pbh_db.clone(),
             args,
             cfg_env,
             block_env,
@@ -132,7 +123,6 @@ where
 
         worldchain_payload(
             self.inner.evm_config.clone(),
-            self.pbh_db.clone(),
             args,
             cfg_env,
             block_env,
@@ -179,11 +169,8 @@ where
     ) -> eyre::Result<PayloadBuilderHandle<OptimismEngineTypes>> {
         let evm_config = OptimismEvmConfig::new(Arc::new((*ctx.chain_spec()).clone()));
 
-        let payload_builder = WorldChainPayloadBuilder::new(
-            evm_config,
-            self.verified_blockspace_capacity,
-            self.pbh_db,
-        );
+        let payload_builder =
+            WorldChainPayloadBuilder::new(evm_config, self.verified_blockspace_capacity);
 
         let conf = ctx.payload_builder_config();
 
@@ -222,7 +209,6 @@ where
 #[inline]
 pub(crate) fn worldchain_payload<EvmConfig, Pool, Client>(
     evm_config: EvmConfig,
-    pbh_db: Arc<DatabaseEnv>,
     args: BuildArguments<Pool, Client, OptimismPayloadBuilderAttributes, OptimismBuiltPayload>,
     initialized_cfg: CfgEnvWithHandlerCfg,
     initialized_block_env: BlockEnv,
@@ -670,11 +656,11 @@ where
 mod tests {
     use crate::{
         node::test_utils::{WorldChainNoopProvider, WorldChainNoopValidator},
-        pbh::payload::PbhPayload,
         pool::{
             ordering::WorldChainOrdering, root::WorldChainRootValidator,
             tx::WorldChainPooledTransaction, validator::WorldChainTransactionValidator,
         },
+        test::get_pbh_transaction,
     };
 
     use super::*;
@@ -741,7 +727,7 @@ mod tests {
         // Init the payload builder
         let verified_blockspace_cap = 50;
         let world_chain_payload_builder =
-            WorldChainPayloadBuilder::new(evm_config, verified_blockspace_cap, db.clone());
+            WorldChainPayloadBuilder::new(evm_config, verified_blockspace_cap);
 
         // Insert transactions into the pool
         let unverified_transactions = generate_mock_pooled_transactions(50, 100000, false);
@@ -857,7 +843,7 @@ mod tests {
         // Init the payload builder
         let verified_blockspace_cap = 10;
         let world_chain_payload_builder =
-            WorldChainPayloadBuilder::new(evm_config, verified_blockspace_cap, db.clone());
+            WorldChainPayloadBuilder::new(evm_config, verified_blockspace_cap);
 
         // Insert transactions into the pool
         let unverified_transactions = generate_mock_pooled_transactions(50, 100000, false);
@@ -975,7 +961,7 @@ mod tests {
         let mut rng = rand::thread_rng();
 
         (0..count)
-            .map(|_| {
+            .map(|i| {
                 let tx = reth_primitives::Transaction::Legacy(TxLegacy {
                     gas_price: 10,
                     gas_limit,
@@ -1000,7 +986,7 @@ mod tests {
                 let pooled_tx = EthPooledTransaction::new(tx_recovered.clone(), 200);
 
                 let pbh_payload = if pbh {
-                    Some(PbhPayload::default())
+                    Some(get_pbh_transaction(i as u16).pbh_payload.unwrap())
                 } else {
                     None
                 };

world-chain-builder/src/pbh/db.rs

@@ -1,4 +1,3 @@
-use alloy_primitives::TxHash;
 use bytes::BufMut;
 use reth_db::cursor::DbCursorRW;
 use reth_db::mdbx::tx::Tx;

world-chain-builder/src/pool/validator.rs

@@ -238,12 +238,12 @@ pub mod tests {
     use crate::pbh::payload::{PbhPayload, Proof};
     use crate::pool::ordering::WorldChainOrdering;
     use crate::pool::root::{LATEST_ROOT_SLOT, OP_WORLD_ID};
-    use crate::test::{get_non_pbh_transaction, get_pbh_transaction, world_chain_validator};
+    use crate::test::{get_pbh_transaction, world_chain_validator};
 
     #[tokio::test]
     async fn validate_pbh_transaction() {
         let validator = world_chain_validator();
-        let transaction = get_pbh_transaction();
+        let transaction = get_pbh_transaction(0);
         validator.inner.client().add_account(
             transaction.sender(),
             ExtendedAccount::new(transaction.nonce(), alloy_primitives::U256::MAX),
@@ -295,7 +295,7 @@ pub mod tests {
     #[tokio::test]
     async fn invalid_external_nullifier_hash() {
         let validator = world_chain_validator();
-        let transaction = get_pbh_transaction();
+        let transaction = get_pbh_transaction(0);
 
         validator.inner.client().add_account(
             transaction.sender(),
@@ -318,7 +318,7 @@ pub mod tests {
     #[tokio::test]
     async fn invalid_signal_hash() {
         let validator = world_chain_validator();
-        let transaction = get_pbh_transaction();
+        let transaction = get_pbh_transaction(0);
 
         validator.inner.client().add_account(
             transaction.sender(),

world-chain-builder/src/test/e2e/mod.rs

@@ -322,25 +322,22 @@ async fn test_dup_pbh_nonce() -> eyre::Result<()> {
 
     let raw_tx_0 = ctx.raw_pbh_tx_bytes(signer.clone(), 0, 0).await;
     ctx.node.rpc.inject_tx(raw_tx_0.clone()).await?;
-
     let raw_tx_1 = ctx.raw_pbh_tx_bytes(signer.clone(), 0, 1).await;
-    ctx.node.rpc.inject_tx(raw_tx_1.clone()).await?;
+
+    // Now that the nullifier has successfully been stored in
+    // the `ExecutedPbhNullifierTable`, inserting a new tx with the
+    // same pbh_nonce should fail to validate.
+    assert!(ctx.node.rpc.inject_tx(raw_tx_1.clone()).await.is_err());
 
     let (payload, _) = ctx
         .node
         .advance_block(vec![], optimism_payload_attributes)
         .await?;
 
-    // Both transactions should be successfully validated
-    // but only one should be included in the block
+    // One transaction should be successfully validated
+    // and included in the block.
     assert_eq!(payload.block().body.transactions.len(), 1);
 
-    // Now that the nullifier has successfully been stored in
-    // the `ExecutedPbhNullifierTable`, inserting a new tx with the
-    // same pbh_nonce should fail to validate.
-    let raw_tx_2 = ctx.raw_pbh_tx_bytes(signer.clone(), 0, 2).await;
-    assert!(ctx.node.rpc.inject_tx(raw_tx_2.clone()).await.is_err());
-
     Ok(())
 }
 

world-chain-builder/src/test/mod.rs

@@ -41,13 +41,13 @@ pub fn get_non_pbh_transaction() -> WorldChainPooledTransaction {
     }
 }
 
-pub fn get_pbh_transaction() -> WorldChainPooledTransaction {
+pub fn get_pbh_transaction(nonce: u16) -> WorldChainPooledTransaction {
     let eth_tx = get_eth_transaction();
     let pbh_payload = valid_pbh_payload(
         &mut [0; 32],
         eth_tx.hash().as_slice(),
         chrono::Utc::now(),
-        0,
+        nonce,
     );
     WorldChainPooledTransaction {
         inner: eth_tx,