# Security Review — Robinhood (chain 4663) CHOP Redeployment

**Target:** 19 on-chain contracts on **Robinhood chain ID 4663**, verified source pulled from Sourcify (`https://sourcify.dev/server/v2/contract/4663/<address>`) — creation + runtime `match` results per the client's own description, not cloned from a git repository. Addresses audited:

| Contract | Address |
|---|---|
| LOG_IMPL (WeightedIndex) | `0x2b9B8e30ca9B794554DC284602AACc0780068b8c` |
| SP_IMPL (StakingPoolToken) | `0xe2007d5F768a700C03F7A4352ed4F1a15e8B6Ee2` |
| REWARDS_IMPL (TokenRewards) | `0x722D5Eef00E6BE68fB803e2f2f081E4F830c9164` |
| LOG_BEACON | `0x9B48F82cf99B31d654DA0120033d4eCAe2580287` |
| SP_BEACON | `0x85eD97DB76B1a05C94FDf7d350d4732280fEADF1` |
| REWARDS_BEACON | `0x93B9b48cC3B09EDE111c2A61B91705466Bd25842` |
| LOG_FACTORY (WeightedIndexFactory) | `0xC1E5d8361fc6ACd0766c3eb0EdF4bD64127aEEd3` |
| INDEX_MANAGER | `0xA8314D71E188903e38EB0fe597b157C2fA7F6D35` |
| PROTOCOL_FEES | `0xa1F1Cd26C9a121Efac0aB483CEF094d7aE20Ad8b` |
| FEE_ROUTER (ProtocolFeeRouter) | `0xA74d5d4f1845de58FA9287d496439b1dAFa63f45` |
| REWARDS_WHITELIST | `0xfd29c5C62d77f68A0f4e9040f3854cf11499F4c7` |
| TWAP_UTILS (V3TwapUtilities) | `0x26FcD93a07470FeF15ba0C4C3EE1980e3006a574` |
| DEX_ADAPTER (UniswapDexAdapter) | `0x2ccd98F299FBb6C7FB4A50c0ED83151b9E4BAEce` |
| INDEX_UTILS | `0x267C5d225d8a3CeA62850a8DDBc0e94B3cE65B8c` |
| LOG_SEEDER (ChopLogSeeder) | `0x15Adba476bE85861c90f607e0AF9F3D408f09452` |
| V4_REWARD_ROUTER | `0x159131C852f965420BC18689A1cDf664AbEaD8ee` |
| WETH example LOG (ChopWeightedIndex) | `0xC9DE159066C6f4b799960344DdEa927c4fE493C5` |
| WETH example STAKING | `0x6E3F22c1695F7D5A5e9B7cB4E7c291Ea43E66B32` |
| WETH example REWARDS | `0x0429d2FF53788c07fd53abd14810d62b42c1A2e2` |

**Client-stated source commit:** `2b1287a46b45b1ba3430a6447ca87afcf36045e9`. **This could not be independently verified** — the job description names a commit but no repository URL, so there is nothing to `git ls-remote` against. This report is pinned to the **on-chain addresses above**, which is the actual artifact audited; treat the commit hash as an unverified client claim.

**Prior audit note:** the job description states this redeployment "replaces the previous instance in the audit inventory" and that "the original addresses remain on-chain and were not upgraded." Per our engagement policy, this audit was run fresh, independent of any prior report on the earlier addresses — every finding below comes from this job's own phase 0 map, phase 1 breadth agents, and phase 2 depth agents.

**Methodology:** four-phase pipeline (x-ray mechanical pre-scan → context/protocol-map → 8-agent ethskills breadth pass → 12-agent pashov-v4 depth hunt, blind to breadth findings → reconciliation). Scope: 17 first-party contracts (excluding interfaces and the two unmodified Uniswap vendor libraries `FullMath.sol`/`TickMath.sol`), 2,887 nSLOC. No test files were present in the Sourcify-verified source bundle (Sourcify mirrors `src`, not `test/`), so test coverage could not be assessed from this artifact. **Coverage note:** 2 of the 12 depth-hunt attack agents (first-principles, numerical-gap) died mid-run on an output-token ceiling and were not respawned (per methodology: a dead agent's slot is never retried within one pass — the remaining agents' coverage stands in for it). 10 of 12 depth agents completed; their findings showed heavy convergence with the 8 breadth agents, so coverage loss from the 2 dead agents is assessed as low.

**Severity counts:** 1 High · 8 Medium · 11 Low · 10 Informational

---

## Findings

### [H-1] `flashMint` lets a caller recycle their own buy/sell tax as repayment, bypassing up to ~85% of the sell tax on every `WeightedIndex`-based log

**Severity:** High

**File:** `contracts/DecentralizedIndex.sol:471-494` (`flashMint`), `:163-187` (`_update`, the fee block), `:233-240` (`_processBurnFee`)

**Description.** `flashMint` snapshots `_balance = balanceOf(address(this))` before minting and, after the recipient's callback, requires `balanceOf(address(this)) >= _balance + _amount + _fee` (`DecentralizedIndex.sol:480,484`). That check protects the pre-existing fee batch, but nothing protects fee revenue that **accrues during the callback itself**. `flashMint` carries the `lock` modifier but not `noSwapOrFee`, so every lTKN transfer the recipient makes inside the callback still pays the buy/sell tax, and that tax is credited straight into `address(this)` (`:175,178,182`) — exactly where the `FMA` repayment check reads its balance from.

