fix(indexer): validate bme price attribute as dec string - #3605
Conversation
A non-numeric price in a bme coin attribute previously flowed unchecked into the numeric(38,18) ledger columns, aborting the whole batch transaction instead of degrading to a parse warning like every other malformed attribute. Also documents that widening the scanned bme event-type set shifts replay ordinals.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — this is a narrowly scoped, well-tested fix.
Extended reasoning...
Overview
This PR adds Dec-string validation to the BME price attribute in parseCoinPrice (apps/chain-indexer/src/bme/bme-deriver.ts), mirroring the existing validation already applied to collateral_ratio. Previously, any non-Dec-string price value (or malformed proto attribute) would pass through unchecked and later fail when written into a numeric(38,18) database column, aborting the whole batch transaction and causing the indexer to retry-loop on that height range. Now a malformed price causes parseCoinPrice to return undefined, which the caller treats as a malformed-coin condition, degrading gracefully to a parse warning consistent with how every other malformed BME attribute is already handled. The PR also adds a doc comment clarifying an existing ordinal-stability caveat, and backfills several previously-missing spec cases (non-Dec price, absent price, unquoted legacy status attributes, malformed coins_to_burn).
Security risks
None. This is purely a defensive-parsing fix for indexer-internal event decoding; no user input, auth, or crypto surfaces are touched. If anything, tightening validation here reduces risk (previously a malformed value could propagate into a numeric DB write and abort a batch).
Level of scrutiny
Low-to-moderate. This is a small, self-contained fix (single function change) to internal blockchain-event parsing logic in a non-critical-path indexer, following an established pattern in the same file (isDecString already existed and was used for collateral_ratio). The change is well covered by new unit tests that exercise exactly the scenarios described in the PR (non-Dec price → warning, absent price → null, legacy unquoted status attrs, malformed coins_to_burn → warning), and I traced asString/isDecString/parseJsonRecord to confirm the null-vs-malformed distinction is preserved correctly.
Other factors
No prior review threads or outstanding comments to address (the only existing comment is an automated CodeRabbit skip notice). The diff is small (size: S label) and touches only one source file plus its spec file. I found no issues beyond what the bug-hunting system already ruled out.
1f618e7
into
feat/indexer-scaffold-chain-indexer-app
1 participant
Why
Part of CON-813. A post-merge CodeRabbit review of #3603 flagged that the BME
priceattribute is the one numeric event attribute written into thenumeric(38,18)ledger columns without validation. The proto field is a free-form string, so a malformed value would abort the whole batch transaction and retry-loop the indexer on that height range. That breaks the deriver's own rule that a malformed event degrades to a parse warning instead of halting the block.What
pricewith the same Dec-string checkcollateral_ratioalready uses. A bad price now becomes a parse warning like every other malformed attribute.bme_status_changesrows.coins_to_burnon a canceled record.