Feature: Manual/Automated Switch to Burn Mode for Exploit Detection#78
Open
machine-god-deus wants to merge 1 commit into
Conversation
WalkthroughThis pull request introduces a "burn mode" feature for emergency emission control and exploit detection. It adds a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/crusades/api/server.py`:
- Around line 110-145: The burn-mode endpoints (burn_mode_router and handlers
activate_burn_mode / deactivate_burn_mode) must require a configured dashboard
API key instead of inheriting the "open if no key" behavior; add an
authorization dependency that enforces a non-empty DASHBOARD_API_KEY (e.g.
create a require_dashboard_api_key dependency that validates
Request.app.state.config.DASHBOARD_API_KEY and the incoming key) and attach it
to the router (APIRouter(..., dependencies=[Depends(require_dashboard_api_key)])
or to each endpoint), or alternatively add a startup check that raises an
exception when DASHBOARD_API_KEY is missing so the app fails to start; ensure
the dependency name and router/handler symbols (burn_mode_router,
activate_burn_mode, deactivate_burn_mode) are used so the protection covers
these routes.
- Around line 72-76: The code caches a MockClient via get_db_client() before the
real SQLite DB is created, causing the mock to persist; to fix, create and
initialize the real Database (use Database, db_path, await db.initialize(), and
assign app.state.db) before any call to get_db_client() or before caching the
dashboard client so get_db_client() will detect the real DB and return the real
client instead of MockClient. Ensure the Database initialization block runs
prior to whatever logic caches the dashboard client or calls get_db_client().
In `@src/crusades/chain/burn_mode.py`:
- Around line 15-20: BurnMode currently allows enabled=True with an empty
reason; add a validation rule so enabling burn mode requires a non-empty reason.
In the BurnMode model add a Pydantic validator (e.g., a `@root_validator` or
`@validator`('reason', always=True)) that checks if values.get("enabled") is True
and (reason is None or reason.strip() == ""), and raise a ValueError like
"reason required when enabled is True" to prevent constructing or persisting an
enabled BurnMode without audit context; update any tests or callers that
construct BurnMode to provide a reason when enabled.
In `@src/crusades/chain/weights.py`:
- Around line 213-221: Filter out burn_mode.blocked_uids before selecting or
promoting a winner so a blocked submission never becomes _previous_winner or
influences the adaptive threshold; change the winner selection flow in the
relevant function to choose the next eligible miner when the chosen winner_uid
is in burn_mode.blocked_uids (do not immediately call _set_burn_only_weights or
burn the full share), and only call self._set_burn_only_weights(f"...") when no
eligible winner remains after filtering; ensure any state updates that ratchet
adaptive thresholds happen after this eligibility check and use the final
elected winner UID.
In `@tests/test_burn_mode.py`:
- Around line 39-45: The tests test_burn_rate_bounds_low and
test_burn_rate_bounds_high use pytest.raises(Exception) which is too broad;
update them to assert the specific validation error raised by BurnMode when
burn_rate_override is out of range (e.g., ValueError or the project-specific
ValidationError used by BurnMode), and optionally use the match= parameter to
verify the error message mentions "burn_rate_override" or "out of range"; change
the pytest.raises(Exception) calls in those two tests to
pytest.raises(<specific-exception>) (and add match="burn_rate" or similar if you
want to assert the message).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15de79e6-5d75-49bc-b1d3-239e430d3f17
📒 Files selected for processing (8)
src/crusades/api/server.pysrc/crusades/chain/burn_mode.pysrc/crusades/chain/weights.pysrc/crusades/storage/database.pytests/__init__.pytests/test_burn_mode.pytests/test_burn_mode_api.pytests/test_burn_mode_weights.py
Comment on lines
+72
to
+76
| # Initialize async database for burn mode operations | ||
| db_path = os.getenv("CRUSADES_DB_PATH", "crusades.db") | ||
| db = Database(url=f"sqlite+aiosqlite:///{db_path}") | ||
| await db.initialize() | ||
| app.state.db = db |
There was a problem hiding this comment.
Initialize the real DB before caching the dashboard client.
On a first boot, get_db_client() above still sees no database file and permanently caches MockClient. This block then creates the real SQLite DB, so burn-mode writes hit the real database while the rest of the API keeps serving mock data until restart.
💡 Simple fix
logger.info("Crusades API starting...")
- get_db_client()
-
# Initialize async database for burn mode operations
db_path = os.getenv("CRUSADES_DB_PATH", "crusades.db")
db = Database(url=f"sqlite+aiosqlite:///{db_path}")
await db.initialize()
app.state.db = db
+ get_db_client()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/crusades/api/server.py` around lines 72 - 76, The code caches a
MockClient via get_db_client() before the real SQLite DB is created, causing the
mock to persist; to fix, create and initialize the real Database (use Database,
db_path, await db.initialize(), and assign app.state.db) before any call to
get_db_client() or before caching the dashboard client so get_db_client() will
detect the real DB and return the real client instead of MockClient. Ensure the
Database initialization block runs prior to whatever logic caches the dashboard
client or calls get_db_client().
Comment on lines
+110
to
+145
| burn_mode_router = APIRouter(prefix="/burn-mode", tags=["burn-mode"]) | ||
|
|
||
|
|
||
| @burn_mode_router.post("/activate") | ||
| async def activate_burn_mode( | ||
| request: Request, | ||
| body: BurnModeActivateRequest, | ||
| ) -> dict[str, Any]: | ||
| """Activate burn mode. Overrides hparams burn_rate and optionally blocks UIDs.""" | ||
| db: Database = request.app.state.db | ||
| burn_mode = BurnMode( | ||
| enabled=True, | ||
| burn_rate_override=body.burn_rate_override, | ||
| blocked_uids=body.blocked_uids, | ||
| reason=body.reason, | ||
| activated_at=datetime.now(UTC), | ||
| activated_by="api", | ||
| ) | ||
| await db.set_burn_mode(burn_mode) | ||
| logger.warning( | ||
| "Burn mode ACTIVATED: rate=%.0f%%, blocked=%s, reason=%s", | ||
| burn_mode.burn_rate_override * 100, | ||
| burn_mode.blocked_uids, | ||
| burn_mode.reason, | ||
| ) | ||
| return burn_mode.model_dump(mode="json") | ||
|
|
||
|
|
||
| @burn_mode_router.post("/deactivate") | ||
| async def deactivate_burn_mode(request: Request) -> dict[str, Any]: | ||
| """Deactivate burn mode. Restores normal hparams-driven weight distribution.""" | ||
| db: Database = request.app.state.db | ||
| burn_mode = BurnMode.inactive() | ||
| await db.set_burn_mode(burn_mode) | ||
| logger.warning("Burn mode DEACTIVATED") | ||
| return {"status": "deactivated"} |
There was a problem hiding this comment.
These control routes need mandatory auth, not optional auth.
These handlers inherit the dashboard's "open if no key is configured" behavior. That's fine for read-only stats, but it makes burn-mode activation/deactivation publicly writable whenever DASHBOARD_API_KEY is missing or empty, which lets anyone who can reach the API suppress emissions or block UIDs. Require a configured key for this router or fail startup when it's absent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/crusades/api/server.py` around lines 110 - 145, The burn-mode endpoints
(burn_mode_router and handlers activate_burn_mode / deactivate_burn_mode) must
require a configured dashboard API key instead of inheriting the "open if no
key" behavior; add an authorization dependency that enforces a non-empty
DASHBOARD_API_KEY (e.g. create a require_dashboard_api_key dependency that
validates Request.app.state.config.DASHBOARD_API_KEY and the incoming key) and
attach it to the router (APIRouter(...,
dependencies=[Depends(require_dashboard_api_key)]) or to each endpoint), or
alternatively add a startup check that raises an exception when
DASHBOARD_API_KEY is missing so the app fails to start; ensure the dependency
name and router/handler symbols (burn_mode_router, activate_burn_mode,
deactivate_burn_mode) are used so the protection covers these routes.
Comment on lines
+15
to
+20
| enabled: bool = False | ||
| burn_rate_override: float = Field(default=1.0, ge=0.0, le=1.0) | ||
| blocked_uids: list[int] = Field(default_factory=list) | ||
| reason: str = "" | ||
| activated_at: datetime | None = None | ||
| activated_by: str = "" |
There was a problem hiding this comment.
Don't allow enabled burn mode without audit context.
BurnMode(enabled=True) is currently valid with an empty reason, so callers can persist an emergency override with no accountable explanation. For a control path that's meant to justify operator action, require a non-empty reason whenever burn mode is enabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/crusades/chain/burn_mode.py` around lines 15 - 20, BurnMode currently
allows enabled=True with an empty reason; add a validation rule so enabling burn
mode requires a non-empty reason. In the BurnMode model add a Pydantic validator
(e.g., a `@root_validator` or `@validator`('reason', always=True)) that checks if
values.get("enabled") is True and (reason is None or reason.strip() == ""), and
raise a ValueError like "reason required when enabled is True" to prevent
constructing or persisting an enabled BurnMode without audit context; update any
tests or callers that construct BurnMode to provide a reason when enabled.
Comment on lines
+213
to
+221
| # If burn mode is active and winner is blocked, redirect to burn | ||
| if burn_mode.enabled and winner_uid in burn_mode.blocked_uids: | ||
| logger.warning( | ||
| "Winner UID %d is blocked by burn mode — redirecting 100%% to burn_uid", | ||
| winner_uid, | ||
| ) | ||
| return await self._set_burn_only_weights( | ||
| f"Winner UID {winner_uid} blocked by burn mode" | ||
| ) |
There was a problem hiding this comment.
Filter blocked winners out before winner-state updates.
By the time this branch runs, the blocked submission may already have been promoted into _previous_winner and used to ratchet the adaptive threshold. It also burns the full winner share instead of passing it to the next eligible miner, so honest miners lose emissions even in the "targeted" path. Exclude blocked UIDs before winner selection/state updates, and only fall back to burn-only if no eligible winner remains.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/crusades/chain/weights.py` around lines 213 - 221, Filter out
burn_mode.blocked_uids before selecting or promoting a winner so a blocked
submission never becomes _previous_winner or influences the adaptive threshold;
change the winner selection flow in the relevant function to choose the next
eligible miner when the chosen winner_uid is in burn_mode.blocked_uids (do not
immediately call _set_burn_only_weights or burn the full share), and only call
self._set_burn_only_weights(f"...") when no eligible winner remains after
filtering; ensure any state updates that ratchet adaptive thresholds happen
after this eligibility check and use the final elected winner UID.
Comment on lines
+39
to
+45
| def test_burn_rate_bounds_low(self): | ||
| with pytest.raises(Exception): | ||
| BurnMode(burn_rate_override=-0.1) | ||
|
|
||
| def test_burn_rate_bounds_high(self): | ||
| with pytest.raises(Exception): | ||
| BurnMode(burn_rate_override=1.1) |
There was a problem hiding this comment.
Narrow the exception assertion in the bounds tests.
pytest.raises(Exception) will also pass on unrelated construction errors, so these tests can go green for the wrong reason. Assert the specific validation failure instead.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 40-40: Do not assert blind exception: Exception
(B017)
[warning] 44-44: Do not assert blind exception: Exception
(B017)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_burn_mode.py` around lines 39 - 45, The tests
test_burn_rate_bounds_low and test_burn_rate_bounds_high use
pytest.raises(Exception) which is too broad; update them to assert the specific
validation error raised by BurnMode when burn_rate_override is out of range
(e.g., ValueError or the project-specific ValidationError used by BurnMode), and
optionally use the match= parameter to verify the error message mentions
"burn_rate_override" or "out of range"; change the pytest.raises(Exception)
calls in those two tests to pytest.raises(<specific-exception>) (and add
match="burn_rate" or similar if you want to assert the message).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BurnModePydantic model insrc/crusades/chain/burn_mode.pywithenabled,burn_rate_override,blocked_uids,reason,activated_at,activated_byfieldsvalidator_stateKV store:get_burn_mode()andset_burn_mode()added tosrc/crusades/storage/database.pyWeightSetter.set_weights()insrc/crusades/chain/weights.pyloads burn mode state at cycle start; when active, overridesburn_ratefrom hparams and redirects blocked UID emissions toburn_uidsrc/crusades/api/server.py:POST /burn-mode/activate,POST /burn-mode/deactivate,GET /burn-mode/statusValidation
uv run pytest tests/ -v— 20/20 passeduv run ruff check— all checks passedBurnModeserialization round-trip, validation bounds onburn_rate_override, andinactive()factory defaultset_burn_mode()/get_burn_mode()persistence and correct fallback to inactive state when no prior state existshttpx.AsyncClient) cover auth enforcement (401 without key), activation/deactivation lifecycle, status reflection, and input validation (422 on out-of-range burn rate)burn_uid, and inactive burn mode falls back to hparamsRisks
blocked_uidscould suppress emissions for honest miners; mitigation is thereasonfield andGET /statusendpoint for auditabilityCloses #77
Summary by CodeRabbit
Release Notes
New Features
Tests