```solidity
// DecentralizedIndex.sol:480-487
uint256 _balance = balanceOf(address(this));
_mint(_recipient, _amount);
IFlashLoanRecipient(_recipient).callback(_data);
require(balanceOf(address(this)) >= _balance + _amount + _fee, "FMA");
_totalSupply -= _fee;
_burn(address(this), _amount + _fee);
```

**Exploit scenario (canonical fee shape: `sell = 50` = 0.5%, `burn = 1500` = 15%, so `s' = s·(1-b) = 0.0425%` of a sale is actually burned and the rest is recoverable):**

1. Attacker holds `A` lTKN they want to sell. They call `flashMint(self, M, data)` for a modest `M` (fee `f = M/1000`).
2. Inside the callback, attacker transfers `S = (A - f)/(1 - s')` lTKN to `V2_POOL`. `_update` taxes the transfer at the sell rate, crediting `tax = S·s` to `address(this)`; `_processBurnFee` burns only `s·b` of that tax, leaving `s·(1-b)·S` sitting in the log's own balance.
3. Attacker calls `IUniswapV2Pair.swap` directly to take the paired-token proceeds of the (1-s)·S lTKN that actually reached the pool.
4. Attacker's remaining `A - S·(1-s') ≈ f` lTKN plus the recycled tax revenue closes the `FMA` check; `_burn(address(this), M+f)` destroys it.

**Net effect:** the attacker spends their own `A` lTKN but delivers `(1-s)/(1-s')·A ≈ 0.9950·A` into the pool instead of the honest `(1-s)·A = 0.995·A` — recovering essentially all of the burned-fee gap and paying only the flat 0.1% flash-mint fee instead of the intended sell tax. At the canonical parameters the effective sell tax drops from 0.5% to ≈0.0754% — **~85% of the sell tax is bypassed**, repeatable on every sale. This is pure fee-revenue theft from LP stakers (ERC20 accounting — `_totalSupply` vs. OZ-internal supply — stays fully consistent throughout; this is not a backing-drain bug).

**Reachability.** `ChopWeightedIndex.sol:18-20` overrides `flashMint` to `revert FlashMintDisabled()`, so canonical chain-4663 CHOP logs are **not** exploitable today. But `DecentralizedIndex.flashMint`/`WeightedIndex` is a fully live, shipped, factory-deployable implementation (`WeightedIndexFactory.setImplementationsAndBeacons` is parameterized with `weightedIndexImpl`), and the only thing preventing exploitation on any such log is a four-line override in one subclass — there is no defense at the base-contract layer, and a future beacon upgrade or a non-Chop log deployment reintroduces it at full severity.

**Fix.** Track fee accrual during an active flash mint and exclude it from the repayment credit (`require(balanceOf(address(this)) >= _balance + _amount + _fee + _feeAccruedDuringFlashMint)`), or block `V2_POOL` transfers while `isFlashMinting == 1`. Move the mitigation into `DecentralizedIndex` itself (an immutable `flashMintEnabled` flag) rather than relying on a subclass override, so it survives implementation swaps on the shared beacon.

---

### [M-1] A 1-wei donation + `sync()` permanently blocks a log's first liquidity add, through both `ChopLogSeeder.seed()` and the raw `addLiquidityV2` path

**Severity:** Medium

**File:** `contracts/ChopLogSeeder.sol:75-77, 188-194`; `contracts/DecentralizedIndex.sol:374-418` (`addLiquidityV2`)

**Description.** `_validateEmptyPair` requires `r0 == 0 && r1 == 0` on the log's V2 pair before any first liquidity is added. The preceding `skim(0xdead)` + `sync()` only clears an **unsynced** donation (`skim` moves `balance - reserve`). Once a reserve is recorded via `sync()`, `skim` moves nothing and the non-zero reserve is permanent: `swap` needs both reserves `> 0`, `burn` divides by a zero LP `totalSupply`, and a direct `mint` would make `totalSupply() != 0`, which `_validatePairWithoutLp` separately rejects.

```solidity
// ChopLogSeeder.sol:75-77
IUniswapV2Pair(pair).skim(address(0xdead));
IUniswapV2Pair(pair).sync();
_validateEmptyPair(pair, logAddress);
// :191
require(r0 == 0 && r1 == 0, "SYNCED_PAIR_DUST_USE_ATOMIC_CREATE");
```

**Exploit.** Any address: (1) `IERC20(paired).transfer(pair, 1)`; (2) `IUniswapV2Pair(pair).sync()`. Cost ≈1 wei + ~50k gas. Every subsequent `ChopLogSeeder.seed()` call for that log reverts at the quoted `require`, **and** `DecentralizedIndex.addLiquidityV2` independently reverts inside the Uniswap router's `UniswapV2Library.quote`, which requires both reserves `> 0`. This is unrecoverable for that specific log: the creator's only escape is a full redeploy (3 new beacon proxies + new pair) through the atomic `createAndSeed` path, which is immune because it creates the pair and seeds it in the same transaction (an attacker cannot `sync()` a pair for a log address that has no code yet). `IndexManager.updateMakePublic` is also permanently blocked for the griefed log (`require(log.totalSupply() > 0 && ... "SEED_FIRST"`).

**Convergence:** independently found and verified by 9 of the 18 breadth/depth agents in this audit (ethskills defi-amm, ethskills general, and 7 of the pashov depth-hunt specialists), including an extension proving it also blocks the raw `addLiquidityV2` path, not just the seeder. This is the single most-corroborated finding in the engagement.

**Fix.** Make `_validateEmptyPair` tolerant of a bounded dust reserve and fold it into the first-liquidity math, or deploy the log/pair pair with an unguessable `CREATE2` salt so a third party cannot pre-target it, or give the log's bootstrap a path that calls `pair.mint` directly (absorbing synced dust) instead of routing through the standard router.

---

### [M-2] `ChopWeightedIndex`'s fee-conversion swap floor leaves only ~1.7–2.7% headroom against a TWAP that can be up to ~30 minutes stale

**Severity:** Medium

**File:** `contracts/ChopWeightedIndex.sol:30-33, 43-73, 85-95`; `contracts/DecentralizedIndex.sol:190-208, 245-259`

**Description.** `_minimumFeeSwapOutput` floors the permissionless fee-conversion swap at `0.97 × amountIn × TWAP`, where `TWAP` is a V2-cumulative-price average spanning 600–1200 seconds and remains usable for a further 1200 seconds (so the price used can be up to ~30 minutes old). The 3% band never subtracts the swap's own cost: at the maximum batch size (`feeBatchAmount` caps it at 1% of the pool's lTKN reserve, `DecentralizedIndex.sol:203`), the achievable V2 output is only `0.98716×` the TWAP-implied amount (0.30% pool fee + price impact), leaving **~1.74%** of real headroom for price drift between the TWAP and spot (≈2.68% at the minimum batch size).

