Minor improvements and cleanup; appease clippy

Author: ImplOfAnImpl

mempool/src/error/ban_score.rs

@@ -54,7 +54,7 @@ impl MempoolBanScore for Error {
             // Internal error
             Error::SubsystemCallError(_) => 0,
 
-            // A configuration error is not the peer's fault.
+            // A configuration error is not peer's fault.
             Error::ConfigError(_) => 0,
         }
     }
@@ -71,7 +71,7 @@ impl MempoolBanScore for MempoolPolicyError {
 
             // Don't punish the peer for this, because max cluster size is technically configurable
             // and the peer doesn't know the node's mempool configuration.
-            MempoolPolicyError::TxSizeExceedsMaxClusterSize => 0,
+            MempoolPolicyError::TxSizeExceedsMaxClusterSize { .. } => 0,
 
             // Errors to do with transaction conflicts and replacements are not punished since the
             // peer may not be aware of all the transactions the node has in the mempool.

mempool/src/error/mod.rs

@@ -98,8 +98,8 @@ pub enum MempoolPolicyError {
     #[error("Transaction size exceeds the maximum block size")]
     TxSizeExceedsMaxBlockSize,
 
-    #[error("Transaction size exceeds the maximum cluster size")]
-    TxSizeExceedsMaxClusterSize,
+    #[error("Transaction size ({tx_size}) exceeds the maximum cluster size ({limit})")]
+    TxSizeExceedsMaxClusterSize { tx_size: usize, limit: usize },
 
     #[error(
         "Replacement transaction has fee lower than the original. Replacement fee is {replacement_fee:?}, original fee {original_fee:?}"

mempool/src/pool/tests/relatives.rs

@@ -312,7 +312,7 @@ async fn test_relatives_and_cluster_tx_count_limit(#[case] seed: Seed) {
         assert_descendants(&mempool, &b5, &[]);
     }
 
-    // A non-orphan subtest with a bigger limit
+    // A non-orphan subtest with a bigger limit
     {
         let mut mempool = create_mempool(5);
 
@@ -388,7 +388,7 @@ async fn test_relatives_and_cluster_tx_count_limit(#[case] seed: Seed) {
         assert_descendants(&mempool, &b2, &[]);
     }
 
-    // A non-orphan subtest with the default limit. All txs will be added and form a single cluster.
+    // A non-orphan subtest with the default limit. All txs will be added and form a single cluster.
     {
         let mut mempool = create_mempool(*MaxClusterTxCount::default());
 

mempool/src/pool/tx_pool/mod.rs

@@ -291,7 +291,10 @@ impl<M: MemoryUsageEstimator> TxPool<M> {
         let max_cluster_size = *self.mempool_config.max_cluster_size_bytes;
         ensure!(
             size <= max_cluster_size,
-            MempoolPolicyError::TxSizeExceedsMaxClusterSize
+            MempoolPolicyError::TxSizeExceedsMaxClusterSize {
+                tx_size: size,
+                limit: max_cluster_size
+            }
         );
 
         Ok(())

mempool/src/pool/tx_pool/store/mod.rs

@@ -851,6 +851,7 @@ impl TxMempoolEntry {
             self.parents.iter().copied(),
             RelativesKind::Ancestors,
             |_| Ok::<_, MempoolStoreInvariantError>(()),
+            Some(self.count_with_ancestors - 1),
         );
 
         match result {
@@ -874,6 +875,7 @@ impl TxMempoolEntry {
             self.children.iter().copied(),
             RelativesKind::Descendants,
             |_| Ok::<_, MempoolStoreInvariantError>(()),
+            Some(self.count_with_descendants - 1),
         );
 
         match result {
@@ -891,9 +893,13 @@ impl TxMempoolEntry {
     ///
     /// This function is mainly intended for testing purposes.
     pub fn collect_cluster(&self, store: &MempoolStore) -> Result<Cluster, MempoolPolicyError> {
-        let cluster = collect_relatives(store, [*self.tx_id()], RelativesKind::Cluster, |_| {
-            Ok::<_, MempoolPolicyError>(())
-        })?;
+        let cluster = collect_relatives(
+            store,
+            [*self.tx_id()],
+            RelativesKind::Cluster,
+            |_| Ok::<_, MempoolPolicyError>(()),
+            Some(self.count_with_ancestors + self.count_with_descendants - 1),
+        )?;
 
         Ok(Cluster::new(cluster))
     }
@@ -940,6 +946,7 @@ impl TxMempoolEntryWithAncestors {
             |collected_size| {
                 ensure_cluster_tx_count_limit(&store.mempool_config, 1 + collected_size)
             },
+            None,
         )?);
         enforce_cluster_size_limit(store, &entry, &cluster)?;
 
@@ -948,6 +955,7 @@ impl TxMempoolEntryWithAncestors {
             parents.iter().copied(),
             RelativesKind::Ancestors,
             |_| Ok::<_, MempoolPolicyError>(()),
+            None,
         )?);
 
         let ancestor_entries_iter = ancestors
@@ -995,17 +1003,26 @@ pub enum RelativesKind {
 ///
 /// After each tx is added to the result, `on_new_tx_added` is called with the current size
 /// of the result as the argument.
+///
+/// `expected_result_size`, if specified, is used to reserve the needed capacity in the result
+/// to avoid redundant reallocations.
 pub fn collect_relatives<E>(
     store: &MempoolStore,
     initial_tx_ids: impl IntoIterator<Item = Id<Transaction>>,
     kind: RelativesKind,
     mut on_new_tx_added: impl FnMut(/*cur_result_size:*/ usize) -> Result<(), E>,
+    expected_result_size: Option<usize>,
 ) -> Result<StoreHashSet<Id<Transaction>>, E>
 where
     E: From<MempoolStoreInvariantError>,
 {
-    let mut stack = Vec::new();
-    let mut result = StoreHashSet::default();
+    let initial_tx_ids = initial_tx_ids.into_iter();
+    let initial_tx_ids_min_size_hint = initial_tx_ids.size_hint().0;
+    let expected_result_size = expected_result_size.unwrap_or(initial_tx_ids_min_size_hint);
+
+    let mut stack = Vec::with_capacity(initial_tx_ids_min_size_hint);
+    let mut result =
+        StoreHashSet::with_capacity_and_hasher(expected_result_size, Default::default());
 
     let mut visit = |stack: &mut Vec<Id<Transaction>>, tx_id: &Id<Transaction>| -> Result<(), E> {
         if result.insert(*tx_id) {

mempool/src/pool/tx_pool/tests/basic.rs

@@ -381,6 +381,7 @@ async fn tx_bigger_than_cluster_size(#[case] seed: Seed) -> anyhow::Result<()> {
         )
         .add_output_n_times(outputs_count, &output)
         .build();
+    let tx_size = tx.encoded_size();
     let mempool_config = MempoolConfig {
         min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(),
         max_cluster_size_bytes: max_cluster_size.into(),
@@ -392,7 +393,11 @@ async fn tx_bigger_than_cluster_size(#[case] seed: Seed) -> anyhow::Result<()> {
 
     assert_eq!(
         mempool.add_transaction_test(tx),
-        Err(MempoolPolicyError::TxSizeExceedsMaxClusterSize.into())
+        Err(MempoolPolicyError::TxSizeExceedsMaxClusterSize {
+            tx_size,
+            limit: max_cluster_size
+        }
+        .into())
     );
     mempool.store.assert_valid();
     Ok(())
@@ -1547,7 +1552,10 @@ async fn accepted_tx_size(
     let result = mempool.add_transaction_test(transaction);
 
     let expected_err = if use_cluster_size_limit {
-        MempoolPolicyError::TxSizeExceedsMaxClusterSize
+        MempoolPolicyError::TxSizeExceedsMaxClusterSize {
+            tx_size,
+            limit: max_cluster_size,
+        }
     } else {
         MempoolPolicyError::TxSizeExceedsMaxBlockSize
     };

wallet/src/account/mod.rs

@@ -976,6 +976,7 @@ impl<K: AccountKeyChains> Account<K> {
         )
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn create_order_tx(
         &mut self,
         db_tx: &mut impl WalletStorageWriteLocked,
@@ -1004,6 +1005,7 @@ impl<K: AccountKeyChains> Account<K> {
         )
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn create_conclude_order_tx(
         &mut self,
         db_tx: &mut impl WalletStorageWriteLocked,
@@ -1199,6 +1201,7 @@ impl<K: AccountKeyChains> Account<K> {
         )
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn create_spend_utxo_tx(
         &mut self,
         db_tx: &mut impl WalletStorageWriteLocked,
@@ -1398,6 +1401,7 @@ impl<K: AccountKeyChains> Account<K> {
         Ok(request)
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn mint_tokens(
         &mut self,
         db_tx: &mut impl WalletStorageWriteLocked,
@@ -1594,6 +1598,7 @@ impl<K: AccountKeyChains> Account<K> {
         )
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn make_change_token_transaction(
         &mut self,
         authority: Destination,

wallet/src/account/utxo_selector/output_group.rs

@@ -24,7 +24,7 @@ use super::UtxoSelectorError;
 /// This helps reduce privacy leaks resulting from address reuse.
 // TODO: at this moment, we always create a separate output group for each utxo, i.e. we don't have
 // by-destination grouping.
-// Note that in Bitcoin, transactions are not grouped unconditionally:
+// Note that in Bitcoin transactions are not grouped unconditionally:
 // *) They are grouped if either m_avoid_partial_spends or m_avoid_address_reuse is true (both are
 //    false by default).
 // *) Otherwise, if m_max_aps_fee is non-negative (it's zero by default), then a second attempt

wallet/src/wallet/mod.rs

@@ -329,7 +329,7 @@ pub struct Wallet<B: storage::Backend + 'static, P: SignerProvider> {
     chain_config: Arc<ChainConfig>,
     // Note: unlike the chain config, the mempool config may potentially change, e.g. if the node
     // is restarted with different parameters. At this moment, node restart requires the wallet
-    // to be restarted as well, so it's not an issue for now. But when we implement the automatic
+    // to be restarted as well, so it's not an issue for now. But when we implement automatic
     // reconnection, the mempool config stored here should be updated each time we reconnect to
     // the node.
     mempool_config: Arc<MempoolConfig>,
@@ -899,6 +899,7 @@ where
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub async fn load_wallet<
         F: Fn(u32) -> WalletResult<()>,
         F2: AsyncFnOnce(StoreTxRo<B>) -> WalletResult<P>,

wallet/wallet-controller/src/lib.rs

@@ -519,6 +519,7 @@ where
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub async fn open_wallet(
         chain_config: Arc<ChainConfig>,
         mempool_config: Arc<MempoolConfig>,