[VPD-1982] Add CollateralGateway for hub deposits - #74
Open
Debugger022 wants to merge 18 commits into
Open
Debugger022 wants to merge 18 commits into
Debugger022 wants to merge 18 commits into
Conversation
Greptile SummaryThe PR introduces a stateless CollateralGateway for depositing wallet assets or existing Core collateral into Liquidity Hub markets, withdrawing combined wallet/Core Hub positions, and supplying to Spoke markets.
Confidence Score: 3/5The PR is not safe to merge until flash migrations prevent caller-selected spenders from accessing preexisting gateway balances. A permissionless migration can approve and invoke an attacker-controlled Hub against gateway-held underlying, allowing stranded funds to be drained through the active callback; withdrawal rounding also spends one complete market receipt for requests smaller than a receipt unit. Files Needing Attention: contracts/CollateralGateway/CollateralGateway.sol
|
| Filename | Overview |
|---|---|
| contracts/CollateralGateway/CollateralGateway.sol | Implements all gateway flows; caller-controlled flash-migration spenders can access stranded balances, and market-share rounding can over-redeem withdrawals. |
| contracts/CollateralGateway/ICollateralGateway.sol | Defines the public API, events, errors, prerequisites, and expected withdrawal semantics. |
| contracts/CollateralGateway/IHub.sol | Adds the minimal ERC-4626-compatible Hub interface required by the gateway. |
| contracts/Interfaces/IComptroller.sol | Adds Core market-entry-for-account and hypothetical-liquidity declarations. |
| contracts/Interfaces/IILComptroller.sol | Adds the isolated-pool market-entry interface with the Spoke-specific argument order. |
| contracts/Interfaces/IVToken.sol | Exposes the flash-loan fee mantissa used to size migration loans. |
| deploy/028-deploy-collateral-gateway.ts | Adds a constructor-free deployment and live-network verification task. |
| tests/foundry/unit/CollateralGateway_Adversarial.t.sol | Covers callback argument validation and balance-delta protection but does not cover a malicious approved spender draining preexisting funds. |
| tests/foundry/unit/CollateralGateway_Withdraw.t.sol | Covers wallet/Core withdrawals and explicitly demonstrates whole-receipt over-redemption for sub-receipt requests. |
| tests/foundry/fork/Fork_CollateralGateway.t.sol | Exercises Core flows against pinned BNB Chain state with an etched pending MarketFacet implementation. |
Reviews (1): Last reviewed commit: "feat(deploy): add CollateralGateway depl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
6 tasks
GitGuru7
reviewed
Sep 16, 2026
Asking for more receipts than the caller holds made the liquidity check report a shortfall, so the call took the flash loan branch and failed later inside the market with a bare "math error". Also scopes the header claim about measured amounts to the redeem and mint paths.
The hub is caller supplied, so every extra read of asset() can return a different token than the one validated against the vToken. The callback path read it again after deposit, which a hub can flip. Migration now carries the validated address and every leg takes it as an argument.
fred-venus
reviewed
Sep 18, 2026
fred-venus
reviewed
Sep 18, 2026
fred-venus
reviewed
Sep 18, 2026
fred-venus
reviewed
Sep 18, 2026
fred-venus
reviewed
Sep 18, 2026
fred-venus
reviewed
Sep 18, 2026
The comptroller was read off the caller-supplied vh market, so nothing tied a call to a real Core deployment and a fake market could name its own. It is an immutable now, and a supply reverts unless that Comptroller lists the market, which pins the hub and the asset behind it.
redeemBehalf accrues the market before it runs its own liquidity check, so asking beforehand read a stale borrow balance. A caller borrowing from the source market could be told there was no shortfall, take the direct path and have the redeem reject it, when the flash loan path would have worked.
The flash loan path wrote ten cold storage slots to carry context into the callback and refunded part of it on delete, with refunds capped at a fifth of the transaction. Transient storage drops the borrowing journey from 1,891,510 to 1,688,054 gas. The deploy script also skips the bare hardhat network, where the Core Comptroller it now takes has no deployment.
Debugger022
force-pushed
the
feat/VPD-1982
branch
from
September 18, 2026 08:16
c894580 to
5879494
Compare
The spoke comptroller renamed enterMarketBehalf to enterMarketForAccount and reordered its arguments to (account, vToken) to match Core. The interface note warning that the arguments were reversed is dropped, since the two signatures now agree.
GitGuru7
reviewed
Sep 18, 2026
GitGuru7
reviewed
Sep 18, 2026
GitGuru7
reviewed
Sep 18, 2026
GitGuru7
reviewed
Sep 18, 2026
A vh market wraps exactly one hub, so passing both meant carrying a fact the market already states and a check for the two disagreeing. Now that the Comptroller is pinned, the market vouches for it: every entry point requires the market to be listed and reads the hub from it, and the source market of a migration is checked the same way.
Migrating a whole position meant reading the receipt balance first and passing it back, so a caller who wanted all of it had to do it in two steps. type(uint256).max now resolves to the caller's balance when the call runs.
No leg of the gateway leaves a balance behind, but tokens sent here by mistake had no way out, because nothing reads the contract's own balance. The owner can now move them, which is the same shape SwapRouter and the leverage manager use.
Every Core market in a call is checked against the pinned Comptroller, but the spoke paths took whatever address the caller passed, so a fake market could take a caller's own tokens through a contract they trusted. The registry pins the market to its pool and asset, and the pool is asked separately whether it is still listed, because the registry keeps its entry after an unlist.
fred-venus
previously approved these changes
Sep 21, 2026
GitGuru7
reviewed
Sep 21, 2026
| if (walletShares != 0) IERC20(hub).safeTransferFrom(msg.sender, address(this), walletShares); | ||
|
|
||
| uint256 freedShares; | ||
| if (walletShares < shares) freedShares = _freeFromMarket(hub, vhMarket, shares - walletShares); |
Contributor
There was a problem hiding this comment.
This passes the uncapped remainder through. The wallet leg above is capped at the balance, but if someone calls with type(uint256).max the remainder here is still ~1.16e77, and _freeFromMarket multiplies by EXP_SCALE before it caps vTokens at the caller's balance, so it overflows and panics before reaching the cap.
Simplest fix is capping the remainder before the multiply so it degrades to "everything you hold". Or add the sentinel properly, for symmetry.
Comment on lines
+227
to
+228
| // Change is measured against this migration's own redeem, never against the balance. Every | ||
| // address here is caller-supplied, so a balance reading would hand a caller anything else |
Contributor
There was a problem hiding this comment.
Every address here is caller-supplied, so a balance reading would hand a caller anything else the gateway happens to hold
with isListed check now , the comment should be updated
| _enterCoreMarket(vhMarket); | ||
|
|
||
| uint256 vTokens; | ||
| if (_wouldCauseShortfall(vToken, vTokenAmount)) { |
Contributor
There was a problem hiding this comment.
If the caller never entered vToken, this reports their pre-existing shortfall rather than anything about this redeem the hypothetical only walks entered markets, so a balance that was never collateral contributes nothing before or after. Core knows that and skips the liquidity check outright for non-members in redeemAllowedInternal, so the redeem goes straight through even for an underwater account.
Net effect is we route those callers into the flash path and charge them the fee for nothing. A checkMembership short-circuit before this call fixes it, and swaps a lens call that loops every entered market with an oracle read each for a single mapping read.
The receipt count was computed before capping at the caller's balance, so shares above about 1e59, including type(uint256).max, overflowed. A request worth more than the receipts now redeems all of them. Also restates comments that still described caller-supplied hubs and Core-only markets.
Core skips the redeem liquidity check for a market the caller never entered, but the gateway still asked the lens, which reported the caller's existing shortfall. An account already under water was sent through the flash loan, paying its fee or reverting when the gateway is not allow-listed.
|
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
3 participants
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.
Summary
CollateralGateway, which turns an underlying balance into collateral in one call on Core (through a Liquidity Hub and its vh market) and on Spoke Pools, and withdraws a vh position back to the wallet. No proxy, no admin, no funds held between calls. The Core Comptroller is fixed at construction.HubRouterin venus-liquidity-hub. The port itself changed only license, imports, types and casts. The review changes since then are listed below and do change behaviour.Related PRs
MarketFacet.enterMarketForAccount(address,address), the ACM gated entry point every Core supply in this gateway calls, and lets PrimeV2 list the 24 decimal vh markets. The gateway's Core paths revert on chain until that facet is cut into the Comptroller (governance step 4 below). The fork tests stand in for that upgrade by etchingtests/foundry/fork/fixtures/MarketFacet.deployed.hex, which is built from #710, so the fixture must be rebuilt whenever #710 changes.Changes
Contract (
contracts/CollateralGateway/)CollateralGateway.sol,ICollateralGateway.sol, and a minimal localIHub(asset,deposit,redeem). License is BSD-3-Clause.IVBep20becomesIVToken,comptroller()returnsIComptrollerrather than an address, andexecuteOperationand the flash loan market array takeIVToken[].Shared interfaces (additions only)
IComptroller:enterMarketForAccount(account, vToken),getHypotheticalAccountLiquidity.IVToken:flashLoanFeeMantissa.IILComptroller: SpokeenterMarketBehalf(vToken, account). The arguments are the reverse of CoreIComptroller.enterMarketBehalf(onBehalf, vToken), so it is not added toIComptroller.Review changes on top of the port
supplyFromCollateralInsufficientReceiptscheckvToken.underlying()redeemBehalfaccrues before its own liquidity check, so asking beforehand read a stale borrow balance and could send a borrowing caller down the direct path, which then revertsThe last change needs
transient, soCollateralGateway.sol, its two local interfaces and the tests moved topragma solidity 0.8.28, with matchingvia_irentries infoundry.toml.Tests (Foundry)
tests/foundry/unit/: Adversarial 9, Spoke 13, Supply 5, Withdraw 13.tests/foundry/fork/, pinned to BSC block121_990_000and readingARCHIVE_NODE_bscmainnet. Setup etches the MarketFacet fixture (built from [VPD-1982]: support PrimeV2 markets with underlyings above 18 decimals venus-protocol#710) and cuts inenterMarketForAccountas the NormalTimelock. The flash loan whitelist and the supply cap are set throughsetWhiteListFlashLoanAccountandsetMarketSupplyCapsas the NormalTimelock.foundry.toml: read access to the fixtures directory, andvia_irfor the three 0.8.28 gateway files.Docs and deploy
contracts/CollateralGateway/README.md.deploy/028-deploy-collateral-gateway.ts: takes the Core Comptroller from theUnitrollerdeployment, tagcollateral-gateway, verified on live networks, skipped on the in-process hardhat network where no Core deployment exists.Test plan
forge test --match-path "tests/foundry/unit/CollateralGateway_*": 40 passed.forge test --match-path "tests/foundry/fork/*"with an archive RPC: 23 passed. With no RPC, all 23 skip.yarn lintandyarn testpass.yarn hardhat deploycompletes on the hardhat network, where028skips.Known gaps
_flashAmountnever run on the fork:treasuryPercentand vUSDTflashLoanFeeMantissaare both 0 at the pinned block. Only a mock with a 1% fee covers them._flashAmountsizing, the_freeFromMarketrounding cap, the wallet and market split inwithdrawPosition, and "holds no funds after any call".minAssetsrevert, a non 18 decimal asset, and the Spoke paths (no Spoke pool is deployed).PoolRegistry. The gateway pulls frommsg.senderand zeroes the approval on the next line, so a fake market can only take the caller's own tokens inside the caller's own transaction. Adding the check needs an immutable registry address and cannot be fork tested until a Spoke pool exists.Governance dependencies (one VIP, all executed by the NormalTimelock
0x939bD8d64c0A9583A7Dcea9933f7b21697ab6396)ProxyAdmin.upgrade(prime, newImpl)diamondCut: Replace for the existing MarketFacet selectors on0x21f8E1471b153f49BE1d645A008E4a57434eEd23, Add for0x2e30a93c. The exact selector list comes from #7100xfD36E2c2a6789Db23113685031d7F16329158384giveCallPermission(comptroller, "enterMarketForAccount(address,address)", gateway)0x4788629ABc6cFCA10F9f969efdEAa1cF70c23555setWhiteListFlashLoanAccount(gateway, true)Steps 5 and 6 need the deployed gateway address, so the gateway is deployed and audited before the VIP is proposed. Spoke
enterMarketBehalf(address,address)grants are not part of this VIP. The PrimeaddMarketfor vh markets follows the decimals fix separately.