ethereum: fix non-transitive trigger ordering causing sort panics

EthereumTrigger::cmp compared Call triggers (and Call/Log pairs) by
transaction_index, but compared Log/Log pairs by log_index alone,
ignoring transaction_index entirely. Those two comparisons only agree
when log_index happens to increase in lockstep with transaction_index
across the whole set being sorted, which isn't guaranteed for every
trigger source. When it doesn't hold, the combined ordering is not
transitive: a Log in an earlier transaction can end up compared as
greater than a Log in a later transaction if the earlier one has a
higher log_index within its own transaction.

This crashes graph-node in production with a panic from Rust's stable
sort ("user-provided comparison function does not correctly implement
a total order") inside BlockWithTriggers::new_with_triggers, which
calls trigger_data.sort() on a freshly-built Vec<Trigger>.

Fix the Log/Log comparison to key on transaction_index first, with
log_index only as a tie-breaker within the same transaction, matching
how Call/Call and Call/Log comparisons already work. Also rewrite the
Call/Log and Log/Call arms with Ordering::then to remove the duplicate
guarded/unguarded match arm pairs, now that both arms share the same
primary-then-secondary-key logic.

Added a regression test (test_trigger_ordering_is_transitive) that
constructs the minimal counter-example and asserts both the pairwise
comparisons and that sorting the resulting Vec doesn't panic.

Author: alter

chain/ethereum/src/tests.rs

@@ -235,3 +235,37 @@ fn test_trigger_dedup() {
 
     assert_eq!(block_with_triggers.trigger_data, expected);
 }
+
+#[test]
+fn test_trigger_ordering_is_transitive() {
+    // Regression test: `EthereumTrigger::cmp` compared `Call` triggers (and `Call`/`Log` pairs)
+    // by `transaction_index`, but compared `Log`/`Log` pairs by `log_index` alone, ignoring
+    // `transaction_index`. Those two comparisons only agree when `log_index` happens to increase
+    // in lockstep with `transaction_index`, which does not hold for every trigger source (for
+    // example log/call triggers that were fetched independently and merged). When it doesn't
+    // hold, the combined ordering isn't transitive, which crashes Rust's sort with "user-provided
+    // comparison function does not correctly implement a total order".
+    //
+    // Here, `log_a` is in an earlier transaction than `log_c` but has a higher `log_index`:
+    let log_a = EthereumTrigger::Log(LogRef::FullLog(create_log(5, 50), None));
+    let call_b = EthereumTrigger::Call(Arc::new(EthereumCall {
+        transaction_index: 5,
+        ..Default::default()
+    }));
+    let log_c = EthereumTrigger::Log(LogRef::FullLog(create_log(6, 10), None));
+
+    // `log_a` and `call_b` share a transaction, so `log_a < call_b` (events before calls in the
+    // same transaction). `call_b` is in an earlier transaction than `log_c`, so `call_b < log_c`.
+    // Transitivity requires `log_a < log_c`.
+    assert_eq!(log_a.cmp(&call_b), std::cmp::Ordering::Less);
+    assert_eq!(call_b.cmp(&log_c), std::cmp::Ordering::Less);
+    assert_eq!(
+        log_a.cmp(&log_c),
+        std::cmp::Ordering::Less,
+        "transitivity violated: log_a < call_b < log_c but log_a is not < log_c"
+    );
+
+    // The actual regression: sorting a `Vec` containing this combination used to panic.
+    let mut triggers = vec![log_c, call_b, log_a];
+    triggers.sort();
+}

chain/ethereum/src/trigger.rs

@@ -378,27 +378,28 @@ impl Ord for EthereumTrigger {
             // Calls are ordered by their tx indexes
             (Self::Call(a), Self::Call(b)) => a.transaction_index.cmp(&b.transaction_index),
 
-            // Events are ordered by their log index
-            (Self::Log(a), Self::Log(b)) => a.log_index().cmp(&b.log_index()),
-
-            // Calls vs. events are logged by their tx index;
-            // if they are from the same transaction, events come first
-            (Self::Call(a), Self::Log(b))
-                if a.transaction_index == b.transaction_index().unwrap() =>
-            {
-                Ordering::Greater
-            }
-            (Self::Log(a), Self::Call(b))
-                if a.transaction_index().unwrap() == b.transaction_index =>
-            {
-                Ordering::Less
-            }
-            (Self::Call(a), Self::Log(b)) => {
-                a.transaction_index.cmp(&b.transaction_index().unwrap())
-            }
-            (Self::Log(a), Self::Call(b)) => {
-                a.transaction_index().unwrap().cmp(&b.transaction_index)
-            }
+            // Events are ordered by their tx index first, and by their log index within a
+            // transaction. Comparing by tx index first (instead of log index alone) keeps this
+            // consistent with the Call/Log orderings below, which also key on tx index first;
+            // log index is not guaranteed to increase in lockstep with tx index for every
+            // trigger source, and a mismatch between the two would make the overall ordering
+            // non-transitive.
+            (Self::Log(a), Self::Log(b)) => a
+                .transaction_index()
+                .cmp(&b.transaction_index())
+                .then_with(|| a.log_index().cmp(&b.log_index())),
+
+            // Calls vs. events are ordered by their tx index; if they are from the same
+            // transaction, events come first.
+            (Self::Call(a), Self::Log(b)) => a
+                .transaction_index
+                .cmp(&b.transaction_index().unwrap())
+                .then(Ordering::Greater),
+            (Self::Log(a), Self::Call(b)) => a
+                .transaction_index()
+                .unwrap()
+                .cmp(&b.transaction_index)
+                .then(Ordering::Less),
         }
     }
 }