fix(prism): unattested execution + master-epoch raw weight push - #56
Conversation
Admit missing constation bundles only when CHALLENGE_UNATTESTED_EXECUTION (or NO_PHALA aliases) is on; stamp accepted results unattested (T19-T22). Forward unattested flags into prism embed.env; default admission_requires_worker to false for the host-trust path.
Use master epoch interval (default 360s) for seal identity instead of challenge scoring epoch_seconds, and advance past ledger last_epoch when wall clock lags force-advanced open epochs so continuous push stays eligible for master ingress.
|
Warning Review limit reached
Next review available in: 50 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 (1)
📝 WalkthroughWalkthroughChangesThe Prism challenge adds configurable unattested execution handling, including server-side attestation projection and a narrowly scoped missing-constation admission path. Embedded Prism environment forwarding is expanded, raw-weight epochs become monotonic with a separate master interval, and tests cover validation, isolation, and epoch behavior. Prism execution and epoch changes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Ingestion
participant ConstationGate
participant CoordinatorDB
Client->>Ingestion: submit proof and result envelope
Ingestion->>ConstationGate: evaluate validation and constation state
ConstationGate-->>Ingestion: admission decision
Ingestion->>CoordinatorDB: persist accepted score
Ingestion-->>Client: return projected response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/challenges/prism/tests/test_unattested_no_bypass.py (2)
712-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test asserts against its own literal and can never fail.
enforcedis defined inline, soMISSING not in enforcedandlen(enforced) >= 4are tautologies — it documents intent but locks nothing. To make it a real structural lock, derive the set from the collected test ids/expected codes in this module, or drop 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 `@packages/challenges/prism/tests/test_unattested_no_bypass.py` around lines 712 - 731, Update test_matrix_documents_enforced_validations to derive enforced validation codes from the module’s collected test IDs or expected-code data instead of defining the set inline. Keep assertions that MISSING is excluded, missing_constation_bundle maps to MISSING via miner_fault_reason, and at least four distinct validations are covered.
52-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win~90 lines of fixtures duplicated across three new test modules.
TINY_ARCH,TINY_TRAIN,_SHARD,_stage_train,_zip_bytes/_zip_b64, and_settingsare byte-identical (moduloWORKER_KEY/DIGEST) intest_unattested_constation_gate.pyandtest_unattested_mark_unforgeable.py. Moving them into a sharedconftest.pykeeps the three suites from drifting.🤖 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 `@packages/challenges/prism/tests/test_unattested_no_bypass.py` around lines 52 - 132, The test fixtures are duplicated across the three unattested test modules. Move TINY_ARCH, TINY_TRAIN, _SHARD, _stage_train, the archive encoding helpers (_zip_bytes/_zip_b64), and _settings into the shared conftest.py, preserving module-specific WORKER_KEY/DIGEST inputs through parameters or existing fixture mechanisms, then update each test module to reuse those shared definitions.packages/challenges/prism/tests/test_unattested_constation_gate.py (2)
132-138: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
_clear_unattested_envis copied three ways, and only one copy scrubs strayCHALLENGE_*vars.test_unattested_mark_unforgeable.pyLines 139-146 deletes unknownCHALLENGE_*keys beforecreate_app, noting that PrismSettings rejects them and that conftest/host env can leak them; the other two copies omit that sweep while running the same HTTP flow, so they are ambient-environment dependent.
packages/challenges/prism/tests/test_unattested_constation_gate.py#L132-L138: replace the local helper with the shared version that also removes unknownCHALLENGE_*keys.packages/challenges/prism/tests/test_unattested_no_bypass.py#L148-L154: same — drop the local copy and use the shared helper so_enable_unattestedinherits the sweep.Extracting the single helper into a package-level
conftest.pyfixes both at once.🤖 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 `@packages/challenges/prism/tests/test_unattested_constation_gate.py` around lines 132 - 138, Extract _clear_unattested_env into the package-level conftest.py, extending it to delete unknown CHALLENGE_* environment variables in addition to the three named keys. Remove the local helper definitions from packages/challenges/prism/tests/test_unattested_constation_gate.py lines 132-138 and packages/challenges/prism/tests/test_unattested_no_bypass.py lines 148-154, and update both test modules to use the shared helper.
377-389: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBranching assertion accepts almost any outcome.
Any 422 whose code merely differs from
MISSINGpasses, so a regression that breaks the unattested path with an unrelated fault would go unnoticed. SinceDockerExecutor.runis stubbed deterministically, pin the expected status/payload instead of accepting either branch.🤖 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 `@packages/challenges/prism/tests/test_unattested_constation_gate.py` around lines 377 - 389, Update the assertions following the /internal/v1/work_units/result request in the unattested-path test to require the deterministic expected status and response payload from the stubbed DockerExecutor.run. Remove the branch that accepts any non-MISSING 422, and assert the exact successful or specifically expected response fields, including the unattested tier behavior.packages/challenges/prism/tests/test_unattested_mark_unforgeable.py (1)
136-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the exported constant instead of the literal
"CHALLENGE_NO_PHALA".The sibling test files import
CHALLENGE_NO_PHALA_ENV; hardcoding it here silently stops clearing the alias if the constant is ever renamed.♻️ Proposed change
from prism_challenge.unattested_execution import ( ATTESTATION_STATUS_UNATTESTED, + CHALLENGE_NO_PHALA_ENV, CHALLENGE_UNATTESTED_EXECUTION_ENV,- for key in (CHALLENGE_UNATTESTED_EXECUTION_ENV, "CHALLENGE_NO_PHALA", NO_PHALA_ENV): + for key in (CHALLENGE_UNATTESTED_EXECUTION_ENV, CHALLENGE_NO_PHALA_ENV, NO_PHALA_ENV): monkeypatch.delenv(key, raising=False)🤖 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 `@packages/challenges/prism/tests/test_unattested_mark_unforgeable.py` around lines 136 - 146, Update _clear_unattested_env to use the imported CHALLENGE_NO_PHALA_ENV constant in the environment-key tuple instead of the literal "CHALLENGE_NO_PHALA", preserving clearing of all relevant variables if the constant is renamed.tests/unit/test_master_embed_env_file.py (1)
219-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest is close to vacuous — AC never reads an env file here.
With
ac_env_file_body=None, the AC child has no env file at all, so these assertions hold even if the prism/AC allowlists were swapped. Passing a distinct AC body (with its own marker key) would make the isolation claim meaningful.🤖 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 `@tests/unit/test_master_embed_env_file.py` around lines 219 - 236, Update test_prism_unattested_flags_do_not_leak_into_ac to provide a distinct ac_env_file_body containing a marker key, then assert that the marker is present in the AC environment while CHALLENGE_UNATTESTED_EXECUTION and NO_PHALA from the Prism file remain absent. Keep the existing Prism-only inputs and isolation assertions so the test verifies the AC file is actually read and the allowlists are not swapped.packages/challenges/prism/src/prism_challenge/unattested_execution.py (1)
129-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused argument and collapse the duplicate returns
project_public_attestation()has no callers,storedis only deleted, and the two return branches differ only byexecution_mode. Remove the parameter and return a single dict with the mode computed fromis_un attested_execution_enabled(), or defer the helper until a caller exists.🤖 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 `@packages/challenges/prism/src/prism_challenge/unattested_execution.py` around lines 129 - 151, Update project_public_attestation to remove the unused stored parameter and delete the corresponding del statement. Collapse the duplicate return dictionaries into one, computing execution mode from is_unattested_execution_enabled() while preserving the existing attestation fields and mode values.
🤖 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 `@docker/master-entrypoint.sh`:
- Around line 212-215: Update the defaulting logic for
PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER in the master entrypoint so it is
false only for unattested/NO_PHALA deployments and remains worker-gated for
attested deployments when the variable is unset. Preserve explicit environment
overrides and the existing CPU_REEXEC_TEST_MODE behavior.
In `@packages/challenges/prism/src/prism_challenge/ingestion.py`:
- Around line 171-177: Thread the per-app unattested setting from create_app’s
PrismSettings into ingestion instead of reading live environment state. Add a
settings-backed flag to the ingestion flow and pass it as settings_flag to both
the response-marking check around mark_result_unattested and
_evaluate_constation_gate, preserving the configured mode for the app instance
throughout its lifetime.
In `@packages/challenges/prism/src/prism_challenge/raw_weight_push.py`:
- Around line 662-663: Update the epoch calculation condition in
packages/challenges/prism/src/prism_challenge/raw_weight_push.py#L662-L663 to
advance only when wall < last, preserving the current epoch when they are equal.
Add an acknowledged epoch=wall case in
packages/challenges/prism/tests/test_raw_weight_push.py#L347-L366 and assert
epoch_fn() == wall.
---
Nitpick comments:
In `@packages/challenges/prism/src/prism_challenge/unattested_execution.py`:
- Around line 129-151: Update project_public_attestation to remove the unused
stored parameter and delete the corresponding del statement. Collapse the
duplicate return dictionaries into one, computing execution mode from
is_unattested_execution_enabled() while preserving the existing attestation
fields and mode values.
In `@packages/challenges/prism/tests/test_unattested_constation_gate.py`:
- Around line 132-138: Extract _clear_unattested_env into the package-level
conftest.py, extending it to delete unknown CHALLENGE_* environment variables in
addition to the three named keys. Remove the local helper definitions from
packages/challenges/prism/tests/test_unattested_constation_gate.py lines 132-138
and packages/challenges/prism/tests/test_unattested_no_bypass.py lines 148-154,
and update both test modules to use the shared helper.
- Around line 377-389: Update the assertions following the
/internal/v1/work_units/result request in the unattested-path test to require
the deterministic expected status and response payload from the stubbed
DockerExecutor.run. Remove the branch that accepts any non-MISSING 422, and
assert the exact successful or specifically expected response fields, including
the unattested tier behavior.
In `@packages/challenges/prism/tests/test_unattested_mark_unforgeable.py`:
- Around line 136-146: Update _clear_unattested_env to use the imported
CHALLENGE_NO_PHALA_ENV constant in the environment-key tuple instead of the
literal "CHALLENGE_NO_PHALA", preserving clearing of all relevant variables if
the constant is renamed.
In `@packages/challenges/prism/tests/test_unattested_no_bypass.py`:
- Around line 712-731: Update test_matrix_documents_enforced_validations to
derive enforced validation codes from the module’s collected test IDs or
expected-code data instead of defining the set inline. Keep assertions that
MISSING is excluded, missing_constation_bundle maps to MISSING via
miner_fault_reason, and at least four distinct validations are covered.
- Around line 52-132: The test fixtures are duplicated across the three
unattested test modules. Move TINY_ARCH, TINY_TRAIN, _SHARD, _stage_train, the
archive encoding helpers (_zip_bytes/_zip_b64), and _settings into the shared
conftest.py, preserving module-specific WORKER_KEY/DIGEST inputs through
parameters or existing fixture mechanisms, then update each test module to reuse
those shared definitions.
In `@tests/unit/test_master_embed_env_file.py`:
- Around line 219-236: Update test_prism_unattested_flags_do_not_leak_into_ac to
provide a distinct ac_env_file_body containing a marker key, then assert that
the marker is present in the AC environment while CHALLENGE_UNATTESTED_EXECUTION
and NO_PHALA from the Prism file remain absent. Keep the existing Prism-only
inputs and isolation assertions so the test verifies the AC file is actually
read and the allowlists are not swapped.
🪄 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: 2a67660a-2027-43f5-a44a-9de4d9991a49
📒 Files selected for processing (9)
docker/master-entrypoint.shpackages/challenges/prism/src/prism_challenge/ingestion.pypackages/challenges/prism/src/prism_challenge/raw_weight_push.pypackages/challenges/prism/src/prism_challenge/unattested_execution.pypackages/challenges/prism/tests/test_raw_weight_push.pypackages/challenges/prism/tests/test_unattested_constation_gate.pypackages/challenges/prism/tests/test_unattested_mark_unforgeable.pypackages/challenges/prism/tests/test_unattested_no_bypass.pytests/unit/test_master_embed_env_file.py
| # PROD POLICY: Prism eval never runs on master — CPU_REEXEC must stay false; | ||
| # miners supply Lium pods; admission_requires_worker must stay true. | ||
| # miners supply Lium pods; admission_requires_worker default false (unattested/host path). | ||
| "PRISM_WORKER_PLANE__CPU_REEXEC_TEST_MODE=${PRISM_WORKER_PLANE__CPU_REEXEC_TEST_MODE:-false}" | ||
| "PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER=${PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER:-true}" | ||
| "PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER=${PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER:-false}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Confirm the admission_requires_worker=false default is intentional for prod, not just the unattested rollout.
This flips embedded Prism from worker-gated admission to open admission for every deployment that doesn't explicitly set the var, including ones running with the unattested flag off. Consider defaulting from the unattested flag instead (e.g. false only when CHALLENGE_UNATTESTED_EXECUTION/NO_PHALA is on) so the attested path keeps its previous guarantee.
🤖 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 `@docker/master-entrypoint.sh` around lines 212 - 215, Update the defaulting
logic for PRISM_WORKER_PLANE__ADMISSION_REQUIRES_WORKER in the master entrypoint
so it is false only for unattested/NO_PHALA deployments and remains worker-gated
for attested deployments when the variable is unset. Preserve explicit
environment overrides and the existing CPU_REEXEC_TEST_MODE behavior.
Unattested ship sets entrypoint default to :-false; align the worker-plane policy landmine with docker/master-entrypoint.sh.
1 participant
Summary
unattested_execution.py+ ingestion gate) with embed env allowlist forCHALLENGE_UNATTESTED_EXECUTION/NO_PHALA.admission_requires_worker=falsefor unattested ship.last_epochwhen wall clock lags force-advanced open epochs.Prod context
Test plan
pytest packages/challenges/prism/tests/test_unattested_*.py tests/unit/test_master_embed_env_file.pypytest packages/challenges/prism/tests/test_raw_weight_push.py(7 passed)BASE_LIUM_TRAINING__ENABLED=trueMandate
keyrelease/Summary by CodeRabbit
New Features
Bug Fixes
Tests