refactor: use &TxHash in Bundle trait methods (#39)

Changed `is_allowed_to_fail` and `is_optional` to accept `&TxHash`
instead of `TxHash`. These methods are primarily used for lookups in
collections, and accepting a reference allows implementations to use
methods like `Vec::contains()` and `HashSet::contains()` directly
without requiring callers to copy the hash.

Author: mattsse

examples/custom-bundle-type.rs

@@ -179,12 +179,12 @@ impl Bundle<CustomPlatform> for CustomBundleType {
 		Eligibility::Eligible
 	}
 
-	fn is_allowed_to_fail(&self, tx: alloy::primitives::TxHash) -> bool {
-		self.reverting_txs.contains(&tx)
+	fn is_allowed_to_fail(&self, tx: &alloy::primitives::TxHash) -> bool {
+		self.reverting_txs.contains(tx)
 	}
 
-	fn is_optional(&self, tx: alloy::primitives::TxHash) -> bool {
-		self.dropping_txs.contains(&tx)
+	fn is_optional(&self, tx: &alloy::primitives::TxHash) -> bool {
+		self.dropping_txs.contains(tx)
 	}
 
 	fn validate_post_execution(

src/payload/exec.rs

@@ -205,7 +205,7 @@ impl<P: Platform> Executable<P> {
 		let mut results = Vec::with_capacity(bundle.transactions().len());
 
 		for transaction in bundle.transactions() {
-			let tx_hash = *transaction.tx_hash();
+			let tx_hash = transaction.tx_hash();
 			let optional = bundle.is_optional(tx_hash);
 			let allowed_to_fail = bundle.is_allowed_to_fail(tx_hash);
 
@@ -224,16 +224,16 @@ impl<P: Platform> Executable<P> {
 				// Optional failing transaction, not allowed to fail
 				// or optional invalid transaction: discard it
 				Ok(_) | Err(_) if optional => {
-					discarded.push(tx_hash);
+					discarded.push(*tx_hash);
 				}
 				// Non-Optional failing transaction, not allowed to fail: invalidate the
 				// bundle
 				Ok(_) => {
-					return Err(ExecutionError::BundleTransactionReverted(tx_hash));
+					return Err(ExecutionError::BundleTransactionReverted(*tx_hash));
 				}
 				// Non-Optional invalid transaction: invalidate the bundle
 				Err(err) => {
-					return Err(ExecutionError::InvalidBundleTransaction(tx_hash, err));
+					return Err(ExecutionError::InvalidBundleTransaction(*tx_hash, err));
 				}
 			}
 		}

src/platform/bundle.rs

@@ -77,11 +77,11 @@ pub trait Bundle<P: Platform>:
 	///
 	/// The `tx` is the hash of the transaction to check, that should be part of
 	/// this bundle.
-	fn is_allowed_to_fail(&self, tx: TxHash) -> bool;
+	fn is_allowed_to_fail(&self, tx: &TxHash) -> bool;
 
 	/// Checks if a transaction with the given hash may be removed from the
 	/// bundle without affecting its validity.
-	fn is_optional(&self, tx: TxHash) -> bool;
+	fn is_optional(&self, tx: &TxHash) -> bool;
 
 	/// An optional check for bundle implementations that have validity
 	/// requirements on the resulting state.
@@ -300,12 +300,12 @@ impl<P: Platform> Bundle<P> for FlashbotsBundle<P> {
 		)
 	}
 
-	fn is_allowed_to_fail(&self, tx: TxHash) -> bool {
-		self.reverting_tx_hashes.contains(&tx)
+	fn is_allowed_to_fail(&self, tx: &TxHash) -> bool {
+		self.reverting_tx_hashes.contains(tx)
 	}
 
-	fn is_optional(&self, tx: TxHash) -> bool {
-		self.dropping_tx_hashes.contains(&tx)
+	fn is_optional(&self, tx: &TxHash) -> bool {
+		self.dropping_tx_hashes.contains(tx)
 	}
 
 	fn hash(&self) -> B256 {

src/platform/ext/bundle.rs

@@ -38,7 +38,7 @@ impl<P: Platform, T: Bundle<P>> BundleExt<P> for T {
 		self
 			.transactions()
 			.iter()
-			.filter(|tx| !self.is_allowed_to_fail(*tx.tx_hash()))
+			.filter(|tx| !self.is_allowed_to_fail(tx.tx_hash()))
 	}
 
 	/// Returns an iterator over all transactions that can be removed from the
@@ -49,7 +49,7 @@ impl<P: Platform, T: Bundle<P>> BundleExt<P> for T {
 		self
 			.transactions()
 			.iter()
-			.filter(|tx| self.is_optional(*tx.tx_hash()))
+			.filter(|tx| self.is_optional(tx.tx_hash()))
 	}
 
 	/// Returns an iterator over all transactions that are required to be present
@@ -61,7 +61,7 @@ impl<P: Platform, T: Bundle<P>> BundleExt<P> for T {
 		self
 			.transactions()
 			.iter()
-			.filter(|tx| !self.is_optional(*tx.tx_hash()))
+			.filter(|tx| !self.is_optional(tx.tx_hash()))
 	}
 
 	/// Returns an iterator over all transactions that must be included and may
@@ -70,8 +70,7 @@ impl<P: Platform, T: Bundle<P>> BundleExt<P> for T {
 		&self,
 	) -> impl Iterator<Item = &Recovered<types::Transaction<P>>> {
 		self.transactions().iter().filter(|tx| {
-			!self.is_allowed_to_fail(*tx.tx_hash())
-				&& !self.is_optional(*tx.tx_hash())
+			!self.is_allowed_to_fail(tx.tx_hash()) && !self.is_optional(tx.tx_hash())
 		})
 	}
 }

src/steps/revert.rs

@@ -132,7 +132,7 @@ impl<P: Platform> Step<P> for RemoveRevertedTransactions {
 						.failed_txs()
 						.filter_map(|(tx, result)| {
 							bundle
-								.is_optional(*tx.tx_hash())
+								.is_optional(tx.tx_hash())
 								.then_some((*tx.tx_hash(), result.gas_used()))
 						})
 						.collect::<Vec<_>>();
@@ -241,7 +241,7 @@ fn has_failures<P: Platform>(checkpoint: &Checkpoint<P>) -> bool {
 			.iter()
 			.map(|r| !r.is_success())
 			.zip(result.transactions().iter().map(|tx| tx.tx_hash()))
-			.any(|(is_failure, tx_hash)| is_failure && bundle.is_optional(*tx_hash));
+			.any(|(is_failure, tx_hash)| is_failure && bundle.is_optional(tx_hash));
 	}
 
 	false