Sovryn Perimeter Delay: Zero - #11
Draft
tjcloa wants to merge 7 commits into
Draft
Conversation
Phase 2 of the Sovryn security perimeter: a voluntary borrower collateral exit that already pays the Perimeter Fee can additionally be held in the ExitDelayQueue for a governance-configured delay, so a detected theft can be frozen or blacklisted and routed to recovery before the funds leave. This change carries the Zero half. - delay hooks on the voluntary collateral-out paths (withdrawColl, the collateral-decreasing adjustTrove, and closeTrove), sharing the surface the fee already uses. The fee leg is paid immediately and only the NET is escrowed, and a single delay quote taken once per exit governs the whole exit — including the full-gross path taken when the fee leg fails, so a fee-vault fault cannot route around the delay; - ActivePool pushes the native RBTC to the queue and the record follows in the same transaction, so a record failure rolls the push back and the exit reverts as a whole rather than leaving value stranded; - fail-open POINTER, fail-closed QUOTE: an unset queue or controller pointer leaves exits paying direct, while a controller that answers incorrectly reverts the exit rather than silently disabling the perimeter. The queue's custom-error selectors propagate unchanged so the off-chain halt watcher can key on them; - BorrowerOperations gains the owner-gated setter for the queue pointer; the pointer lives in an unstructured slot, so neither hook adds state to any upgradeable proxy (asserted by the storage-layout zero-diff guard, whose baseline now covers the delay hooks as well). Redemptions, liquidations and Stability Pool operations stay untouched, as does the surplus claim, which remains exempt from the delay and keeps a pinning test to prove it. The delay ships disabled and is enabled only by governance after post-deployment verification.
Matches the lending repo: contracts, interfaces, directories, the Echidna tester and the test suite now use Perimeter naming, and the surface constants become PERIMETER_SURFACE_ZERO_*, each hashing a literal identical to its own name rather than a prefixed one. The surface ids therefore change value, exactly as on the lending side, and need configuring against the new hashes before these hooks can charge. Whether that is a migration or simply part of a first activation depends on the release shape recorded in the runbook. Two strings deliberately keep their spelling: the controller and delay-queue pointer slots, "sovryn.exitFeeController" and "sovryn.exitDelayQueue". Neither contains the old name, and both are shared with the lending integration — renaming them here alone would put the two products on different slots for the same pointer. Also fixes the surface-id helper the tests use. It built ids by prefixing the constant name, so the rename left it hashing a doubly-prefixed string and every surface assertion failed; under the new scheme the string is the constant name and no prefix is applied. 70 tests passing, including the storage-layout zero-diff.
The pointer slots read "sovryn.exitFeeController", "sovryn.exitDelayQueue" and "sovryn.borrowerExitPerimeterOps" — names that say what is stored but not which system owns it, in a protocol that has several kinds of fee. They are now sovryn.perimeterExitFeeController, sovryn.perimeterExitDelayQueue and sovryn.perimeterBorrowerExitOps, so every slot the perimeter owns carries the same prefix. These strings are shared across the lending and Zero integrations and the perimeter contracts: the slot address is the hash of the string, so the three repos must carry identical spellings or the same pointer would live at different addresses in each. The rename is applied to all three together for that reason. Free to do right now, and checked rather than assumed: every one of these slots reads zero on mainnet today, on both the protocol and the Zero BorrowerOperations proxy. Nothing has been pinned, because pinning needs the activation proposal and that has not executed. Once it has, moving a slot would mean re-pinning it.
BorrowerOperations was 685 bytes over the EIP-170 limit once the exit delay joined the fee hook. Move the settlement body into BorrowerOperationsPerimeterOps and invoke it with delegatecall -- the pattern TroveManager already uses for TroveManagerRedeemOps. The companion shares BorrowerOperationsStorage, so it settles in the proxy's own context, reads the proxy's pointer slots and emits the proxy's events. The hook address is a plain storage variable appended after feeDistributor, mirroring troveManagerRedeemOps, and is rotatable by the owner. BorrowerOperations 25,261 -> 24,125 (451 under the limit); the companion is 3,840.
The selector-propagation regression pulled the revert payload out of the node's eth_call error object. Hardhat cannot attach a custom-error payload when it fails to build a stack trace through the settlement delegatecall, so the assertion saw an empty string even though the selector bubbles intact. Read the returndata through PerimeterRawCatcher instead: that is what the EVM hands the caller, independent of how the node formats errors. The storage-layout guard forbade any appended state on every target. BorrowerOperations now holds the settlement hook in one appended slot, so its policy becomes append-only -- the deployed prefix must stay byte-identical and additions must land beyond every baseline slot. ActivePool and CollSurplusPool stay zero-diff.
Mirror 4-TroveManagerRedeemOps: deploy the companion, then set it on BorrowerOperations -- through the multisig on testnet, as a proposal line on mainnet, directly otherwise.
The surplus claim charged a fee but was always paid straight out, because the deployed claimCollWithFee hard-sends the net to the claimant with no way to redirect it. Add claimCollWithFeeTo, which pays the remainder to a named recipient under the same accounting and the same CEI ordering, so the perimeter can escrow it. A zero fee now takes no leg at all rather than making a zero-value call that would report a fee never charged. claimCollWithFee is left exactly as deployed: it is the selector the shipped BorrowerOperations calls, and the rollback target. The whole settlement moved into the companion as claimSurplusWithPerimeter and claimCollateral became a thin wrapper, which is why BorrowerOperations came out smaller: 24,125 -> 23,233, 1,343 under the limit. The delay is quoted once, up front, so a fee-vault failure still escrows the gross behind the hold. The fee leg stays fail-open; the delay leg is fail-closed, so an unwired queue or a reverting record reverts the claim and the claimant keeps their balance. The companion no longer inherits BorrowerOperationsStorage. It never shared those slots: BorrowerOperations also inherits LiquityBase, so every plain variable sat four words earlier here, and reading collSurplusPool would have returned liquityBaseParams. It was harmless only because nothing read plain storage, and this change needed to. The companion now declares none, takes what it needs as arguments, and a regression pins the declared layout as empty. ClaimSurplus.notouch retired for ZeroClaimSurplus.delay (11 tests): 79 perimeter tests, 831 in the base suite, none failing.
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 Zero half.
A voluntary borrower collateral exit that already pays the Perimeter Fee can additionally be held in the
ExitDelayQueuefor a governance-configured period, so a detected theft can be frozen or blacklisted and routed to recovery before the funds leave.What this carries
withdrawColl, the collateral-decreasingadjustTrove, andcloseTrove— sharing the surface the fee already uses. The fee leg is paid immediately and only the net is escrowed, and a single delay quote taken once per exit governs the whole exit, including the full-gross path taken when the fee leg fails.ActivePoolpushes the native RBTC to the queue and the record follows in the same transaction, so a record failure rolls the push back and the exit reverts as a whole rather than leaving value stranded.BorrowerOperationsgains the owner-gated setter for the queue pointer. The pointer lives in an unstructured slot, so neither hook adds state to any upgradeable proxy — asserted by the storage-layout zero-diff guard, whose baseline now covers the delay hooks as well.Redemptions, liquidations and Stability Pool operations are untouched, as is the surplus claim, which remains exempt from the delay and keeps a pinning test to prove it. The delay ships disabled and is enabled only by governance after post-deployment verification.
Base branch
Opened against
sovryn-perimeter-feerather thandevelopmentso the diff is the delay delta alone. Re-target todevelopmentonce SIP-0094 is approved and executed.Not ready to merge — known blocker
Contract size (EIP-170).
BorrowerOperationsreaches 25,242 bytes with the delay hooks, against a 24,576-byte limit, so it cannot be deployed as it stands. The deployed Phase-1 implementation is 23,917 bytes, leaving 659 bytes of headroom that the hooks exceed. The candidate fix is theTroveManagerRedeemOpspattern — split the delay legs into a companion contract reached bydelegatecall.Also planned for this branch
The surplus claim is currently exempt from the delay. That is being reversed: surplus withdrawals will route into the vault like every other delayed exit. It needs a new
CollSurplusPoolfunction, because the deployedclaimCollWithFeesends the net straight to the claimant with no parameter to redirect it — so it carries a second pool implementation upgrade.Verification so far
BorrowerOperations,CollSurplusPoolandActivePool.