Skip to content

feat(psr): support multiple pool registries - #168

Open
Debugger022 wants to merge 5 commits into
developfrom
feat/psr-multi-pool-registry
Open

Debugger022 wants to merge 5 commits into
developfrom
feat/psr-multi-pool-registry

Conversation

@Debugger022

Copy link
Copy Markdown
Contributor

Summary

Hub-funded spoke pools ship their own PoolRegistry so indexers and tooling can tell them apart from the isolated pools. PSR stores a single poolRegistry and rejects any non-core pool that registry does not know, so pointing it at the spoke registry would break every live isolated pool: updateAssetsState is called by vTokens both on reduceReserves and on the protocol-seize path, which means those pools stop taking income and their liquidations revert on-chain.

This PR lets PSR resolve markets through more than one registry. poolRegistry stays the primary and is probed first; extra registries are added alongside it.

Changes

  • addPoolRegistry / removePoolRegistry, owner gated, bounded by maxLoopsLimit
  • updateAssetsState resolves through the primary registry, then the additional set
  • New views: getPoolRegistries, totalAdditionalPoolRegistries, isMarketRegistered
  • setPoolRegistry keeps its behaviour and ABI, plus a guard against promoting a registry that is already in the additional set

Why keep poolRegistry instead of only the array + mapping

  1. It cannot simply be deleted. poolRegistry sits at slot 301 and assetsReserves at 302. Removing the declaration shifts assetsReserves onto 301 and corrupts live accounting, so a dead placeholder slot would have to stay anyway, holding the same address the array would then duplicate. Storage ends up less clean, not more.
  2. No ABI change. poolRegistry() and setPoolRegistry(address) stay on the deployed ABI, so off-chain consumers are unaffected.
  3. The upgrade stays safe on its own. PSR is live on 16 chains. As written, a chain with no spoke pool can take the implementation upgrade with zero config and nothing breaks. With an array-only design the array starts empty, so every chain's upgrade would have to be paired with its own addPoolRegistry call or that chain's isolated-pool liquidations revert until it lands.

Storage safety

PSR is a leaf contract with no trailing gap. New state is appended at slots 305/306; 301-304 are untouched, so the upgrade needs no reinitializer and no migration. Pinned by tests/ProtocolReserve/storageLayout.ts.

Rollout

Upgrade and addPoolRegistry(<spoke registry>) must go in the same VIP proposal, per chain. setPoolRegistry is never called.

Testing

  • 25 unit tests covering add/remove/promote guards, resolution through either registry, removal re-blocking, and the core-pool bypass
  • 6 storage-layout tests pinning the deployed slots
  • 12 fork tests on bscmainnet that upgrade the live proxy through DefaultProxyAdmin, assert poolRegistry, all 18 distribution targets and the income ledger survive, and prove a live isolated pool and a second-registry pool both resolve at the same time

Out of scope

  • distributionTargets is global, not per-pool, so spoke income will split across the same destinations as core income. Routing it to the treasury is a separate product decision.
  • RiskFundConverter holds its own poolRegistry and is untouched; nothing in this change or its rollout repoints it.

Hub-funded spoke pools ship their own PoolRegistry so indexers and tooling
can tell them apart from the isolated pools. Repointing `poolRegistry` at
the spoke registry would make every existing isolated pool fail
`updateAssetsState`, which vTokens call both when reducing reserves and when
seizing the protocol's share of liquidated collateral. Those pools would
stop taking income and their liquidations would revert on-chain.

`poolRegistry` now stays the primary registry and the spoke registry is
added alongside it, so pools that resolve today keep resolving on the same
single external call.

- add owner gated `addPoolRegistry` / `removePoolRegistry`, bounded by
  maxLoopsLimit
- resolve a market through the primary registry first, then the additional set
- add `getPoolRegistries`, `totalAdditionalPoolRegistries` and
  `isMarketRegistered` views
- reject promoting a registry that is already in the additional set, so no
  registry is probed twice
- append new state after `distributionTargets`, so the upgrade needs no
  reinitializer and no migration
