Repository navigation
Conversation
Phase 2 of the Sovryn security perimeter. Where the Perimeter Fee takes a cut of a user-initiated exit, the Perimeter Delay can hold the remainder for a governance-configured period, so a detected theft can be frozen or blacklisted and routed to recovery before the funds leave the protocol. - ExitDelayQueue: a per-request escrow holding the user leg of an exit until its unlock time. Requests are immutable once recorded, ingress is restricted to registered product sources, and the payout is settled before any external call. Only the originator or the position owner may execute a request — the receiver never can, so a withdrawal split into a fee leg and a delayed leg is always completed by the same actor who started it; - three ways out of the queue: normal execution after unlock, a pre-registered recovery route for funds belonging to a blacklisted party, and an owner-level catch-all bounded to requests that are blocked, paused or still locked, so a healthy in-flight exit can never be touched by governance; - self-service recovery for an undeliverable payout: the stored receiver is attempted first and an alternative is paid only if that genuinely bounces, which keeps a healthy exit from being redirected; - ExitFeeController gains the delay extension: a global kill switch independent of the fee switch, one global delay, and bypass tiers mirroring the fee tiers (actor, sub-product, surface) plus a surface-scoped passthrough registry for contracts that withdraw on a user's behalf; - deploy and verification scripts for the queue, its host wiring, and the go-live gates; unit, invariant and Echidna coverage for the queue. The delay ships disabled and is enabled only by governance after post-deployment verification.
The delay extension declared `securityPerimeterEnabled` and `globalDelaySeconds` ahead of `admin`. All three are small enough to share one slot, so Solidity packed them together and moved `admin` from offset 0 to offset 5 of slot 257 — a slot the deployed controller already uses. Upgrading the live proxy to that layout would have reinterpreted the stored admin address: the perimeter would have read as enabled with no governance action, the global delay as roughly 112 years, and `admin` as an address nobody holds. Nothing would have reverted. `admin` is now declared first and alone, exactly where the fee release put it, and the two delay scalars move into a slot reclaimed from `__gap`. Slot 271 is then closed with an explicit reservation: left half-used, its 27 free bytes would capture the next field any future upgrade appends, landing it outside the reserved gap and tripping the same class of check. The layout mirrors in the upgrade-safety fixtures move with it. tools/diff-storage-layouts.py now reports the candidate upgrade-safe against the recorded mainnet layout, where it previously failed; the fixture harness accepts the positive case and still rejects both negatives. Behaviour is unchanged — no logic, no interface, no ABI difference — and the suite is green at 389 tests.
The storage section had drifted into narrating how the layout came about — which release shipped which slot, why one placement was chosen over another, what a verification tool would or would not accept. None of that helps someone reading or integrating the contract, and it goes stale the moment it deploys. That material belongs in the spec repo, the commit log and the PR. The layout block now states the current layout as fact and keeps the forward-looking rule: new state consumes from __gap, nothing is reordered, and nothing is declared before `admin` or packed into the free bytes of its slot. Also drops __slot271Reserved. It consumed no slot of its own and guarded nothing about the present layout; it existed only so that a later upgrade could not pack a small field into the 27 spare bytes of slot 271 — writing into guaranteed-zero padding, which is safe, and which the layout checker rejects only out of conservatism. A storage variable that exists to satisfy a tool, and needs a paragraph of justification to explain itself, is worse than the spare bytes it was protecting. The upgrade-safety fixture now demonstrates a packed addition in a fresh slot instead. Layout unchanged where it matters: `admin` at slot 257 offset 0, delay scalars packed at 271, __gap[29] at 272..300. Checker still reports upgrade-safe against the recorded mainnet layout, the fixture harness passes all three scenarios, and the suite is green at 389 tests.
…with it Brings this branch up to the reviewed state of the fee line and carries the delay work alongside it: the pinned surface ids asserted against literal bytes, the corrected README env name, and NatSpec naming the events this interface actually declares. Resolution where the two lines differ: src keeps this branch's side, since the fee line's interface lacks the delay bypass types this branch's controller compiles against. The one correction from the fee line is applied on top rather than by taking its file wholesale. 416 tests passing.
The printed (to, value, data) triple already matched submitTransaction's signature; the output now says where to take it - Blockscout's read/write tab on the Admin multisig, method 20, value 0 - and that the threshold confirmation executes the call itself.
disable-perimeter / enable-perimeter preview and print the calldata for setSecurityPerimeterEnabled on the controller, with the same rules as the queue levers: read-only, no-op guard, submit-from-multisig output. The disable wording states what it is - a liveness escape that makes every charged exit pay straight out and releases nothing already escrowed - so it cannot be mistaken for an incident lever. _emitCalldata now takes the target address since the two levers submit to different contracts. Suite 416 -> 420.
…ination guards, go-live gate Contract correctness: - recoverStuckExit gave the stored-receiver payout 63/64 of remaining gas, so a caller could pick a gas limit that out-of-gases it (caught as a bounce) while the retained 1/64 completed the altReceiver payout -- redirecting a healthy exit. The attempt now runs under a fixed gas budget the caller must cover, so an under-funded call reverts instead of redirecting. - A plain freeze on an already-blacklisted address zeroed its recorded trigger and re-emitted with reason 0, erasing blacklist provenance. A freeze carrying no evidence now leaves the blacklist trigger/reason intact. - resolveBySIP and setRecoveryRoute lacked the this/token/WRBTC destination guard recoverStuckExit already enforces; a fat-fingered destination could trap the escrow. Guard added to both. Go-live and observability: - 06_VerifyActivation now asserts the queue is not left paused before enabling the perimeter -- a pause set during drills would otherwise freeze every withdrawal system-wide the moment the perimeter goes live. - InspectController drove the delay-bypass dump off the surface policy, so a soft-retired surface bypass (the documented disable-while-auditable state) printed as absent. It now consults the surface-bypass key index. Merge casualties restored (weakened when the fee line merged in): - upgrade test re-checks admin + perimeter state survive upgradeTo; non-owner upgrade pins the exact revert; overflow guard pins its exact boundary both ways; removeActorPolicies pins pre-state and stored-policy clearing; disabled-gate test regains its positive control. Deploy shape and comments: - 05 no longer forbids admin == owner: that is the launch shape (the governance Safe holds both roles), and the header NatSpec claiming a hard separation is corrected to match the contract. - Reviewer-finding ids stripped from comments and on-chain revert strings; '(Finding N)' pointers dropped, keeping the property each stated. Suite 420 -> 425.
…yments Squashed adoption of sovryn-perimeter-fee into the delay line: bring in the authoritative Phase-1 deployment records on RSK mainnet, the deployment env template, and the proxy full-match verification helper. Per the decision to complete Phase 2 (delay) before Phase 1's SIPs land. Core suite 425 passing.
The adoption of sovryn-perimeter-fee was squashed with a soft reset, which collapsed it to a single-parent commit and dropped the merge relationship. The tree was correct, but git no longer knew the branches were reconciled, so the merge base stayed at the old ancestor and pull requests recomputed conflicts that are not real. This commit changes no content: its tree is exactly the tested delay tree. It only records sovryn-perimeter-fee as a second parent.
…ction The console was a Foundry script that took request ids typed by hand. Under incident pressure that is the wrong shape: the operator has to source ids from somewhere else and transcribe them, and a single wrong id reverts the whole atomic batch after signatures have been collected. This adds a served page that closes that loop: - requests are read from chain and listed, so ids are selected rather than typed; only Queued rows can be selected, since a terminal id would revert the batch - filtering by one or more addresses resolves through the queue's own per-party index, so it is exact rather than a scan - the parties a selection would block -- originator and owner always, receiver only on request -- are shown before any calldata exists - the generated calldata comes with its destination and value, and the multisig submission instructions live in a tab beside it rather than a separate document It signs nothing and holds no key; the levers stay owner/admin-gated and the transaction is submitted from the Admin multisig. Verified against a queue deployed on a local chain: the request decoder round-trips an abi-encoded struct field for field, the calldata encoder is byte-identical to cast for the same inputs, address filtering returns exactly the indexed requests, and the generated call simulates clean from the admin while reverting for a non-admin caller. The Foundry script stays as the no-browser path and still owns the pause, the clears and the controller kill switch.
…sole - The pause lever was missing. It is the answer to the case selection cannot solve: when too much is happening to pick out the fraudulent requests, pausing stops every payout from the vault while new withdrawals still escrow. It sits above the list and shows the current state rather than making the operator infer it. - Levers now read 'Freeze batch requests' / 'Blacklist batch requests', matching the batch call they build. - Links had no colour rule, so they fell back to the browser's dark blue and were unreadable on the dark ground. They take the accent now. - The README points at the admin panel's Perimeter page as the surface for non-technical operators, and frames this page as the local fallback. Pause calldata cross-checked against cast; the whole page re-verified against a queue on a local chain.
Blocking is by address: freezeFromRequest resolves the parties behind the given ids and blocks those addresses, and executeExit gates on address state. So a block named from one request also holds that party's other queued withdrawals, and any they queue afterwards. Nothing asserted it, though the operator surface depends on it — a fraudulent actor must never be left half-blocked with some exits still executable. The test freezes naming ONE of three requests belonging to the same parties and proves the other two are held, that a request queued after the block is held too, and that an unrelated party still pays out.
The global pause is the locked door: it stops payouts so an incident can be worked out, and blocking the parties responsible has to remain possible underneath it. Nothing gated blocking on the pause, but nothing pinned that either, so a modifier added later could have taken the workflow away. The test pauses first, blocks by request and by address, then lifts the pause and shows the holds standing while an unrelated party is paid.
Scopes Slither to src/ only (lib, test, script excluded). No source change: this cycle's findings all resolved to false positives or accepted design choices on triage, none required a code fix.
A mutation campaign against ExitDelayQueue.sol found 51 lines the unit suite reaches but never depends on. 46 were genuine gaps: the owner-block check in execution, the single-id freeze/blacklist authority gate, the recovery gas floor's exact threshold, three of four ingress entry points skipping the shared validation guard in tests, escrow arithmetic only ever exercised where subtraction and other operators coincide, missing active-index cleanup and solvency checks on two of the three settlement legs, and nearly every config-setter's event going unchecked. Each gets a test that fails on the exact mutated line and passes on the real source. The other 5 survivors are confirmed equivalent (redundant OZ init calls, a reentrancy-guard default that behaves like its initialized value, an empty upstream library function, and Solidity's own fallthrough-return default) and are left undocumented in code, since no test can meaningfully distinguish them.
… fuzz and invariant harnesses Four new invariants pin what the operator levers promise: a blocked party is never paid, a paused queue pays nobody, a route resolves only blacklisted funds, and blocking by a request blocks exactly that request's parties. The same actions and properties are mirrored in the Echidna harness, and a release-gate script runs both at campaign depth outside CI. Each invariant is a "this never happened" assertion, so a companion test drives the same handler through a scripted sequence and proves every guarded path was actually entered before the counters are read.
…he same floor The gate swallowed a failed Echidna phase: both phases ended in `|| true`, and the summary grep over an empty log then killed the script under pipefail before it could say anything. Each phase now reports its exit code, a phase that prints no verdict fails the gate, and the deep campaign passes its own --timeout so the config's five-minute wall clock cannot truncate a 500k-call run into a green result. A phase argument lets either half run on its own. Blocking by request now holds a flagged receiver to the same floor as the source parties. Checking only that it was blocked at all would accept a blacklist that left the receiver merely frozen, and a freeze is clearable and does not authorize a route resolution. The Echidna harness keeps permanent reachability counters so its coverage can be re-checked rather than taken on trust, and a new property keeps those counters consistent with the payout ledger they are measured against.
A mistyped phase argument matched neither block, so both phases were skipped, the failure counter stayed at zero and the gate exited 0 having done no work — the same silent-success shape the exit-code handling was added to remove.
… and ledger recovery payouts
Bring in the fee release landing on main and the foundry-toolchain pin bump that makes CI install Foundry again.
Three things about the block state machine were wrong at once, and they all live in the same function. The trigger request id is the only on-chain link from a blocked address back to the evidence that explains it, and every re-block was overwriting it. A responder who blocks forty addresses by request id and then runs the flat freeze batch over the same list for safety was left with forty zeroes. The trigger is now written only by a call that carries evidence, or by the first block on a clear address, and the announcement names the trigger that holds after the call so the log alone reconstructs it. A plain freeze over an already-blacklisted address changed nothing and said nothing, and a successful transaction reads as "this address is now frozen" — weaker than the truth. It reverts now. The by-request levers carry a trigger and a reason, so they stay unconditional on a known id: over a blacklisted party they hold the stronger state, record the evidence and announce it. That left no way down from a blacklist except clearing it and re-freezing, with the address completely unblocked in between. downgradeToFrozen does it in one call, keeps the evidence, and emits its own transition. The operator preview gains the downgrade action and now refuses the flat freeze that would revert, instead of printing "no state change" and spending a Safe round to find out.
The launch shape has one governance Safe holding both roles, and the equality checks that once enforced a separation are gone from both contracts. The deploy script's usage block still told the operator the Admin "must != Owner", contradicting its own code a hundred and fifty lines below and inviting a second signer set nobody asked for. The two contract notes beside it were written as an account of what changed rather than of what the code does, which is not what a reader of the source needs; they now state the shape and the consequence of it.
The two hosts the gate verified are the ones that hold the queue pointer. They are not the addresses the queue sees. On the lender surface each iToken proxy calls the queue from its own context, so the allowed-source check is evaluated against each pool separately — and a deploy that registers iXUSD and misses iDOC passed this gate green. Every iDOC withdrawal would then revert from the moment the delay was armed: no funds lost, the pool simply shut, and the check that exists to catch exactly that reported success. The checked list is now the union of the two pointer-holders and every hooked record caller named in QUEUE_ALLOWED_SOURCES — the same variable the deploy script registers the allow-list from, so the deploy and the gate cannot describe different systems. Any length, deduplicated, and every entry held to both existing predicates. An empty list is refused the same way a missing host already was, and a blank entry is always a typo. What it still cannot see is a pool left out of that variable and off the chain at once. The banner says so now instead of claiming the record callers are out of scope.
… be found Neither is a defect, and both were being inherited silently. The queue promises that every reachable held state has a way out. That promise holds over its own state machine and stops at the token's edge: all four disposal legs end in the same transfer, so an escrowed token that later refuses to move this contract's balance — an upgradeable one adding a blocklist, a pause, a migration — takes the escrow with it, beyond the reach of the owner included. A test now drives all four legs against exactly that token and proves the request survives every one, so the claim is bounded by something executable rather than by a sentence. The other is a number a caller has to know. Stuck-exit recovery reserves three million gas for the attempt on the stored receiver, and refuses the call when the frame cannot hold it — that reservation is what stops a caller starving the attempt into a false bounce and redirecting a healthy exit. It is far above what a wallet's own estimate offers, so a caller who lets the wallet decide is refused outright. The ABI documentation says so now.
…y off A global delay of zero switched the delay off for every non-bypassed exit while securityPerimeterEnabled still read true and the console still showed the delay holding. The queue's floor cannot catch it: a zero quote means the hook pays direct and never calls the queue. setGlobalDelaySeconds now reverts on zero, and setSecurityPerimeterEnabled refuses to switch on while the length is unset, so a freshly upgraded controller cannot be enabled into that state. Switching off is never blocked. Tests that pinned zero-as-off now pin the refusal; fuzz inputs are bounded away from zero; the never-set case keeps its own test.
A contract that withdraws is recorded as the request's originator and owner, and only those two could deliver it. A contract with no code that calls the queue therefore left its users' money in escrow until an operator resolved it by hand. When the recorded owner has code, anyone may deliver after unlock. The delivery still goes only to the receiver fixed at escrow time, the redirect lever stays with the originator and owner, the three recorded parties are still block-gated, and the executor — whoever it is — must not be frozen or blacklisted. Wallet-owned requests are unchanged.
A contract that withdraws is the caller the hook sees and the owner of what it withdraws, so the queue records it as both; blocking it stops every withdrawal through it, and the person it pays is reachable through the receiver. The passthrough only ever let such a contract's users deliver their own requests, and the queue now lets anyone deliver a contract-owned request. What remained was a second owner lever that looked like the bypass in every read and did nothing for a caller that names itself as receiver. The setter, the identity rewrite, the key sets, the event and the inspector rows go. The delay quote returns the originator and owner unchanged; its signature is untouched. The four slots the registry held are reclaimed by the reserved gap, which grows from 29 to 33; nothing deployed carried them.
Removing the passthrough deleted its three storage declarations and grew the reserve from 29 to 33 slots. That moved the bypass discovery set from slots 267-268 to 265-266 and the packed delay switch and length from 271 to 267. A proxy already running the delay build and upgraded in place would read its bypass-set length as the switch and the length: switched on with a zero length, exactly the state the two setters now refuse, reached without either setter running. The design note keeps those slots declared and never written so the layout only appends. Slots 265, 266 and 269-270 are declared again with their original types, under names that say they are unused; the reserve is back to 29 and every later variable is back at its position. A test pins the positions by reading the proxy's slots. Both layout fixtures mirror the restored layout, so the upgrade-safety regression accepts the safe candidate again (GoodPackedV2, reserve 29 -> 27) and still rejects BadV2 and BadV3. Before this change it rejected the safe candidate.
The controller now refuses to switch the withdrawal delay on while its length is unset. The block script's enable-perimeter action still emitted that call whenever the switch read off. Through the multisig the inner revert does not fail the outer transaction, so an operator could execute it, see success, and leave every withdrawal paying out at once. The script now reads the length, prints it, and refuses before producing calldata while it is 0, naming setGlobalDelaySeconds as the Owner's prior step. After producing either switch call it says to read the switch back, because the multisig reports success even when the call inside failed. Its messages now describe only the delay switch; the Perimeter fee has its own. The controller inspector printed a zero length bare. It now says that 0 means unset and that switching on is refused while it is, and warns when the switch reads on with no length. Two comments described the removed passthrough model: the queue interface said recovery shares delivery's caller set, and the inner delay quote spoke of an "already-effective actor" rather than the originator.
Ingress refused only the zero receiver. A request whose receiver is the queue itself goes terminal on delivery while the money never leaves: the payout sends it back into the queue, escrow drops, solvency still holds, and the amount sits as surplus the Owner can sweep. The same happens when native RBTC is paid to WRBTC, whose deposit credits the sender - the queue - with wrapped tokens. Every ingress path now reverts InvalidReceiver(receiver) for the queue as receiver, and for WRBTC as receiver whenever delivery sends native RBTC: native escrow, or WRBTC escrow that unwraps on delivery. A request that pays WRBTC as a token, without unwrapping, is unchanged. Tests cover all four ingress functions, both refusals, and the token case that stays accepted.
…wn behavior The comment named "the review" that caught this case rather than describing what the test pins. Reworded to state the behavior only: a blocked stored receiver refuses recovery even with a clean alternate receiver named. Documentation only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Exemption Granting an actor-tier exemption took two separate calls (setActorPolicy, then setActorBypass), each its own Exchequer multisig transaction with its own confirmation round. Between the two executing, the actor is either fee-exempt but still held, or - if the delay half lands first - fee-charged but paid instantly with no hold at all, defeating the perimeter for that actor during the gap. --half fee / --half delay make a permanently half-applied exemption directly reachable. grantExemption writes both entries in one call, mirroring revokeExemption's existing two-write atomicity in the opposite direction, so the hardhat tooling in the lending repository can submit one multisig transaction for a full grant instead of two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oup correctly _verifyByRequest determined whether a request's receiver belonged to the clean group (blocked alongside it) or the held group (left alone) by checking queue.blockTrigger(receiver) == this request's id. That trigger is last-write-wins: when two ids in the same clean-group call resolve to the same receiver - the shape a set of malicious withdrawals routed to one payout address produces - only the later-processed id's trigger survives, so the earlier one reads as held even though its receiver was genuinely blocked, and verify wrongly reports NOT CONFIRMED. Replace the single-id trigger check with _receiverIncludedInBatch, which also accepts a trigger that names any other id in the same verify batch resolving to the same receiver. Add coverage for two clean requests sharing one receiver, under both freeze and blacklist.
…uest verify batch scan _receiverIncludedInBatch only matched another id's own receiver field against the address in question, so a request whose receiver was also touched through a different id's originator or owner role in the same batch (several malicious withdrawals from one wallet, one paying out to an address the wallet also used to originate a different withdrawal) was misread as held even though the on-chain freeze fully succeeded - verify-freeze/verify-blacklist reverted NOT CONFIRMED for a genuinely successful block. Extend the same-batch scan to originator and owner too, and add regression tests pairing a receiver with another request's originator and with its owner.
…clean The by-request verify classified a receiver as clean or held purely from its current, mutable blockTrigger. A clean request's freeze call blocks its receiver alongside it - but an Admin can separately unfreeze just the receiver afterward, a real onlyAdminOrOwner action unrelated to the batch under review. That clears the receiver's trigger, so verify misread it as "held, receiver correctly left alone" and printed a clean CONFIRMED even though the receiver no longer reads blocked. Freeze only ever submits the clean group, always with the receiver included, so a request whose own call is evidenced on its originator and owner but whose receiver reads excluded either way was clean, not held - its exclusion means a later action cleared what this call set, not that the call skipped it by design. That case is now flagged rather than confirmed, since current chain state alone cannot tell "never touched" apart from "touched, then cleared". Split the receiver check into its own function to keep the loop inside the stack limit, and add a regression test that freezes a clean request, unfreezes its receiver alone, and asserts verify does not print a clean CONFIRMED for it.
…ims was actually reachable executedWhileBlockedAttempts incremented as soon as a party was observed blocked, before the call ran - but _executeOne checks status, the unlock time and the executor before it ever reaches _requireNotBlocked, so a call that reverted on NotUnlocked or NotExecutor for reasons unrelated to the block gate still counted, letting leversReached() read true without the block check ever firing. Gate the counter on those same preconditions (plus not-paused, since the pause check runs before anything else in _executeOne) in both execute and executeMany, mirroring _executeOne's own order. Also add recoveredAsOriginator/recoveredAsOwner/recoveredAsReceiver: recoveredPayouts alone only shows some recovery authority fired at least once, not that the harness exercised all three independently, since it is always the recovered request's own originator, owner, or receiver for the call to succeed at all. This changes what the currently running Echidna campaign measures for executedWhileBlockedAttempts, since it is the harness the campaign is already running against; the campaign's own record names the harness it ran on. Not run here - forge build/test only.
…ust one The single-guardian identity assertion's comment claimed the controller's local admin is used ONLY by the kill switch, but onlyAdminOrOwner also gates setFeeReceiver, which re-points where the Perimeter fee is paid - a second, distinct lever. Runtime behavior is unaffected; this corrects what the comment tells a maintainer reading it before the identity assertion.
…cleared after the fact R3-2's fix (4560527) scoped the by-request verify guard to target == Frozen only, reasoning that blacklist's held group already has a legitimate excluded-receiver shape and so needed no equivalent guard. That reasoning covers the held group but not the clean one: blacklist escalates a held party exactly as it does a clean one, so a held id's own call is evidenced (_requestEvidenced) exactly like a clean id's. The identical false-CONFIRMED shape R3-2 fixed for freeze therefore reproduces on blacklist for the clean group - a clean request's blacklist call blocks its receiver, an Admin unblacklist on the receiver alone later clears it, and by-request verify printed a clean CONFIRMED for a receiver that no longer reads Blacklisted. Extending the guard naively (dropping the target == Frozen check) would also misfire on blacklist's genuinely-held group: unlike freeze, where a held id's call never runs at all (so it is never evidenced), blacklist submits the held group too, just without its receiver - so a held id's own evidence looks identical to a clean id's whose receiver was blocked then cleared. There is no on-chain signal left that tells these two cases apart once the receiver's own trigger has been cleared, so by-request verify can no longer soundly confirm a held id's receiver claim under blacklist either way. It now refuses rather than guess, and a held id's parties - which never made a receiver claim in the first place - are verified by address instead (BLOCK_ACTORS), which was already sound and unaffected by any of this. test_verify_blacklist_confirms_both_groups updated to verify the clean group by request and the held group by address, matching the new, honest split. New regression test test_verify_blacklist_does_not_confirm_a_receiver_unblacklisted_after_the_fact reproduces the reviewer's proof of concept against the pre-fix behavior (fails first) and passes at head. Ref: sovryn-spec/sovryn-perimeter/docs/reviews/2026-09-17-contracts-codex-round3.md, re-verification of R3-2.
_reachesBlockGate's party check compared msg.sender - the address Echidna used to call the harness's own external execute/executeMany - against r.originator/r.owner. But _executeOne actually sees the harness's own address as its msg.sender, since execute/executeMany call queue.executeExit/executeExits directly. echidna.yaml sets no custom sender list, so Echidna's default fuzzing senders never equal address(this)/actors[], making party effectively always false and collapsing the check to r.owner.code.length > 0 alone - inconsistent with executeMany's own pre-existing pair check four lines above, which already tests b.originator == address(this) || b.owner == address(this). Effect was under-counting only (never over-counting, so the R3-3 false-positive shape does not reopen): executedWhileBlockedAttempts missed every genuinely-reachable attempt via the originator role with a non-contract owner - a combination the harness's own actor-keyed request generation produces routinely. Fixed by comparing address(this) instead, mirroring executeMany's own convention. Not forge-testable directly: the harness is driven by the Echidna campaign, not forge test (no forge test targeted it before this fix either). Verified by forge build and the full forge test suite (543 unit, 11 invariant) staying green - this file is inert to both, since neither instantiates the Echidna harness. The fix only widens when `party` reads true, and every earlier gate (paused, status, unlock time) is untouched, so the reachability counters still only advance past a genuine reach and never over-count. Ref: sovryn-spec/sovryn-perimeter/docs/reviews/2026-09-17-contracts-codex-round3.md, re-verification of R3-3.
executeExit/executeExits treated the receiver as a payout destination only; the originator or owner had to press the button (or, when the owner is a contract, anyone). The receiver can now deliver too, since it already knows its own request is unlocked and has every incentive to move it. _activeByParty now indexes a request under the receiver alongside the originator and owner, so getActive(receiver, ...) lists it while Queued. _removeActive takes the receiver and prunes it from all three sets on every terminal transition (execute, recover, resolve-to-protocol, resolve-by-owner), de-duplicating when the receiver coincides with the originator or owner. Storage layout is unchanged (no new slots, no fields added). NatSpec in ExitDelayQueue.sol and IExitDelayQueue.sol updated to match.
Unit tests: the receiver now delivers its own request, single and batch, after unlock; it is refused before unlock, while paused, on a terminal request, and while the originator or owner is frozen/blacklisted, mirroring the existing originator/owner coverage. getActive(receiver, ...) lists a request after enqueue and drops it after each of the four terminal paths (execute, recover, resolve-to-protocol, resolve-by-owner) with no double add or double remove when the receiver coincides with the originator or owner. A caller matching none of the three parties is still NotExecutor when the owner carries no code. Two pre-existing tests asserted the receiver could NOT execute; flipped to match the new rule (one now proves the outsider-only revert, the other proves the receiver succeeds). Invariant handler + forge invariant test: the active-index consistency check now covers the receiver's set alongside originator/owner, and the executeMany batch-pairing heuristic treats the receiver as a valid co-executor. Echidna harness: the block-gate reachability predicate and the batch-pairing heuristic both gained the receiver as a valid executor, mirroring the contract's own rule. recoverStuckExit's own three-party rule is untouched.
… path Adds ExitDelayQueuePayeeReentrancy.t.sol, nine tests where the payee is also the caller and re-enters the queue from inside its own payout: - executeExit and executeExits, native payout - executeExit, WRBTC-unwrap payout - executeExit, hooked-ERC20 payout (hook called after the balance moves) - executeExit, hooked-ERC20 payout with the hook called BEFORE the balance moves -- the case that briefly exposes a transient surplus equal to the payout amount - recoverStuckExit, a healthy receiver (paid; the alternate goes unused) - recoverStuckExit, a receiver that bounces after its attempts (the alternate is paid exactly once) - resolveByOwner, to a destination that is itself the re-entering payee - sweepSurplus, to a destination that is itself the re-entering payee Each test has the payee attempt six re-entry paths while it is being paid (executeExit, executeExits, recoverStuckExit, payoutExternal, recordNativeExit, recordReceivedNativeExit) and asserts the concrete outcome: five attempts stopped by the shared reentrancy guard, one by payoutExternal's self-only check, the paid request's status/index/escrow already updated while the payee runs, and a balance that moved exactly once for exactly the paid amount. The pre-hook case additionally asserts the transient surplus is recorded as escrow by no record* call and moved by no sweep while a sibling payout sits in the queue. Ingress reentrancy and resolveToProtocol reentrancy already had re-entering-payee coverage elsewhere in the suite; this file covers the remaining value-out paths. New mocks, self-contained in the file: StipendWRBTC (the WETH9-style 2300-gas withdraw already used by ExitDelayQueueUnwrapStipend.t.sol), HookToken (calls a registered address before or after its own transfer), Source (a registered allowed-source helper, matching the suite's existing SourceHarness/Source pattern), and Hostile (the re-entering payee).
The per-address filter matches a request whenever the address is the originator, owner OR receiver. Add one sentence where the filter is described: filtering by a suspect address and choosing "Select all shown" therefore also selects requests where that address is only the receiver, pulling in their originators and owners -- the payers -- for a from-request freeze or blacklist. The selection panel already lists every address that would be blocked before anything is sent; the sentence just states the consequence next to the filter it follows from. No other change to the tool.
…d escrow Covers the state where the queue holds less of an asset than its recorded escrow (negative rebase, clawback, a token that takes from its holder), for ERC20 and native. Ingress: recordReceivedERC20Exit and recordReceivedNativeExit measure a zero surplus, so a deposit smaller than, equal to or larger than the shortfall is refused with ReceivedAmountMismatch(asset, max(deposit - shortfall, 0), deposit), leaving the escrow total, request count and queue balance unchanged. A partial top-up still refuses; a full top-up lets the same deposit record normally. A fuzz test per path covers the general case. The ERC20 pull path, which measures its own transfer and never compares against the escrow, is pinned as unaffected. Value-out: executeExit, executeExits, recoverStuckExit, resolveToProtocol and resolveByOwner are each refused with SolvencyViolated while backing is short, leave the requests queued and the escrow untouched, and pay once a third party tops the backing up. sweepSurplus is refused and moves nothing while short; after the top-up it succeeds and moves exactly the excess over the escrow.
…view Refuse zero block actors and duplicate clear/downgrade actors before collecting multisig signatures. Add five regressions proven failing on the original preview and passing after repair; keep idempotent freeze/blacklist repeats. Qualify caller-gas recovery delivery guarantees and correct exemption and queue interface documentation. Preserve the owner-approved no-cap runtime behavior pending review of the reproduced healthy-receiver redirect.
… readiness Validate exact Foundry and Echidna inventories, requested run/depth/call counts, passing statuses and the expected reachability exit rather than loose log markers. Clear inherited selection filters, pin the default profile, and compare pristine tracked source/config/dependency snapshots before and after execution. Add sixteen deterministic positive and negative gate checks, including seven meaningful failures on the original guard, and the mutation-proven paused activation regression. Contract runtime, recovery policy, dependency pins and unit-only CI policy are unchanged.
…ion receipts Add an explicit read-only receipt route instead of fabricating a Foundry 99 broadcast. Bind reviewed artifact/profile and real implementation creation, exact wallet intent and 3-of-7 quorum, ordered proxy/wallet logs, canonical blocks, slot/code, preserved controller state and rollback record before atomic conflict-safe update. Keep direct finalization unchanged and support receipt-backed source verification with source/settings profile guards. Add real receipt-shaped positive, wrong-receipt/target/data/slot/state/quorum and conflict/tamper/replay controls; all 27 tooling tests pass. Runtime, recovery policy, CI and dependency pins are unchanged; genuine fork proof follows through the coordinator.
…zation Validate timestamp and timestampUnit alongside the other stable chain-derived record fields before accepting an idempotent replay. Cover timestamp-only, unit-only and combined corruption with refusal and exact unchanged-record assertions.
Describe sampled nested queue-call budgets rather than exact minimum or live block-limit proof. Distinguish ordinary MockERC20 call success from native receiver gas consumption and qualify the stored-payment case to its supplied harness gas. Change notices and diagnostic labels only; retain assertion conditions and recovery behavior.
…nits Promote distinct unwrap rollback and underpayment properties, atomic four-mode 86400-second floor checks, and typed denial checks for uncovered public entry variants. Strengthen four existing owner-error assertions so later eligibility errors cannot hide a missing guard. Keep production behavior, roles, recovery policy and unit-only CI unchanged; verify in isolated outputs with meaningful mutation controls.
Preserve capped-address metadata and show a persistent partial-read warning alongside the selected-request workflow. Update the warning with replacement rows so a pending refresh cannot hide the old partial state. Keep valid rows, exact selection, calldata and read-only operation unchanged; cover complete and capped boundaries plus meaningful removed-warning controls.
Phase 2 ("Perimeter Delay") step 1 deploys ONLY a new ExitFeeController
implementation: the ERC1967 proxy is already live on chain 30
(0x99994b45...440E) and owned by the Exchequer 3-of-7 multisig, so the
upgrade itself must be a multisig upgradeTo transaction, never a script.
Until now 03_DeployController unconditionally deployed impl + new proxy
(running it on mainnet would create an unwanted second proxy) and
finalize-deployment.sh had no shape for an impl-only broadcast.
03_DeployController.s.sol: detect a live recorded proxy via
deployments/<chainId>/ExitFeeController.json plus an on-chain code check;
with a live proxy, deploy only `new ExitFeeController()` (never
initialized, never upgradeTo) and print a loud warning block: staged
impl, existing proxy, current on-chain owner() read through the proxy,
multisig vs governance-timelock upgrade guidance (EXCHEQUER_MULTISIG
env-confirmed, else the known mainnet Exchequer hint marked unverified,
else both paths printed), a do-not-merge-until-upgrade-verified rule,
and the exact finalize command. Fresh-deploy behaviour is unchanged when
no live proxy exists.
finalize-deployment.sh: recognise a third broadcast shape -- exactly one
CREATE, no proxy CREATE, no upgradeTo. With an existing artifact naming
the proxy, record the new impl as a nested pendingImplementation object
(implAddress, on-chain implBytecodeHash via cast code, block, tx,
timestamp, gitSha) WITHOUT touching the live implAddress/implBytecodeHash
(the proxy still runs the old impl until the upgrade executes). Refuse
impl-only broadcasts with no recorded proxy as ambiguous. Usage header
updated.
finalize-multisig-upgrade.py: when the receipt-backed multisig upgrade
lands, require the manifest implementation to match the staged
pendingImplementation (when present), promote it to the live fields and
clear the staged record; rollback keeps the original staged JSON.
Verified locally only: forge build, forge test (622 pass),
bash -n, jq dispatch fixtures for all three shapes, and anvil
simulation of both fresh-deploy and impl-only script paths. No network
broadcasts, no mainnet calls.
Add KNOWN_MAINNET_TIMELOCK_ADMIN (0x6c94c8aa...29FB13) and KNOWN_MAINNET_TIMELOCK_OWNER (0x967c84b7...A7F69f) from the Sovryn-smart-contracts rskSovrynMainnet hardhat deployment data. When the live proxy owner() matches either timelock, the staged-impl warning block now names it and states the required SIP path (GovernorAdmin/GovernorOwner) instead of falling through to the generic both-paths message. Hints are marked UNVERIFIED, same policy as the Exchequer hint. Print-only change: forge build clean, forge test 622/622 pass.
Impl-only deploy via script 03 (commit 4cf3805), tx 0xcc1bcce36c751a1c1749ea25c0679bd9936d37446f2d1f5d8aa41355a3c2ee50 at block 9317153, 3175549 gas, deployer 0x163463B7...f9c4. Runtime code byte-identical to the reviewed artifact template with the five self-address immutables hydrated; source verified on Rootstock Blockscout in the same run. Artifact records pendingImplementation; live proxy 0x99994b45...440E still on impl 0x50EC5c1C...552d, owner Exchequer, exitFeeEnabled true. Activation only via Exchequer 3-of-7 upgradeTo + --multisig-upgrade finalizer.
Impl-only mode now reuses a recorded pendingImplementation whose on-chain code hash matches (no second CREATE). Reads the proxy's live ERC-1967 impl and routes the upgrade: already-upgraded prints finalize guidance; Exchequer-owned proxy with an Exchequer-owner signer submits the multisig upgradeTo transaction itself (3-of-7, with the 3,000,000-gas executing confirmation and state-readback rules printed); non-owner signer gets the exact calldata to submit later; TimelockOwner/TimelockAdmin owners get the SIP-required + do-not-merge warning. Verified: forge build clean, 622/622 tests, mainnet simulation (reuse + calldata print), and a full anvil-fork 3-of-7 rehearsal (submit/confirm/confirm@3M gas -> executed, impl slot updated, securityPerimeterEnabled false, exitFeeEnabled true).
…ay-build impl 0xe85e7e5c (exec tx 0x80ef954d, block 9321661); receipt-backed record with rollback
…86f…c50a (block 9322578); owner/admin Exchequer, floor 86400, no hosts wired (DEFER_HOSTS)
…sion + ExitDelayQueue deploys The 03_DeployController run-latest record now reflects the owner run that submitted the Exchequer multisig upgrade of the live ExitFeeController proxy 0x99994b45...3440E to the delay-build implementation 0xe85e7e5c...18e1 (submitTransaction 0x918ff4ef..., block 9319660, multisig tx 2218), replacing the earlier step-1 CREATE record. The 05_DeployQueueAndWire broadcast records hold the two ExitDelayQueue CREATEs on mainnet (chain 30): implementation 0xB19C086f...c50a and ERC1967 proxy 0x6A281FCB...Dc45 (0xe799dc6d... / 0x71cb6343..., block 9322578, hosts deferred). Deployment state was already finalized in 951a139 and d74a577; these are the raw broadcast receipts. test/phaseG/ (Phase G fork-test mock host) is intentionally untracked.
Stand-in for the product host contracts when fork-testing the live ExitDelayQueue (proxy 0x6A281FCB...Dc45): wraps the four record* ingress functions, models the fee-then-hold product flow (quote the live ExitFeeController, pay the fee leg to the vault, escrow the net into the queue in one transaction), and doubles as a bouncing receiver for the recoverStuckExit leg. Used in the 2026-10-14 post-upgrade on-chain test campaign against mainnet fork state (evidence in sovryn-spec KB, logs-2026-10-14/phase-g/). Not deployed; test-only.
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.
Phase 2 of the Sovryn security perimeter — the core contracts.
Where the Perimeter Fee takes a cut of a user-initiated exit, the Perimeter Delay can hold the remainder for a governance-configured period, so a detected theft can be frozen or blacklisted and routed to recovery before the funds leave the protocol.
What this carries
ExitDelayQueue— a per-request escrow holding the user leg of an exit until its unlock time. Requests are immutable once recorded, ingress is restricted to registered product sources, and payouts settle state before any external call. Only the originator or the position owner may execute a request; the receiver never can, so a withdrawal split into a fee leg and a delayed leg is always completed by the actor who started it.ExitFeeControllerdelay extension: a kill switch independent of the fee switch, one global delay, bypass tiers mirroring the fee tiers (actor, sub-product, surface), and a surface-scoped passthrough registry for contracts that withdraw on a user's behalf.The delay ships disabled and is enabled only by governance after post-deployment verification.
Storage layout
The controller is upgraded in place, so its layout has to stay compatible with the implementation already live on RSK mainnet.
adminkeeps the slot the fee release shipped, alone and at offset 0; all new state comes from__gap. Slot 271 is closed with an explicit reservation so a future upgrade cannot accidentally pack into its free bytes and land outside the reserved gap.An earlier revision of this work declared the two delay scalars ahead of
admin. All three fit in one slot, so Solidity packed them and movedadminwithin it — which would have made the live proxy read the perimeter as enabled, the delay as roughly 112 years, andadminas an address nobody holds, with nothing reverting. That is corrected here, andtools/diff-storage-layouts.pynow reports the candidate upgrade-safe against the recorded mainnet layout.Verification
forge fmtclean.tools/diff-storage-layouts.pyagainstdeployments/30/ExitFeeController.json: upgrade-safe.tools/test-upgrade-safety.sh: positive fixture accepted, both negative fixtures rejected.Still outstanding
Delta and human security review, an independent custody audit of
ExitDelayQueue, deeper fuzz/Echidna, the reentrancy audit gate (Stage 2 introduces the first user-facing payout paths from the vault), and the cross-repo mainnet-fork integration test. The product-side branches carry a separate EIP-170 contract-size blocker.