causal sorting

[insipx]

Add causal sorting of envelopes and dependency extraction via xmtp_api_d14n::protocol::sort::causal and Envelope::depends_on()

Introduce causal in-place sorting that removes envelopes with unmet dependencies and advances TopicCursor; add DependsOnExtractor and extend Envelope with cursor() and depends_on(); update VectorClock and GlobalCursor to support apply(&Cursor); expose TopicCursor and rework Sort to return Result<Option<Missing>, EnvelopeError>.

📍Where to Start

Start with the causal sorter entrypoint causal(...) in [file:xmtp_api_d14n/src/protocol/sort/causal.rs] and then review trait changes in [file:xmtp_api_d14n/src/protocol/traits/sort.rs] and [file:xmtp_api_d14n/src/protocol/traits/envelopes.rs].

---

Macroscope summarized de38fb0.

[insipx]

This PR is part of a stack of 11 bookmarks:

1. trunk()
2. insipx/dependency-resolution
3. insipx/causal-sort ← this PR
4. insipx/resolution
5. insipx/network-backoff
6. insipx/ordered-combinator
7. insipx/test-resolution
8. insipx/test-ordering
9. insipx/add-published-in-epoch
10. insipx/sql-queries
11. insipx/sync-conn
12. insipx/set-dependencies

---

Created with jj-stack

[claude]

Claude finished @insipx's task —— View job

---

PR Review - Causal Sorting Implementation

Reviewed at 2025-11-13 21:38 UTC

Todo List

• Read core implementation files
• Analyze code quality and identify issues
• Post review feedback

---

Code Quality & Architecture

Strong points:

• Clean separation of concerns with trait-based design (Sort, VectorClock, Envelope)
• Well-documented causal sort algorithm with ASCII diagrams
• Good use of property-based testing with proptest

Potential Issues

1. Memory inefficiency in causal sort (xmtp_api_d14n/src/protocol/sort/causal.rs:24)

missing.push(self.envelopes.remove(i));

The Vec::remove(i) operation is O(n) as it shifts all elements. With potentially many missing dependencies, this creates O(n²) worst-case behavior. Consider using swap_remove or restructuring to process in reverse order if ordering of missing envelopes doesn't matter.

2. Clone overhead in depends_on extractor (xmtp_api_d14n/src/protocol/extractors/depends_on.rs:40)

self.cursor = e.aad.as_ref().and_then(|a| a.depends_on.clone().map(Into::into));

The GlobalCursor clone happens on every extraction. Given that GlobalCursor wraps a HashMap, this could be expensive for large dependency graphs. The TODO comment references issue #2691, but this should be addressed.

3. Inconsistent error handling in TopicCursor::get_or_default (xmtp_proto/src/types/topic_cursor.rs:16)

pub fn get_or_default(&mut self, topic: &Topic) -> &GlobalCursor {
    self.inner.entry(topic.clone()).or_default()
}

This clones the Topic on every call. Consider taking topic: Topic by value since it's cloned anyway, or document why the reference is needed.

4. Missing documentation on TopicCursor::add (xmtp_proto/src/types/topic_cursor.rs:35)

The add method lacks documentation explaining when to use it vs. ApplyCursor::apply. The difference in behavior (direct insert vs. merge) isn't obvious.

Performance Considerations

VectorClock::dominates optimization (xmtp_api_d14n/src/protocol/impls/vector_clock.rs:8-10)

fn dominates(&self, other: &Self) -> bool {
    other.iter().all(|(&node, &seq)| self.get(&node) >= seq)
}

This iterates over other and calls self.get(&node) which does a HashMap lookup each time. For large clocks, this could be optimized by early-exit on first non-dominating node.

Test Coverage

Strengths:

• Excellent property-based tests with sophisticated strategies
• Tests verify both correct sorting and missing dependency detection
• Good use of #[should_panic] to document limitations

Gaps (per codecov):

• DependsOnExtractor has 0% coverage - needs unit tests
• TopicCursor::add method is untested
• Several VectorClock trait methods lack direct tests

Specific recommendations:

1. Add unit tests for DependsOnExtractor::visit_client with various AAD configurations
2. Test TopicCursor::get_or_default with multiple topics
3. Add tests for VectorClock::merge and compare edge cases (empty clocks, concurrent updates)

Security

No security concerns identified. The implementation correctly handles untrusted input through proper validation and doesn't expose sensitive data.

Minor Issues

1. Proptest regression file - Good practice including regression seeds, but the file shows recent failures. Ensure these are expected test cases or fix the underlying issues.

2. Timestamp::sort uses sort_unstable_by_key (xmtp_api_d14n/src/protocol/sort/timestamp.rs:14) - This is correct for performance, but the unstable sort means envelopes with identical timestamps may be reordered. Document this behavior if it matters for protocol correctness.

Summary

The implementation is solid with good architecture and testing. Main concerns are performance optimizations (O(n²) removal, cloning overhead) and test coverage gaps in the new extractors. Consider addressing the clone issues referenced in #2691 before merging if this code will process large message batches.

---

[codecov]

Codecov Report

❌ Patch coverage is 65.67797% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.27%. Comparing base (281e730) to head (de38fb0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
xmtp_api_d14n/src/protocol/sort/causal.rs	77.63%	34 Missing ⚠️
...mtp_api_d14n/src/protocol/extractors/depends_on.rs	0.00%	13 Missing ⚠️
xmtp_api_d14n/src/protocol/traits/envelopes.rs	27.77%	13 Missing ⚠️
xmtp_api_d14n/src/protocol/impls/vector_clock.rs	0.00%	7 Missing ⚠️
xmtp_proto/src/types/topic_cursor.rs	50.00%	6 Missing ⚠️
xmtp_api_d14n/src/protocol/sort/timestamp.rs	50.00%	4 Missing ⚠️
xmtp_api_d14n/src/protocol/traits/sort.rs	71.42%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2770      +/-   ##
==========================================
- Coverage   77.28%   77.27%   -0.02%     
==========================================
  Files         371      375       +4     
  Lines       53461    53692     +231     
==========================================
+ Hits        41316    41488     +172     
- Misses      12145    12204      +59     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.