- pin the deployed storage slots in a test, and cover the upgrade against the
  live proxy in a bscmainnet fork suite
@Debugger022
Debugger022 marked this pull request as ready for review September 2, 2026 11:21
@Debugger022 Debugger022 self-assigned this Sep 2, 2026
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

The PR extends ProtocolShareReserve market resolution to support a bounded set of additional pool registries while preserving the existing primary registry and deployed storage layout.

  • Adds owner-controlled registry addition and removal with duplicate and loop-limit checks.
  • Resolves non-core markets through the primary registry followed by additional registries.
  • Adds registry inspection views and comprehensive unit, storage-layout, and BSC fork coverage.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

The existing primary-registry behavior and ABI remain intact, additional registry management is bounded and owner-controlled, and deployed storage evidence confirms the new variables occupy previously unused slots.

Important Files Changed

Filename Overview
contracts/ProtocolReserve/ProtocolShareReserve.sol Adds bounded multi-registry storage, governance controls, views, and primary-first market resolution without changing existing accounting slots or the updateAssetsState ABI.
tests/ProtocolReserve/ProtocolShareReserve.ts Covers access control, duplicate and limit guards, swap-and-pop removal, views, primary and additional registry resolution, removal behavior, and fund distribution.
tests/ProtocolReserve/storageLayout.ts Pins inherited and deployed storage through slot 304 and verifies the new registry state is appended at slots 305 and 306.
tests/fork/ProtocolShareReserve.ts Upgrades the live BSC proxy in a fork and verifies preserved configuration and accounting alongside simultaneous primary and additional registry resolution.

Reviews (1): Last reviewed commit: "feat(psr): support multiple pool registr..." | Re-trigger Greptile

fred-venus
fred-venus previously approved these changes Sep 2, 2026

@fred-venus fred-venus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@GitGuru7 GitGuru7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Debugger022

Debugger022 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor Author

RiskFundConverter will need the same change. _releaseFund calls updateAssetsState
on every destination, and RiskFundConverter still resolves through its own single registry,
so it reverts MarketNotExistInPool for a spoke comptroller and takes releaseFunds down
with it. Fine as a follow-up, but it should land before a spoke pool goes to mainnet.

Note: RiskFundConverter currently isn't a PSR distribution target, so this does not affect the Spoke Pool launch.

If it is added as a distribution target in the future, it would need to support the Spoke registry as well. However, there is also an existing compatibility issue: RiskFundConverter calls updatePoolState(...), which is not implemented by RiskFundV2 (RiskFundConverter.sol#387-391).

So the registry change alone would not be sufficient. This should be addressed as a follow-up if RiskFundConverter is ever used as a PSR distribution target.

cc: @fred-venus

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Health
Interfaces 100% 100% ✔
ProtocolReserve 96% 83% ✔
Test 100% 100% ✔
Test.Mocks 73% 53% ➖
TokenConverter 89% 74% ✔
Utils 100% 100% ✔
helpers 97% 83% ✔
Summary 90% (821 / 910) 76% (283 / 370) ✔

@fred-venus

Copy link
Copy Markdown
Contributor

RiskFundConverter will need the same change. _releaseFund calls updateAssetsState on every destination, and RiskFundConverter still resolves through its own single registry, so it reverts MarketNotExistInPool for a spoke comptroller and takes releaseFunds down with it. Fine as a follow-up, but it should land before a spoke pool goes to mainnet.

Note: RiskFundConverter currently isn't a PSR distribution target, so this does not affect the Spoke Pool launch.

If it is added as a distribution target in the future, it would need to support the Spoke registry as well. However, there is also an existing compatibility issue: RiskFundConverter calls updatePoolState(...), which is not implemented by RiskFundV2 (RiskFundConverter.sol#387-391).

So the registry change alone would not be sufficient. This should be addressed as a follow-up if RiskFundConverter is ever used as a PSR distribution target.

cc: @fred-venus

I dont think we will ever use reuse RiskFundConverter, its relatively low efficient, so i am ok to skip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants