# Audit report > TOP PRIORITY: the treasury (FeeVault) must never be at risk. Prove or break: (a) ARB already in the vault when the strategy takes ownership (the bank) can never be split, spent, laid in the wall or sent anywhere by any path, by anyone, in any order of calls; (b) the strategy can only ever withdraw from the vault what pot/pendingWall count, and pot + pendingWall can never exceed new fees received since handover (plus ARB returned via _toVault); (c) no external caller, flash loan, reentrancy through the vault/hook/PoolManager, or manipulated price can make the strategy call vault.withdraw for more, or to an address other than the PoolManager during a settled swap/add; (d) the owner's exit (releaseVault) can never be blocked; (e) withdrawWall returns the wall to the vault as bank, never re-counted as a fee. Then: verify the round-1 fixes (see README): activation re-snapshot on handover, anchor-gated buildWall, buy limit min(spot, anchor, last hourly close) + slip, INSIDE_SLIP_TICKS, holding flag, burnWall/moveWall, lowerAth. | | | |---|---| | Repository | https://github.com/ch3rryhq/drawdown-burn-audit.git | | Commit | `ccb0a75c4774e638152a0633e8c75167cd03a068` | | Job | `f2721f67-9f52-48d9-b6f6-f38b6fe1d598` | | Judged | 2026-10-16 00:04 UTC | | Findings | 1 high · 2 low · 3 info | Four agents audited the code as it is at `ccb0a75`, each in one area (math, permissions, economics, control flow), and a judge reproduced, merged and ranked what they found, then read the code once more itself. Nothing in the repository was changed or deployed. ## Findings ### 1. High: Bank is snapshotted only at the first handover: after releaseVault and a re-handover, ARB in the vault is split and spent by buybacks and the wall `src/DrawdownBurnStrategy.sol:453` ``` if (!a.active) { ``` Invariant (a) says ARB already in the vault when the strategy takes ownership (the bank) may never be split, spent or laid in the wall, in any order of calls; (b) says pot + pendingWall never exceed fees received since handover. The round-1 fix 'activation re-snapshot on handover' is a one-shot flag: _observe() snapshots `accounted` and sets anchor.active the first time it finds vault.owner() == address(this), and nothing ever clears the flag. releaseVault() (line 751) hands the vault back without touching it. Three consequences, one root cause: (1) while released, every call that reaches _observe() (the hook's vault.onFeeReceived on each sell -- vault.strategy still points here -- the permissionless vault.onFeeReceived, buyback(), buildWall(), burnHeld()) keeps splitting totalReceived - accounted into _pot/_pendingWall although the strategy does not own that ARB; (2) at a re-handover (vault.transferOwnership(strategy), the documented way to repoint vault.strategy since the strategy has no setStrategy passthrough) no re-snapshot happens, so ARB that is in the vault at that moment -- bank by the README's definition -- is already counted; (3) stale _pot/_pendingWall from before the release survive even if the interim owner withdrew them, and _due() caps only at the vault balance (line 487-488), so the next buyback()/in-swap buyback pays the phantom pot out of the new bank through vault.withdraw, and _build() lays it in the wall. Merged from audit_economics 419bb2f0, audit_flow 3e7dee9a, audit_permissions 4b00986c and audit_math 311a2c58, all of whose proofs fail on this code for the stated reason. Rated high because the top-priority invariant is broken by the contract's own documented operational cycle (release, change strategy pointer, re-install) with no attacker needed, and the loss (ARB swapped for tokens and burned) is irreversible. Fix (keeps the design): in releaseVault() set `anchor.active = false;` before transferOwnership (a storage write cannot revert, so (d) holds), and in the activation branch of _observe() reset `_pot = 1; _pendingWall = 1;` alongside `accounted = vault.totalReceived(reserve);` so whatever the vault holds at any handover is bank and no stale counter survives. Verified: with exactly those three lines applied all five specialist proof tests pass; without the counter reset the stale-pot case (this finding's proof) still fails. **Reproduction** Vault holds 100 ARB; vault.setStrategy(strat); vault.transferOwnership(strat); one reading (buyback() or vault.onFeeReceived) activates: pot+pendingWall == 0. 50 ARB fees arrive and sync; next reading: pot+pendingWall == 50e18. Owner: strat.releaseVault(); vault.withdraw(ARB, owner, 150e18) -> vault balance 0. New 1000 ARB bank arrives, vault.sync (the hook does this on every swap), owner: vault.transferOwnership(strat); next reading. Expected: pot+pendingWall == 0 and vault balance >= bank. Actual: pot+pendingWall == 1050e18 > 1000e18 balance; the next buyback()/buildWall() withdraws the new bank to the PoolManager. Simpler variant (Proof_4b00986c, also reproduced): release, 50 ARB lands and vault.onFeeReceived runs, re-handover with 150 ARB in the vault -> pot == 50e18 at handover; buyback() in the next block takes the vault from 150e18 to 100e18. Ran: forge test --match-path test/scratch/Proof_311a2c58e8a0.t.sol -> FAIL 'bank at re-handover is spendable: 2050000000000000000000 > 1000000000000000000000'. **Proof**: a Foundry test that fails on this code and passes once it is fixed. ```solidity // SPDX-License-Identifier: MIT pragma solidity 0.8.26; import {Test} from "forge-std/Test.sol"; import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {PoolManager} from "v4-core/src/PoolManager.sol"; import {IPoolManager} from "v4-core/src/interfaces/IPoolManager.sol"; import {IHooks} from "v4-core/src/interfaces/IHooks.sol"; import {PoolKey} from "v4-core/src/types/PoolKey.sol"; import {Currency} from "v4-core/src/types/Currency.sol"; import {TickMath} from "v4-core/src/libraries/TickMath.sol"; import {FeeVault} from "src/FeeVault.sol"; import {Token} from "src/Token.sol"; import {DrawdownBurnStrategy} from "src/DrawdownBurnStrategy.sol"; import {IFeeStrategy} from "src/interfaces/IFeeStrategy.sol"; contract MockARB is ERC20 { constructor() ERC20("Arbitrum", "ARB") {} function mint(address to, uint256 amount) external { _mint(to, amount); } } /// @notice Invariant (a): ARB already in the vault when the strategy takes ownership is the bank and /// must never be split or spent. The strategy snapshots the bank only on its first activation. /// After releaseVault() and a later re-handover, everything the vault holds at that second /// handover is read as fresh fees and split into pot / pendingWall. contract RehandoverBankTest is Test { PoolManager pm; Token token; MockARB arb; FeeVault vault; DrawdownBurnStrategy strat; PoolKey key; address owner = address(0xA11CE); function setUp() public { pm = new PoolManager(address(this)); token = new Token("Tok", "TOK", 1_000_000_000e18, address(this)); // currency0 must be the token: mine a salt so the reserve sorts after it. bytes32 initHash = keccak256(type(MockARB).creationCode); uint256 salt; while (uint160(vm.computeCreate2Address(bytes32(salt), initHash, address(this))) <= uint160(address(token))) { salt++; } arb = new MockARB{salt: bytes32(salt)}(); require(address(token) < address(arb), "ordering"); key = PoolKey({ currency0: Currency.wrap(address(token)), currency1: Currency.wrap(address(arb)), fee: 3000, tickSpacing: 60, hooks: IHooks(address(0)) }); // price 1e-6 ARB per token: tick ~ -138162 pm.initialize(key, TickMath.getSqrtPriceAtTick(-138162)); vault = new FeeVault(owner); address[] memory ex = new address[](0); // athMcap_ = 0 -> ATH = current mcap -> tier 0% (everything to pendingWall), so no swap is attempted. strat = new DrawdownBurnStrategy(pm, vault, key, token, address(arb), 0, ex, 1e18, 60, owner); vm.startPrank(owner); vault.setStrategy(IFeeStrategy(address(strat))); vm.stopPrank(); } function _reading() internal { vm.roll(block.number + 1); strat.buyback(); // unlock -> unlockCallback -> _observe } function test_rehandover_bank_is_split() public { // First handover with a 100 ARB bank: correctly ignored. arb.mint(address(vault), 100e18); vm.prank(owner); vault.transferOwnership(address(strat)); _reading(); assertEq(strat.pot() + strat.pendingWall(), 0, "first bank must not be counted"); // 50 ARB of fees arrive and are split. arb.mint(address(vault), 50e18); vault.sync(address(arb)); _reading(); assertEq(strat.pot() + strat.pendingWall(), 50e18, "fees counted"); // The owner exits and takes everything out of the vault. vm.startPrank(owner); strat.releaseVault(); vault.withdraw(address(arb), owner, 150e18); vm.stopPrank(); assertEq(arb.balanceOf(address(vault)), 0); // Later the vault is funded with a new 1000 ARB bank and handed back to the same strategy. uint256 bank = 1000e18; arb.mint(address(vault), bank); vault.sync(address(arb)); // the hook does this on every swap; anyone may vm.prank(owner); vault.transferOwnership(address(strat)); _reading(); // Expected by invariant (a): nothing new arrived since this handover, the 1000 ARB is bank, // so pot + pendingWall == 0 and the vault covers bank + counters. // Actual: pot + pendingWall == 1050e18 (the 1000 bank was split, plus the stale 50 the // previous owner already withdrew), so the strategy may withdraw the bank via _payFromVault. uint256 counted = strat.pot() + strat.pendingWall(); assertLe(counted + bank, arb.balanceOf(address(vault)), "bank at re-handover is spendable"); assertEq(counted, 0, "re-handover bank must not be counted"); } } ``` ### 2. Low: lowerAth writes only athMcap: the stale lastMcap re-lifts the ATH at the next hour's first reading `src/DrawdownBurnStrategy.sol:704` ``` athMcap = mcap; ``` lowerAth is the documented way back from an ATH lifted by a pump held over an hour's close. It writes athMcap only. _anchorNow() (lines 427-430) applies the hourly close lazily: on the first reading of a new clock hour it sets ath = lastMcap if that is higher, and lastMcap is only rewritten by a reading. So whenever lastMcap > the lowered value and the next reading falls in a new hour (two cases, same cause: the pump's reading was the last one of hour H and the owner lowers in H+1 before any reading; or the ATH was already lifted by a close, the owner lowers it, and no further reading happens before the hour ends), that reading restores the pumped ATH over the owner's value. The tiers are then read against the pumped ATH and wallTarget() places a new wall under it. The owner can call lowerAth again, so the effect is bounded; hence low. Merged from audit_economics e51d2b53, audit_flow 854d956f and audit_math 78633c80. Fix: have lowerAth settle the pending close first (call _observe()) and clamp the running close, `if (lastMcap > mcap) lastMcap = mcap;`, so a stale reading cannot out-vote the owner. **Reproduction** Deploy at tick 0 with athMcap_ = 0 (ATH = lastMcap = m0). Pump to tick 150 with one swap; next L1 block call buyback() (a reading): lastMcap ~ 1.015*m0, athMcap still m0. Warp +1 hour with no reading; owner lowerAth(0.9*m0) -> athMcap == 0.9*m0. Next block buyback(). Expected: athMcap stays 0.9*m0. Actual: athMcap == ~1.015*m0 (assertGt passes in test_lowerAth_reliftedByStaleLastMcap, test/scratch/Judge.t.sol). Variant 2 in the same test: with the ATH now at the pumped value, owner lowerAth(0.9*m0), warp +1 hour, one reading -> athMcap back to the pumped value. ### 3. Low: Activation does not require vault.strategy == this: a vault handed over while its strategy pointer names another contract lets anyone distribute() the bank there `src/DrawdownBurnStrategy.sol:455` ``` if (vault.owner() != address(this)) return; ``` FeeVault.distribute is permissionless and transfers the vault's whole reserve balance (bank included) to vault.strategy, relying on the recipient's onFees to revert; DrawdownBurnStrategy.onFees does revert (DistributeUnsupported), but that only protects while vault.strategy == this strategy. The strategy exposes no passthrough to FeeVault.setStrategy, so once it owns the vault the pointer is frozen; if the handover (vault.transferOwnership(strategy)) is done before vault.setStrategy(strategy), or while the pointer still names the previous LiquidityStrategy or any contract whose onFees returns, any address can move the entire bank to that contract until the owner runs releaseVault + setStrategy + re-handover (which itself triggers the high finding above). The strategy activates and reports a bank without noticing. This is an ordering/trust gap, not a guard bypass, hence low. Reproduced from audit_permissions 729dc015. Fix: require vault.strategy() == address(this) in the activation branch of _observe() (so a mis-ordered handover is loud and nothing is counted until it is corrected), or give the strategy an owner-only setVaultStrategy passthrough. **Reproduction** vault.setStrategy(oldStrategy) where oldStrategy.onFees returns; mint 100 ARB to the vault; vault.transferOwnership(drawdownStrategy) without setStrategy; drawdownStrategy.buyback() -> anchor.active == true, bank 100. An unrelated EOA calls vault.distribute(ARB). Expected per (a): the bank never leaves the vault. Actual: vault balance 0, oldStrategy holds 100e18 (test_distributeBankToOldStrategy in test/scratch/Judge.t.sol). ### 4. Info: Any unlock holder can force an in-swap buyback (buyInside, INSIDE_SLIP_TICKS) through the permissionless vault.onFeeReceived `src/DrawdownBurnStrategy.sol:286` ``` if (!poolManager.isUnlocked()) return; ``` The deployed FeeVault.onFeeReceived has no caller check and the strategy's onFeeReceived only requires msg.sender == vault. A contract can call poolManager.unlock and, inside its own unlockCallback, vault.onFeeReceived(ARB): the strategy sees isUnlocked() true, no synced currency, no fee delivered and no hook involved, and runs buyInside with the pool in whatever state the caller arranged, once per L1 block. Checked against (b)/(c): no loss found -- the amount is capped by pot and maxPerBuy, the limit is min(spot, anchor, last hourly close) + 60 ticks so a pre-pump makes _buy return early and a pre-dump only buys cheaper, the withdraw goes to the PoolManager and _requireSettled checks the strategy's own deltas; the sandwich exposure is the same one the permissionless buyback() already accepts (with a wider 200-tick slip). Reported so the README's trigger list (trader's swap or buyback()) is complete and the gas/ordering assumptions in onFeeReceived are reviewed with a hostile unlock holder in mind. From audit_economics c225fbf7. **Reproduction** pot = 10e18, maxPerBuy = 1e18, strategy active. Attacker contract calls poolManager.unlock(""); its unlockCallback calls vault.onFeeReceived(ARB). Expected per README: in-swap buybacks run only inside a fee-paying swap or via buyback(). Actual: vault balance falls by exactly 1e18 (== maxPerBuy), pot 10e18 -> 9e18, the ARB goes to the PoolManager (test_forcedInsideBuy in test/scratch/Judge.t.sol). ### 5. Info: INSIDE_SLIP_TICKS is a fixed 60 ticks, not the pool's tickSpacing `src/DrawdownBurnStrategy.sol:116` ``` int24 public constant INSIDE_SLIP_TICKS = 60; ``` The round-1 fix describes the in-swap slip as one tick spacing so a buyback inside a swap crosses at most a tick or two of liquidity seeded above the price and stays under the vault's 400k gas cap. The constant is 60 while tickSpacing is an immutable read from the pool key, and the constructor does not check they agree. For a pool with spacing below 60 the in-swap buyback may cross up to 60/spacing initialised ticks, weakening the gas argument; above 60 it is merely conservative. Only matters if the strategy is reused on a pool whose spacing is not 60. From audit_economics fe922831. Fix: use `tickSpacing` at the call site in onFeeReceived, or require key.tickSpacing == INSIDE_SLIP_TICKS in the constructor. **Reproduction** Constructor with key.tickSpacing = 10 is accepted (wallWidth validation only needs a multiple of 10). _buy(amount, INSIDE_SLIP_TICKS) then sets limit = base + 60, six spacings above the base; an LP seeding ticks at base+10 ... base+60 makes the in-swap buyback cross six initialised ticks. Expected per README: at most one spacing. Actual: 60 ticks regardless of spacing (by inspection of lines 116, 245 and 498; the deployed pool's spacing is not stated in the repository). ### 6. Info: lowerAth accepts values that push the wall target below MIN_TICK, making buildWall and moveWall revert until the next hour close `src/DrawdownBurnStrategy.sol:703` ``` if (mcap >= athMcap || mcap == 0) revert InvalidAth(); ``` lowerAth only rejects 0 and values not below the current ATH. With a tiny ATH (e.g. 1 or 2) level = ath * 4000 / 10000 is 0, sqrt is 0 and wallTarget() clamps sqrtP to MIN_SQRT_PRICE, whose tick is MIN_TICK (-887272); _floor rounds that to -887280 and lower = upper - wallWidth = -887340, both below MIN_TICK. _build() then reverts in TickMath.getSqrtPriceAtTick (InvalidTick) while no wall is laid, so buildWall() reverts and moveWall() cannot complete (it takes the old wall down and then reverts in _build, so the whole call reverts). The ATH is restored to lastMcap at the next hour close, so the effect is bounded owner misuse. From audit_math 4c405005. Fix: in wallTarget clamp `upper` so that upper - wallWidth >= the spacing-aligned MIN_TICK, or reject mcap values in lowerAth below a sane floor. **Reproduction** Pool spacing 60, wallWidth 60, pendingWall 10e18, no wall. Owner lowerAth(1) succeeds. wallTarget() returns (lower -887340, upper -887280). Next block buildWall(). Expected: a wall at a clamped valid range or a clean no-op. Actual: revert InvalidTick(-887340) (test_lowerAthTiny_buildWallReverts in test/scratch/Judge.t.sol). --- Judge's submission `4b7afa063a0a29d7ee20763a3d65067ce5379a1101d81cf3ca2028401c060b7e`, accepted on the IdentityMD network. Acceptance means the report met the job's checks; it is not a guarantee that the code has no other defects.