feat: add witnessed suffix admission shell skeleton#322
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds serialization infrastructure for witnessed suffix admission judgment, introducing ABI request/response/outcome types with strict CBOR encoding, domain logic with deterministic Blake3 digest computation for overlap verification, and comprehensive round-trip and negative-case tests across both layers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 19 minutes and 30 seconds.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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/warp-core/src/witnessed_suffix.rs`:
- Around line 397-401: The helper len_to_u64 currently masks conversion failures
by returning u64::MAX which creates ambiguous, non‑deterministic encodings;
change len_to_u64 to fail deterministically on overflow instead of saturating:
replace the Err branch with a hard failure (e.g. use u64::try_from(len).expect
or unwrap with a clear message) so any usize > u64::MAX panics immediately;
update callers if they expect a Result, but prefer keeping the function
signature u64 len_to_u64(usize) that panics on overflow and include a clear
error message referencing len_to_u64.
🪄 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
Run ID: 2c9725c2-ad3f-4867-b1bb-5d601ffde311
📒 Files selected for processing (6)
crates/echo-wasm-abi/src/kernel_port.rscrates/echo-wasm-abi/src/lib.rscrates/echo-wasm-abi/src/witnessed_suffix_tests.rscrates/warp-core/src/lib.rscrates/warp-core/src/witnessed_suffix.rscrates/warp-core/src/witnessed_suffix_tests.rs
![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: 9e23ed4d4d
ℹ️ 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".
| /// Target basis used while judging admission. | ||
| pub target_basis: ProvenanceRef, | ||
| /// Optional target-basis evidence for strand/parent realization cases. | ||
| pub basis_report: Option<SettlementBasisReport>, |
There was a problem hiding this comment.
WitnessedSuffixAdmissionRequest is marked with deny_unknown_fields, but its basis_report payload reuses SettlementBasisReport, which does not deny unknown keys. In JSON/self-describing formats, this means extra fields inside basis_report are still silently accepted, so the new admission boundary does not fully block field smuggling when callers nest unexpected transport/sync/status data under that object.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks. I am not applying this in this PR because it would tighten pre-existing settlement DTO decode behavior outside the witnessed-suffix shell skeleton. The shell DTOs added here use #[serde(deny_unknown_fields)] at their own boundary, and the nested SettlementBasisReport reuse is intentionally existing ABI vocabulary rather than a broad ABI strictness redesign. This should be handled separately if we decide all settlement evidence DTOs should reject unknown fields.
|
Follow-up fixes pushed after initial PR checks:
Local verification after these fixes:
Push hooks also passed the local critical gates on both follow-up pushes. |
1 participant
Summary
Adds the first shape-only witnessed suffix admission shell skeleton.
This PR follows the METHOD sequence already landed on main:
7adf880 docs: design witnessed suffix admission shella6802f2 test: add red tests for witnessed suffix admission shell9e23ed4 feat: add witnessed suffix admission shell skeletonWhat changed
WitnessedSuffixShell,WitnessedSuffixAdmissionRequest,WitnessedSuffixAdmissionResponse, andWitnessedSuffixAdmissionOutcome.WorldlineId,WorldlineTick,ProvenanceRef, settlement basis evidence, overlap revalidation evidence,ConflictReason, and reading residual posture vocabulary.#[serde(deny_unknown_fields)]on the new ABI DTOs so transport/sync/status field smuggling is rejected at the boundary.Non-goals preserved
Verification
cargo fmt --all -- --checkcargo test -p echo-wasm-abi --lib witnessed_suffixcargo test -p warp-core --lib witnessed_suffixcargo clippy -p warp-core --all-targets -- -D warnings -D missing_docsPush hook also passed the full local gate: fmt, guards, clippy-core, runtime tests, warp-core tests, and rustdoc.
Summary by CodeRabbit
Release Notes
New Features
Tests