Conversation
WalkthroughUpdate Holochain crate versions in two Cargo.toml files, add an extern validate callback that verifies parents of PerspectiveDiffEntryReference on StoreRecord (returning UnresolvedDependencies when parents are missing), and replace a knownLinkLanguages entry in two mainnet seed JSON files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Node
participant HC as Holochain Host
participant IZ as Integrity Zome (perspective_diff_sync_integrity)
Node->>HC: StoreRecord(entry: PerspectiveDiffEntryReference)
HC->>IZ: validate(op: StoreRecord)
rect rgba(220,235,255,0.35)
note over IZ: Extract parents from entry\nQuery/check each parent record
IZ->>HC: get_record(parent_hash) [repeated]
alt Any parent missing
IZ-->>HC: ValidateCallbackResult::UnresolvedDependencies (Hashes: missing)
HC-->>Node: Reject StoreRecord (UnresolvedDependencies)
else All parents present
IZ-->>HC: ValidateCallbackResult::Valid
HC-->>Node: Accept StoreRecord
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bootstrap-languages/p-diff-sync/hc-dna/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/Cargo.toml(1 hunks)bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/Cargo.toml(1 hunks)bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs(1 hunks)cli/mainnet_seed.json(1 hunks)rust-executor/src/mainnet_seed.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
rust-executor/src/mainnet_seed.json (1)
7-7: Mirrors CLI seed update; keep seeds in lockstep.Change matches cli/mainnet_seed.json. Keep these in sync going forward.
You can reuse the verification script from cli/mainnet_seed.json to confirm parity.
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/Cargo.toml (1)
16-19: Holochain crate bump mirrored; good.Integrity and coordinator zomes are aligned on versions and moved SB to registry pin. Proceed.
Use the same rg-based check from the other Cargo.toml comment to confirm alignment.
cli/mainnet_seed.json (1)
7-7: Verified — knownLinkLanguages CID updated and no stale refs remain. cli/mainnet_seed.json and rust-executor/src/mainnet_seed.json both contain QmzSYwdePGBz4TB2fB2Zwh5YQnX9WJBwXctajWfM9UuREdFJ2LH; no occurrences of the old CID QmzSYwdiuaBqw812TNcudpKwvU5U3HM9tdxWqjhkp26XEc228xe found in the repo.bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/Cargo.toml (1)
25-27: Holochain crate bump: cross‑zome versions verified.
hdk=0.5.6, hdi=0.6.6, holochain_serialized_bytes="=0.0.56" are consistent between bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/Cargo.toml and perspective_diff_sync_integrity/Cargo.toml; holo_hash is declared only in the integrity zome and is 0.5.6 (matches hdk).
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs (1)
246-255: Return UnresolvedDependencies for missing parents (don’t hard-fail).If a parent hasn’t gossiped yet,
must_get_valid_recordwill err; returningInvalidpermanently rejects the commit. UseUnresolvedDependenciesso validation retries when parents arrive.Apply:
- if let Some(parents) = pdiff_ref.parents { - for parent_action_hash in parents { - // Ensure each declared parent exists and is valid in the source chain/DHT - if must_get_valid_record(parent_action_hash).is_err() { - return Ok(ValidateCallbackResult::Invalid( - "Parent action not found or invalid".into(), - )); - } - } - } + if let Some(parents) = pdiff_ref.parents { + let mut missing: Vec<AnyDhtHash> = Vec::new(); + for parent_action_hash in parents { + if let Err(_) = must_get_valid_record(parent_action_hash.clone()) { + missing.push(parent_action_hash.into()); + } + } + if !missing.is_empty() { + return Ok(ValidateCallbackResult::UnresolvedDependencies(missing)); + } + }
🧹 Nitpick comments (3)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs (3)
247-254: Optional: Ensure parents are PDERR entries (defense-in-depth).After fetching the parent record, verify its entry decodes to
PerspectiveDiffEntryReference; otherwise reject as invalid reference.Apply atop the earlier change:
- for parent_action_hash in parents { - if let Err(_) = must_get_valid_record(parent_action_hash.clone()) { - missing.push(parent_action_hash.into()); - } - } + for parent_action_hash in parents { + match must_get_valid_record(parent_action_hash.clone()) { + Ok(parent_record) => { + let is_pdiff = parent_record + .entry() + .to_app_option::<PerspectiveDiffEntryReference>() + .ok() + .flatten() + .is_some(); + if !is_pdiff { + return Ok(ValidateCallbackResult::Invalid( + "Parent action does not contain a PerspectiveDiffEntryReference entry".into(), + )); + } + } + Err(_) => missing.push(parent_action_hash.into()), + } + }
236-263: Refactor duplication into a helper (keeps StoreRecord/StoreEntry paths in sync).Extract a small helper to validate parents from a
PerspectiveDiffEntryReference; call it from both match arms.I can draft the helper and wire-up calls if you want.
236-263: Sanity check complete — only this file defines validate and it only handles Op::StoreRecord.Location: bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs (validate). No other validate callbacks or Op::StoreEntry matches found in the repo.
If you need compatibility with hosts that emit Op::StoreEntry, add a matching Op::StoreEntry arm mirroring the StoreRecord branch; otherwise no change required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rs
Show resolved
Hide resolved
2 participants
Adds an integrity zome
validatecallback that enforces parent existence forPerspectiveDiffEntryReferenceentries in the p-diff-sync bootstrap language.bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync_integrity/src/lib.rsmust_get_valid_record()duringOp::StoreRecordto verify each parent.Problem
Without validation, the DHT can accept a diff that references parents which are not yet present. If an agent shares later commits in the linearized history and goes offline before earlier commits are published, peers may end up in a state where subsequent syncs never converge: newer data references missing ancestors that never arrive, causing resolution to stall.
Solution
validatecallback that:PerspectiveDiffEntryReferencefrom theStoreRecordop.must_get_valid_record()to assert presence and validity.Summary by CodeRabbit
New Features
Chores