DepegGuard / StableGuard — Second-Pass Technical Review
Matthew, this will be a long description because the issue is not a single defect; it affects the structure of the code as a whole.
Appreciate for the implementation update and for publishing the contracts, workflow, tests, and ACTION_RECORD.md openly. The first three invariants are no longer merely described; they are represented in code and tests:
- an alert for an unregistered exposure cannot enter the pause path;
- destination slots reject writes after they leave
PENDING;
- lookup-before-create preserves one active event per coin and makes terminal closure explicit.
The separation between the detection record and the action record is also the correct direction, and the explicit list of conditions that remain unproven is materially better than presenting local tests as production evidence.
This second pass is therefore not a rejection of that work. It is the next layer exposed by the fact that the original layer was implemented successfully. The remaining problems now sit mainly between individually reasonable state machines:
observation state
≠
asset-incident state
≠
physical vault state
≠
delivery-attempt state
The current branch still collapses some of those identities into one another. That produces several reachable cases in which the event ledger and the physical system can disagree.
Review target:
- implementation branch:
feat/automatic-recovery
- implementation commit:
8ffd1dd7845bd8d2288e3fd1b5ffe2a3535737bf
- documentation commit:
999baaa4bcc47f25c5df1475bf4c75228083d819
The reviewed implementation reports 85/85 tests passing. The analysis below is state-space and code-path review: it focuses on cross-object combinations that the current tests do not represent, so the existing pass count does not close these findings.
Executive conclusion
I would not connect the current receiver to a real multi-asset vault or real asynchronous delivery path yet.
Five items are production blockers:
- independent asset incidents can independently unpause one shared vault;
resumeProtectionTracking() can manufacture an incident for the wrong asset from the single fact that the vault is paused;- report acceptance authenticates the Forwarder but not the intended workflow identity;
- report replay and duplicate asset entries can satisfy
stabilityWindow without new observations;
- transferring the registry controller to the receiver makes several documented control and recovery operations unreachable.
The remaining findings concern attempt identity, partial-delivery truth, reconciliation, TTL determinism, evidence lineage, asset identity, and configuration hardening. They are all repairable without discarding the work already completed.
I. Blocking invariants
B-01 — Per-asset incidents control one global vault actuator
Severity: Blocker
Status: Confirmed from the current control flow
StableGuardCREReceiver has one immutable vault, but it loops over an array of coins and advances a separate active event for each coin. When any one coin reaches RECOVERY_PENDING, the receiver calls vault.unpause() directly. There is no check for another active protected incident attached to the same vault.
Relevant code:
Reachable trace
USDC incident = PROTECTED
USDT incident = PROTECTED
vault.paused() = true
USDC then produces stabilityWindow accepted stable calls
USDC incident -> RECOVERY_PENDING
receiver processes USDC
receiver calls vault.unpause()
USDC destination -> COMPLETE
USDC may eventually -> NORMAL
USDT incident is still PROTECTED
but vault.paused() = false
The USDC event has authority over a physical actuator that is also enforcing the USDT event. The logical scope is per asset; the actuator scope is per vault. Those scopes are not equivalent.
This is not solved by making pause() and unpause() idempotent. Idempotence prevents a revert; it does not preserve the conjunction of active protection requirements.
Required invariant
For every vault V:
V may be unpaused
iff
there is no active protection hold requiring V to remain paused.
Formally:
unpauseAllowed(V) <=> activeHoldCount(V) == 0
Recommended repair
Introduce a vault-level protection coordinator or equivalent hold ledger:
struct ProtectionHold {
bytes32 holdId;
bytes32 rootIncidentId;
bytes32 assetId;
address vault;
bool active;
}
mapping(address vault => uint256 count) public activeHoldCount;
mapping(bytes32 holdId => ProtectionHold) public holds;
Protection becomes:
acquire hold for (vault, asset, rootIncident)
if activeHoldCount changed 0 -> 1:
physically pause vault
Recovery becomes:
release only this incident's hold
if activeHoldCount changed 1 -> 0:
physically unpause vault
else:
keep vault paused
The physical action must be derived from the aggregate hold set, not from the state of one coin event.
Minimal alternative
If a vault is intentionally single-asset, make that restriction structural:
one receiver instance
+ one vault
+ one canonical assetId
+ no multi-coin action loop
That is a valid smaller design. What is unsafe is advertising a multi-coin receiver while retaining a one-bit actuator with no aggregate authority model.
Missing test
1. Register USDC and USDT exposure for the same vault.
2. Drive both incidents to PROTECTED.
3. Feed stabilityWindow stable reports for USDC only.
4. Continue feeding HEDGE for USDT.
5. Assert USDC can close its own hold.
6. Assert vault remains paused while the USDT hold remains active.
B-02 — resumeProtectionTracking() reconstructs causality from the wrong fact
Severity: Blocker
Status: Confirmed; batch order changes the result
The continuation branch is evaluated when:
eventId == bytes32(0) && vault.paused()
It then creates a fresh PROTECTED event for the coin currently being processed. This happens before the exposure gate, and it does not require:
- a prior event for that coin;
- a prior event that expired;
- a parent event ID;
- an active hold belonging to that coin;
- evidence that this coin caused the pause;
- evidence that the existing pause belongs to DepegGuard at all.
Relevant code:
Single-report reproduction
Take one report with ordered arrays:
coins = [USDC, USDT]
signalLevels = [HEDGE, STABLE]
Assume both coins have no active event and USDC is registered as an exposure.
The receiver loop executes sequentially:
USDC:
processReport -> CONFIRMED_DEPEG
initiateProtection
vault.pause()
USDC -> PROTECTED
USDT:
processReport(STABLE) -> (0, NORMAL)
eventId == 0
vault.paused() == true
resumeProtectionTracking(USDT, ...)
USDT -> PROTECTED
The system has now created a protected USDT incident even though the same report said USDT was stable and no prior USDT incident existed.
Worse, reversing the array order changes the result:
[USDT STABLE, USDC HEDGE]
USDT is processed before the vault is paused, so no USDT event is created. The final state therefore depends on array order, not only on the observations.
That violates permutation invariance for a report whose coin observations are logically independent.
Consequence
The fabricated USDT event can later accumulate stable calls, enter RECOVERY_PENDING, and invoke vault.unpause() while the real USDC depeg remains active. In combination with B-01, this creates a complete false-unpause path.
Required invariant
A continuation event may exist only if it inherits a verified, still-active
protection hold for the same (vault, assetId, rootIncidentId).
A global physical bit such as paused() can confirm physical state, but it cannot identify the cause of that state.
Recommended repair
Do not infer incident lineage from vault.paused().
Either remove resumeProtectionTracking() entirely by separating hold lifetime from event-epoch lifetime, or require explicit lineage:
function resumeProtectionTracking(
bytes32 parentEventId,
bytes32 rootIncidentId,
bytes32 holdId,
bytes32 assetId,
bytes32 observationId
) external returns (bytes32 newEventId);
The registry must verify:
parentEvent is terminal for an allowed continuation reason
parentEvent.assetId == assetId
hold[holdId].active == true
hold[holdId].assetId == assetId
hold[holdId].rootIncidentId == rootIncidentId
hold[holdId].vault == destination vault
A new event epoch may be created, but it must not create a new causal history.
Missing tests
[A=HEDGE, B=STABLE] must not create a B incident.- Reversing the order to
[B=STABLE, A=HEDGE] must produce the same logical state.
- A vault paused for an unrelated reason must not permit any continuation event.
- An expired A event must not authorize continuation for B.
- A continuation must fail if the corresponding hold was already released.
B-03 — The receiver authenticates a Forwarder, not the intended workflow
Severity: Blocker before state-changing production use
Status: Confirmed security-boundary gap
The receiver checks only:
if (msg.sender != forwarder) revert UnauthorizedForwarder(msg.sender);
The metadata argument is explicitly ignored. Therefore, the consumer does not distinguish the intended DepegGuard workflow from another valid workflow delivered through the same Chainlink Forwarder.
Relevant code:
The Forwarder check proves the delivery channel. It does not, by itself, prove the application-level author of the report.
Required invariant
A state-changing report is accepted only when:
caller == configured Forwarder
AND workflowId == expectedWorkflowId
AND workflowOwner == expectedWorkflowOwner
AND destination chain == configured chain
AND receiver == address(this)
Workflow name may be checked as an additional label, but it should not be used without owner validation; the official template makes that distinction explicitly.
Recommended repair
Inherit from or reproduce the relevant checks from ReceiverTemplate and configure at least:
expected Forwarder
expected workflow ID
expected workflow owner/author
Also ensure that chain selector and receiver identity are authenticated and bound before any state mutation, whether through authenticated CRE metadata, a signed report envelope, or an equivalent domain-separated mechanism, so a report cannot be validly reused in another domain.
Missing tests
- correct Forwarder + wrong workflow ID → revert;
- correct Forwarder + wrong workflow owner → revert;
- correct workflow + wrong receiver domain → revert;
- correct workflow + wrong chain selector → revert;
- correct metadata and report → accepted.
B-04 — stabilityWindow counts calls, not fresh observations
Severity: Blocker
Status: Confirmed
While an event is PROTECTED, each call to processReport() with a score below watchThreshold increments stableCount. The registry receives no observation sequence, source timestamp, report ID, or digest that must be unique. The receiver also does not reject the same coin appearing more than once in a report array.
Relevant code:
Chainlink’s own documentation states that signed reports can be replayed on another chain or resubmitted on the same chain and that state-changing consumers must embed and verify protective metadata.
Two direct failure modes
A. Report replay
With stabilityWindow = 3:
one genuine stable observation
same signed report delivered three times
stableCount: 0 -> 1 -> 2 -> recovery
The system interprets repeated delivery of one observation as three consecutive observations.
B. Duplicate coin entries inside one accepted report
coins = [USDC, USDC, USDC]
signalLevels = [STABLE, STABLE, STABLE]
The receiver loops three times and calls processReport() three times in one transaction. The same payload can satisfy the entire stability window immediately.
Required invariant
stableCount may advance at most once per asset per accepted observation sequence.
A stronger definition is:
accepted observation n+1 must have:
sequence > lastAcceptedSequence
sourceObservedAt > lastAcceptedSourceObservedAt
sourceObservedAt within an allowed freshness window
assetId unique within the report
Recommended report envelope
struct ReportEnvelope {
bytes32 workflowId;
uint64 sourceChainSelector;
address receiver;
uint64 sequence;
uint64 sourceObservedAt;
uint64 validUntil;
bytes32 payloadDigest;
CoinObservation[] observations;
}
struct CoinObservation {
bytes32 assetId;
bytes32 feedId;
int192 rawPrice;
bytes32 evidenceDigest;
}
The receiver should reject:
sequence <= lastAcceptedSequence[workflowId]
sourceObservedAt <= lastObservedAt[assetId]
block.timestamp > validUntil
sourceObservedAt too far in the future
sourceObservedAt older than maxReportAge
repeated assetId within one envelope
wrong chain selector
wrong receiver
wrong workflow identity
Only after those checks should the registry receive an accepted observation ID and update stableCount.
Missing tests
- replay the exact same stable report
stabilityWindow times; count must advance once;
- submit the same sequence with modified payload; reject;
- submit a lower sequence; reject;
- submit a stale timestamp; reject;
- include the same asset twice in one report; reject atomically;
- submit two distinct fresh reports; count advances twice.
B-05 — Controller transfer creates an authority dead-end
Severity: Blocker for documented operations
Status: Confirmed integration defect
DepegEventRegistry has one controller. The documented deployment flow transfers that role to StableGuardCREReceiver. The integration test does exactly that.
Relevant code:
After transfer, only the receiver contract can call controller-gated methods. However, the receiver exposes no governance or forwarding entry point for:
retryFailedDestinations();supersede();- manual
initiateRecovery();
- future
transferController();
- a customer-held early-unlock or override path described in
ACTION_RECORD.md.
An EOA or multisig cannot call those registry functions because it is no longer controller. The receiver cannot spontaneously call them because no external function instructs it to do so.
This means several advertised recovery and governance paths become unreachable in the deployed composition even though they are callable in isolated registry tests.
Recommended repair
Replace the single controller with explicit capabilities:
REPORTER_ROLE
accepts authenticated observations and score transitions
ACTION_ROLE
records destination callbacks and dispatch results
RETRY_ROLE / KEEPER_ROLE
retries or times out delivery attempts
GOVERNANCE_ROLE
changes configuration, supersedes eligible incidents,
authorizes exceptional recovery, rotates roles
PAUSE_COORDINATOR_ROLE
acquires/releases physical vault holds
A multisig or timelocked governance contract should retain governance and role-rotation authority. The CRE receiver should receive only the minimum roles required for automatic operation.
If the single-controller model is retained, the receiver needs explicit, access-controlled forwarding methods for every operation that must remain reachable. That is less clean but still better than an unreachable authority graph.
Missing test
Deploy exactly as intended, transfer control to the receiver, and prove that the designated governance identity can still:
- retry a failed destination;
- apply an allowed override;
- rotate the receiver/controller;
- supersede a pre-protection incident;
- recover from a receiver upgrade or key compromise.
II. Asynchronous delivery and reconciliation
H-01 — A destination slot has no delivery-attempt identity
Severity: High before real CCIP/asynchronous integration
Status: Confirmed design gap
A Destination contains only:
chainSelector
vault
state
A callback contains only:
eventId
destIndex
newDestState
A retry changes only:
FAILED -> PENDING
Relevant code:
Dangerous ordering
attempt 1 dispatched
attempt 1 reports FAILED
slot becomes FAILED
attempt 2 is queued
slot becomes PENDING
late callback from attempt 1 arrives before attempt 2 callback
slot is currently PENDING
old callback is accepted as the result of attempt 2
The sticky-slot rule correctly prevents a callback from overwriting COMPLETE. It does not distinguish two different attempts that occupy the same PENDING state at different times.
The existing stale-callback test covers:
retry succeeds -> slot COMPLETE -> stale FAILED arrives
That case is rejected. The untested and dangerous interval is:
retry requeued -> slot PENDING -> stale old callback arrives -> new callback has not arrived yet
Required identity
Destination identity != Delivery-attempt identity
Recommended attempt record:
enum Phase { PROTECTION, RECOVERY }
enum AttemptState { PENDING, COMPLETE, FAILED, TIMED_OUT, SUPERSEDED }
struct DeliveryAttempt {
bytes32 eventId;
Phase phase;
bytes32 destinationKey;
uint32 attemptNo;
bytes32 messageId;
uint64 startedAt;
uint64 deadline;
AttemptState state;
}
The callback must bind at least:
eventId
phase
destinationKey
attemptNo
messageId
result
A callback for attempt n must never be able to settle attempt n+1.
Missing test
1. attempt 1 -> FAILED
2. queue attempt 2 -> PENDING
3. deliver late COMPLETE or FAILED from attempt 1
4. assert rejection because messageId/attemptNo is stale
5. deliver attempt 2 callback
6. assert only attempt 2 changes current destination state
H-02 — A retried destination can become permanently non-retryable
Severity: High
Status: Confirmed liveness gap
retryFailedDestinations() is allowed only from PARTIALLY_PROTECTED or PARTIALLY_RECOVERED and changes a failed slot to PENDING. settlePending() is explicitly not available in the PARTIALLY_* states.
If the retried asynchronous message never returns:
slot remains PENDING
retry cannot be called again because slot is not FAILED
settlePending cannot be called because event is PARTIALLY_*
The only remaining bound is the outer event TTL. That can leave the system unable to retry for the full incident lifetime.
Recommended repair
Give each attempt its own deadline and a permissionless timeout transition:
PENDING --after attemptDeadline--> TIMED_OUT
TIMED_OUT -> eligible for next attempt
The event’s outer TTL should cap the incident; it should not substitute for per-attempt liveness.
Missing test
PARTIALLY_PROTECTED
failed destination retried
new attempt never callbacks
attempt deadline passes
timeout marks attempt TIMED_OUT
next retry is permitted before outer eventTTL
H-03 — pendingTTL collapses partial physical truth into global FAILED
Severity: High
Status: Confirmed
Consider two destinations:
destination A = COMPLETE
destination B = PENDING
Because not all destinations are settled, the event remains PROTECTION_PENDING. After pendingTTL, either settlePending() or a subsequent callback terminates the entire event as FAILED.
Relevant code:
The ledger then says:
Event = FAILED
while physical reality says:
A is protected
B timed out
This discards exactly the partial-delivery state that PARTIALLY_PROTECTED and PARTIALLY_RECOVERED were introduced to preserve.
Recommended repair
At pending timeout:
- mark only still-
PENDING attempts as TIMED_OUT;
- preserve already
COMPLETE destinations;
- evaluate the aggregate state;
- produce:
PROTECTED if all effective destinations completed;PARTIALLY_PROTECTED if at least one completed and at least one failed/timed out;FAILED only if none completed;
- mirror the same logic for recovery.
The event state should summarize destination truth, not erase it.
Missing tests
- one complete + one pending at timeout →
PARTIALLY_PROTECTED;
- one complete + one pending at recovery timeout →
PARTIALLY_RECOVERED;
- all pending time out →
FAILED;
- completed destination remains queryable and unchanged after timeout.
H-04 — Physical action and ledger acknowledgement can diverge permanently
Severity: High
Status: Confirmed reconciliation gap
The receiver performs the physical action first and then reports the result to the registry.
Recovery divergence
vault.unpause() succeeds
registry.destinationCallback(...) reverts
The receiver emits RegistryCallbackFailed, but the physical vault is already unpaused. On the next report, the event may still be RECOVERY_PENDING; however, because vault.paused() is now false, the receiver skips the callback block entirely and only attempts finalizeRecovery(). finalizeRecovery() cannot succeed while the destination remains PENDING.
Relevant code:
The event can therefore remain pending or eventually become failed/expired while the vault is physically unpaused.
Protection divergence
A real Pausable implementation commonly reverts on pause() when already paused. The current mock appears to model idempotent behavior, which hides the distinction between:
pause call reverted because vault was already safely paused
and:
pause action failed and vault remains unprotected
The receiver treats any pause() revert as FAILED, even if the postcondition vault.paused() == true is already satisfied.
Required invariant
The action record must be based on verified postcondition, not only call return.
For pause:
success <=> vault.paused() == true
For unpause:
success <=> vault.paused() == false
AND aggregate active hold count == 0
Recommended repair
Use explicit reconciliation:
1. read desired physical state from hold coordinator;
2. attempt action only if current state differs;
3. re-read physical state;
4. record COMPLETE if the required postcondition is true;
5. if the registry callback fails, retain a reconciliation job keyed by
event/phase/destination/attempt and retry the acknowledgement independently.
Do not make future acknowledgement conditional on repeating the physical action.
Missing tests
- unpause succeeds, callback reverts once, next report reconciles the destination to
COMPLETE without calling unpause again;
- pause reverts because vault is already paused, postcondition check records protection success;
- pause reverts and vault remains unpaused, destination records failure;
- registry says complete but physical state disagrees, reconciliation detects and corrects/escalates.
H-05 — The declared outer-TTL priority is not global
Severity: High
Status: Confirmed deterministic-state defect
The code comment says eventTTL always wins, and some paths enforce it. Other mutating paths do not.
Examples:
processReport() / _applyScoreTransitions() checks outer TTL;destinationCallback() checks outer TTL;initiateRecovery() checks outer TTL;finalizeRecovery() does not;settlePending() does not first apply outer TTL;supersede() does not first apply outer TTL;retryFailedDestinations() does not first apply outer TTL.
Relevant code:
After the same outer deadline, terminal state can depend on which function is called first:
settleExpired() -> EXPIRED
settlePending() -> FAILED
finalizeRecovery() -> NORMAL
supersede() -> SUPERSEDED (for eligible pre-protection states)
If the specification says the outer TTL has highest priority, this is nondeterministic relative to call ordering.
Recommended repair
Centralize expiry as a guard on every mutating entry point:
modifier applyOuterExpiry(bytes32 eventId) {
DepegEvent storage ev = _events[eventId];
_requireExists(ev);
if (!_isTerminal(ev.state) && block.timestamp >= ev.createdAt + eventTTL) {
_terminate(eventId, _coinKey(ev.coin), State.EXPIRED);
return;
}
_;
}
Because Solidity modifiers with early return need careful design, an internal helper returning a boolean may be clearer:
if (_expireIfDue(eventId)) return;
Apply one documented precedence order everywhere. If some transitions are intentionally allowed after outer TTL, state that explicitly instead of calling G13 universal.
Missing tests
At createdAt + eventTTL, invoke every mutating path in multiple orders and assert the same terminal result.
H-06 — Protection and recovery overwrite one another’s destination history
Severity: High for auditability and asynchronous correctness
Status: Confirmed
initiateRecovery() deletes the protection destinations and repopulates the same array for recovery. Auto-recovery reuses the protection array and resets every slot to PENDING.
Relevant code:
Consequences:
- the final onchain event no longer contains the protection delivery record;
- a late protection callback cannot be distinguished structurally from a recovery callback if the same index is now pending;
- the meaning of
destinations[i].state changes by phase without phase being part of the slot identity;
- forensic reconstruction requires event logs and assumptions rather than durable phase records.
Recommended repair
Use separate append-only phase records:
struct PhaseRecord {
Phase phase;
uint64 openedAt;
uint64 closedAt;
DeliveryAttempt[] attempts;
}
mapping(bytes32 eventId => PhaseRecord protection);
mapping(bytes32 eventId => PhaseRecord recovery);
Or store a monotonically increasing phase ID and make every callback phase-qualified. Do not mutate a protection result into a recovery request.
H-07 — Auto-recovery resurrects SUPERSEDED destinations
Severity: High
Status: Confirmed semantic contradiction
During protection, SUPERSEDED is treated as settled but excluded from the effective destination count. This allows the event to reach PROTECTED without that destination completing.
Auto-recovery then loops over every stored destination and sets every state to PENDING, including destinations previously marked SUPERSEDED.
Relevant code:
A destination declared no longer relevant during protection becomes required again during recovery without a reactivation decision.
Recommended repair
Either:
- keep
SUPERSEDED excluded across all later phases for the incident; or
- create an explicit governance transition
reactivateDestination() with a reason and new destination generation.
A blanket reset must not erase exclusion semantics.
Missing test
Protect three destinations with one SUPERSEDED, trigger auto-recovery, and assert the superseded destination remains excluded unless explicitly reactivated.
H-08 — recoveryCooldown currently delays ledger closure, not physical reopening
Severity: High if cooldown is intended as a safety delay; otherwise specification mismatch
Status: Confirmed behavior, policy meaning must be clarified
The receiver unpauses the vault as soon as the incident enters RECOVERY_PENDING. finalizeRecovery() applies recoveryCooldown only before changing the event to NORMAL.
Therefore:
physical vault reopening happens first
cooldown delays only bookkeeping closure
Relevant code:
If the intended policy is “observe stability for N reports, then wait a cooldown before reopening,” the current implementation does not enforce it. If the intended policy is “reopen immediately after N stable reports but delay terminal record closure,” then the name and documentation should say that explicitly.
Recommended repair
Choose one of two explicit policies:
Safety-delay policy
PROTECTED
-> STABILITY_CONFIRMED
-> wait recoveryCooldown
-> authorize hold release
-> physically unpause if aggregate hold count reaches zero
-> NORMAL after reconciled completion
Immediate-reopen policy
PROTECTED
-> stabilityWindow satisfied
-> release hold immediately
-> cooldown applies only to archival/closure
The first is safer. In either case, the physical transition and its authority must be represented explicitly.
H-09 — Event expiry and physical protection lifetime are coupled incorrectly
Severity: High
Status: Confirmed architectural cause of the resume problem
The current continuation logic exists because an event can expire while the vault remains paused. The system then creates a new independent PROTECTED event to continue counting stability.
This reveals that one object is being used for two lifetimes:
incident/event epoch lifetime
physical protection-hold lifetime
Those lifetimes are not necessarily equal. A record may need to close, rotate, or archive while the physical protection obligation remains active.
Recommended repair
Separate them:
RootIncident
durable causal identity
EventEpoch
bounded observation/processing window
may expire and be replaced
ProtectionHold
remains active until an authorized recovery condition releases it
An event epoch expiry must never fabricate, remove, or transfer a physical hold. A successor epoch references the same rootIncidentId and active holdId.
This eliminates the need to infer continuation from paused() and prevents indefinite chains of unrelated fresh incidents.
III. Evidence, identity, and source binding
E-01 — The Data Streams evidence chain disappears before the action record
Severity: High for evidence-backed automation
Status: Confirmed
The workflow retrieves fullReport and validFromTimestamp. It forwards fullReport in the payload, but the receiver decodes and discards it. The receiver passes bytes32(0) as evidenceRoot to every registry transition. The registry records its own block.timestamp rather than the source observation time.
Relevant code:
The resulting action event is not cryptographically linked to the Data Streams evidence that caused it.
Required separation
Store at least:
sourceObservedAt = time claimed/verified by source report
acceptedAt = block.timestamp when consumer accepted it
workflowId = authenticated workflow identity
sequence = monotonic accepted report sequence
evidenceDigest = digest of the verified source evidence and normalized observation
Do not overwrite the only root on every extension. Use an append-only event or rolling accumulator:
newAccumulator = keccak256(oldAccumulator, observationId, evidenceDigest)
For forensic access, emit one ObservationAccepted event per asset with the full identity tuple.
Gas-conscious design
The full signed report does not need to be stored onchain. Store its digest and preserve the full bytes offchain in a content-addressed evidence package. The digest must be computed before discarding the bytes.
E-02 — Feed-to-asset binding is positional and not independently verified
Severity: High
Status: Confirmed
The workflow requests one report per configured coin, stores results in an array, then later pairs dsReports[i] with coins[i]. It does not verify that the returned report.feedID equals the configured coin.feedId before using the price.
Relevant code:
The onchain receiver also receives no feedId, so it cannot validate the mapping independently.
Recommended repair
For every observation, carry and verify:
assetId
feedId
source domain
raw source price
sourceObservedAt
evidenceDigest
The workflow must verify report.feedID == configured feedId. The receiver should maintain an allowlist:
allowedFeed[assetId] == feedId
If verification of the full Data Streams report occurs elsewhere, bind the verified digest to that exact tuple.
E-03 — Exposure binding proves configured intent, not actual holdings
Severity: High for claim accuracy; implementation-dependent for runtime risk
Status: Confirmed scope mismatch
ExposureRegistry is a manual admin registry. Its own comment says Phase 1 uses manual registration. Yet receiver comments and ACTION_RECORD.md describe the vault as “demonstrably” or “provably” holding the asset.
Relevant code and documentation:
What is currently proven is narrower:
an asset not registered in the configured exposure set cannot enter the pause path
What is not proven is:
registered exposure == current actual vault holdings
A stale or incorrect admin registration passes the gate even if the vault no longer holds the asset.
Two valid resolutions
A. Narrow the claim
Rename it ConfiguredExposureRegistry and state exactly what it proves.
B. Strengthen the mechanism
Reconcile configured exposure against authoritative vault accounting or token balances, with a clearly defined treatment of:
- wrapped assets;
- debt exposure;
- LP-token indirect exposure;
- cross-chain representations;
- temporarily zero balances;
- delegated/custodied positions.
The second option needs an explicit exposure ontology; a simple ERC-20 balanceOf check is not sufficient for every vault model.
E-04 — Asset identity is ambiguous and not chain-qualified
Severity: High in a multi-chain design
Status: Confirmed
ExposureRegistry calls its key a symbol and gives keccak256("USDC") as an example. The receiver and tests instead use a token address padded into bytes32. The production and staging configs run on Sepolia but contain Ethereum mainnet token addresses.
Relevant code:
An address is not globally unique across chains, and a symbol is not unique even within one ecosystem.
Recommended canonical identity
bytes32 assetId = keccak256(
abi.encode(
sourceChainSelector,
canonicalTokenAddress,
representationType
)
);
If the system acts on a destination-chain representation, distinguish:
canonical economic asset
source feed identity
destination token contract
vault position identity
Do not call all four “coin” or “symbol.”
E-05 — The receiver trusts mutually redundant fields without checking consistency
Severity: High
Status: Confirmed
The payload contains:
price
deviationBps
signalLevel
compositeScore
marketStress
The receiver stores or acts on these values but does not verify that they agree. A report can claim:
price = $1.00
deviation = 0
signalLevel = EXIT
and the action path follows signalLevel.
The workflow is expected to be trusted after workflow authentication, but redundant unverified fields widen the semantic attack and error surface. They also make future workflow upgrades capable of changing action semantics without an onchain invariant.
Recommended repair
Choose one canonical source input and derive cheap fields onchain, or validate consistency:
recompute deviation from raw price and configured peg
recompute signal from threshold configuration
recompute composite score from accepted per-asset signals
If gas cost requires offchain derivation, commit to a versioned policy hash:
policyId / thresholdSetHash / scoringVersion
and verify that the report names an allowed policy version.
E-06 — Source timestamp and freshness are not enforced
Severity: High
Status: Confirmed
validFromTimestamp is retrieved but not used in the action payload. The payload uses nowSec from workflow execution. The receiver writes lastObservedAt without monotonicity or maximum-age checks, and the registry replaces it with block.timestamp.
A stale source report can therefore be transformed into a fresh-looking workflow report.
Recommended repair
Carry both:
sourceObservedAt
workflowGeneratedAt
Verify:
sourceObservedAt <= workflowGeneratedAt
workflowGeneratedAt <= block.timestamp + allowedClockSkew
block.timestamp - sourceObservedAt <= maxSourceAge
sourceObservedAt > lastSourceObservedAt[assetId]
sequence > lastSequence
Time is not a substitute for sequence, but both are useful.
IV. Receiver and workflow hardening
Several items in this section are hardening consequences or implementation surfaces of earlier root findings rather than separate root defects. In particular, duplicate-asset handling within R-01 supports B-04, R-09 is part of B-04/E-06, and R-10 is an efficiency/evidence consequence of E-01. They remain listed for implementation completeness, but should not be counted as independent findings where those dependencies apply.
R-01 — Array shape, uniqueness, and batch bounds are unchecked
Severity: Medium to High
Status: Confirmed
The receiver decodes parallel arrays and indexes prices[i], deviationsBps[i], and signalLevels[i] for every coins[i] without first requiring equal lengths. It also does not bound batch size, reject zero addresses, or reject duplicate asset identities.
Consequences:
- a short parallel array reverts the whole report;
- duplicate assets can advance one incident multiple times;
- an oversized report can make action execution exceed practical gas limits;
- one malformed coin blocks every well-formed coin in the same batch.
Recommended checks
coins.length > 0
coins.length <= MAX_BATCH
prices.length == coins.length
deviationsBps.length == coins.length
signalLevels.length == coins.length
fullReports.length == coins.length
no coin == address(0)
no duplicate assetId
signalLevel within enum range
Decide deliberately whether malformed input rejects the whole signed report or isolates one observation. For authenticated reports, atomic rejection is usually preferable because partial interpretation of a malformed envelope can hide workflow defects.
R-02 — Per-coin failure isolation is incomplete
Severity: Medium
Status: Confirmed
processReport(), pause(), unpause(), and some registry callbacks are wrapped in try/catch. However, external view calls such as vault.paused() and exposureRegistry.isExposed() are outside try/catch. A revert from either dependency reverts the entire onReport() transaction.
The current comment suggests per-coin isolation, but it is not complete across dependency boundaries.
Recommended repair
Either:
- deliberately make the report atomic and remove the partial-isolation implication; or
- move every external dependency behind a per-coin adapter call that can be caught and recorded.
For safety actions, atomicity may be the cleaner default if a partially applied multi-asset report would create inconsistent interpretation. The correct choice should be explicit.
R-03 — Negative Data Streams prices are converted to positive values
Severity: Medium
Status: Confirmed fail-open conversion
The workflow applies absolute value:
const price18 = raw >= 0n ? raw : -raw
with the comment that stablecoin prices are never negative.
Relevant code:
If negative values are impossible by specification, receiving one is evidence of malformed data, decoding error, feed mismatch, or upstream corruption. Converting it into a plausible positive value destroys that evidence.
Recommended repair
require raw > 0
require raw within configured plausible bounds
fail closed on violation
emit/store reason in the detection record
R-04 — Configured chain selector is not the selector used by the workflow
Severity: Medium
Status: Confirmed
Config includes chainSelectorName, but main.ts constructs EVMClient(SEPOLIA_CHAIN_SELECTOR) using a hardcoded constant. The configuration therefore appears authoritative while not controlling execution.
Relevant code:
Recommended repair
Resolve the configured chain selector to the exact numeric selector and use that value for:
- EVM client construction;
- report domain binding;
- asset identity;
- receiver configuration;
- destination validation.
Reject configuration if those values disagree.
R-05 — The workflow computes a cooldown policy surface that does not control the payload
Severity: Medium
Status: Confirmed dead-policy surface
The workflow reads COOLDOWN(), computes triggerable = results.filter(...), and comments that the receiver enforces lastTriggered. The encoded payload nevertheless contains all results, and the current receiver has no lastTriggered cooldown guard matching that comment.
Relevant code:
Dead policy code is dangerous because reviewers may believe a limit is enforced when it is not.
Recommended repair
Either remove the obsolete cooldown read/filter and update comments, or define exactly which transition it controls and enforce it in one authoritative layer.
R-06 — Constructor and destination configuration validation is insufficient
Severity: Medium
Status: Confirmed
DepegEventRegistry assigns constructor parameters without validating:
- zero controller;
watchThreshold <= confirmedThreshold;- nonzero
stabilityWindow;
- nonzero and coherent TTLs;
- relation between
pendingTTL, recoveryCooldown, and eventTTL.
Destination inputs are not checked for zero vault addresses or duplicates.
Recommended minimum validation
controller != 0
watchThreshold < confirmedThreshold
stabilityWindow > 0
pendingTTL > 0
eventTTL > pendingTTL
recoveryCooldown < eventTTL
all destination vaults != 0
all destination keys unique within a phase
The precise inequalities depend on policy, but invalid states should be rejected at construction rather than discovered after deployment.
R-07 — Empty effective destination sets have no defined closure
Severity: Medium
Status: Confirmed
If every destination is SUPERSEDED, effectiveTotal == 0. _evaluateProtection() and _evaluateRecovery() return and wait for TTL. finalizeRecovery() also rejects effectiveTotal == 0.
This leaves an incident in a nonterminal state despite having no remaining effective work.
Recommended repair
Specify one policy:
- terminal
SUPERSEDED for the phase/incident;
- governance-required closure;
NO_EFFECTIVE_DESTINATIONS explicit state;- hard rejection of superseding the last effective destination.
Do not leave the result implicit until TTL.
R-08 — Callback result values are not phase-restricted
Severity: Medium
Status: Confirmed
destinationCallback() accepts any DestState value supplied by the controller, including PENDING. The enum is ABI-range checked, but semantic transition validity is not.
A callback should normally settle an attempt into a terminal attempt state, not rewrite it to PENDING. SUPERSEDED may also require governance authority rather than a delivery callback.
Recommended repair
Separate functions or validate by origin:
recordDeliveryResult(... COMPLETE | FAILED)
markAttemptTimedOut(...)
supersedeDestination(...) only governance
queueRetry(...) creates new PENDING attempt
R-09 — lastObservedAt can move backward
Severity: Medium
Status: Confirmed
The receiver assigns lastObservedAt = observedAt for every accepted call without checking monotonicity. A replayed or stale report can roll the public latest-observation field backward even before considering state transitions.
This should be closed as part of B-04/E-06 with sequence and freshness checks.
R-10 — Large evidence bytes are transmitted onchain and then ignored
Severity: Medium / efficiency
Status: Confirmed
fullReports are included in the ABI payload, decoded, and discarded. This increases calldata and verification cost without contributing to onchain evidence.
If the receiver does not verify or store the full reports, transmit only a digest plus the normalized verified fields. If onchain verification is intended later, make that boundary explicit and do not describe the current path as evidence-bound until it exists.
V. Documentation consistency
D-01 — ACTION_RECORD.md contains two incompatible recovery descriptions
Severity: Documentation defect
Status: Confirmed
Section 1.3 says recovery is not wired into the CRE workflow and requires an explicit controller call. Section 3.5 later says the default path is fully automatic and no manual call is required.
Relevant locations:
Update section 1.3 to describe the current branch and then list the unresolved authority/reconciliation conditions identified above.
D-02 — Replay safety is currently stated too broadly
Severity: Documentation/assurance defect
Status: Confirmed
The sticky-slot rule proves:
once this slot is no longer PENDING, a later callback cannot overwrite it
It does not prove:
a callback belongs to the current attempt
After a retry returns the slot to PENDING, an old callback is acceptable unless attempt identity is added. The assurance claim should be narrowed until H-01 is fixed.
D-03 — “Correct by construction under asynchronous CCIP” is premature
Severity: Documentation/assurance defect
Status: Confirmed
The local state machine is stronger than before, but asynchronous correctness requires at least:
- phase-qualified callback identity;
- attempt/message identity;
- per-attempt timeout;
- stale-attempt rejection;
- physical/ledger reconciliation;
- preserved partial-delivery truth.
Until those are implemented and tested with realistic delayed/reordered callbacks, the honest statement is:
The local destination-state guards are implemented and tested.
End-to-end asynchronous correctness remains unproven.
That wording is fully consistent with the transparent style already used elsewhere in the record.
VI. Two valid implementation routes
The current system can be completed in either of two coherent ways.
Route A — Scope reduction: one asset per receiver/vault domain
This is the smaller route.
Structural rules
one receiver instance binds exactly one assetId
one receiver instance binds exactly one vault
one active root incident domain
no multi-coin action loop
reports for any other asset are rejected
Still required
Even with scope reduction, the implementation still needs:
- exact workflow identity validation;
- report sequence/freshness/replay protection;
- delivery-attempt identity;
- deterministic TTL precedence;
- physical/ledger reconciliation;
- evidence digest lineage;
- reachable governance and controller rotation.
Benefit
It eliminates the shared-actuator conflict and the cross-asset fabricated-resume path by construction. It is appropriate if the intended product is a dedicated protection controller per stablecoin vault.
Route B — Full multi-asset architecture
This is the more general route and fits the current batched monitoring design.
Required additional object
Introduce a VaultProtectionCoordinator that owns the physical pause/unpause authority and maintains asset/incident-scoped holds.
Observation accepted
-> asset incident advances
-> incident acquires/releases its own hold
-> coordinator derives global vault desired state
-> actuator reconciles physical state
-> action acknowledgement is recorded independently
Benefit
Multiple assets can share one vault safely, and one recovered asset cannot release another asset’s protection requirement.
Cost
More state and more tests, but the semantics become explicit and extensible to cross-chain destinations.
VII. Recommended target model
The cleanest completion separates four identities.
1. Observation identity
struct Observation {
bytes32 observationId;
bytes32 workflowId;
uint64 sequence;
bytes32 assetId;
bytes32 feedId;
uint64 sourceObservedAt;
uint64 acceptedAt;
bytes32 evidenceDigest;
uint8 policyVersion;
}
Invariant:
one observation can affect one asset incident at most once
2. Root incident and event epoch identity
struct IncidentEpoch {
bytes32 eventId;
bytes32 rootIncidentId;
bytes32 parentEventId;
bytes32 assetId;
IncidentState state;
uint64 createdAt;
uint64 expiresAt;
uint32 stableObservationCount;
uint64 lastObservationSequence;
}
Invariant:
an epoch may expire, but causal lineage remains explicit
3. Physical hold identity
struct VaultHold {
bytes32 holdId;
bytes32 rootIncidentId;
bytes32 assetId;
address vault;
bool active;
}
Invariant:
physical unpause occurs only when the aggregate active hold set is empty
4. Delivery-attempt identity
struct DeliveryAttempt {
bytes32 attemptId;
bytes32 eventId;
Phase phase;
bytes32 destinationKey;
uint32 attemptNo;
bytes32 messageId;
uint64 startedAt;
uint64 deadline;
AttemptState state;
}
Invariant:
only the callback bound to the current attempt may settle that attempt
VIII. Required regression matrix
The following tests should be added before describing the system as complete.
Shared actuator and causal lineage
- USDC and USDT both
PROTECTED; USDC recovers; USDT remains HEDGE; vault remains paused.
[USDC=HEDGE, USDT=STABLE] does not create a USDT continuation.- Reversing the array order produces the same logical result.
- A pre-paused vault with no DepegGuard hold creates no incident.
- An expired USDC event cannot authorize a USDT continuation.
- A successor epoch must reference the same root incident and active hold.
- Releasing the final hold unpauses; releasing a nonfinal hold does not.
Report authority, replay, and freshness
- wrong workflow ID rejected;
- wrong workflow owner rejected;
- wrong chain domain rejected;
- wrong receiver domain rejected;
- identical report replayed three times advances stable count once;
- duplicate asset within one report rejected;
- lower/equal sequence rejected;
- stale
sourceObservedAt rejected;
- future timestamp beyond allowed skew rejected;
- feed ID mismatch rejected;
- negative or zero source price rejected.
Delivery attempts
- attempt 1 late callback after attempt 2 is queued is rejected;
- attempt 1 and attempt 2 may have opposite results without ambiguity;
- retried attempt can time out and be retried again;
- callback with wrong phase rejected;
- callback with wrong destination key rejected;
- callback with wrong message ID rejected.
Partial truth and reconciliation
- one complete + one timed out →
PARTIALLY_PROTECTED;
- one recovered + one timed out →
PARTIALLY_RECOVERED;
- unpause succeeds, registry acknowledgement fails once, next cycle reconciles;
- already-paused real vault is recognized by postcondition rather than marked failed;
- physical state disagreement with ledger is detected;
- protection history remains queryable after recovery starts;
SUPERSEDED destination remains excluded through recovery.
TTL and closure determinism
- every mutating function called after outer TTL produces the same specified result;
settlePending after outer TTL cannot produce a different terminal state;finalizeRecovery after outer TTL follows the documented precedence;- empty effective destination set has an explicit closure state;
- event epoch expiry does not release or fabricate a physical hold.
Authority and upgradeability
- after deployment, governance can still retry, rotate roles, and apply documented overrides;
- compromised receiver can be revoked without redeploying the registry;
- governance cannot forge an observation unless explicitly granted reporter authority;
- automatic receiver cannot use governance-only supersede/override paths.
IX. Implementation order
Phase 0 — Do not attach real value-moving authority yet
Keep the current branch test-only until B-01 through B-05 are resolved.
Phase 1 — Repair object boundaries
- choose Route A or Route B;
- introduce asset-qualified holds or enforce one-asset scope;
- remove pause-bit-based causal inference;
- add root incident and parent epoch lineage;
- separate registry roles.
Phase 2 — Secure report acceptance
- validate workflow ID/owner metadata;
- add domain binding;
- add monotonic sequence and freshness;
- reject duplicate assets and malformed arrays;
- bind feed ID and asset ID;
- derive or validate action fields.
Phase 3 — Complete asynchronous semantics
- add phase and attempt identity;
- add per-attempt deadlines;
- preserve partial truth on timeout;
- separate protection and recovery records;
- preserve
SUPERSEDED semantics;
- implement physical/ledger reconciliation.
Phase 4 — Evidence and hardening
- carry evidence digest and source time;
- update exposure claim or strengthen reconciliation;
- validate constructors/configuration;
- remove dead cooldown policy code;
- update
ACTION_RECORD.md to one consistent current description;
- run the full regression matrix, then real delayed/reordered delivery tests.
X. Completion criteria
I would consider this layer complete when the following invariants are structural rather than procedural assumptions:
I1. An observation is accepted once, for the intended workflow and domain.
I2. An asset incident cannot be created, resumed, or closed from another
asset's physical state.
I3. One incident can release only its own protection hold.
I4. A vault is physically unpaused only when its aggregate active hold set is empty.
I5. Every destination callback is bound to one phase and one delivery attempt.
I6. Timeout preserves completed physical work and records partial truth.
I7. Event terminal state is independent of transaction ordering at the same deadline.
I8. Physical actuator state and the action ledger are eventually reconciled after
either side succeeds while the other side temporarily fails.
I9. The action record is cryptographically linked to the accepted source evidence.
I10. Governance, retry, override, and role-rotation paths remain reachable after
the receiver is installed, without giving the receiver unnecessary authority.
Final assessment
The implementation has advanced materially. The original three invariants were not ignored or cosmetically patched; they were integrated into the state machine and documented. The next defects are therefore mostly second-order defects: they arise where asset-scoped logic, vault-scoped physical authority, report identity, and asynchronous delivery identity meet.
The central correction is not another isolated guard. It is to stop treating these pairs as equivalent:
processReport call == fresh observation
coin event == vault protection authority
vault.paused() == proof of this coin's incident
PENDING destination slot == current delivery attempt
one mutable destination list == complete phase history
Once those identities are separated, the rest of the repair becomes systematic rather than ad hoc. The existing work can remain as the event-policy layer; it needs a qualified observation boundary above it, a hold coordinator beside it, and an attempt-qualified delivery ledger below it.
That would turn the current local state-machine proof into a coherent end-to-end protection architecture.