007697c8 - Alert before protocol equity reaches MINIMUM_EQUITY - #84
Merged
Conversation
deuro.equity() was already read every cycle and persisted as DeuroState.reserveEquity, but nothing alerted on it. Two paths in Equity.sol are gated on that value falling below MINIMUM_EQUITY (1'000 dEURO): _calculateShares takes its bootstrap branch and assigns a flat 10'000'000 nDEPS for a single deposit, and restructureCapTable becomes callable and burns the balances of the holders passed to it. Both gates are evaluated per transaction, so a single block below the threshold is enough. The watcher computes headroom = reserveEquity - MINIMUM_EQUITY from the value already fetched — no new contract call — and pages on two tiers: a configurable warning floor, and critical once headroom is gone or the trend projects it gone within a configurable horizon. deuro_state is a single upserted row and keeps no history, so the trend had nowhere to come from. A new equity_samples table takes one sample per cycle (30-day retention) and the watcher fits an ordinary least-squares slope over the last 24 h of it; a series that is too thin or too short is never projected from. Dedup, escalation and re-arm follow the PositionState.miniLifetimeAlertedAt pattern: the marker is written only after telegramService.sendCriticalAlert confirms delivery, so a failed send is retried next cycle. A WARNING -> CRITICAL step bypasses the repeat window, a demoted level never lowers the stored marker, and only a recovery above the floor clears it and sends a resolve message. MINIMUM_EQUITY is a private constant of Equity.sol with no on-chain getter (eth_call reverts), so it is hardcoded next to the logic. The alert body names the mitigation, because it is neither obvious nor expensive: equity() is balanceOf(reserve) - minterReserve() and reserve() is the Equity contract itself, so a plain dEURO transfer to that contract raises the headroom 1:1 without minting shares and without a vote.
Review findings on the first commit: equity() is clamped at zero — DecentralizedEURO.sol returns 0 when balanceOf(reserve) <= minterReserve() instead of a negative number. The mitigation text claimed a transfer raises the headroom 1:1, which is false during a reserve deficit: equity() stays at 0 and the transfer has no effect until the deficit is covered. An operator following that text under pressure would act into the void. The copy now states the clamp, and the alert gains a "Reserve deficit" line — computed from reserveTotal and reserveMinter, which the state already carries — that names how much has to arrive before the headroom moves at all. Both contract gates compare strictly (capitalBefore < MINIMUM_EQUITY and require(dEURO.equity() < MINIMUM_EQUITY)), so at exactly zero headroom neither path is open yet. Alerting there stays as a safety margin, but the wording no longer claims the paths are already live; the reason text now distinguishes reaching the boundary from crossing it. Three edge cases in the pure logic: - computeTrend accepted non-finite timestamps. NaN comparisons are always false, so neither the span guard nor the zero-variance guard caught them and the result was usable:true with a NaN slope. Guarded at both ends now. - formatDeuro printed -0.00 for sub-cent negative amounts, because the sign was taken before cent truncation. - warningFloorWei=0n is documented to mute the warning tier, but a negative headroom still collected a "below warning floor of 0.00" reason into the critical message. The formatDeuro doc comment overstated its precision guarantee and now says what actually holds. Four tests added for the new edge cases (62 total).
Second review round on this branch. Every headroom message went through sendCriticalAlert, which stamps "🚨 CRITICAL ALERT" on whatever it is handed. A warning arrived under a critical banner, and so did the all-clear — the graduated severity this watcher is built around never reached the operator. TelegramService gains sendAlert(icon, header, message); sendCriticalAlert keeps its signature and behaviour and now delegates to it, so the twelve existing call sites are untouched. The watcher passes its own header per level, and the first line of the body no longer repeats what the header already says. The message also claimed both gated contract paths go live the moment headroom turns negative. Only restructureCapTable does. Equity._invest reverts with InsufficientEquity when post-transfer equity is still below MINIMUM_EQUITY, so the bootstrap branch needs a deposit large enough to lift equity back over the line in the same transaction. Said properly now, in the alert body and in the trigger reason. The reserve-deficit line was off by one wei: equity() clamps at balance <= minterReserve, so covering exactly the deficit reaches equality and still yields zero. It now says the headroom starts moving once the balance exceeds the minter reserve. Comment corrections, all of them things that were not true as written: - the watcher registration still described the pre-correction boundary - "NaN/Infinity comparisons are always false" holds for NaN only - the varX === 0 branch is unreachable behind the span guard, and now says so - escalation bypasses the repeat window only out of a WARNING marker; once a reminder has raised the stored level to CRITICAL, a relapse waits for the window. That is the price of the flutter guard and is now written down - the second inconsistency throw is redundant (strictNullChecks is off), kept as fail-loud defence Tests: the "identical timestamps" case was passing for the wrong reason — it never reached the zero-variance guard it claimed to cover, the span guard returns first. Renamed to what it actually exercises. The non-finite guard is now parametrised over NaN, Infinity and -Infinity, and a new case pins the accepted CRITICAL → reminder → relapse suppression (65 total).
Third review round on this branch. Two hardenings and two comments that described something the code does not do. computeTrend documented TREND_WINDOW_HOURS as its lookback but never enforced it — only the caller's DB query did. That works today and fails quietly the moment anything else calls it: a single old sample outside the window can turn a steeply falling series into a rising slope and the projection then never fires. It now filters relative to its newest sample, so the constant is enforced where it is defined; the caller's query stays as an efficiency measure. Trigger (2) fired on an already-breached floor as well, because hoursToDepletion is 0 there. The message then carried both "equity is below MINIMUM_EQUITY" and "projected to deplete in 0.0 h" — a forecast of something that has happened. The projection now requires positive headroom. Classification is unchanged; trigger (1) already covers that case. decideAction now rejects a non-positive repeat window instead of turning every cycle into a reminder. Not reachable through the current caller, which passes a constant, but this module fails loud rather than degrade. Comment corrections: the module header still described the bootstrap branch as opening on any deposit below MINIMUM_EQUITY (it needs one large enough to lift equity back over the line), and hoursToDepletion is documented as null-when-not-falling while it returns 0 for a breached headroom. Ten tests added (75 total): empty and single-sample series, the window filter, a non-finite slope reached through a finite series, a non-linear irregularly spaced fixture with a hand-derived OLS expectation that a naive two-point slope would fail, and the classification boundaries — headroom exactly at the floor, depletion exactly at the horizon, a muted floor that still allows a critical projection, and a breached headroom that no longer claims to project.
Fourth review round. Neither lens found a behaviour defect this time; what was left was prose that had drifted and tests that did not pin what their names claimed. The same contract mechanic is described in four places — the logic module header, the checkEquityHeadroom docstring, .env.example and the alert text itself. The qualifier added earlier (the bootstrap branch needs a deposit large enough to lift equity back over MINIMUM_EQUITY, otherwise Equity._invest reverts with InsufficientEquity) had only reached two of them. All four now say the same thing, and the three short ones stay short so they are less likely to drift apart again. The watcher registration comment claimed the alert fires when equity reaches MINIMUM_EQUITY. It has three triggers: the configured warning floor, well above that; a projected breach within the horizon; and the boundary itself. computeTrend's docstring described the regression but not the window it enforces since the last commit, so the returned sampleCount and spanHours looked like they described the raw input. Documented, along with why the caller's DB query is a load bound rather than the window: the query cuts at wall-clock now, computeTrend cuts relative to the newest sample, and since that sample is never in the future its cutoff is always the earlier one — the internal filter can never drop what the query returned. Four tests added (79 total): the window boundary is inclusive to the millisecond, a series that only becomes unusable after filtering reports the filtered metadata, zero headroom with a falling trend emits no depletion projection (a guard against >= creeping back into that condition), and the repeat-window guard rejects negative values too — the existing test only covered zero, so narrowing it to === 0n would have gone unnoticed.
Fifth review round. The two new thresholds fell back to their defaults through `||`, which also swallows a variable that is set but empty. These two values decide whether the alert fires at all, so an unparseable value has to reach the existing validation instead of vanishing. The default now applies only when the variable is unset. Verified against the real factory: unset gives 5000, "0" stays 0 (it mutes the warning tier), and both an empty value and garbage now fail validation at boot. Two comments claimed more than the code guarantees — the same failure the last round was about: - computeTrend's docstring said sampleCount and spanHours always describe the filtered window. On the early non-finite-timestamp path they do not: it returns the raw sorted length with a span of 0, before any window is formed. - The note on the sample query said computeTrend's cutoff can never drop what the query returned. That holds only while no sample timestamp sits in the future. If the clock jumps backward, computeTrend's window becomes the later bound and does drop samples — which is its documented semantics, but not what the comment promised. The test for a series that only becomes unusable after filtering asserted against a hardcoded span of 93 hours rather than the span of the samples it had just built, so a drifting fixture would have kept the sanity check green. It now derives the span from the timestamps.
Sixth review round.
The feature's core promise — the dedup marker is written only after
Telegram confirms delivery — had no test. Neither did the recovery path.
deuro.service.spec.ts now drives checkEquityHeadroom against a stubbed
repository and a stubbed sender: a quiet state alerts nothing, a confirmed
warning persists the marker, a failed send leaves it untouched so the next
cycle retries, a confirmed recovery clears it, a failed all-clear does not,
and each level arrives under its own header.
The thresholds still accepted a partially numeric value: parseInt('72h')
is 72, so EQUITY_HEADROOM_PROJECTION_HOURS=72h would have booted with a
plausible-looking wrong horizon. Number() is not the fix — Number('')
is 0, and 0 on the warning floor means "mute the warning tier", so an
empty value would silently disable the warning instead of failing. A
strict integer check rejects both. monitoring.config.spec.ts pins the
whole contract: unset, "0", explicit value, empty, non-numeric, partially
numeric, surrounding whitespace, and 0 for the horizon, which @min(1)
must reject even though the floor accepts it.
Two comment corrections. decideAction said an escalation "must always
page" — it is a pure function that can only decide; delivery belongs to
the service layer. And the note on the sample query described the clock
skew case but not the ordinary one: after an outage the wall-clock query
is the tighter bound, so the regression runs over less than
TREND_WINDOW_HOURS — fewer data points, not a wrong result.
.env.example now states that the defaults apply only when a variable is
unset, and that anything set but unparseable stops the boot.
94 tests across 5 suites.
TaprootFreak
changed the title
Seventh review round. Three orchestration paths had no service-level test. The escalation is the one that matters in a real incident: a stored WARNING and a headroom that has since reached zero must page immediately, in the middle of the repeat window, or the deterioration is swallowed by the dedup it was supposed to escape. Also covered now: the reminder that fires once the window has elapsed, and the very first run, where no state is persisted yet and the watcher must return without touching Telegram or the marker. parseIntegerEnv accepted '-1' as a well-formed integer and left the rejection to @min(0), but refused '+5000' outright. The asymmetry was arbitrary; both signs parse now, and the tests pin which layer rejects what — format for exponential and hexadecimal notation, the value constraint for a negative floor. 101 tests across 5 suites.
Eighth review round, one finding. The test named "rejects a negative warning floor via @min(0), not the format regex" asserted only that something throws. Drop the minus from the integer pattern and '-1' would fail at the format check instead — earlier, for a different reason — and the test would stay green while its name became false. It now asserts that the validation error names the min constraint and does not name isNumber, so the layer that rejects the value is pinned, not just the fact that it is rejected.
Ninth review round, one comment. The note on the negative-floor test said a broken regex would make @isnumber "fail first", implying class-validator stops at the first violation. It does not: NaN violates @min(0) and @isnumber at once and both names land in the message. The assertion was right for that reason, not despite it — the comment now says so.
Contributor
Author
|
Nine review passes were needed to reach zero findings, across two independent lenses (conformity to the surrounding code, and logic/correctness). The last pass found nothing. What the reviews actually changed, beyond wording:
Deliberately not changed, and why:
Verification for the final commit: build clean, 101 tests across 5 suites, |
TaprootFreak
marked this pull request as ready for review
July 29, 2026 09:26
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.
Closes #83.
What
deuro.equity()was already read every cycle and persisted asDeuroState.reserveEquity, but nothing alerted on it. This adds a watcher that pages before the value reachesMINIMUM_EQUITY.Two paths in
Equity.solare gated on exactly that comparison:_calculateSharestakes its bootstrap branch (capitalBefore < MINIMUM_EQUITY) and assigns a flat10_000_000 * ONE_DEC18nDEPS for a single deposit instead of the proportional_fifthRootformula.restructureCapTablebecomes callable (require(dEURO.equity() < MINIMUM_EQUITY)) and burns the balances of the addresses passed to it; beyond that gate only the 2 % vote quorum stands in the way.Both are evaluated per transaction, so a single block below the threshold is enough.
Measured state at implementation time
Mainnet, block 25 630 604 (2026-07-28), read from
0xbA3f535bbCcCcA2A154b573Ca6c5A49BAAE0a3ea/0xc71104001A3CCDA1BEf1177d765831Bd1bfE8eE6and cross-checked against the live monitoring API:equity()MINIMUM_EQUITYAll 16 historical
Loss(address,uint256)events of the dEURO contract sum to 441 040.49, matching the service's owndeuroLossaggregate. The largest single ones (248 707.56 and 149 009.49) date from the June 2025 WFPS liquidation and exceed any headroom the protocol can currently hold; the most recent one is 4 623.86 (block 24 959 311, 2026-04-25).The default warning floor is therefore calibrated to 5 000 dEURO — above that most recent realized single
coverLoss, not above the daily drift, as #83 asks. A floor above the 2025 tail would be permanently breached and carry no information.Consequence, deliberately accepted: at the current headroom the warning tier fires on the first cycle after deployment. That is the true state, not a false positive.
How
headroom = reserveEquity − MINIMUM_EQUITY, computed from the value already fetched. No new contract call.EQUITY_HEADROOM_WARNING_DEURO, default 5000;0mutes the tier).EQUITY_HEADROOM_PROJECTION_HOURS(default 72).deuro_stateis a single upserted row and keeps no history, so there were no rows to fit a slope over. A newequity_samplestable takes one sample per cycle (30-day retention) and the watcher fits an ordinary least-squares slope over the last 24 h. A series that is too thin (< 6 samples), too short (< 2 h) or has zero time variance is never projected from.PositionState.miniLifetimeAlertedAtpattern: the marker is written only aftertelegramService.sendCriticalAlertconfirms delivery, so a failed send retries next cycle. AWARNING → CRITICALstep bypasses the repeat window (an escalation must never be swallowed by dedup); a demoted level never lowers the stored marker (flutter guard); only a recovery above the floor clears it and sends a resolve message. While a level stays unresolved the alert repeats at most every 24 h.MINIMUM_EQUITYis hardcoded next to the logic: it is aprivate constantofEquity.soland has no on-chain getter — aneth_calltoMINIMUM_EQUITY()reverts.equity()isbalanceOf(reserve) − minterReserve()clamped at zero, andreserve()is the Equity contract itself: a plain dEURO transfer to that contract needs neither a share mint nor a governance vote, but it only moves the headroom oncebalanceOf(reserve)exceedsminterReserve(). When the reserve is in deficit the alert therefore also prints how much has to arrive before the headroom moves at all — otherwise an operator transfers into a clamped zero and sees no effect.<), so at exactly zero headroom neither path is open yet. The watcher still pages there as a safety margin, and the reason text distinguishes reaching the boundary from crossing it.Decision logic, trend fit and the alert state machine live in
equity-headroom.logic.ts— framework-free, no clock of its own — mirroring theposition-guard.logic.tssplit, with 34 unit tests.Telegram markdown
Every identifier containing an underscore is wrapped in backticks in the alert text. Alerts are sent with
parse_mode: 'Markdown', where a stray_opens italics and an unpaired one makes the API reject the whole message.Verification
npm run build— cleannpx jest— 62 passed (34 new), 3 suitesnpx prettier --check "src/**/*.ts"— the same 7 files as ondevelop; the two new files are cleannpx eslint "src/**/*.ts"— 14 errors, identical count todevelop(pre-existing, see Fix pre-existing lint errors (unused imports) sonpm run lintis clean #77). No new lint debt.Pre-existing formatting inside
deuro.service.tsandmonitoring.service.tswas deliberately left untouched so the diff stays reviewable.Migration
0006_equity_headroom_alert: createsequity_samples, addsequity_headroom_alerted_at/equity_headroom_alert_leveltodeuro_state. Additive only, no backfill — the trend becomes usable ~2 h after deployment; until then the watcher alerts on level only and reports the trend as not usable yet.Out of scope, found while reviewing
Two adjacent weaknesses are pre-existing and deliberately not touched here:
telegram.service.tsenvTag()escapes only[and].CHAIN/ENVIRONMENTare free-form env values, so a value likeEth_Mainnetputs an unpaired_into a legacy-Markdown message and the Telegram API rejects every alert with HTTP 400. A central escaper for dynamic values belongs in its own change.deuro_stateis overwritten andequity_samplesare pruned by age only, so a sample written on an orphaned block stays inside the 24 h trend window. Systemic, not introduced here.