Uh oh!
There was an error while loading. Please reload this page.
fix(prism): reuse sealed BYOK Lium key on infra retry - #132
Conversation
Admin/auto retries that need another GPU run were rejecting with missing_lium_api_key when X-Lium-Api-Key was omitted, even though the payer vault still held the submission seal. Fall back to the vault and refresh the seal TTL on requeue.
📝 WalkthroughWalkthroughRetry handling now uses a sealed payer-vault Lium key when the request has no key header. Successful retries re-seal the key. A regression test covers reuse and missing-key errors. ChangesRetry key fallback
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/prism-challenge/src/api.rs (1)
773-775: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftProve that retry extends the vault TTL.
Line 775 only proves that the key remains present immediately after retry. The assertion also passes if Line 475 is removed because intake already inserted the key.
Use a controllable vault clock or expiry observation. Assert that the key survives beyond its original TTL after the retry re-seals it.
🤖 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 `@crates/prism-challenge/src/api.rs` around lines 773 - 775, Strengthen the retry test around the vault lookup and status assertions to verify TTL extension, not merely key presence. Use the controllable vault clock or expiry-observation mechanism already available in the test setup, advance time past the original TTL while remaining within the retry-extended TTL, and assert the key is still present after retry re-seals it.
🤖 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 `@crates/prism-challenge/src/api.rs`:
- Around line 717-802: Keep retry_reuses_payer_vault_lium_key focused on
vault-backed retry behavior, and add a separate end-to-end challenge
verification test. Exercise happy-path intake, failure probes,
challenge-specific validation, leaf emission, raw weight submission, and bundle
sealing through the public flow, then call GET /v1/weights/latest and assert the
response reports sealed: true.
---
Nitpick comments:
In `@crates/prism-challenge/src/api.rs`:
- Around line 773-775: Strengthen the retry test around the vault lookup and
status assertions to verify TTL extension, not merely key presence. Use the
controllable vault clock or expiry-observation mechanism already available in
the test setup, advance time past the original TTL while remaining within the
retry-extended TTL, and assert the key is still present after retry re-seals it.
🪄 Autofix
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: f6deef6a-1c2c-4dc0-953d-3c8c2827de8f
📒 Files selected for processing (1)
crates/prism-challenge/src/api.rs
| /// Infra retry on live Lium must reuse the sealed BYOK vault when the | ||
| /// miner does not resend `X-Lium-Api-Key` (regression: dropped key → | ||
| /// `missing_lium_api_key` instead of replaying the real infra error). | ||
| #[tokio::test] | ||
| async fn retry_reuses_payer_vault_lium_key() { | ||
| let vault = Arc::new(prism_lium_payer::PayerKeyVault::new()); | ||
| let st = Arc::new(AppState { | ||
| store: Arc::new(MemoryPrismStore::new()), | ||
| eval_store: Arc::new(crate::MemoryEvalStore::new()), | ||
| epoch: std::sync::atomic::AtomicU64::new(7), | ||
| netuid: 541, | ||
| backend_mode: "lium", | ||
| retry_max: 2, | ||
| gating: None, | ||
| metagraph: None, | ||
| admin_token_hashes: vec![], | ||
| payer_vault: Some(Arc::clone(&vault)), | ||
| logs: std::sync::Arc::new(prism_orphan::LogBuffer::new()), | ||
| }); | ||
| let app = submission_router(Arc::clone(&st)); | ||
| let req = crate::example_valid_request(); | ||
| let id = prism_pipeline::submission_id(&req); | ||
| let body = serde_json::to_vec(&req).unwrap(); | ||
| let (s, v) = call( | ||
| app.clone(), | ||
| Request::post("/v1/submissions") | ||
| .header("content-type", "application/json") | ||
| .header("x-lium-api-key", "sk_test_miner_lium") | ||
| .body(Body::from(body)) | ||
| .unwrap(), | ||
| ) | ||
| .await; | ||
| assert_eq!(s, StatusCode::ACCEPTED, "{v}"); | ||
| assert_eq!(vault.get(&id).as_deref(), Some("sk_test_miner_lium")); | ||
| st.store | ||
| .apply( | ||
| &id, | ||
| &StatePatch { | ||
| status: Some(Stage::Failed), | ||
| final_score: Some(FinalScore::NoScore( | ||
| NoScoreReasonCode::ChallengeInternal as u8, | ||
| )), | ||
| ..StatePatch::default() | ||
| }, | ||
| None, | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
| // No Lium header — vault must satisfy the live retry gate. | ||
| let (s, v) = call( | ||
| app.clone(), | ||
| Request::post(format!("/v1/submissions/{id}/retry")) | ||
| .body(Body::empty()) | ||
| .unwrap(), | ||
| ) | ||
| .await; | ||
| assert_eq!(s, StatusCode::ACCEPTED, "{v}"); | ||
| assert_eq!(v["status"], "queued"); | ||
| assert_eq!(vault.get(&id).as_deref(), Some("sk_test_miner_lium")); | ||
| // Without vault entry, live infra retry still demands the header. | ||
| vault.remove(&id); | ||
| st.store | ||
| .apply( | ||
| &id, | ||
| &StatePatch { | ||
| status: Some(Stage::Failed), | ||
| final_score: Some(FinalScore::NoScore( | ||
| NoScoreReasonCode::ChallengeInternal as u8, | ||
| )), | ||
| ..StatePatch::default() | ||
| }, | ||
| None, | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
| let (s, v) = call( | ||
| app, | ||
| Request::post(format!("/v1/submissions/{id}/retry")) | ||
| .body(Body::empty()) | ||
| .unwrap(), | ||
| ) | ||
| .await; | ||
| assert_eq!(s, StatusCode::BAD_REQUEST, "{v}"); | ||
| assert_eq!(v["code"], "missing_lium_api_key"); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add separate end-to-end challenge verification.
This test directly patches the failed submission state. It only verifies the retry route response.
Keep this focused regression test. Add a separate verification that executes intake, failure probes, challenge validation, leaf emission, raw weight submission, bundle sealing, and confirms GET /v1/weights/latest returns sealed: true.
As per coding guidelines, “Challenge verification must simulate an end-to-end submission, including happy-path intake, failure probes, challenge-specific validation, leaf emission, raw weight submission, sealing, and confirmation of sealed: true.”
🤖 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 `@crates/prism-challenge/src/api.rs` around lines 717 - 802, Keep
retry_reuses_payer_vault_lium_key focused on vault-backed retry behavior, and
add a separate end-to-end challenge verification test. Exercise happy-path
intake, failure probes, challenge-specific validation, leaf emission, raw weight
submission, and bundle sealing through the public flow, then call GET
/v1/weights/latest and assert the response reports sealed: true.
Source: Coding guidelines
Uh oh!
There was an error while loading. Please reload this page.
Summary
/v1/submissions/{id}/retrynow reuses the sealed payer-vault Lium key forsubmission_idwhenX-Lium-Api-Keyis omitted, instead of failingmissing_lium_api_key.retry_reuses_payer_vault_lium_key.Context
OpenRouter 401 outage forced mass infra retries; miners without resending BYOK headers could not recover even with vault seals present.
Test plan
cargo test -p prism-challenge retry_reuses_payer_vault_lium_keycargo clippy -p prism-challenge --all-targets -- -D warningsSummary by CodeRabbit
New Features
Bug Fixes