feat(sync): 009 - SubtreePrefetch Sync Strategy

[xilosada]

Summary

• Add SubtreePrefetch sync protocol types for deep trees with clustered changes
• Optimized for tree depth > 3, divergence < 20%, clustered changes
• O(1) round trips per subtree vs HashComparison's O(depth)

Types Added

• SubtreePrefetchRequest: Request subtrees by root hash with depth limit
• SubtreePrefetchResponse: Contains fetched subtrees and not-found roots
• SubtreeData: Single subtree with entities for CRDT merge
• should_use_subtree_prefetch(): Heuristic for protocol selection

Security

• MAX_SUBTREE_DEPTH (64): Prevents deep traversal attacks
• MAX_SUBTREES_PER_REQUEST (100): Limits request size
• MAX_ENTITIES_PER_SUBTREE (10,000): Bounds per-subtree data
• MAX_TOTAL_ENTITIES (100,000): Caps total response size
• is_valid() methods on all types for post-deserialization validation
• Saturating arithmetic in total_entity_count() to prevent overflow

Test Plan

• 39 unit tests covering:
  • Request/Response construction and serialization
  • Depth clamping and validation
  • Entity count limits
  • Memory exhaustion prevention
  • Overflow protection
  • Heuristic boundary conditions
  • Edge cases (empty, zeros, max values)

---

Note

Low Risk
Mostly additive sync wire/type definitions and tests with explicit size/depth validation; minimal impact on existing behavior aside from exposing new protocol types/constants.

Overview
Adds a new SubtreePrefetch sync protocol primitives module (sync/subtree.rs) with Borsh-serializable request/response/data types and a heuristic (should_use_subtree_prefetch) for choosing this strategy on deep, low-divergence, clustered changes.

Updates sync.rs to register the new submodule and re-export its types/constants, including explicit is_valid() bounds checks (depth, subtree counts, per-subtree and total entity limits) plus extensive unit tests covering serialization roundtrips and resource-exhaustion edge cases.

^{Written by Cursor Bugbot for commit abdf8ee. This will update automatically on new commits. Configure here.}

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 33% | Review time: 99.8s

🟡 1 warnings, 💡 1 suggestions, 📝 1 nitpicks. See inline comments.

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-cb995cc9}

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 33% | Review time: 110.2s

🟡 1 warnings, 💡 1 suggestions, 📝 1 nitpicks. See inline comments.

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-c5e2be46}

[cursor]

Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.

• ✅ Fixed: New threshold constants duplicate magic numbers in select_protocol
  • Replaced hardcoded magic numbers (3 and 0.2) in select_protocol() with imports of DEEP_TREE_THRESHOLD and MAX_DIVERGENCE_RATIO from subtree.rs to ensure a single source of truth.

Or push these changes by commenting:

@cursor push 174dba2cc8

Preview (174dba2cc8)

diff --git a/crates/node/primitives/src/sync/protocol.rs b/crates/node/primitives/src/sync/protocol.rs
--- a/crates/node/primitives/src/sync/protocol.rs
+++ b/crates/node/primitives/src/sync/protocol.rs
@@ -5,6 +5,7 @@
 use borsh::{BorshDeserialize, BorshSerialize};
 
 use super::handshake::{SyncCapabilities, SyncHandshake};
+use super::subtree::{DEEP_TREE_THRESHOLD, MAX_DIVERGENCE_RATIO};
 
 // =============================================================================
 // Protocol Kind (Discriminant-only)
@@ -236,7 +237,11 @@
     }
 
     // Rule 4: Deep tree with localized changes
-    if remote.max_depth > 3 && divergence < 0.2 {
+    #[expect(
+        clippy::cast_possible_truncation,
+        reason = "DEEP_TREE_THRESHOLD is always small (currently 3)"
+    )]
+    if remote.max_depth > DEEP_TREE_THRESHOLD as u32 && divergence < MAX_DIVERGENCE_RATIO {
         return ProtocolSelection {
             protocol: SyncProtocol::SubtreePrefetch {
                 subtree_roots: vec![], // Will be populated during sync

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 33% | Review time: 133.2s

🟡 1 warnings, 📝 1 nitpicks. See inline comments.

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-e7c1cabf}

[meroreviewer]

🟡 Test doesn't actually verify total entity limit

The test creates 101 subtrees (100,000/1,000 + 1), which exceeds MAX_SUBTREES_PER_REQUEST (100), so is_valid() fails on the subtree count check before ever reaching the total entity count check.

Suggested fix:

Use fewer subtrees (e.g., 20) with more entities each (e.g., 5,001) so total exceeds 100,000 while each subtree stays under 10,000 and subtree count stays under 100.

[meroreviewer]

📝 Nit: Consider documenting NaN/Inf behavior for divergence_ratio

The heuristic function silently rejects NaN values (returns false) since NaN < anything is false; this is probably correct but undocumented.

Suggested fix:

Add a note in the doc comment that NaN/Inf values will cause the function to return false.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before Autofix could start.}

[cursor]

Test passes for wrong reason, total entity limit untested

Medium Severity

The test_subtree_response_validation_total_entity_limit test doesn't actually exercise the MAX_TOTAL_ENTITIES check. With entities_per_subtree = 1000, num_subtrees computes to (100_000 / 1000) + 1 = 101, which exceeds MAX_SUBTREES_PER_REQUEST (100). So is_valid() returns false at the subtree count check (line 287) before ever reaching the total entity count check (line 295). The assertion passes for the wrong reason, leaving the MAX_TOTAL_ENTITIES validation effectively untested — if that check were removed, no test would catch it.

Additional Locations (1)

• crates/node/primitives/src/sync/subtree.rs#L284-L301