fix(indexer): remove legacy monitored value monitors#3311
Open
baktun14 wants to merge 1 commit into
Open
Conversation
The Address/Deployment Balance Monitors are leftovers from the old Blockspy/Cloudmos system and are unused by the deploy tool. The DeploymentBalanceMonitor in particular has been throwing non-stop (5180+ events) whenever it can't fetch a deployment's escrow balance. Nothing in the monorepo reads the monitoredValue table, so this removes the two monitors, their scheduled tasks, the shared Sequelize model, and drops the table via migration 0009. Fixes CLOUDMOS-SYNCER-5C
Contributor
📝 WalkthroughWalkthroughThis PR removes the ChangesIndexer monitored value removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/indexer/drizzle/0009_drop_monitored_value.sql`:
- Line 2: The DROP TABLE IF EXISTS statement for "monitoredValue" on line 2
performs an irreversible hard drop without any safeguard. Add a data-retention
gate before dropping the table by implementing a check that verifies the table
is empty or has no dependent data, causing the migration to fail explicitly if
data still exists rather than silently deleting rows. This can be done by adding
a guard query (such as checking row count) before the DROP TABLE statement, or
implementing a two-phase migration where the table is first marked for
deprecation before being dropped in a subsequent migration.
🪄 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: CHILL
Plan: Pro
Run ID: 9c91cde2-fdd1-4ef5-a4a0-bd4b3568daf3
📒 Files selected for processing (12)
apps/indexer/README.mdapps/indexer/drizzle/0009_drop_monitored_value.sqlapps/indexer/drizzle/meta/_journal.jsonapps/indexer/drizzle/schema.tsapps/indexer/src/db/buildDatabase.tsapps/indexer/src/index.tsapps/indexer/src/monitors/addressBalanceMonitor.tsapps/indexer/src/monitors/deploymentBalanceMonitor.tsapps/indexer/src/monitors/index.tspackages/database/dbSchemas/base/index.tspackages/database/dbSchemas/base/monitoredValue.tspackages/database/dbSchemas/index.ts
💤 Files with no reviewable changes (9)
- packages/database/dbSchemas/base/index.ts
- apps/indexer/src/monitors/addressBalanceMonitor.ts
- packages/database/dbSchemas/base/monitoredValue.ts
- apps/indexer/src/monitors/index.ts
- apps/indexer/src/monitors/deploymentBalanceMonitor.ts
- apps/indexer/README.md
- apps/indexer/drizzle/schema.ts
- apps/indexer/src/index.ts
- apps/indexer/src/db/buildDatabase.ts
| @@ -0,0 +1,2 @@ | |||
| -- Drop legacy Blockspy monitoredValue table (no longer used) | |||
| DROP TABLE IF EXISTS "monitoredValue"; | |||
Contributor
There was a problem hiding this comment.
Add a deprecation/data-retention gate before dropping the table.
Line 2 performs an immediate hard drop, which can irreversibly delete existing monitoredValue rows during rollout/rollback. Add an explicit guard (or two-phase deprecate-then-drop sequence) so the migration fails safely if data still exists.
Proposed migration hardening
-- Drop legacy Blockspy monitoredValue table (no longer used)
-DROP TABLE IF EXISTS "monitoredValue";
+DO $$
+BEGIN
+ IF EXISTS (SELECT 1 FROM "monitoredValue" LIMIT 1) THEN
+ RAISE EXCEPTION 'monitoredValue is not empty; deprecate/archive before dropping';
+ END IF;
+END
+$$;
+
+DROP TABLE IF EXISTS "monitoredValue";As per coding guidelines, raw SQL migrations should flag destructive DROP COLUMN/TABLE operations without prior deprecation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| DROP TABLE IF EXISTS "monitoredValue"; | |
| DO $$ | |
| BEGIN | |
| IF EXISTS (SELECT 1 FROM "monitoredValue" LIMIT 1) THEN | |
| RAISE EXCEPTION 'monitoredValue is not empty; deprecate/archive before dropping'; | |
| END IF; | |
| END | |
| $$; | |
| DROP TABLE IF EXISTS "monitoredValue"; |
🤖 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 `@apps/indexer/drizzle/0009_drop_monitored_value.sql` at line 2, The DROP TABLE
IF EXISTS statement for "monitoredValue" on line 2 performs an irreversible hard
drop without any safeguard. Add a data-retention gate before dropping the table
by implementing a check that verifies the table is empty or has no dependent
data, causing the migration to fail explicitly if data still exists rather than
silently deleting rows. This can be done by adding a guard query (such as
checking row count) before the DROP TABLE statement, or implementing a two-phase
migration where the table is first marked for deprecation before being dropped
in a subsequent migration.
Source: Coding guidelines
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3311 +/- ##
==========================================
- Coverage 68.81% 67.42% -1.40%
==========================================
Files 1096 1007 -89
Lines 26837 24583 -2254
Branches 6435 5995 -440
==========================================
- Hits 18468 16574 -1894
+ Misses 7326 6997 -329
+ Partials 1043 1012 -31
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Why
The indexer's
DeploymentBalanceMonitorhas been throwing non-stop in production — Sentry CLOUDMOS-SYNCER-5C, 5180+ events since 2026-05-08 — whenever it can't fetch a deployment's escrow balance.That monitor and its sibling
AddressBalanceMonitorare leftovers from the old Blockspy/Cloudmos system (the indexer README flagged them "Blockspy specific" and "not used in the deploy tool"). Nothing in the monorepo reads themonitoredValuetable they write to, so the whole feature is dead weight that only generates error noise.Fixes CLOUDMOS-SYNCER-5C
What
Removes the legacy MonitoredValue feature end to end:
apps/indexer/src/monitors/) and unregistered their scheduled tasks inindex.ts.MonitoredValue.sync()call (buildDatabase.ts) and the shared Sequelize model (packages/database), including its entry inbaseModels— also consumed byapps/api, which never queries the table (verified, type-checks clean).schema.tsand removed the two task rows from the indexer README.0009_drop_monitored_value.sqlrunsDROP TABLE IF EXISTS "monitoredValue". No code in this PR reads or writes the table anymore, so the drop is non-blocking and safe. Historical migrations andmeta/snapshots are intentionally left intact as immutable history.Summary by CodeRabbit