Skip to content

Commit 4cbad7a

Browse files
carlaKCclaude
andcommitted
simln-lib: give PaymentEvent a total ordering
The payment event heap previously ordered events by execution time alone. When two events are scheduled for the same instant, the BinaryHeap pop order was unspecified, making otherwise-seeded runs non-reproducible. Break ties on the source node's public key. The heap holds at most one event per source at a time, so (execution_time, source) is a total order, and Eq is made consistent with the new Ord. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 66daf69 commit 4cbad7a

1 file changed

Lines changed: 40 additions & 2 deletions

File tree

simln-lib/src/lib.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,12 @@ struct ExecutorPaymentTracker {
644644

645645
impl Ord for PaymentEvent {
646646
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
647-
self.execution_time.cmp(&other.execution_time)
647+
// Order primarily by execution time, then break ties on the source node's public key. The heap only ever holds
648+
// at most one event per source at a time, so `(execution_time, source)` is a total order. Without the tie-break,
649+
// events scheduled for the same instant pop in an unspecified order, which makes runs non-reproducible.
650+
self.execution_time
651+
.cmp(&other.execution_time)
652+
.then_with(|| self.source.cmp(&other.source))
648653
}
649654
}
650655

@@ -656,7 +661,7 @@ impl PartialOrd for PaymentEvent {
656661

657662
impl PartialEq for PaymentEvent {
658663
fn eq(&self, other: &Self) -> bool {
659-
self.execution_time == other.execution_time
664+
self.execution_time == other.execution_time && self.source == other.source
660665
}
661666
}
662667

@@ -1669,6 +1674,39 @@ mod tests {
16691674
assert_eq!(seq1, seq1_again);
16701675
}
16711676

1677+
#[test]
1678+
fn test_payment_event_orders_ties_by_source() {
1679+
use crate::PaymentEvent;
1680+
use std::cmp::Reverse;
1681+
use std::collections::BinaryHeap;
1682+
use std::time::{Duration, SystemTime};
1683+
1684+
let nodes = test_utils::create_nodes(2, 100_000);
1685+
let (mut low, mut high) = (nodes[0].0.clone(), nodes[1].0.clone());
1686+
// Order our two nodes so that `low` has the smaller public key.
1687+
if low.pubkey > high.pubkey {
1688+
std::mem::swap(&mut low, &mut high);
1689+
}
1690+
1691+
let when = SystemTime::UNIX_EPOCH + Duration::from_secs(10);
1692+
let event = |source: &NodeInfo, destination: &NodeInfo| PaymentEvent {
1693+
source: source.pubkey,
1694+
execution_time: when,
1695+
destination: destination.clone(),
1696+
amount: 1_000,
1697+
};
1698+
1699+
// Push the higher-keyed source first to prove that pop order is decided by the tie-break, not by
1700+
// insertion order.
1701+
let mut heap: BinaryHeap<Reverse<PaymentEvent>> = BinaryHeap::new();
1702+
heap.push(Reverse(event(&high, &low)));
1703+
heap.push(Reverse(event(&low, &high)));
1704+
1705+
// Both events share an execution time, so the min-heap must pop the smaller public key first.
1706+
assert_eq!(heap.pop().unwrap().0.source, low.pubkey);
1707+
assert_eq!(heap.pop().unwrap().0.source, high.pubkey);
1708+
}
1709+
16721710
mock! {
16731711
pub Generator {}
16741712

0 commit comments

Comments
 (0)