feat(research): verify immutable ready-event catalog - #348
Conversation
|
Warning Review limit reached
Next review available in: 23 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds authenticated Polymarket evidence objects, ChangesPolymarket evidence catalog
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Catalog
participant Qualification
participant EvidenceVerifier
participant ReceiptStore
Catalog->>Qualification: authenticate and parse qualification JSON
Catalog->>EvidenceVerifier: seal and verify evidence triplet
EvidenceVerifier-->>Catalog: return sequence, coverage, and digest metadata
Catalog->>Catalog: classify qualification and evidence compatibility
Catalog->>ReceiptStore: append receipt by receipt_sha256
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f90b04249
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs (1)
478-484: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid recomputing the SHA-256 for the returned digest.
The digest of
bytesis already computed on Line 478 for the equality check, and after it passes it is provably equal toexpected(and to the validatedexpected_sha256). Rehashing up to 4 MiB on Line 482 is redundant.♻️ Reuse the validated digest
Ok(AuthenticatedPolymarketEvidenceObject { - sha256: format!("{:x}", Sha256::digest(&bytes)), + sha256: hex_digest(expected), bytes, })🤖 Prompt for 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. In `@rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs` around lines 478 - 484, Reuse the SHA-256 digest already computed for the equality check in the artifact validation flow instead of calling Sha256::digest(&bytes) again when constructing AuthenticatedPolymarketEvidenceObject. After validation succeeds, derive the returned sha256 string from the validated digest or expected value while preserving the existing mismatch error and bytes field.
🤖 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
`@rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/catalog.rs`:
- Around line 286-307: Update classify and its caller verify_and_append so
carrier schema, empty verifier contract, missing contract, and invalid contract
count validation failures are converted into a typed Rejected receipt via
rejected_from_evidence(...), matching the existing unparseable-qualification
path instead of propagating the error from classify(...)?.
---
Nitpick comments:
In
`@rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs`:
- Around line 478-484: Reuse the SHA-256 digest already computed for the
equality check in the artifact validation flow instead of calling
Sha256::digest(&bytes) again when constructing
AuthenticatedPolymarketEvidenceObject. After validation succeeds, derive the
returned sha256 string from the validated digest or expected value while
preserving the existing mismatch error and bytes field.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da3cd563-119c-40f5-9a43-a9ce24c3329d
📒 Files selected for processing (6)
rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.jsonrust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.jsonrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rsrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/catalog.rsrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/mod.rsrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/verified.rs
|
Follow-up hardening is in 6e139f0.
Kept out of this #319 contract: a new slot-alignment requirement and extra consistency rules for untrusted producer clock fields. |
c62dde2 to
13a66df
Compare
13a66df to
390a7f7
Compare
Change contract
Publish append-only, digest-keyed Polymarket BTC 5m event receipts. Ready is derived only after independently reopening and rehashing the evidence triplet and producer carrier; all validation failures produce typed Rejected receipts.
Acceptance evidence
cargo test -p ploy-market-data --locked(70 passed)cargo clippy -p ploy-market-data --locked --all-targets --no-deps -- -D warningscargo fmt --checkandgit diff --checkOut of scope
Snapshot/cohort construction, Mission admission, evaluator/MCTS, database/catalog service, mutable latest pointers, and collector deployment.
Dependency / merge order
#336 and #337 are merged. This PR was rebased on
main@b8670874after independent #347 and #346 merges. Merge this before reworking #331/#322.Rollout / rollback
Research-only fail-closed catalog API; no live execution or deployment path. Revert this one commit to remove the catalog contract.
Scope exception
874 non-generated lines exceed the 750-line assessment threshold, but artifact binding, verified evidence facts, receipt construction, and counterexample tests are one inseparable trust contract and cannot be safely rolled back independently. Explicit reviewer approvals: Codex standards reviewer
review_standards_319; Codex spec reviewerreview_spec_319.Summary by CodeRabbit