**Two independent, concretely numeric consequences, confirmed by five independent agents with matching thresholds:**
- **Liveness failure:** any ordinary price decline faster than ~1.7–2.7% within the TWAP's staleness window makes every `processFees()`/`processPreSwapFeesAndSwap()` call revert with `INSUFFICIENT_OUTPUT_AMOUNT` until the price recovers or the oracle catches up — LP-staker reward conversion simply stops.
- **Sandwich on a rising market:** because `updateFeeOracle()` is permissionless and `processFees()` carries only a 20-second cooldown, a searcher can, in one bundle, depress spot to the stale floor, call `processFees()` to buy the fee batch at the lagging price, then restore spot — extracting up to the full 3% gap repeatedly while fee inventory lasts.

**Fix.** Subtract the known pool fee + price impact from the expected amount before applying the slippage band (so the 3% is a true tolerance, not partially consumed by the swap's own mechanics); shrink `FEE_ORACLE_MAX_QUOTE_AGE` well below the window length; and/or add an owner-tunable fallback so the pipeline cannot stall indefinitely on one side and cannot be gamed on the other.

---

### [M-3] `WeightedIndex`'s inherited fee-conversion swap has **no** slippage floor at all (`_minimumFeeSwapOutput` returns `0`) — only `ChopWeightedIndex` overrides it

**Severity:** Medium

**File:** `contracts/DecentralizedIndex.sol:245-264`

```solidity
function _minimumFeeSwapOutput(uint256) internal view virtual returns (uint256) {
    return 0;
}
```

**Description.** The base `WeightedIndex` implementation — a fully shipped, factory-deployable contract, not a hypothetical — executes the entire accrued fee batch through `DEX_HANDLER.swapV2Single(..., 0, ...)` with zero slippage protection and no deadline worth the name (`block.timestamp`). `processFees()`/`processPreSwapFeesAndSwap()` are permissionless and re-callable every 20 seconds. Any attacker can, in one bundle: depress the log's price into `V2_POOL`, call `processFees()` (which also raises `feeBatchAmount`'s cap, since it's sized off `balanceOf(V2_POOL)`), and buy back — extracting essentially all of a fee batch's value. Confirmed independently by three agents (two breadth, one depth) with matching mechanics.

**Fix.** Do not default to `0`: make `_minimumFeeSwapOutput` abstract so every concrete index must supply a real floor, or derive a base-layer floor from the existing `V3_TWAP_UTILS` dependency and revert when no quote is available, mirroring `ChopWeightedIndex`'s own protection.

---

### [M-4] `TokenRewards.depositFromPairedLpToken` sends the `yieldBurn` fee slice to the admin sink instead of burning it, doubling the admin's effective take to up to 40%

**Severity:** Medium

**File:** `contracts/TokenRewards.sol:181-192` (bug) vs. `:264-288` (`_depositRewards`, the correct sibling path)

```solidity
// TokenRewards.sol:184-191 — LEAVE_AS_PAIRED_LP_TOKEN branch
(, uint256 _yieldBurnFee) = _getYieldFees();
uint256 _burnAmount = _yieldBurnFee == 0 ? 0 : (_amountTkn * _yieldBurnFee) / PROTOCOL_FEE_ROUTER.protocolFees().DEN();
_adminAmt += _burnAmount;          // <-- burn slice added to the admin fee, not burned
_amountTkn -= _burnAmount;
if (_adminAmt > 0) { _processAdminFee(_adminAmt); }   // both slices sent to the same sink
```

Compare the correct handling in the sibling function, two call-sites away:

```solidity
// TokenRewards.sol:276-281
if (_yieldBurnFee > 0) {
    uint256 _burnAmount = (_amountTotal * _yieldBurnFee) / PROTOCOL_FEE_ROUTER.protocolFees().DEN();
    if (_burnAmount > 0) { _burnRewards(rewardsToken, _burnAmount); }   // <-- actually burned (or swept on burn failure)
    _depositAmount -= _burnAmount;
}
```

**Description.** `ProtocolFees` caps `yieldAdmin` and `yieldBurn` at 20% each specifically so the admin's take is bounded at 20% and the burn slice benefits remaining holders/is destroyed. On the `LEAVE_AS_PAIRED_LP_TOKEN` deposit path, the burn slice is folded into `_adminAmt` and sent to the **same** admin sink as the admin fee — with both fees at their 20% cap, the admin collects up to 40% of that deposit instead of the intended 20%, with the other 20% simply never reaching either stakers or a burn.

**Fix.** Replace `_adminAmt += _burnAmount;` with a call to `_burnRewards(PAIRED_LP_TOKEN, _burnAmount);`, matching `_depositRewards`'s handling of the same fee parameter.

---

### [M-5] `IndexUtils.bond` / `addLPAndStakeSingleSided` grant a caller-chosen "index fund" contract standing approval over `IndexUtils`'s own token balances

**Severity:** Medium

**File:** `contracts/IndexUtils.sol:38-64` (`bond`), `:175-193` (`_addAndStakeSingleSided`)

**Description.** `bond(IDecentralizedIndex _indexFund, ...)` takes `_indexFund` directly from the caller with no check that it is a real, registered protocol log (no `IIndexManager.isRegistered` check anywhere in the function). It then asks the named contract what assets/amounts it needs (`getAllAssets()`, `getInitialAmount()`), pulls that much from the caller, and **approves the named contract to spend `IndexUtils`'s own balance** of each asset:

```solidity
// IndexUtils.sol:52-54
IERC20(_assets[_i].token).safeTransferFrom(_msgSender(), address(this), _amountNeeded);
IERC20(_assets[_i].token).safeIncreaseAllowance(address(_indexFund), _amountNeeded);
```

**Exploit.** An attacker supplies a contract they control as `_indexFund`, reporting a benign-looking asset list and a `getInitialAmount` that matches whatever they want approved. `bond()` pulls the attacker's own funds (refunded to them at the end — self-funded, no net cost) but leaves the approval on `IndexUtils`'s balance of that asset **standing after the call returns**, since nothing resets it. A duplicate-asset variant (`getAllAssets()` returning the same token twice — impossible for a real log, enforced at `WeightedIndex.sol`'s `require(!_isTokenInIndex[...])`, but not checked by `IndexUtils`) drains `IndexUtils`'s pre-existing dust balance of that token directly in the same call. `IndexUtils` is a shared, multi-user contract that deliberately retains 1 wei of every asset and paired token it has ever touched (by design, for gas reasons), so a standing unbounded allowance to an attacker-controlled contract is a live, if currently dust-sized, risk that grows with the contract's residual balances over time.

**Convergence:** independently found by two separate depth-hunt specialists with matching technical detail.

**Fix.** Require `IIndexManager(indexManager).isRegistered(address(_indexFund))` before use, and reset each allowance to zero after `_indexFund.bond(...)` returns.

---

### [M-6] `IndexManager.removeIndex` + `addIndex` let any `authorized` address reassign a log's `creator`, hijacking the first-bond bootstrap gate

**Severity:** Medium

**File:** `contracts/IndexManager.sol:134-154` (`addIndex`/`_addIndex`), `:156-167` (`removeIndex`)

**Description.** `removeIndex` (gated `onlyAuthorized`) deletes a registry entry with no check on who created it; `addIndex` (also `onlyAuthorized`) then re-registers the same log address with an arbitrary new `creator`, and — critically — `ChopWeightedIndex._beforeBond`'s bootstrap gate (`WeightedIndexFactory.canBootstrap`) reads that `creator` field directly to decide who may take the log's fee-free, price-setting first bond.

**Exploit.** Any `authorized` address `A` (a role the owner must grant broadly — e.g. to `ChopLogSeeder` itself, since `ChopLogSeeder.createAndSeed` needs `_authorizedOrOwner` to deploy on a user's behalf): calls `removeIndex(idx)` on a creator `C`'s deployed-but-unseeded log, then `addIndex(log, A, false, false, false)`. `A` is now the registered `creator`; `A` alone now passes `canBootstrap`'s `getIndex(log).creator == account` check and can take the first, fee-free bond at the fixed launch price, permanently locking out `C`. A variant that calls `removeIndex` without re-adding bricks the log's first bond for **everyone**, because `canBootstrap`'s downstream `getIndex` call reverts `UNKNOWN_INDEX` on an unregistered log rather than returning false.

**Convergence:** independently found by two separate agents (one breadth, one depth) with matching mechanics.

**Fix.** Record `creator` in a separate, write-once mapping that `_addIndex` can never overwrite for a log `isDeployedLog` already recognizes, and restrict `removeIndex` to the owner (not the broader `authorized` set) or forbid removing a log that has not yet had its creator's bootstrap consumed.

---

### [M-7] `IndexManager.addIndex` can set `makePublic = true` with none of the guards `updateMakePublic` enforces for the identical state change

**Severity:** Medium

**File:** `contracts/IndexManager.sol:134-154` (`addIndex`/`_addIndex`) vs. `:184-200` (`updateMakePublic`)

**Description.** `updateMakePublic` requires `verified`, a passing `validateIndex(_index)`, and a seeded pool (`log.totalSupply() > 0 && pool.totalSupply() > 0`) before setting `makePublic = true`. `_addIndex` writes the same field through `addIndex` with **none** of those checks — `validateIndex` only runs when the caller also sets `_verified = true`, which `addIndex` lets any `authorized` caller leave `false` while still setting `makePublic = true`.

**Exploit.** `authorized` caller → `addIndex(anyAddress, caller, false, false, true)`. `anyAddress` needs no code, no CHOP integration, no liquidity — it is registered as "public" with none of the canonical preflight that the guarded door enforces. Anything (a front-end, an integrator) that reads `makePublic` as "this log passed verification" is misled.

**Fix.** Either reject `addIndex(..., _makePublic=true)` outright (force callers through `updateMakePublic`), or factor `updateMakePublic`'s three checks into a shared internal function both entry points call.

---

### [M-8] A malicious/reentrant underlying token exploits the `noSwapOrFee` global flag to bypass buy/sell fees during `bond`/`debond`/`addLiquidityV2`/`removeLiquidityV2`

**Severity:** Medium

**File:** `contracts/DecentralizedIndex.sol:77-81` (`noSwapOrFee` modifier), `:163-187` (`_update`)

**Description.** `noSwapOrFee` sets a single storage flag `_swapAndFeeOn = 0` for the duration of the modified function's body, and `_update`'s fee computation is gated entirely on that flag (`if (_swapping == 0 && _swapAndFeeOn == 1)`). The flag is **global per-log state**, not scoped to a specific transfer. `WeightedIndex._bond` (reached from `bond`, which carries `noSwapOrFee`) calls `transferFrom` on every underlying asset the creator configured for the log — if that asset's `transferFrom` runs attacker-supplied code (a classic ERC777-style or malicious-token hook), that code executes **while `_swapAndFeeOn == 0`**, and any lTKN transfer it makes (e.g. straight to `V2_POOL`) pays zero buy/sell/transfer tax. The same window exists in `debond`, `addLiquidityV2`, and `removeLiquidityV2`.

**Reachability.** Requires the log's creator to have configured (or the index to otherwise accept) an underlying/paired asset with hostile transfer-hook behavior — a real but bounded precondition, in the same family the codebase already defends against for fee-on-transfer/rebasing tokens elsewhere. Impact falls on the log's fee revenue (hence its stakers), not only on the party who chose the asset.

**Fix.** Scope the fee exemption to the specific counterparties the router/adapter actually needs (e.g. check `msg.sender`/the actual caller context) rather than a global flag that silently exempts any code that gains control during the window.

---

### [L-1] `ProtocolFeeRouter.setProtocolFees` lets the owner swap in a `ProtocolFees` implementation that is not actually bound by the documented 20% yield-fee caps

**Severity:** Low (centralization — no unprivileged amplifier found; recorded because the documented invariant is not actually enforced where it is read)

**File:** `contracts/ProtocolFeeRouter.sol:18-26`; `contracts/TokenRewards.sol:377-391`

`_checkFees` validates only `code.length > 0 && fees.DEN() > 0` — it never checks the new contract's `yieldAdmin()`/`yieldBurn()` against any bound, and `TokenRewards` reads both values live against whatever `DEN()` the currently-installed contract reports. An owner (or compromised owner key) can install a `ProtocolFees` with `yieldAdmin = 99/100` and redirect up to 99% of every reward deposit to the admin sink, instantly, with no timelock. **Fix:** pin `DEN()` to the protocol constant and bound `yieldAdmin + yieldBurn` in `_checkFees` itself, and add a timelock to `setProtocolFees`.

### [L-2] `WeightedIndex._bond`'s first-deposit branch uses native multiplication where its own `convertToShares` preview uses 512-bit `FullMath`, overflowing (reverting) for high-supply 18-decimal underlyings

**Severity:** Low (DoS only — a smaller first-bond amount works around it; no fund loss)

**File:** `contracts/WeightedIndex.sol:150` vs. `:93`

```solidity
// :150 — reverts above ~1.46e30 raw units (~1.46e12 whole 18-decimal tokens)
_tokensMinted = (_amount * FixedPoint96.Q96 * 10 ** decimals()) / indexTokens[_tokenIdx].q1;
// :93 — the identical quantity, via 512-bit math, does not revert
_shares = FullMath.mulDiv(_assets, FixedPoint96.Q96 * 10 ** decimals(), indexTokens[0].q1);
```

Confirmed independently by two agents with matching overflow thresholds. **Fix:** use `FullMath.mulDiv` at line 150 (and the equivalent non-first-in computations at `:147,152` and `debond`'s `:185`), matching the view.

### [L-3] `convertToShares`/`convertToAssets` always apply the bond/debond fee even on the branches where `_bond`/`debond` actually waive it

**Severity:** Low (integration-correctness — misleads slippage-parameter calculations, no direct fund loss)

**File:** `contracts/WeightedIndex.sol:90-100, 117-130` vs. `_canWrapFeeFree` (`DecentralizedIndex.sol:286-292`) and `_isLastOut` (`:303-305`)

The view always subtracts `_fees.bond`/`_fees.debond`; the executing functions waive the fee entirely on first-bond and on `_isLastOut`/whitelisted-caller debonds. An integrator sizing `_amountMintMin`/`_amountOut` off the view is off by exactly the fee amount on exactly the transaction where price-setting matters most (the first bond). Confirmed independently by four agents. **Fix:** move the fee deduction inside the non-waived branch in both views.

### [L-4] Beacon upgrades are protocol-wide, instant, and un-timelocked for every log/staking-pool/rewards contract

**Severity:** Low (documented centralization risk, out of source-code scope to fully bound — beacon ownership itself was not part of the audited application-layer source)

**File:** `contracts/WeightedIndexFactory.sol:18-25, 46-83`

Each of `weightedIndexBeacon`/`stakingPoolTokenBeacon`/`tokenRewardsBeacon` governs *every* deployed instance with no per-instance override, no timelock, and (per `DecentralizedIndex.setup()`'s `renounceOwnership()` on the per-log `StakingPoolToken`) no local owner left after setup to object. **Fix:** put each beacon behind a timelocked multisig and have `IndexManager.validateIndex` assert the implementation codehash matches an approved value.

### [L-5] The `ChopWeightedIndex` beacon swap leaves the new fee-oracle storage zeroed with no reinitializer, bricking `processFees()` on every existing log until manually primed

**Severity:** Low (one-time migration defect, recoverable by calling `updateFeeOracle()` twice ~10 minutes apart per log)

**File:** `contracts/ChopWeightedIndex.sol:37-40, 85-95`

A beacon upgrade from `WeightedIndex` to `ChopWeightedIndex` gives every existing log the new `_minimumFeeSwapOutput` override, but slots 80-83 (`_feePriceCumulativeLast` etc.) are freshly zeroed with no `reinitializer`. Until someone calls `updateFeeOracle()` twice, ~600-1200s apart, `processFees()`/`processPreSwapFeesAndSwap()` revert `FeeOracleNotReady` for that log, stranding accrued lTKN fees. **Fix:** ship a `reinitializer(2)` that seeds the first observation as part of the upgrade, and/or make `_minimumFeeSwapOutput` degrade to `0`/skip rather than revert when unprimed.

### [L-6] `WeightedIndex` declares no storage gap of its own between `DecentralizedIndex.__gap` and `ChopWeightedIndex`'s new state

**Severity:** Low (latent — becomes a critical storage-corruption issue on the *next* `WeightedIndex` revision, not reachable today)

**File:** `contracts/WeightedIndex.sol` (no state, no gap); `contracts/DecentralizedIndex.sol:63`; `contracts/ChopWeightedIndex.sol:36-40`

Any future addition of storage to `WeightedIndex` itself (the natural place to add a shared feature) will alias `ChopWeightedIndex`'s existing slots 80+. **Fix:** add `uint256[50] private __gap;` to `WeightedIndex` now, before any mainnet log exists on this beacon lineage, and add CI storage-layout diffing for every future upgrade pair.

### [L-7] `flashMint` is disabled only by a subclass override; the base contract has no defense-in-depth, and `convertToAssetsPreFlashMint` is permanently dead on every canonical log as a side effect

**Severity:** Low

**File:** `contracts/ChopWeightedIndex.sol:18-20`; `contracts/WeightedIndex.sol:112-115`

Covered mechanically by [H-1]'s reachability discussion; recorded separately because `convertToAssetsPreFlashMint` — part of the public `IDecentralizedIndex` ABI — now unconditionally reverts `NO_FLASH_MINT` for every canonical log, which can surprise an integrator that calls it defensively. **Fix:** move the disable flag into `DecentralizedIndex` itself; have the view degrade to `convertToAssets` instead of reverting when flash-minting is disabled.

### [L-8] `V4RewardRouter.keeper` is an unrecoverable single point of failure for all non-CHOP-paired reward distribution, and revoking a router via `IndexManager.setRewardRouterAllowed` has no effect on live `TokenRewards`

**Severity:** Low

**File:** `contracts/TokenRewards.sol:69, 106-113, 170-231`; `contracts/IndexManager.sol:54-58`

Once `rewardRouter != address(0)` for a log, the only conversion path for its queued paired-token rewards is `processPairedRewards`, gated to `IV4RewardRouter(rewardRouter).keeper()` — with no setter, no timeout, and no permissionless fallback. `approvedRewardRouters` is checked only at log-creation/validation time, never at spend time, so revoking a compromised router's approval does nothing to stop `TokenRewards` from continuing to use it. **Fix:** add a keeper-staleness fallback (anyone may convert after N days with their own `minOut`) and re-check `approvedRewardRouters` at spend time in `processPairedRewards`.

### [L-9] `RewardsWhitelist.setPaused` is checked only on the claim side, so pausing a reward token freezes withdrawals of already-earned rewards while deposits/accrual continue silently

**Severity:** Low

**File:** `contracts/TokenRewards.sol:342-344` (checked) vs. `_depositRewards`/`_isValidRewardsToken` (not checked)

Because `rewardsToken == CHOP` for every canonical log, pausing CHOP freezes claims protocol-wide in one transaction while admin-fee skims and `_rewardsPerShare` accrual continue unaffected; `claimReward`'s per-token `try/catch` additionally swallows the paused case silently (no distinguishing event). **Fix:** check `paused` on the deposit side instead of (or in addition to) the claim side, and bound any pause with a maximum duration.

### [L-10] `Zapper`'s V2 and Curve swap branches apply no fallback minimum when the caller passes `0`, unlike the V3 branch, which derives a TWAP-based floor

**Severity:** Low

**File:** `contracts/Zapper.sol:88-92, 205-224` (unprotected) vs. `:159-172` (`_swapV3Single`, protected)

On chain 4663, `zapMap` is empty unless the owner populates it (the constructor only seeds mainnet addresses), so every `_zap` call falls through to the unprotected V2 branch today. `IndexUtils.addLPAndStake` forwards the caller's `_amountPairedLpTokenMin` unchanged, and a caller/integrator that passes `0` (a plausible default, since the function's own floor is only `require(_amountOut > 0)`) gets a fully sandwichable swap. **Fix:** mirror `_swapV3Single`'s TWAP-derived fallback in `_swapV2`/`_swapCurve`, or reject a zero minimum outright.

### [L-11] `DecentralizedIndex.flash`'s DAI fee can be routed to `address(0)` with no guard, unlike every other admin-fee sink in the codebase

**Severity:** Low

**File:** `contracts/DecentralizedIndex.sol:450-453` vs. `contracts/TokenRewards.sol:256-257, 367-368, 426-427` (all have `require(sink != address(0), "ZERO_SINK")`)

If `V3TwapUtilities`'s owner ever renounces ownership (a plain one-step `Ownable`, no override), every subsequent `flash()` either silently burns 10 DAI per call to the zero address (if DAI doesn't block it) or bricks `flash()` entirely (if it does) — with no admin recovery path, since `DAI`/`FLASH_FEE_AMOUNT_DAI`/`V3_TWAP_UTILS` have no setters. **Fix:** add the same `ZERO_SINK` guard used everywhere else in the codebase.

---

### Informational

- **[I-1]** `TokenRewards.getUnpaid` omits the `lockedRewards` subtraction that `_claimRewardToken` applies, so the view over-reports claimable balances for wallets with immature stake tranches. No on-chain caller of the view was found in scope. *(5 independent agents.)*
- **[I-2]** `TokenRewards._burnRewards` uses a bare `try { ICHOP(_token).burn(_burnAmount) }` with no post-call balance check; a phantom/no-op `burn(uint256)` (e.g. via a proxy whose implementation lacks it) silently strands the tokens rather than falling through to the sink transfer.
- **[I-3]** `DecentralizedIndex.addLiquidityV2`'s `safeIncreaseAllowance(DEX_HANDLER, 0)` is a no-op, not a reset (OZ `safeIncreaseAllowance(x,0)` increases by zero); harmless only because the shipped adapter happens to consume the full allowance it's given.
- **[I-4]** `StakingPoolToken`/`TokenRewards` reward-token loops (`_setShares`/`_resetExcluded`/`_lockImmatureRewards`) scale with `MAX_REWARD_TOKENS` (13) × 11 lock buckets with no gas cap on `stake`/`unstake`/transfer; `claimReward`'s hardcoded `{gas: 750_000}` per-token stipend was estimated by three independent agents at 70-95% of its own ceiling in the worst case (11 populated lock buckets). Not currently reachable to a hard revert at the canonical single-reward-token configuration, but owner-controlled growth of the reward-token list narrows the margin.
- **[I-5]** `WeightedIndex.sol:68-71`'s weight-normalization math and `TokenRewards.sol:204-209`'s V3-TWAP reward-quote math both use native (non-`FullMath`) multiplication on oracle-derived or weight-derived values with theoretical overflow/divide-by-zero bounds; unreachable at the canonical single-asset, realistic-decimals configuration enforced elsewhere in the codebase, but live surface for any future non-canonical configuration.
- **[I-6]** `V2CumulativePrice.sol:37-38`'s `_priceX112 * _elapsed` sits outside its surrounding `unchecked` block despite a comment claiming it always fits; the true bound is `2^224 × 2^32 = 2^256`, tight but not strictly within range. Requires reserve ratios and elapsed times far outside any realistic pool; not reachable in practice.
- **[I-7]** `_isLastOut`'s `>= 99%` threshold (rather than `== 100%`) lets a near-total holder debond fee-free while a residual minority remains, and the waiver composes (repeatable down to a 1-unit residual), shifting a small amount of the debond fee's benefit away from genuinely-remaining holders.
- **[I-8]** `DecentralizedIndex._update`'s `require(!_blacklist[_to], "BK")` check has no corresponding writer anywhere in scope (the storage slot is kept only for proxy layout compatibility per its own comment) — dead code that makes the token look blacklist-capable to integrators when it is not.
- **[I-9]** Rebasing or share-accounted underlying assets (not explicitly excluded by `IndexManager._validateCreation`, which only checks `code.length > 0`) desync the cached `_totalAssets[token]` counter from the real balance, in both directions — a negative rebase creates a last-debonder-eats-the-shortfall shape, a positive rebase permanently strands the surplus yield. Requires the log creator to choose (or a token to later become) a non-standard-balance asset.
- **[I-10]** `IndexManager.validateIndex` omits the `stakeUserRestriction`/`stakingToken` cross-checks that `ChopLogSeeder._validate` separately applies to the same canonicality question — the two validators of "is this log canonical" check different things, and a log reaching the registry by a route other than `deployNewIndex` is not fully covered by either.

---

## Coverage Gate

- **Entrypoints:** 51 external/public state-changing functions enumerated in the grep-verified access-control inventory (`x-ray/entry-points.md`), 51 addressed across the findings above or the negative-result log below.
- **Threat-catalog rows:** every actor×entrypoint×asset row in `protocol-map.md` §4 is answered by a finding above or an explicit "examined, no issue" note in this gate.
- **Invariants:** 6 single-contract + 4 cross-contract + 2 economic inferred rows from the x-ray pre-scan, all answered:
  - **I-1 (conservation), I-3 (fee bounds at write-site, excepting L-1's read-side bypass), I-4 (bond/debond snapshot ordering), E-2 (donation-immune first-bond pricing):** **hold**, confirmed by multiple independent agents, no gap found.
  - **I-2 (flash-mint / custom-`_totalSupply` boundary):** **holds** for the specific mechanism it names (the shared `lock` does block every share-math reentrancy path) — but see **H-1**, a *different* mechanism (fee-revenue recycling through the unguarded `_update` path) achieves a related exploit the invariant did not anticipate.
  - **I-5/I-6 (reward-lock state machine, fee-oracle readiness state machine):** hold structurally; **see M-2** for the oracle's economic insufficiency, which is a distinct issue from the state machine's correctness.
  - **X-1 (trackingToken ABI assumption):** holds for the canonical factory-paired deployment; silent-degrade risk only on a hypothetical future non-canonical pairing (recorded as a lead by multiple depth agents, no live path found).
  - **X-2 (stake/reward-lock bucket collision — the pre-scan's highest-priority flagged gap):** **REFUTED.** Independently falsified by nine separate agents (both breadth and depth) via exact modular-arithmetic proof: with `unlockAt ∈ [t+600, t+659]` and 11 buckets, any two tranches sharing a bucket are forced ≥660 seconds apart, so the "colliding" predecessor has always already matured before the new write lands. `LOCK_COLLISION`/`REWARD_LOCK_COLLISION` cannot fire. This is the correct resolution of the lead the pre-scan raised — recorded here as **holds**, not as a finding.
  - **X-3 (keeper trust boundary):** holds as a documented trust assumption; see **L-8** for its single-point-of-failure consequence.
  - **X-4 (pre-lock non-canonical deployment window):** holds as intentional bootstrap sequencing; see **M-6/M-7** for the `authorized`-role gaps that interact with it.
- **Holes closed this pass:** 0 (every entrypoint/threat-row/invariant-row above was examined by at least one phase; none were left completely unaddressed by both phases simultaneously).

## Invariant Verification

| ID | Property | On-chain | Verdict |
|---|---|---|---|
| I-1 | Custom `_totalSupply` conservation across bond/debond/burn/flashMint | Yes | Holds |
| I-2 | Flash-mint window cannot be used to exploit undiluted `_totalSupply` via share-math reentrancy | Yes (conditioned on `lock`) | Holds for its named mechanism; **H-1** is a different, unblocked mechanism |
| I-3 | Fee/weight percentage fields bounded at write sites | Yes (write-side) | Holds at write sites; **L-1** shows a read-side bypass via contract substitution |
| I-4 | Bond/debond snapshot-before-mutate ordering | Yes | Holds |
| I-5 | Stake/reward lock bucket one-shot-per-bucket accumulation | Yes | Holds |
| I-6 | Fee-oracle readiness state machine internal correctness | Yes | Holds structurally; **M-2** is an economic-sufficiency gap, not a state-machine break |
| X-1 | `TokenRewards`↔`StakingPoolToken` ABI pairing | No (by design, degrades silently) | Holds for canonical deployment; no live non-canonical path found |
| X-2 | Stake/reward lock bucket collision DoS | No (was flagged as a gap) | **Holds — refuted as a finding by 9 independent agents** |
| X-3 | Keeper fully trusted for V4 conversion pricing | No (by design) | Holds as documented trust boundary; see L-8 |
| X-4 | Pre-canonical-lock deployment window | Yes (documented sequencing) | Holds; interacts with M-6/M-7 |
| E-1 | Admin yield extraction bounded at ≤40% combined | Yes (at write sites) | Holds at write sites; see L-1 for the read-side gap |
| E-2 | First-bond price immune to pool-donation manipulation | Yes | Holds |

---

*This review was performed using an automated multi-agent AI pipeline (x-ray mechanical pre-scan, context-building, 8-agent checklist breadth pass, 12-agent adversarial depth hunt — 10 of 12 completed, 2 died to an output-token ceiling and were not respawned per methodology). AI analysis can never verify the complete absence of vulnerabilities and no guarantee of security is given. A team security review, a bug bounty program, and on-chain monitoring are strongly recommended before and after any mainnet deployment of this redeployment.*