Harden trusted runtime WAL recovery integrity#661
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Code Lawyer post-merge auditFollow-up to #659. The merged branch was green, but the final audit identified the following remaining integrity defects:
Local gate
@codex review please @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 661 --repo flyingrobots/echo --json title,body,files,commits,additions,deletions,changedFiles > /tmp/pr.json
cat /tmp/pr.json | jq '.title, .additions, .deletions, .changedFiles'
jq -r '.files[].path' /tmp/pr.jsonLength of output: 598 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr diff 661 --repo flyingrobots/echo > /tmp/pr.diff
wc -l /tmp/pr.diff
sed -n '1,400p' /tmp/pr.diffLength of output: 18978 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '400,499p' /tmp/pr.diffLength of output: 4973 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "witnessed_submissions" crates/warp-core/src/trusted_runtime_host.rs | head -50Length of output: 1539 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '200,270p' crates/warp-core/src/trusted_runtime_host.rs
echo ---
sed -n '1150,1200p' crates/warp-core/src/trusted_runtime_host.rsLength of output: 5485 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,90p' docs/topics/WAL.md | grep -n "^First\|^Second\|^Third\|^Fourth\|^Fifth\|^Sixth\|^Seventh\|^Eighth"Length of output: 855 Confirmed: exactly seven items are enumerated, matching the corrected count. The implementation checks out. Everything lines up with the stated objectives.
Minor observations, non-blocking:
No correctness issues found. LGTM. ✅ Action performedReview finished.
|
Code Lawyer Activity SummaryFinal reviewed head:
Gate
The repository still reports |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d0571fa9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .find(|frame| frame.header.record_kind == WalRecordKind::SubmissionAcceptedRecorded) | ||
| .ok_or_else(missing_trusted_runtime_record)?; | ||
| let frame = | ||
| required_unique_transaction_frame(transaction, WalRecordKind::SubmissionAcceptedRecorded)?; |
There was a problem hiding this comment.
With a committed SubmissionIntake transaction that contains two identical SubmissionAcceptedRecorded frames, this new uniqueness check is not exercised by TrustedRuntimeWal::recover_read_only: that path builds submissions via recover_submission_index(&report), which scans every acceptance frame and treats identical repeats as idempotent. As a result, read-only recovery can still accept and expose a duplicate singular acceptance claim even though the new WAL invariant says duplicate singular authority frames must be rejected; enforce the same per-transaction uniqueness before or inside the recovered submission index path.
Useful? React with 👍 / 👎.
| .iter() | ||
| .map(|record| (record.submission.submission_id, &record.envelope)) | ||
| .collect::<BTreeMap<_, _>>(); | ||
| for correlation in receipt_correlations { |
There was a problem hiding this comment.
When a scheduler tick uses the legacy digest-only RuntimeStateDeltaRecorded payload, recover_runtime_state_delta_material records the state delta as missing and never emits a ReceiptCorrelationPersistenceRecord, but recover_read_only has already populated recovery.receipts from recover_receipt_index. Because this validation loop only checks receipt_correlations, a WAL can pair a retained envelope with mismatched correlation parents plus a legacy state delta and still expose the bad parent set through the public receipt index; validate the raw receipt-index correlations as well or reject before returning the recovery report.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
This is a post-merge integrity follow-up to #659. The final audit found that correlation-parent validation occurred during runtime restoration but not at the public
TrustedRuntimeWal::recover_read_onlyboundary. It also found that the duplicate-frame correction covered tick receipts and correlations but left submission acceptance, retained-envelope, and state-delta cardinality inconsistent.The result is one validation boundary for recovered causal-parent evidence and one exact-cardinality helper for every singular authority frame.
Validation
cargo +1.90.0 xtask pr-preflight --base origin/maincargo test -p warp-core --lib --features native_rule_bootstrap,trusted_runtime trusted_runtime_host::tests:: -- --nocapturecargo test -p warp-core --test external_consumer_contract_fixture_tests --features native_rule_bootstrap,trusted_runtime inverse_intent_resolves_one_admitted_transition_after_restart -- --exact --nocapturecargo test -p warp-core --test trusted_runtime_host_loop_tests --features native_rule_bootstrap,trusted_runtime,host_testcargo clippy -p warp-core --lib --features native_rule_bootstrap,trusted_runtime,host_test -- -D warnings -D missing_docs