VDB-53: VIP-657: wire Cash+ and Ceffu FRV vaults into the Liquidity Hub, raise FRV caps to 50% - #753
Conversation
… to 50% Registers the Asseto CASH+ fixed-rate vault as a resource on the U Hub's FRV yield source (behind the shared AdapterFRV) with its inner deposit/ withdraw queues, and raises the FRV percentage-of-TVL cap from 30% to 50% (absolute cap unchanged at 5,000,000) on both the U and USDT Hubs. All commands are direct Normal Timelock calls (roles granted by VIP-650/651); no ACM grants. The Ceffu/USDT vault is not yet deployed, so its resource registration is left to a follow-up VIP. Adds a 4-part fork simulation (19 assertions, green at block 116780000) including a behavioral proof that an Operator reallocate push into the vault reverts ResourceNotRegistered pre-VIP and ResourceCapacityExceeded post-VIP, proving registration went live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review — VIP-665 (Cash+ FRV vault → U Liquidity Hub, FRV cap 30%→50%)Reviewed by a three-dimension squad (code quality / functional correctness / security). All on-chain claims below were re-verified independently against BNB Chain mainnet at head ~116,792,100–116,792,252, not taken from the PR description. The five-command payload itself verifies clean — every address matches the VIP-650 address book / The material finding is not about the payload — it is about what registration imports: a 3-of-6 Safe with no delay can extract Hub funds from the CASH+ vault once capital sits in it (Security #1, HIGH). Not exploitable while the vault is pre-Fundraising with Counts: Code Quality 8 · Functional Correctness 1 · Security 5. Code QualityComments
Complexity
Organization
Checked, no finding: the vendored ABIs are plain ABI arrays with no bytecode and Functional Correctness
Nothing else. The five-command payload verifies clean; details below. Verified against BNB Chain mainnet (head ~116,792,100, not the fork block — no archive RPC on this host, so the 19/19 sim run itself was not reproduced)
Intent note (not a code defect): VDB-53 asks for SecurityReviewed the diff adversarially and traced every suspicion into 1. The VIP verifies the caller's roles but not the role set that registration imports. Traced end-to-end:
So Precondition is a privileged multisig, not a permissionless exploit — but this is a unilateral, zero-delay extraction primitive over user funds, which is stronger than the pause-only powers guardians normally hold. Before this VIP, zero Hub funds were reachable this way ( Fix: exclude 2. The per-source cap is doing all the work here:
Scale: the U Hub holds only ~10.005 U, so its 50% is immaterial today — but the USDT Hub holds 2,700,575 USDT, whose FRV headroom rises 810k → 1.35M USDT. Fix: set a non-zero 3. The Operator's push goes 4. The address is correct — verified independently of the PR's comments: EIP-1167 minimal proxy → impl 5. The vault is pre-Fundraising ( Checked and clean
Escalation: Security finding 1 crosses three repos ( |
…edupe addresses - drop setInnerDepositQueue command (out of VDB-53 scope + pre-arms FRV cascade) - checksum CASH_PLUS_VAULT (EIP-55) - import Hub/FRV/adapter/cap constants from vip-650 address book - tighten effective-cap assertion to exact equality; drop bogus tolerance - hoist frvGroups typed decl + expectFrvCaps helper; trim redundant docs - record sweep/setTreasury + redeemFeeBps guardrails in the VIP description Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the Ceffu fixed-rate vault leg to VIP-665 alongside the existing Cash+ leg, per review feedback. The Ceffu vault is not yet deployed; its address is the deterministic CREATE2 clone address read live from the InstitutionalVaultController (predictVaultAddress(CEFFU_INSTITUTION) == 0x086fd7972510dF9d9cFdc4efB8677fc72d290103), verified on-chain at HEAD and at the sim fork block. VIP now, per USDT Hub FRV source: addResource(Ceffu, AdapterFRV) + setInnerWithdrawQueue([Ceffu]); the USDT Hub FRV cap raise to 50% was already present. No new ACM grants (Normal Timelock already holds the roles on the USDT FRV source, verified). Sim: assert the controller still predicts CEFFU_VAULT, etch a minimal FRV-vault stub at that address (asset()==USDT), and mirror the Cash+ pre/post registration + reallocate-push behavioural checks. 27/27 green on the bscmainnet fork; eslint + prettier clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review — VIP-665 round 2 (
|
Option A: keep the single 6-command VIP with the Ceffu leg; the VIP is only proposed after the Ceffu vault is deployed. Reword meta.description and the CEFFU_VAULT comment so voters are not told the fork-pinned predictVaultAddress assertion proves the address at execution time — a fork at a fixed block returns the same value forever. The real guarantee is the deploy-first sequencing plus the atomic revert (addResource fails ResourceNotContract on a codeless address). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review — VIP-665 round 3 (
|
| # | Round-2 finding | Status under Option A |
|---|---|---|
| 1 | :188 atomic revert if the vault is undeployed at execution |
Relocated, not resolved. Deploy-first removes the cause, but the enforcement is a manual pre-proposal step: verified live that the vault is still undeployed and isRegistered == false, and no repo artifact gates queueing on it. Residual coupling (48h delay, 14d grace) is unchanged. |
| 2 | :65 prediction stable only while nonce and vaultImplementation are unchanged |
Moot in substance — once the vault is deployed the prediction stops mattering, the address is occupied by real code. But the incomplete caveat survives verbatim in the description (:97) and now decorates a claim that is false anyway — folded into finding 1 below. |
| 3 | sim :132-137 drift guard cannot fail |
Still open, untouched — and now actively harmful. See finding 2. |
| 4 | description "the fork simulation asserts the prediction still holds" | Fixed in the voter-facing text, and the replacement ResourceNotContract claim verifies out. But the identical false assurance still sits in-repo at simulations/vip-665/bscmainnet.ts:133-135 — it was deleted from the description only. |
| 5 | :113 "which it will" — the address commits to code, not terms |
Relocated. Deploy-first genuinely improves this: the config becomes inspectable before the vote. Nothing in the PR inspects it — see finding 6. |
| 6 | sim :143-144 asserts absence, etches a stub |
Still open, untouched, and now directly contradicts the sequencing the description commits to. See finding 3. |
Net: Option A resolves none of the six in the repo. It resolves #1/#2/#5 in principle, conditional on
a manual step, and leaves #3/#4/#6 — all three in the simulation — exactly as they were.
Findings
-
vips/vip-665/bscmainnet.ts:93-102— the description's one voter-verifiable claim is guaranteed false at the moment a voter can check it:createVaultdoesinstitutionNonce[institution]++(InstitutionalVaultController.sol:205) andpredictVaultAddressreads the current nonce (:544-548), so once the vault is deployed (nonce 0→1)predictVaultAddress(0x8972E6F8…6262)returns the nonce-1 clone, not0x086fd797…0103; a voter running the check the description hands them gets a mismatch, and the description's own caveat "(this holds only while the institution's nonce is unchanged)" then tells them the address is stale — so a correct VIP reads as broken. Same paragraph opens "The Ceffu vault is not yet deployed" and closes "submitted only after the Ceffu vault has been deployed" — flatly self-contradictory. — Fix: delete the CREATE2/prediction paragraph entirely and state the deploy-first facts a voter can verify at proposal time:eth_getCode(CEFFU_VAULT) != 0x,controller.isRegistered(CEFFU_VAULT) == true,IInstitutionalLoanVault(CEFFU_VAULT).asset() == 0x55d3…7955,institutionalConfig().institutionOperator == CEFFU_INSTITUTION. -
simulations/vip-665/bscmainnet.ts:136— the drift guard is not merely inert, it inverts: as shipped,forking(116780000)freezesinstitutionNonce == 0so no mainnet drift is visible and CI stays green; and after the obvious repair (bump the fork past the deployment, which Option A requires) nonce is 1 and this assertion fails precisely when the VIP is correct. It cannot be fixed by moving the block. Its comment at:133-135still carries the round-2 VIP-123: Resilient Oracle #4 falsehood ("If the institution deploys a different vault first, this fails…"). — Fix: delete the test; assertexpect(await controller.isRegistered(CEFFU_VAULT)).to.equal(true)instead — monotonic (set atInstitutionalVaultController.sol:207, never cleared;cancelVault:231-237leaves it true). -
simulations/vip-665/bscmainnet.ts:143-144— the sim asserts the exact state Option A commits the proposal not to be in (getCode == 0x) and then etches a 195-byte stub whoseasset()is a hard-codedreturn 0x55d3…7955, so no CI run ever executes against the real vault. Trigger: the deployed Ceffu vault is created withsupplyAsset != USDT— nothing in the address commits it, the salt iskeccak256(abi.encode(institution, nonce))(InstitutionalVaultController.sol:193) andAdapterFRV.validateRegistrationis anexternal pureno-op (AdapterFRV.sol:183-186) — mainnet revertsResourceAssetMismatchand takes all six commands down while the sim passes green. — Fix: delete:143-144, the etch andMOCK_FRV_VAULT_BYTECODE; assertgetCode != "0x"and readAdapterFRV.asset(CEFFU_VAULT)off the real vault. -
simulations/vip-665/bscmainnet.ts:268-275— the hard-codedResourceCapacityExceeded(PUSH, 0)breaks as soon as the fork block moves into the deploy-first state. Verified the current assertion is right only because the stub returnsmaxDeposit == 0:depositResourcechecksamount > room → ResourceCapacityExceededbefore_minDeposit(YieldGroupBase.sol:336-347). Trigger: fork past a Ceffu deployment whose vault has been opened intoFundraising(VaultState.Fundraising == 2,IVaultTypes.sol:15-28) —room > 0, so the push either succeeds outright or revertsResourceBelowMinimumDepositvia_minDeposit → IFRVVault.config().minSupplierDeposit(YieldGroupFRV.sol:104-106), and the test fails on the wrong error. The stub cannot reach that branch at all — it has noconfig()selector. — Fix: assert the realvault.state()explicitly and branch the expected behaviour on it instead of hard-coding the pre-Fundraising revert. -
vips/vip-665/bscmainnet.ts:25-29and:57-64— the header's on-chain verification record and the constant-block comment are falsified by the very step Option A adds.:25"Ceffu (USDT) vault: NOT yet deployed",:28-29"predictVaultAddress(CEFFU_INSTITUTION) == CEFFU_VAULT (checked at HEAD and at 116,780,000, so the institution's nonce has not advanced)",:57"NOT yet deployed",:63"The sim's predictVaultAddress assertion is only a sanity check". Trigger: the vault is deployed (the precondition for proposing) — the banner then records a verification that no longer reproduces, and the banner is what a future reader uses to re-derive the address. — Fix: re-run the verification after deployment and recordgetCode != 0x,isRegistered,asset()and the vault's lifecycle state instead of the prediction. -
vips/vip-665/bscmainnet.ts:119and:161— the description is still written in the pre-deployment tense for facts that become checkable at proposal time under Option A::119"which it will" (action 4's asset match) and:161"Ceffu vault (USDT, predicted, pending deployment)". Materially: deploy-first makesconfig()/institutionalConfig()/riskConfig()inspectable, but the description discloses no vault terms — so voters approve wiring the USDT Hub (verified live TVL 2,700,594.93 USDT, FRV headroom rising 810,178.53 → 1,350,297.46) to a counterparty whose coupon, lock length, settlement window and collateralisation appear nowhere in the proposal. — Fix: after deployment, replace "which it will" with the read value and add the approvedconfig()terms to the description. -
simulations/vip-665/bscmainnet.ts:243-249— the outer-queue regression guard is asserted for the U Hub only, while the description at:139claims it for "each Hub". Verified live that both Hubs are currentlyouterDepositQueue == [Core, Flux]andouterWithdrawQueue == [Flux, Core, FRV], so the claim is true today. Trigger: the Operator (0x83f42623…6a7A) holdssetOuterDepositQueue(address[])on the USDT Hub with no timelock — verified viaACM.hasRole— so an Operator queue change before execution puts USDT FRV into the outer deposit queue, lender deposits then cascade into the Ceffu vault automatically, contradicting the description's "FRV is filled only by the Operator's reallocate" (:90-91), and this sim passes. The unguarded Hub is the one holding 2.7M USDT and gaining its first FRV destination. — Fix: iterate the assertion over both stacks.
Does the reframed meta.description describe what a voter is approving?
Not yet. The ResourceNotContract fail-closed claim (:98-99) is accurate — verified live, and
YieldGroupBase.addResource:115 fires the resource.code.length == 0 guard before the adapter and asset
probes. The deploy-first claim (:100-102) is a correct statement of intent. But the paragraph they sit in
is self-contradictory (finding 1), its one actionable check is designed to fail (finding 1), and the
surrounding text — :93, :97, :119, :161 — is uniformly written for the pre-deployment world the
proposal is committing to leave. Two further notes:
:124-125"the Hub's raise guard accepts (absolute unchanged, percentage strictly increases)" — verified
correct:raiseYieldGroupCaprevertsNotIncreasingonly if a value decreases or both are unchanged
(Hub/Hub.sol:225-228→HubAdminLib.setYieldGroupCap). One dimension strictly rising is sufficient.:147-148"Not exploitable while a vault is pre-Fundraising (maxDeposit == 0), so this VIP ships safely"
— under Option A the Ceffu vault exists for the whole voting window, so this premise is now an unverified
runtime property rather than a structural one. Flagged here as a description-accuracy issue; the
underlying exposure is Security's, below.
Checked and clean this round
addResource's guard ordering (YieldGroupBase.sol:111-124: access → zero-address → already-registered →
code.length → adapter code → asset match → validateRegistration) confirms the fail-closed claim.
setInnerWithdrawQueue validation (:203-209, _validateQueue:710-719,
_requireWithdrawQueueCoversFundedResources:725-735) is satisfied by a freshly registered zero-balance
resource. The six description actions still map 1:1 and in order onto the six makeProposal commands.
Both FRV groups verified live at (5e24, 3000 bps, registered, unpaused) with resources() == []; CASH+
verified state() == 1 (MarginDeposited), asset() == U, maxDeposit == 0, and AdapterFRV.asset(CASH_PLUS_VAULT) == U.
Round-1's effAfter.eq(half) fix remains correct and was not weakened.
Security
All state re-verified live at block ~116,819,900.
eth_getCode(0x086fd797…0103)==0x— the Ceffu vault is still not deployed. Option A's
pre-proposal gate is not yet met.institutionNonce[0x8972E6F8…6262]==0,predictVaultAddress(CEFFU_INSTITUTION)==CEFFU_VAULT,
vaultImplementation==0xe87A1eFC…5bdC(matchesfixed-rate-vaults/deployments/bsc_mainnet_addresses.json),
EIP-1967 impl/admin slots both empty, 40,061 bytes — all unchanged since round 2.- ACM role holders on the controller, unchanged since round 2, and wider than round 2 recorded:
createVault/openVault/cancelVault/closeVault/partialPauseVault/unpauseVault/
sweep→ NormalTimelock + FastTrackTimelock (6h) + CriticalGuardian (3-of-6 Safe, zero delay);
setTreasury→ NormalTimelock + CriticalGuardian;setVaultImplementation→ NormalTimelock +
FastTrackTimelock. Noaddress(0)wildcard grants on any of them. - USDT Hub
totalAssets2,700,595 USDT, FRV effective cap 810,178 → 1,350,297 after this VIP,
redeemFeeBps == 0,resources() == []. U Hub 10.0054 U,redeemFeeBps == 0. - Governance window: REGULAR voting period 192,384 blocks ≈ 1.0 day at the measured 0.45 s/block,
NormalTimelockdelay() == 172800(48 h),GRACE_PERIOD() == 1209600(14 d). So under Option A the
deployed vault sits live and Safe-reachable for ≥3 days, up to 17 days, between proposal and
execution — plus however long deployment precedes proposal.
Triage of the seven round-2 security findings
| # | Round-2 finding | Round-3 verdict |
|---|---|---|
| 1 | :65,188 config chosen after the vote by a zero-delay 3-of-6 Safe |
Still open. Option A makes the config inspectable but the PR adds no inspection step, and three risk params stay mutable after inspection. |
| 2 | :188 sweep/setTreasury now spans the 2.7M USDT Hub |
Still open, unchanged. Roles re-verified identical; and the separation-of-duties argument that bounded it is weaker than round 2 stated (see the LOW below). |
| 3 | :138-143 guardrail note understates exposure |
Still open and now factually wrong, not merely incomplete — Option A moves the vault's lifecycle start before execution. |
| 4 | sim :132-137 predictVaultAddress guard is inert |
Still open. The delta relabels it "only a sanity check" in comments; the assertion is unchanged and is still the only automated check touching the Ceffu address. |
| 5 | :198 30%→50% raise on the USDT Hub no longer inert |
Still open, and widened by Option A — the registered vault may now hold real capacity at execution. |
| 6 | :185-194 atomic coupling to an external deployment |
Closed as a revert risk, replaced by a worse one. Deploy-first removes it by consuming the only on-chain guard the Ceffu leg had. |
| 7 | sim :268-275 behavioural proof runs against an etched stub |
Still open, and worse. The stub now stands in for a contract that will actually exist at proposal time. |
Round-1 items #3 (setInnerDepositQueue dropped) and #4 (checksums) remain fixed.
What Option A closes, and what survives it
Closes: the round-2 functional exposure that the whole six-command proposal reverts because the
vault never landed (round-2 security #6, LOW). It also closes nonce/vaultImplementation drift in
principle — once deployed, CEFFU_VAULT is verifiable by eth_getCode + isRegistered rather than
by a prediction. Note the consequence: after deployment institutionNonce becomes 1, so
predictVaultAddress will no longer return CEFFU_VAULT, and any live re-derivation must switch
to checking the deployed vault directly.
Survives, one by one, with the check that would close each:
- Config chosen after the vote. Not closed. Deployment fixes
VaultConfig/InstConfig/RiskConfig
atcreateVaulttime, but nothing in this PR requires anyone to read them. Re-verified
_validateVaultConfig(InstitutionalVaultController.sol:600-633) bounds only sanity: non-zero
durations,0 < fixedAPY ≤ 10000,minBorrowCap ≤ maxBorrowCap,idealCollateralAmount != 0,
marginRate ≤ 1e18, LT·LI < 1. There is no bound tyingidealCollateralAmountto
maxBorrowCap— a vault withmaxBorrowCap = 5,000,000 USDTandidealCollateralAmount = 1 wei
passes validation. Closing check: pin the approved(supplyAsset, fixedAPY, minBorrowCap, maxBorrowCap, openDuration, lockDuration, settlementWindow, idealCollateralAmount, marginRate, liquidationThreshold, liquidationIncentive, latePenaltyRate)inmeta.description, and add a
pre-queue script that readsconfig()/institutionalConfig()/riskConfig()off the deployed
vault and diffs them against those literals. - Address drift. Closed if and only if the pre-proposal gate is enforced and verified. Closing
check: before queueing, asserteth_getCode(CEFFU_VAULT) != 0x,controller.isRegistered(CEFFU_VAULT) == true,
AdapterFRV.asset(CEFFU_VAULT) == USDT, andruntime().state ∈ {WaitingForMargin, MarginDeposited}.
Do not re-assertpredictVaultAddress— it is guaranteed to be wrong post-deployment. sweep/setTreasuryreaching the 2.7M USDT Hub. Not closed at all — orthogonal to sequencing.
Re-verifiedBaseVault.sweep:162-170transfers the full balance of any token to
controller.treasury()with no supply-asset exclusion, andAdapterFRV._breakdown:227marks a
Lock-state position fromrt.totalRaised, i.e. bookkeeping, not the vault's actual balance — so
a sweep does not reduce the Hub's marked NAV by one wei. Closing change: narrowsweep,
setTreasury,createVault,openVault,cancelVaultandcloseVaultto the Normal Timelock and
add the supply-asset exclusion, before any reallocate.- 30%→50% cap raise. Not closed; widened. Closing change: set a non-zero
redeemFeeBps, or split
the USDT cap raise out and leave it at 3000 bps until a funded vault exists.
New exposure introduced by deploy-first
ResourceNotContract fires on exactly one condition — no code at the address. Deploy-first satisfies
that condition deliberately, before the vote. After deployment the guard can never fire again, and
the only remaining on-chain property is AdapterFRV.asset(vault) == USDT. Verified: addResource
(YieldGroupBase.sol:111-135) checks non-zero args, not-already-registered, code.length != 0,
adapter code, asset match, then AdapterFRV.validateRegistration — which is an external pure no-op
(AdapterFRV.sol:183-187). There is no lifecycle-state check anywhere on the registration path.
That makes a deployed-but-wrong vault at the pinned address strictly more likely than under the
undeployed framing, and gives three concrete new paths, all live for ≥3 days:
cancelVault(CEFFU_VAULT)— CriticalGuardian, zero delay. Transitions toFailed
(InstitutionalLoanVault.sol:185-199); the clone keeps its code andasset() == USDT. The VIP then
registers a dead vault, and the replacement Ceffu vault lands at nonce 1 — a different address
needing another VIP.openVault(CEFFU_VAULT)— same Safe. VerifiedmaxDeposit(BaseVault.sol:378-383) returns 0
unlessFundraising, so the description's "not exploitable while a vault is pre-Fundraising" is a
claim about vault state at execution time that Option A no longer guarantees.- Fundraising expiry — a self-inflicted deadlock. If
openVaultis called during the window,
_checkAndAdvanceState(InstitutionalLoanVault.sol:551-563) sends the vault toFailedonce
block.timestamp >= openEndTimewithtotalRaised < minBorrowCap, andupdateVaultState()is
permissionless. The Hub is the intended supplier but cannot deposit until this VIP executes. So
anyopenDurationshorter than the ~3-day proposal→execution window guarantees the vault fails
before the wiring that was supposed to fund it goes live.
Findings
-
vips/vip-665/bscmainnet.ts:67,194— the VIP registers a vault whose economic terms are set outside governance and never checked. Under Option A the config is fixed at deployment but nothing in the PR reads it:addResourceenforces only code-present +asset() == USDT+ apureno-opvalidateRegistration, and_validateVaultConfigplaces no bound betweenidealCollateralAmountandmaxBorrowCap, so a 5,000,000-USDT-cap vault collateralised at 1 wei passes.createVaultis held by NormalTimelock, FastTrackTimelock (6h) and the zero-delay CriticalGuardian 3-of-6 Safe. HIGH, confidence high. — Pin the full approved config inmeta.descriptionand add a pre-queue script that diffs the deployed vault'sconfig()/institutionalConfig()/riskConfig()against those literals; abort the proposal on any mismatch. -
vips/vip-665/bscmainnet.ts:194—sweep/setTreasuryextend the round-1 extraction primitive to the Hub holding 2,700,595 USDT, with FRV headroom raised to 1,350,297 by this same VIP.BaseVault.sweep:162-170moves the entire balance of any token tocontroller.treasury()with no supply-asset exclusion, andAdapterFRV._breakdown:227marks a Lock position fromrt.totalRaisedrather than the vault's balance, so the Hub's NAV does not move when the vault is drained. Both roles verified still held by the zero-delay CriticalGuardian Safe (sweepalso by FastTrack). HIGH, confidence high on mechanism, medium on exploitability (needs the Operator to have reallocated in first). — Narrow both roles to the Normal Timelock and add the supply-asset exclusion before any reallocate. -
vips/vip-665/bscmainnet.ts:194—closeVault(address)is a second, unreported extraction-equivalent primitive on the same authority set, missed by rounds 1 and 2. Verified:BaseVault.closeVault:118-123accepts any terminal state (Matured/Failed/Liquidated) with no check that suppliers have withdrawn (the docstring says governance "should only call this once all suppliers have withdrawn" — there is no such guard);maxWithdrawreturns 0 forClosed(BaseVault.sol:401-409); andAdapterFRV._breakdown:213deliberately values aClosedposition at 0. So between a vault maturing and the Operator pulling funds back out, one zero-delay 3-of-6 Safe call permanently strands the Hub's entire FRV position and writes it to zero. HIGH, confidence high on mechanism, medium on exploitability. — AddcloseVault(andcancelVault,partialPauseVault,completePauseVault) to the set narrowed to the Normal Timelock; round 2's escalation list was incomplete. -
vips/vip-665/bscmainnet.ts:146-149— the on-chain, voter-facing guardrail bullet says "Not exploitable while a vault is pre-Fundraising (maxDeposit == 0), so this VIP ships safely." Option A falsifies this and the delta did not update it: the vault now exists for the ≥3-day (up to 17-day) proposal→execution window, and oneopenVaultcall by the same zero-delay CriticalGuardian Safe puts it inFundraisingwithmaxDeposit == maxBorrowCap, so the VIP can execute onto a vault with live capacity. Voters are given a safety property the sequencing change removed. MEDIUM, confidence high. — Rewrite the bullet to state that the vault is live throughout voting and the timelock delay and that its lifecycle state is Safe-controlled, and add a pre-queue assertion thatruntime().state ∈ {WaitingForMargin, MarginDeposited}. -
vips/vip-665/bscmainnet.ts:194— deploy-first disarms the only on-chain control the Ceffu leg has.ResourceNotContractfires solely on an empty address; satisfying it before the vote means it can never fire, andaddResourceperforms no lifecycle check, so acancelVault'd (Failed) vault — or one that fell toFailedbecause itsopenDurationexpired during the ~3-day window while the Hub, its intended supplier, was still blocked on this very VIP — registers successfully and is permanently wired into the USDT Hub plus its inner withdraw queue. Removal needs another VIP.cancelVaultis zero-delay CriticalGuardian; the expiry path needs no privilege at all, sinceupdateVaultState()is permissionless. MEDIUM, confidence high. — Add a pre-queue check onruntime().state, and sequenceopenVaultto happen after this VIP executes, not before; state that ordering constraint in the description. -
simulations/vip-665/bscmainnet.ts:132-137,143-144— the only automated check on the Ceffu address is structurally incapable of observing the deployment the whole safety argument now rests on.forking(FORK_BLOCK)pins state at 116,780,000, sopredictVaultAddressandgetCode(CEFFU_VAULT) == "0x"return the same values forever. The delta demotes the assertion to "only a sanity check" in a comment but leaves it in place and adds no replacement, so after the vault is deployed CI stays green while asserting the vault's absence — andinstitutionNoncewill then be 1, making thepredictVaultAddressassertion false against any live block. MEDIUM, confidence high. — Replace both with a pre-queue live-provider script (getCode != 0x,isRegistered,AdapterFRV.asset == USDT,runtime().state, config diff); a pinned fork cannot express this check. -
vips/vip-665/bscmainnet.ts:204— the 30%→50% USDT raise is widened further by Option A: it now lifts headroom 810,178 → 1,350,297 USDT on a Hub that gains its first FRV destination in the same transaction, and that destination may hold live capacity at execution rather than being certainly pre-Fundraising.redeemFeeBps == 0re-verified on both Hubs, so the secondary brake is still off while the primary containment is widened 67%. The mitigation offered is an off-chain commitment that the Operator will hold off. MEDIUM, confidence high. — Set a non-zeroredeemFeeBpsin this VIP, or hold the USDT FRV percentage at 3000 bps and raise it in the VIP that authorises the first reallocation. -
simulations/vip-665/bscmainnet.ts:268-275— the Ceffu behavioural proof still runs againstMOCK_FRV_VAULT_BYTECODEetched at the address, and Option A makes this worse rather than better: the stub now stands in for a contract that will genuinely exist at proposal time, so the sim substitutes a fiction for a state it could test for real. It proves only that the Hub rejects a zero-capacity resource; none of the realInstitutionalLoanVaultlifecycle, valuation or settlement surface is exercised. MEDIUM, confidence high (raised from round-2 LOW). — BumpFORK_BLOCKpast the deployment and skip the etch whengetCode(CEFFU_VAULT) != "0x", so CI exercises the vault the VIP will actually register. -
vips/vip-665/bscmainnet.ts:146-149— the separation-of-duties argument that bounds thesweepfinding is weaker than round 2 recorded. Round 2 noted thatreallocateis Operator-only and CriticalGuardian does not hold it. Verified live, the Operator0x83f426…6a7Ais itself a 3-of-6 Safe sharing two of its six owners (0x358f758D…DFF2,0x209D6B1E…319f) with the CriticalGuardian Safe — so funding the vault and draining it requires four distinct signers, not six, and the two Safes are not independent principals. LOW, confidence high (defence-in-depth; it does not create the exposure, it shrinks the margin). — Record the actual overlap wherever the "Operator-only" separation is relied on, and prefer disjoint signer sets for the fund-in and fund-out roles. -
vips/vip-665/bscmainnet.ts:93,161— under Option A the permanent on-chain description will be factually false at vote time: "The Ceffu vault is not yet deployed" and "Ceffu vault (USDT, predicted, pending deployment)". The vault will be deployed before the proposal is submitted — that is the whole commitment. Telling voters the target is unbuilt actively suppresses the one check they should perform, which is to inspect the deployed vault's config. LOW, confidence high. — Rewrite both to state the vault is deployed at that address, and cite the deployment tx plus the pinned config so voters approve terms rather than an address.
Checked and clean this round
- No arbitrary code can still land at
CEFFU_VAULT. Re-verified: hitting the address requires the
clone implementation to be exactly0xe87A1eFC…5bdC; that contract's EIP-1967 implementation and
admin slots are both empty (40,061 bytes), so it is not upgradeable, andsetVaultImplementation
(NormalTimelock + FastTrack only) would move the prediction rather than change the code at the
pinned address. Round-2's conclusion holds under Option A. CEFFU_INSTITUTION0x8972E6F8…6262still has no code,institutionNonce == 0, no vault ever
created. It cannot create or open the vault itself.- No wildcard grants.
keccak256(abi.encodePacked(address(0), sig))holds no member among
NormalTimelock / FastTrack / Critical / Guardian / CriticalGuardian for any of the eight controller
functions checked. - The payload is byte-identical to round 2 — same six commands, same targets, same params. The
Normal Timelock still holds all six(contract, signature)pairs. - All four address literals round-trip EIP-55 cleanly (
CEFFU_VAULT,CASH_PLUS_VAULT,
CONTROLLER,CEFFU_INSTITUTION) — re-confirmed by the lead. AdapterFRV.asset→IFRVVault(resource).asset()confirmed, so a deployed vault whose
supplyAsset != USDTrevertsResourceAssetMismatchand the VIP fails closed on that axis.- No secrets, keys, URLs or injection surfaces introduced by the delta.
Consolidated asks
Correctness and security converged independently on the same two work items. Neither is optional under
Option A — the sequencing commitment is what makes the current simulation wrong.
A. Rebuild the simulation for the post-deployment state. As shipped it asserts the vault's
absence and etches a stub, which is the exact state Option A commits the proposal not to be in.
- Bump
FORK_BLOCK(:54) past the CeffucreateVaulttx, in the same "prepare VIP-665 for
proposal" chore commit the repo already uses for this (cf.cfb0113for VIP-656).forking()
takes a literal block number (src/vip-framework/index.ts:71). - Delete the prediction plumbing — the guard (
:132-137),CONTROLLER_ABI(:30), the controller
handle (:91). Replace withisRegistered(CEFFU_VAULT) == true(monotonic) and
institutionalConfig().institutionOperator == CEFFU_INSTITUTION— that is what the prediction was
actually trying to establish. - Delete the etch (
:139-147,MOCK_FRV_VAULT_BYTECODE:32-46). AssertgetCode != "0x"and
AdapterFRV.asset(CEFFU_VAULT) == usdtStack.asset— the exact calladdResourcemakes
(YieldGroupBase.sol:121), now against real vault code. - Pin the approved terms:
vault.config(),vault.riskConfig(),vault.state(). Structurally
impossible against the stub, and this is what converts "voters approved an address" into "voters
approved these terms". - Rewrite the two behavioural tests (
:259-275) to branch on the realstate()instead of
hard-codingResourceCapacityExceeded(PUSH, 0). - Extend the outer-queue guard (
:243-249) and the effective-cap equality assertion (:232-241) to
the USDT leg — both are currently U-only, and USDT is the Hub holding the funds.
B. Add a pre-proposal gate, and make it an artifact rather than a promise. A pinned fork cannot
express any of these; they need a live-provider script run immediately before queueing, with the
proposal aborted on any mismatch:
eth_getCode(CEFFU_VAULT) != 0xandcontroller.isRegistered(CEFFU_VAULT) == trueAdapterFRV.asset(CEFFU_VAULT) == USDTandinstitutionalConfig().institutionOperator == CEFFU_INSTITUTIONruntime().state ∈ {WaitingForMargin, MarginDeposited}- full
config()/riskConfig()diff against literals pinned inmeta.description - an explicit written commitment that
openVaultis not called until after this VIP executes - do not re-assert
predictVaultAddress— post-deployment it is guaranteed to return the wrong address
C. Rewrite the meta.description for the world it will ship into. Delete the CREATE2/prediction
paragraph (:93-102); keep the ResourceNotContract fail-closed sentence, which is correct and is
the one genuinely useful thing the delta added. Fix the pre-deployment tense at :93, :119, :161
and the banner at :25-29. Correct the pre-Fundraising guardrail bullet at :146-149. Add the
approved vault terms.
Escalation
The round-2 recommendation stands, and Option A strengthens the case for it rather than satisfying
it. Sequencing is an off-chain process control; it changes when the vault exists, not who can
act on it. The authority set is re-verified byte-for-byte unchanged, and this round adds three
primitives round 2 did not list — cancelVault, closeVault and the timing reachability of
openVault — all held by the same zero-delay 3-of-6 Safe, all now reachable during the governance
window precisely because the vault is deployed early. A full multi-researcher audit of the
InstitutionalVaultController trust boundary (vips → venus-liquidity-hub → fixed-rate-vaults)
should happen before the Ceffu leg ships, not before the first reallocation.
Note: the PR body above is still the stale 5-command / Ceffu-deferred / 19-of-19 text and the bot
cannot edit it; the on-chain meta.description is the artifact this review assesses.
…security note Round-3 review fixes (all compatible with Option A — single 6-command VIP, proposed only after the Ceffu vault is deployed): - Sim: reframe the predictVaultAddress assertion as a build-time nonce-0 preimage check, not a voter-reproducible safeguard. Verified on-chain that createVault increments institutionNonce, so post-deployment the predictor returns the institution's next clone, not CEFFU_VAULT (both already-deployed institutions sit at nonce 1 and predict a different, codeless address than their live vault). Assert nonce == 0 and document the caveat; make the etch test honest that the real vault cannot be exercised in a fork and is verified off-fork by the gate. - VIP description: rewrite the Ceffu paragraph for the post-deployment ship state (no predictVaultAddress-as-voter-check; verify the deployed vault's code/asset/registration instead), and extend the security disclosure to the CriticalGuardian 3-of-6 Safe's zero-delay openVault/closeVault/cancelVault authority alongside sweep/setTreasury (verified on-chain). - scripts/checkCeffuVaultReady.ts: deploy-first gate that fails unless the Ceffu vault holds code, is registered on the controller for the Ceffu institution, and reports asset() == USDT. Run before proposing. Sim 27/27 green @116780000; eslint + prettier clean; gate script exits 1 today (vault undeployed) as expected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| signature: "addResource(address,address)", | ||
| params: [CASH_PLUS_VAULT, ADAPTER_FRV], | ||
| }, | ||
| { |
There was a problem hiding this comment.
remove setInnerWithdrawQueue, we prefer doing deposit/withdraw manually
| params: [CEFFU_VAULT, ADAPTER_FRV], | ||
| }, | ||
| { | ||
| target: USDT_FRV_SOURCE, |
There was a problem hiding this comment.
remove this cmd, same reason as above
…gement) Fred's review: remove setInnerWithdrawQueue on both FRV sources — same rationale as the earlier setInnerDepositQueue removal (team prefers to wire queues manually). VIP now has 4 commands (addResource + raiseYieldGroupCap per hub). Sim updated: ACM role check trimmed, expectEvents drops InnerWithdrawQueueSet, post-VIP inner queue tests now assert both queues remain empty. 25/25 green; eslint+prettier clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renumber vip-665 -> vip-657 (on-chain proposalCount 656) and correct the description to state what is actually true at proposal time: the Ceffu USDT vault at 0x086fd797...0103 is not yet deployed, so the deploy-first gate has to pass before execution rather than before proposal. The VIP fails closed atomically if it is still missing then. Also drop inline backticks from meta.description (the venus.io governance UI strips their contents) and fix the "six commands" count to four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fred-venus
changed the title
3 participants
VIP-657 [BNB Chain] Wire Cash+ and Ceffu FRV vaults into the Liquidity Hub and raise FRV caps to 50%
Renumbered from the placeholder
vip-665tovip-657— on-chainproposalCount()is 656, so this is the next proposal id.safe-vote.jsonvotes on 657.What it does
BNB Chain mainnet,
REGULAR(Normal Timelock), 4 commands, one atomic transaction, no ACM grants (VIP-650/651 already gave the Normal Timelock the full Governance role set):0x30908edd…9143addResource(CASH_PLUS_VAULT 0x41179fc6…bcea, AdapterFRV 0x1FA0365b…5902)0x0e5AA174…88B0raiseYieldGroupCap(U FRV source, 5,000,000e18, 5000 bps)0x621eF38c…4bE7addResource(CEFFU_VAULT 0x086fd797…0103, AdapterFRV)0x18AfDACF…7226raiseYieldGroupCap(USDT FRV source, 5,000,000e18, 5000 bps)Absolute FRV cap is unchanged at 5,000,000; only the percentage-of-TVL dimension moves 3000 → 5000 bps. Registering a vault does not auto-route deposits: the FRV source stays out of each Hub's outer deposit queue, so FRV is filled only by the Operator's
reallocate. Inner deposit/withdraw queues are deliberately left empty.Corrects the previous version of this description, which claimed the Ceffu
addResourcehad been deferred to a follow-up VIP. It has not — command 3 above is in the proposal.0x086fd7972510dF9d9cFdc4efB8677fc72d290103holds no code today. The deploy-first gate confirms it:The address is fixed in advance regardless: it is the deterministic CREATE2 clone the InstitutionalVaultController (
0x6D9e91cB…6E85) mints for the Ceffu institution (0x8972E6F8…6262) at that institution's nonce 0 —salt = keccak256(institutionOperator, nonce)— andpredictVaultAddress(CEFFU_INSTITUTION)returns exactly that address today, withinstitutionNonce == 0.The safety property is atomic fail-closed, not trust in the address:
addResourcerevertsResourceNotContractagainst a codeless address, so if the Ceffu vault is not live at execution time the whole proposal reverts — the CASH+ registration and both cap raises included. Nothing lands by halves.Operational requirement: the fixed-rate-vaults workstream must deploy the vault at that address inside the propose → execute window. Run
scripts/checkCeffuVaultReady.tsbefore queueing and again immediately before executing. Once the vault is live, verify it by its on-chain code,asset() == USDT, andInstitutionalVaultController.isRegistered(…)— not by re-runningpredictVaultAddress, which returns the institution's next clone aftercreateVaultbumps the nonce.The Asseto CASH+ vault (
0x41179fc6…bcea) is live, withasset() == U.Description rewrite
meta.descriptionwas rewritten in this commit so the on-chain text matches reality:all six commands→all four commands.Verification
resources() == [], both FRV yield groups sit at(5,000,000e18, 3000 bps)registered and unpaused, and the Normal Timelock holds every role called.reallocatepush revertsResourceNotRegisteredpre-VIP andResourceCapacityExceededpost-VIP (both vaults are pre-Fundraising,maxDeposit == 0). The sim assertsgetCode(CEFFU_VAULT) == "0x"and thenhardhat_setCodes a minimal USDT-asset stand-in, so it proves the Hub-side wiring only — it is not evidence the real vault exists.propose()gas 1,116,868 — 6.65% of the 16,777,216 per-tx cap (EIP-7825 headroom fine).proposecalldata decoded withcast: 4 targets in order,values [0,0,0,0],proposalType 0(REGULAR), params byte-checked (0x422ca8b0a00a425000000= 5e24,0x1388= 5000).yarn prettierclean,yarn lint0 errors.scripts/checkCeffuVaultReady.tsalso renumbered — it still importedvips/vip-665/bscmainnetand crashed on module resolution.Standing security note (unchanged from review)
Do not
reallocatefunds into either vault yet. On the InstitutionalVaultController,sweep(address,address),setTreasury(address),openVault,closeVaultandcancelVaultare all held by the CriticalGuardian 3-of-6 Safe (0x7B1AE5Ea599bC56734624b95589e7E8E64C351c9) with no timelock delay, andBaseVault.sweepdoes not exclude the supply asset — that Safe could move vault capital while AdapterFRV still marks full principal. Not exploitable while the vaults are pre-Fundraising, so this VIP ships safely, but funds must wait until the controller's authority is audited, those roles are narrowed to the Normal Timelock, and thesweepsupply-asset guard is added. The secondary exit-fee brake is also still off (redeemFeeBps == 0); this VIP sets no redeem fee, as none was specified in the approved scope.