Introduce the Sovryn Perimeter Fee across lending and borrowing - #580
Open
tjcloa wants to merge 33 commits into
Open
Introduce the Sovryn Perimeter Fee across lending and borrowing#580tjcloa wants to merge 33 commits into
tjcloa wants to merge 33 commits into
Conversation
Phase 1 of the Sovryn security perimeter (SIP-0094): a minimal exit fee on user-initiated withdrawal surfaces, funding continuous exit monitoring. This change carries the lending/borrowing half of the system: - exit-fee hooks on the LM/WrbtcLM iToken burn paths (beacon modules) and on borrower collateral withdrawal and loan closing (protocol modules), quoting through the shared ExitFeeController (Sovryn-perimeter repo) and paying the fee leg to the ExitFeeVault; every hook fails open — a fee fault forgoes the fee, never blocks a withdrawal; - ExitFeeModule (governance setters for the protocol controller pointer) and ColFeeBorrowerExitOps (the borrower-exit charge hook); - SIP-0094 builders (three executable parts across the two governors) with self-validating creation guards, deploy scripts for exactly the shipped artifact set, storage-layout zero-diff guards in CI, and the fork rehearsal suites incl. an execution-day dress run against a script-deployed stack; - explorer verification wiring (Rootstock Blockscout) in the deploy flow. The fee system deploys disabled and enables only by governance after post-deployment verification. Perimeter call graph: docs/PERIMETER_FEE_CALL_GRAPH.md.
Following the repo convention for contracts deployed outside this repo: deployment records (address + ABI) in external/deployments/rskMainnet, and hardhat-shaped artifacts in external/artifacts. ExitFeeController 0x8C1abf364Bf214E41221562693BD9Fb26D6Fa563 ExitFeeVault 0x2ba389B021fA4A5F50cc1758EFD23Ca066d0Be08 Both are the PROXY addresses — what every consumer calls and what SIP-0094 pins — carrying the implementation ABI. Deployed from Sovryn-perimeter 881c74d and source-verified on the Rootstock Blockscout explorer. This is what lets the SIP-0094 builders resolve the controller from a saved record rather than requiring COLFEE_EXIT_FEE_CONTROLLER in the environment at proposal-creation time.
Eight artifacts deployed 2026-08-13 by 0x163463b7ddbce853832037a059f5c5e6606bf9c4 (nonces 17-24), all inert until SIP-0094 Part 1 registers the modules: LoanTokenLogicLM 0x6acf70d9E194a9cbe60c1283aaF30fFb252358AC LoanTokenLogicWrbtcLM 0x96E4f6D9276A36e1c51485119406667F5eAe12ee ColFeeBorrowerExitOps 0x381fFaf5bbFa532a6E4A33E6bffCa97755C99cB3 SwapsImplSovrynSwapLib 0xFE2bb2d345452673C4E90622147c4F515F2F4CE0 LoanClosingsRollover 0x2aed71b3fdD6D51f38405Fd5912624515dB46783 LoanClosingsWith 0xbC97D6BdB4eb0741179eDA636aCdFed34bB9d67E ExitFeeModule 0x4C0eAc3f96a4fc25e5251dBA6Ec2369db0CA768B LoanMaintenance 0x9958b841d4af6459E4c520876F238Ec59a4Bd83D SwapsImplSovrynSwapLib redeployed although its source is untouched: it imports ISovryn.sol, which this branch extends, and the import ripples into the library's metadata hash, changing its creation bytecode — so hardhat-deploy correctly saw a different artifact. The library is stateless and rides only inside the link references of the new modules; live modules keep the previously recorded library. No governance action touches it. LoanClosingsLiquidation and the three non-burn-route LoanToken modules are deliberately absent (byte-identical or not re-registered by this release).
LoanClosingsWith redeployed at 0x279D307C09E30d282aA96C3F4D216051277787f7 (2026-08-13, deployer nonces 27-29), linked against the previously-deployed, explorer-verified SwapsImplSovrynSwapLib at 0x47Dc479cA058B4BB7C4AD9D56EE3a029236b88A8 — whose record this restores. Verified on-chain: the new bytecode differs from the Phase-B deployment in exactly the 20 link-address bytes; everything else is byte-identical. Motivation: the Phase-B library copy (byte-identical code, different metadata) cannot be verified on the explorer — its call-protection self-address defeats the current matcher, and its 240-source compilation bundle would be hostile to auditors even if it could. Linking the verified library gives voters a clean, already-verified source view. The lean- interface extraction that removes the root cause is scheduled post- activation. The run also emitted redundant copies of LoanClosingsRollover (0x2384580d8c8F76310CeA498CF17ec8A082299294) and LoanMaintenance (0x2386AAF789fc59BEfd21932dd00A340527f40571): hardhat-deploy treats the libraries option as part of the skip-check, so passing the old library address re-deployed modules whose runtime bytecode does not even reference it. Their records are reverted to the Phase-B deployments, which the SIP registers; the copies are inert orphans. ExitFeeModule correctly reused.
…tions Each Part's on-chain proposal description now carries the commit-anchored document link (SIPS 3fdc85f336359fa6575d8e629514eef0d6ddeec3, pushed) and the document's sha256 (8a54a1d2ec447e6e609017940742f8a6bedce1ff0f72bb58d024f6c5f42c712d, computed from the git object at that commit), in the repo's established Details format. The document is placeholder-free as of that commit — deployed addresses, rollback anchors, and runtime code hashes all final — so this pin is the P9 freeze; assertDescriptionFinalized now passes on mainnet. If the document changes during the review week, this pin must be re-cut in a single commit.
The document was corrected during its review week: the library disclosure now names both linked deployments and which module links which, the Part 1 action table makes good on the summary's promise of action-by-action call data, and a stray character was removed. Editing the document changes its hash, so all three proposal descriptions are re-pinned to the new commit and sha256. The pin can only be re-cut before sips:create — once a proposal exists its description is immutable on-chain. No contract source is affected.
Matches how every other proposal in this file cites its SIP document; the previous pin used the full 40-character hash. Only the link text changes — the document sha256 in each description is untouched, since it hashes SIP-0094.md rather than this file.
Phase 1 ships this and nothing else. The storage-pointer slots and the surface ids are keccak256 of a name, so the name is the value: they change here, and Phase 2 then inherits them unchanged instead of having to migrate every pointer off a live fee while the old slots are left holding unreachable state. sovryn.exitFeeController -> sovryn.perimeterExitFeeController sovryn.colFeeBorrowerExitOps -> sovryn.perimeterBorrowerExitOps SURFACE_* -> PERIMETER_SURFACE_* Code identifiers and paths follow the values, so the deployed artifact names match what Phase 2 expects: ColFeeLib is PerimeterLib, ColFeeBorrowerExit(Ops) is BorrowerExitPerimeter(Ops), IColFeeEvents is IPerimeterEvents, and the colfee directories are perimeter. Deploy-input and fork-harness env names are deliberately untouched -- the delay branch keeps them too, and renaming them would break saved command lines without moving anything on chain. Identifiers.pinned pins each slot and surface twice: to its preimage and to the literal 32 bytes it must hash to. Pinning the preimage alone is not enough, because a bulk rename rewrites the preimage and the assertion together and stays green -- which is how the first pass through this produced sovryn.borrowerExitPerimeterOps, a plausible-looking name that hashes somewhere else entirely and would have forced Phase 2 to re-pin after all. Perimeter suite 79 passing plus 9 pinned identifiers; storage layout unchanged against the pre-Perimeter baseline, as constants occupy no slots.
The Zero fixtures still carried Phase-1 bytecode. Their embedded preimages were COLFEE:SURFACE_ZERO_WITHDRAW_COLL and COLFEE:SURFACE_ZERO_CLAIM_SURPLUS, while the rehearsal configures the controller with the re-cut ids -- so the fork run would have deployed a consumer quoting surfaces no policy was ever written for. Fail-open means that reads as no charge rather than an error, which is the one failure mode this release is supposed to make impossible. Both Zero fixtures now embed the PERIMETER_SURFACE_* preimages and the renamed controller slot. The four unchanged fixtures take a provenance refresh only. Worth recording, because it explains a bytecode delta that looked unexplained: solc 0.6.11 and 0.5.17 do NOT fold keccak256 of a string literal assigned to a `constant`. The preimage is stored in the contract and hashed at run time on every quote. That is why every perimeter consumer grew by exactly three bytes -- PERIMETER_SURFACE_LENDING_LENDER_WITHDRAW is three characters longer than COLFEE:SURFACE_LENDING_LENDER_WITHDRAW -- and why the id can be read straight out of deployed bytecode, which is how the stale fixtures were caught.
The doc called out Perimeter FeeLib, IPerimeter FeeEvents, Perimeter FeeBorrowerExitOps and setPerimeter FeeBorrowerExitOps -- none of which exist. An earlier sweep on the Phase-1 branch had expanded ColFee to the product name "Perimeter Fee" inside symbol names, so the release documentation has been naming contracts nobody could find for a while. The four remaining "Perimeter Fee" occurrences are the product name and stay. Fixtures re-pinned after the Zero NatSpec correction moved its metadata.
Decision: modules whose bytecode moved only in the metadata tail stay out of Phase 1. That decision does not hold by itself. getProtocolModules() returns every protocol module, hardhat-deploy redeploys any whose bytecode differs from its record, and 2080 proposes a replaceContract for each new address. Renaming the perimeter identifiers changed the metadata hash of seven modules carrying no perimeter code, so the machinery would have grown the release by seven contracts -- past the ten-action cap that split Phase 1 into three proposals. The test compares each module's runtime body against RSK mainnet with the metadata tail stripped and library links normalised, then asserts both halves of the decision: the four perimeter-bearing modules genuinely differ and must ship, and the seven left out are byte-identical in executable code so omitting them changes nothing on chain. The second assertion is the one that earns its keep. If a future change puts real code into an omitted module, it fails and says so -- either the change belongs in the release and the action count needs rechecking, or it was accidental. Today the omission is provably free.
Second own review cycle. The three perimeter-core fixtures still named commit c9548db -- the revert that kept src/ byte-reproducible against the deployed controller. That revert was itself reverted when the deployed contracts moved into the redeploy set, so the fixture carried a controller built from source that no longer exists on any branch. Same class as the stale Zero fixtures, one layer along, and caused by a commit made after the review that found the first pair. Only the controller's bytes actually moved; the vault and proxy take a provenance refresh.
A stale fixture is the worst kind of test failure because it does not fail. The rehearsal configures the controller with current surface ids, a stale contract quotes the old ones, no policy resolves, and the fail-open perimeter reports that as no fee -- so the gate that exists to prove the fee is charged passes while exercising the wrong contract. It happened twice on this branch: once caught by external review, once by a later commit invalidating fixtures regenerated an hour earlier. Neither was caught by a test, because none existed. This reads the identifiers straight out of the fixture bytecode, which works because solc 0.5.17 and 0.6.11 store the preimage and hash it at run time rather than folding it. Any Phase-1 preimage present means the fixture predates the re-cut; the Zero fixture must additionally embed the two surfaces and the pointer slot it is supposed to quote, so a fixture that stops referencing a surface fails too.
Second adversarial pass attacked the guards written after the first, and it was right about all three: each was green for the correct reason today and would have stayed green through the next mistake. The fixture guard only rejected known Phase-1 strings. The stale controller fixture that a later commit invalidated contained none of them -- it was simply built from source that no longer existed -- so every assertion passed. Freshness now means the fixture names the commit its source repo is actually on, checked against the repo on disk, and the test fails loudly rather than passing quietly when no source repo is available to check against. The release-set test duplicated the module list instead of reading it. A module could be added to or removed from getProtocolModules(), which the deploy scripts iterate and 2080 proposes replacements from, and the test would say nothing. It now asserts every deployed module is classified and nothing classified has stopped being one. It also asserts each module links only the swap library, because normalising link addresses away would otherwise hide a swap to a different ABI-compatible library. The identifier pin read the raw source, and every constant sits directly under a comment repeating its own preimage -- so the containment check passed on the comment while the executable derivation could say anything. Comments are stripped first. Verified by corrupting only the derivation and leaving the comment: 1 failing, as it should be.
Caught by the freshness guard added an hour ago, on its first real outing: it named all three core fixtures as pinned to a commit the perimeter repo had moved past. Only the controller's bytes changed -- comments feed the metadata hash -- but the point is that nothing had to notice by hand.
The rehearsal could not start. deployments.fixture() asks the node for each previous deployment's transaction so hardhat-deploy can decide whether the bytecode moved, and a forked node does not serve pre-fork transactions by hash. Both RSK endpoints return that transaction perfectly well when asked directly; the fork simply will not proxy the call. So it fails on every contract with a recorded deploy tx, which is all of them, and no endpoint or archive depth changes that. The delay branch had already solved it by deploying the release contracts from local artifacts. This is that helper, scoped to exactly what SIP-0094 Part 1 registers rather than the delay branch's larger set, so the rehearsal exercises the release and not a superset: two hooked beacon modules, three replaced protocol modules, the admin module and the borrower-exit charge hook, over a freshly deployed swaps library the linked ones bind to. Rehearsal green against a mainnet fork: both SIPs execute through governance, every registration is asserted, live exits charge and the no-touch paths stay uncharged.
Mutation testing on the perimeter suites. Most bit hard: neutering the borrower charge hook fails 10 tests, the lender path 16, Zero 20, and injecting real code into a module the release set excludes fails the release test. The liquidation no-touch tests did not. They survive deleting both exemption guards in _exitFeeChargeable and making it return true unconditionally -- and that is not a weak assertion, it is because liquidation never reaches the charging code at all. It calls the plain _withdrawAsset; charged exits call _withdrawAssetChargingExitFee. The exemption is structural. Which means their stated reason was wrong. "Exempt by CloseOrigin.Liquidation" points at a runtime gate that does not protect this path, and a reader trusting it would think the origin check covers liquidation. It does not. The added assertion fails on the change that would actually break the property: wiring a charging helper into the liquidation module. Verified by doing exactly that -- 1 failing, restored.
The release redeploys LoanMaintenance for the perimeter hook on withdrawCollateral, which puts its unrelated entry points on new bytecode. The unit suite covers them; the fork rehearsal did not, so they had never run against real state on the new build. depositCollateral and extendLoanDuration now do, and each is asserted twice: no fee fires -- position adjustments are deliberately outside the perimeter -- and the operation actually did its job. The second half matters, because a regression that bricked either would satisfy "no fee charged" perfectly. Verified the assertions execute by inverting one: 1 failing, restored. Also recorded, since it cost a re-run to be sure of: the rehearsal takes ~295s against a cold fork cache and ~38s warm. Fast is not skipped work.
…ng against deployed contracts Two additions Tyrone asked for. reduceLoanDuration now runs in the rehearsal alongside extendLoanDuration. It is indefinite-term only, which is what iToken.borrow opens, so the existing borrow loan qualifies; the withdrawal is sized from what is actually left on the loan rather than a guessed constant, so it cannot trip the "amount too high" bound. Zero's adjustTrove joins it in both increase directions -- adding collateral and drawing more debt. Every one asserts no fee fires AND that state actually moved, because a regression that reverted any of them would emit nothing at all and satisfy a fee-absence check perfectly. PERIMETER_DEPLOYED_ADDRESSES points at a JSON file of name-to-address and every helper attaches instead of deploying: the lending release set, controller and vault, and both Zero implementations. Names absent from the file still deploy, so a partial file works. This is the mode for re-running after the release is redeployed, so the rehearsal exercises the bytecode that shipped rather than a fresh local copy of it. Two deliberate details. Each attached address is checked for code on the fork and throws if empty -- otherwise the rehearsal passes by calling nothing, which is the failure mode this whole review has been chasing. And an attached controller keeps its on-chain policy instead of being re-bootstrapped: after redeployment it is already configured, and rewriting it here would test configuration this run invented rather than what the release carries. Verified: rehearsal green with the new operations; inverting the last new assertion fails it, which also proves everything before it was reached; and a deliberately codeless address is refused with the address named.
…it stays so The perimeter rename left this library's runtime body byte-identical while moving its metadata trailer, because that trailer hashes every source file in the compilation unit and ISovryn.sol changed. hardhat-deploy compares full bytecode, so a deploy run would have produced a functionally identical copy — and consumers link whatever address the record holds, so every module would have bound to it. That copy is also unverifiable on Blockscout, whose verifier does not mask the call-protection self-address. The previous deployment hit exactly this and had to relink afterwards. 2060 now decides by executable code. It compares runtime bodies with the metadata trailer stripped: identical means keep the existing address, anything else means deploy, logged loudly because every module linking it then needs redeploying and re-verifying too. A blanket skipIfAlreadyDeployed was rejected on the way — it fixes the metadata case and silently breaks its opposite. The stripping lives in deployment/helpers and is shared with ReleaseSet.pinned, so the decision to skip a redeploy and the test asserting that omission is safe cannot answer differently. Documented there: it is unsafe for contracts with immutables, whose deployed body legitimately differs from the artifact's. ReleaseSet.pinned gained two assertions. The library's body must match what is deployed, so a real code change forces the link-don't-deploy decision to be revisited rather than assumed. And every module must link the verified library, with the two existing deployments that link an older copy listed by name so a new one linking a wrong copy still fails — that list is to be emptied once those two are redeployed. Comments across the perimeter tests and the deploy script were rewritten to state properties rather than history. They had drifted into phase names, dated decisions and pointers to design documents that are not in this repository; the dress-run prerequisite now names the deploy command a reader can actually run.
The release linked 0x47Dc479c, live since March and verified only as a partial match. The copy deployed in the previous round, 0xFE2bb2d, verified on Blockscout on 2026-08-16 as a FULL match — six days after the decision to avoid it was taken, and on the strength of it being unverifiable at the time. All three builds of this library — March, the previous round, and the current one — are byte-identical once the metadata trailer and each copy's own call-protection self-address are set aside, so this changes no executable code. It changes which deployment a voter lands on when rebuilding a module's bytecode, and a full match is the stronger evidence. The deployment record is the authentic one from the previous round, carrying its own transaction hash and block rather than an address transplanted into another deployment's receipt. The release-set test's exception list now names the current on-chain LoanClosingsWith, which links the March copy and is redeployed this round. The list empties itself once that lands; if it does not, the redeploy did not do what it claims.
Seven artifacts deployed 2026-08-24 by the release deployer, all inert until SIP-0094 Part 1 registers them: BorrowerExitPerimeterOps 0xe533dc6D03Ae97977Da3357A98AA94b769738AB5 LoanTokenLogicLM 0x593DB96E61F59F1278068742486Fc85CDBe9D872 LoanTokenLogicWrbtcLM 0x6c8f59D321560f4bEF84c99e83dd7FF31122e14f LoanClosingsRollover 0xdB4fF0a861bF0c714B31E678c919c2B7EfD05329 LoanClosingsWith 0xa3FCC9F88De9A7f0258eda6cD8d6F7D39ef6fb8d LoanMaintenance 0xa87Bd1eF82FA2BE049473c73eAaa97d6AB9C4399 ExitFeeModule 0x0562e396f5fF7e819c8e9CcF25DCb68e16A2672F The swaps library was not redeployed: its executable body is unchanged, so the deploy script kept the existing deployment and all three consumers now link the same fully verified copy — the split the previous round left behind is gone. LoanClosingsWith is 24,473 bytes, 103 under the size limit.
Co-authored-by: tjcloa <77393391+tjcloa@users.noreply.github.com>
The "differs from mainnet" comparison read the hardhat-deploy record for rskSovrynMainnet, which says what was last deployed rather than what the protocol has registered. Deploying the release overwrote that record, so all four shipping modules compared equal to themselves and the gate went red the moment it had done its job. Pin the baseline instead. The three replaced modules are the targets the protocol resolves today -- 0xc7AEa780, 0x47eCc939, 0x4BC462e2, the same addresses SIP-0094 names as rollback anchors -- stored as the sha256 of the metadata-stripped, link-normalised runtime body. Each was verified equal to the live code at its address before being written. ExitFeeModule has no entry because its selector resolves to the zero address on chain: it is new, has no rollback anchor, and nothing on mainnet to differ from. The exemption is derived from the baseline rather than hand-listed, so a module that loses its entry fails instead of quietly excusing itself. Also empties KNOWN_UNRELINKED. LoanClosingsWith now links the fully verified library copy along with the other two consumers, which is what the redeploy claimed and this list existed to check.
Both source repos moved forward by deployment-record commits that touch no contract source, which is enough to trip the freshness gate: it compares commit identity, not bytes. Regenerated against the current checkouts -- regenerate.js reports six fixtures written and zero with new bytes, so only the recorded commit moved.
…lease
Attach mode had never run end to end -- it could not, until the contracts
existed on chain. Three things stood between it and a run, all of them the
rehearsal describing a stack it had deployed itself rather than the one that
shipped:
- Addresses arrive as text from an operator-written file, and ethers leaves
contract.address exactly as passed. A lowercase address failed the identity
assertions on case alone, against contracts that were in fact correct.
attachDeployed now checksums before attaching and before saving the record.
- The fee rate was a 1% constant. The shipped controller carries 10 bps, so
every fee assertion would have been arithmetic about a policy nobody
deployed. The rate now comes off the attached controller, and the four
surfaces are required to agree -- a split policy fails loudly rather than
mismeasuring one surface quietly.
- setExitFeeEnabled is onlyAdminOrOwner, and neither is a signer this test
holds: the deployer EOA owns the controller until the Exchequer accepts, and
the Exchequer is the admin. The run now impersonates the on-chain authority,
which is also the path production takes.
Adds the deployed address set so the run is reproducible:
PERIMETER_DEPLOYED_ADDRESSES=./tests-onchain/perimeter/deployed.rskSovrynMainnet.json \
COLFEE_FORK_BLOCK=9180100 npx hardhat test \
tests-onchain/perimeter/perimeterActivationSips.test.js --network rskForkedMainnet
Green on the round-2 deployment: both proposals build from the deployed
addresses, execute in order, charge 10 bps where they should, and leave the
no-touch paths uncharged.
All three parts now link the SIPS commit carrying the deployed addresses, with the matching sha256, so a voter can bind the on-chain calldata to the document that describes it. The previous pin named the pre-redeploy text. This is re-cuttable only until sips:create -- a created proposal's description is immutable on chain.
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
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 1 of the Sovryn security perimeter (SIP-0094): a minimal exit fee on user-initiated withdrawal surfaces, funding continuous exit monitoring. This is the lending/borrowing half of the system; the shared fee rail lives in Sovryn-perimeter and the Zero half in zero-contracts#10.
What this carries
ExitFeeControllerand paying the fee leg to theExitFeeVault. Every hook fails open: a fee fault forgoes the fee, never blocks a withdrawal.ExitFeeModule(governance setters for the protocol controller pointer) andColFeeBorrowerExitOps(the borrower-exit charge hook).The fee system deploys disabled and is enabled only by governance after post-deployment verification. Perimeter call graph:
docs/PERIMETER_FEE_CALL_GRAPH.md.Merge timing
This PR is intended to stay open until SIP-0094 has been approved and executed on-chain — mainline follows the chain rather than leading it. The deployment runs from this branch's commit, so no contract file here may change between deploy and execution; the proposal builders compare the live runtime code hash at creation, so a stale implementation produces a refused proposal rather than a silent mismatch.
Supersedes #579, which was merged prematurely and has been rolled back off
development.