feat(sync): rfc-003 phase a.2.2.1 — payload_hash + hlc total-order helpers - #54
Conversation
…lpers Phase A.2.2 building block. Three pure functions in a new src/payload_hash.rs module that the apply pipeline handlers will consume in A.2.2.2 / A.2.2.3. - canonical_serialize(fields, hlc, origin_device_id) -> Vec<u8> Wraps the synced entity fields under a top-level "fields" key alongside "hlc" + "origin_device_id" so a field name colliding with the HLC names can't shadow the clock. Keys sorted recursively via BTreeMap so the byte form is identical on every platform. Arrays keep source order (multi-artist tag order is semantically significant). - compute_payload_hash(...) -> [u8; 32] BLAKE3-256 over the canonical bytes, sized for direct BYTEA bind into Postgres without a hex round-trip. - hlc_strict_gt(incoming, existing) -> bool RFC-003 §2 total order on (wall, logical, origin_device_id). Uses Rust's derived lex Ord, which gives None < Some(any UUID) automatically — matches A.1.1's "legacy backfilled rows lose to any v2 op" intent. Module is unit-test only (no Postgres). 11 cases cover key-order determinism, field-value sensitivity, HLC sensitivity, origin UUID sensitivity, array-order preservation, nested object sort, plus each branch of the total-order comparator (wall / logical / origin_device_id tiebreaks, None < Some, equal-triple no-op). Pure addition — no apply pipeline change yet. A.2.2.2 wires this into the profile-scoped handlers (playlist / library / track); A.2.2.3 wires into the user-scoped ones (liked / rating). Signed-off-by: InstaZDLL <github.105mh@8shield.net>
📝 WalkthroughWalkthroughAjout du fichier ModificationsModule payload_hash : ordre total HLC + hash canonique BLAKE3
Diagramme de séquencesequenceDiagram
participant Appelant
participant canonical_serialize
participant canonicalize
participant compute_payload_hash
participant blake3
rect rgba(70, 130, 180, 0.5)
Note over Appelant,canonicalize: Sérialisation canonique
Appelant->>canonical_serialize: fields, hlc, origin_device_id
canonical_serialize->>canonicalize: Value JSON brut
canonicalize-->>canonical_serialize: Value avec clés triées (BTreeMap)
canonical_serialize-->>Appelant: octets canoniques
end
rect rgba(60, 179, 113, 0.5)
Note over Appelant,blake3: Calcul du hash
Appelant->>compute_payload_hash: fields, hlc, origin_device_id
compute_payload_hash->>canonical_serialize: délègue
canonical_serialize-->>compute_payload_hash: octets canoniques
compute_payload_hash->>blake3: hash(bytes)
blake3-->>compute_payload_hash: hash 32 octets
compute_payload_hash-->>Appelant: hash BYTEA Postgres
end
Effort de revue estimé🎯 3 (Moderate) | ⏱️ ~20 minutes PRs potentiellement liées
Labels suggérés
Poème
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Review fix on #54: the wrapper map + the hlc sub-map were only sorted by insertion-order accident — "fields" / "hlc" / "origin_device_id" and "logical" / "wall" happened to be inserted in alphabetical order, so the byte form was deterministic. But if serde_json gets compiled with the `preserve_order` feature (IndexMap backing for Map), insertion order is the serialisation order, and a future edit that swapped two inserts would silently flap every existing hash. Fix: run the FULL tree through `canonicalize()` at the end so the top-level wrapper + hlc sub-map go through the BTreeMap sort explicitly, regardless of how `serde_json::Map` is backed. New test `canonical_serialize_top_level_keys_are_sorted` asserts on the raw byte form to lock the lex order in — catches a future regression where someone removes the canonicalize wrap. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/payload_hash.rs`:
- Around line 43-140: Move the pure RFC-003 protocol logic functions and types
from this file to the waveflow-core repository instead of keeping them in
waveflow-server. Specifically, extract and relocate the HlcTriple struct,
hlc_strict_gt function, canonical_serialize function, and compute_payload_hash
function to waveflow-core since they implement reusable protocol rules with no
server-specific dependencies. Remove these definitions from src/payload_hash.rs
and remove any exports of these functions from src/lib.rs, then add the
necessary imports from waveflow-core in the server code where these functions
are needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ae037fe-775a-451d-b754-6cc4bcdfe9d1
📒 Files selected for processing (2)
src/lib.rssrc/payload_hash.rs
What
Phase A.2.2 building block. Three pure functions in a new
src/payload_hash.rsmodule that the apply pipeline handlers will consume in A.2.2.2 (profile-scoped:playlist/library/track) and A.2.2.3 (user-scoped:liked/rating).Follows #50 / #51 / #52 / #53. Pure addition — no apply pipeline change yet.
API
canonical_serialize(fields, hlc, origin_device_id)Vec<u8>fields/hlc/origin_device_idcompute_payload_hash(fields, hlc, origin_device_id)[u8; 32]BYTEAbindhlc_strict_gt(incoming, existing)bool(wall, logical, origin_device_id)HlcTriplewraps the three §2 components and derivesOrdso the comparator is just a tuple>.Design notes
hlc/origin_device_idcan't shadow the clock. Hash stays unambiguous.BTreeMaprecursive sort for objects — bytes-identical across platforms regardless of source serde_json emission order.[Tyler, Earl]is semantically distinct from[Earl, Tyler]. Sorting them here would let the apply pipeline silently swap primary / secondary on re-emit.Option<Uuid>tiebreaker uses Rust's derivedOrd(None < Some(any UUID)), which matches A.1.1's "legacy backfilled rows lose to any v2 op" intent — legacy rows backfill withorigin_device_id = NULL, every v2 op then strictly outranks them on tiebreak.What this does NOT do
metadata_digest_versionbump helper — that's a thin SQL helper that lives insrc/db.rsnext to the entity writes (added in A.2.2.2).payload_hashon legacy rows is Phase B.Test plan
cargo check --all-targets --all-features— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test --all-features --lib payload_hash— 11 unit tests pass (~10 ms, no Postgres):canonical_serialize_is_deterministic_across_key_ordercanonical_serialize_changes_with_field_valuecanonical_serialize_changes_with_hlccanonical_serialize_changes_with_origin_device_idcanonical_serialize_array_order_is_preservedcanonical_serialize_nested_objects_are_sortedhlc_strict_gt_compares_wall_firsthlc_strict_gt_tiebreaks_on_logicalhlc_strict_gt_tiebreaks_on_origin_device_idhlc_strict_gt_none_loses_to_somehlc_strict_gt_rejects_equal_tripleRefs
payload_hashconstruction)Summary by CodeRabbit
Notes de version