Protect score from outside assignment and document the requirements

backend/rust-gbt/src/audit_transaction.rs

@@ -21,7 +21,8 @@ pub struct AuditTransaction {
     pub ancestor_fee: u64,
     pub ancestor_weight: u32,
     pub ancestor_sigops: u32,
-    pub score: f64,
+    // Safety: Must be private to prevent NaN breaking Ord impl.
+    score: f64,
     pub used: bool,
     pub modified: bool,
     pub dirty: bool,
@@ -53,6 +54,9 @@ impl PartialOrd for AuditTransaction {
 
 impl Ord for AuditTransaction {
     fn cmp(&self, other: &AuditTransaction) -> Ordering {
+        // Safety: The only possible values for score are f64
+        // that are not NaN. This is because outside code can not
+        // freely assign score. Also, calc_new_score guarantees no NaN.
         self.partial_cmp(other).expect("score will never be NaN")
     }
 }
@@ -80,4 +84,20 @@ impl AuditTransaction {
             dirty: false,
         }
     }
+
+    #[inline]
+    pub fn score(&self) -> f64 {
+        self.score
+    }
+
+    /// Safety: This function must NEVER set score to NaN.
+    #[inline]
+    pub fn calc_new_score(&mut self) {
+        self.score = (self.ancestor_fee as f64)
+            / (if self.ancestor_weight == 0 {
+                1.0
+            } else {
+                self.ancestor_weight as f64 / 4.0
+            });
+    }
 }

backend/rust-gbt/src/gbt.rs

@@ -185,7 +185,7 @@ pub fn gbt(mempool: &mut ThreadTransactionsMap) -> Option<GbtResult> {
                             *overflowed,
                             TxPriority {
                                 uid: *overflowed,
-                                score: overflowed_tx.score,
+                                score: overflowed_tx.score(),
                             },
                         );
                     } else {
@@ -261,12 +261,7 @@ fn set_relatives(txid: u32, audit_pool: &mut AuditPool) {
         tx.ancestor_fee = tx.fee + total_fee;
         tx.ancestor_weight = tx.weight + total_weight;
         tx.ancestor_sigops = tx.sigops + total_sigops;
-        tx.score = (tx.ancestor_fee as f64)
+        tx.calc_new_score();
-            / (if tx.ancestor_weight == 0 {
-                1.0
-            } else {
-                tx.ancestor_weight as f64 / 4.0
-            });
         tx.relatives_set_flag = true;
     }
 }
@@ -303,30 +298,25 @@ fn update_descendants(
             descendant.ancestor_fee -= root_fee;
             descendant.ancestor_weight -= root_weight;
             descendant.ancestor_sigops -= root_sigops;
-            let current_score = descendant.score;
+            let current_score = descendant.score();
-            descendant.score = (descendant.ancestor_fee as f64)
+            descendant.calc_new_score();
-                / (if descendant.ancestor_weight == 0 {
-                    1.0
-                } else {
-                    descendant.ancestor_weight as f64 / 4.0
-                });
             descendant.dependency_rate = descendant.dependency_rate.min(cluster_rate);
             descendant.modified = true;
             // update modified priority if score has changed
-            if !descendant.modified || descendant.score < current_score {
+            if !descendant.modified || descendant.score() < current_score {
                 modified.push_decrease(
                     descendant.uid,
                     TxPriority {
                         uid: descendant.uid,
-                        score: descendant.score,
+                        score: descendant.score(),
                     },
                 );
-            } else if descendant.score > current_score {
+            } else if descendant.score() > current_score {
                 modified.push_increase(
                     descendant.uid,
                     TxPriority {
                         uid: descendant.uid,
-                        score: descendant.score,
+                        score: descendant.score(),
                     },
                 );
             }