fix: handle stale parents during ancestor eviction

Author: chenyukang

tx-pool/src/component/pool_map.rs

@@ -600,31 +600,37 @@ impl PoolMap {
             return Ok(evicted);
         }
 
-        if ancestors_count.saturating_sub(cell_ref_parents.len()) <= self.max_ancestors_count {
-            // if ancestors count exceed limitation,
-            // try to evict some conflicted transactions due to ref cells
-
-            // sort them to find out the transactions with lowest fees
-            let evict_candidates: Vec<ProposalShortId> = self
-                .entries
-                .iter_by_evict_key()
-                .filter(move |entry| cell_ref_parents.contains(&entry.id))
-                .map(|x| x.id.clone())
-                .collect();
-
-            let mut iter = evict_candidates.iter();
-            while ancestors_count > self.max_ancestors_count {
-                if let Some(next_id) = iter.next() {
-                    let removed = self.remove_entry_and_descendants(next_id);
-                    ancestors_count = ancestors_count.saturating_sub(1);
-                    parents.remove(next_id);
-                    evicted.extend(removed);
-                } else {
-                    break;
+        if ancestors_count.saturating_sub(cell_ref_parents.len()) > self.max_ancestors_count {
+            return Err(Reject::ExceededMaximumAncestorsCount);
+        }
+
+        // if ancestors count exceed limitation,
+        // try to evict some conflicted transactions due to ref cells
+
+        // sort them to find out the transactions with lowest fees
+        let evict_candidates: Vec<ProposalShortId> = self
+            .entries
+            .iter_by_evict_key()
+            .filter(move |entry| cell_ref_parents.contains(&entry.id))
+            .map(|x| x.id.clone())
+            .collect();
+
+        let mut iter = evict_candidates.iter();
+        while ancestors_count > self.max_ancestors_count {
+            if let Some(next_id) = iter.next() {
+                let removed = self.remove_entry_and_descendants(next_id);
+                for removed_id in removed.iter().map(|entry| entry.proposal_short_id()) {
+                    parents.remove(&removed_id);
                 }
+                ancestors_count = self
+                    .links
+                    .calc_relation_ids(parents.clone(), Relation::Parents)
+                    .len()
+                    + 1;
+                evicted.extend(removed);
+            } else {
+                break;
             }
-        } else {
-            return Err(Reject::ExceededMaximumAncestorsCount);
         }
 
         // some txs in `parents` are removed, now `ancestors` need to re-caculate,

tx-pool/src/component/tests/proposed.rs

@@ -695,6 +695,58 @@ fn test_max_ancestors_with_dep() {
     assert_eq!(pool.edges.inputs_len(), 1);
 }
 
+#[test]
+fn test_max_ancestors_evicts_stale_parent_descendant() {
+    let mut pool = PoolMap::new(2);
+    let dep_hash: Byte32 = h256!("0x1").into();
+
+    let tx1 = build_tx_with_dep(vec![(&Byte32::zero(), 0)], vec![(&dep_hash, 0)], 1);
+    let tx1_id = tx1.proposal_short_id();
+    let tx1_hash = tx1.hash();
+
+    let tx2 = build_tx(vec![(&tx1_hash, 0)], 1);
+    let tx2_id = tx2.proposal_short_id();
+    let tx2_hash = tx2.hash();
+
+    let tx3 = build_tx(vec![(&dep_hash, 0), (&tx2_hash, 0)], 1);
+    let tx3_id = tx3.proposal_short_id();
+
+    pool.add_proposed(TxEntry::new(
+        dummy_resolve(tx1, |_| None),
+        MOCK_CYCLES,
+        MOCK_FEE,
+        MOCK_SIZE,
+    ))
+    .unwrap();
+    pool.add_proposed(TxEntry::dummy_resolve(
+        tx2,
+        MOCK_CYCLES,
+        MOCK_FEE,
+        MOCK_SIZE,
+    ))
+    .unwrap();
+
+    let (inserted, evicted) = pool
+        .add_entry(
+            TxEntry::dummy_resolve(tx3, MOCK_CYCLES, MOCK_FEE, MOCK_SIZE),
+            Status::Proposed,
+        )
+        .unwrap();
+    assert!(inserted);
+
+    let evicted_ids: HashSet<_> = evicted
+        .iter()
+        .map(|entry| entry.proposal_short_id())
+        .collect();
+    assert_eq!(evicted_ids.len(), 2);
+    assert!(evicted_ids.contains(&tx1_id));
+    assert!(evicted_ids.contains(&tx2_id));
+    assert!(pool.get(&tx1_id).is_none());
+    assert!(pool.get(&tx2_id).is_none());
+    assert!(pool.get(&tx3_id).is_some());
+    assert!(pool.calc_ancestors(&tx3_id).is_empty());
+}
+
 #[test]
 fn test_container_bench_add_limits() {
     use rand::Rng;