Done. Committed as a7a69e0.

What landed

foundry.toml        solc 0.8.26 pin, cancun, bytecode_hash=none, explicit remappings
src/Placeholder.sol imports v4-core's Hooks library
test/Placeholder.t.sol
lib/                6 dependencies, 580 files, ordinary committed blobs
README.md
.gitignore

forge build and forge test both pass (2 tests). forge fmt --check is clean.

Vendoring

No submodules, no gitlinks, no manifest — verified with git ls-tree -r HEAD finding zero mode-160000 entries. Pins are the set the Uniswap repos pin each other at, so it's a combination upstream tests together:

lib	pin
forge-std	77041d2 (v1.9.7)
v4-core	59d3ecf (v1.0.2)
v4-periphery	3779387
openzeppelin-contracts	dbb6104 (v5.0.2)
permit2	cc56ad0
solmate	4b47a19

Two judgment calls worth flagging:

v4-periphery is pinned behind main deliberately. Commit 5da22e6 moved BaseHook and HookMiner out of the repo entirely; main has no src/utils/. I pinned 3779387, the last commit that still ships both, since the next task writes a hook. It pins the same v4-core and permit2 as main does, so nothing is lost.

Each library is vendored source-only — Solidity plus licence and README. Audit PDFs, docs, JS test tooling and broadcast artifacts are ~35MB of the ~41MB upstream and compile to nothing; dropping them puts the bundle at 701K. v4-core's and v4-periphery's test/ dirs are kept, because hook tests are built on v4-core/test/utils/Deployers.sol. Because that removes the nested remappings.txt files, foundry.toml sets auto_detect_remappings = false and lists all nine prefixes explicitly.

Verification beyond the required commands

The delivered tree was exported with git archive into a non-git directory and built there, which is the verifier's situation. In that clean room I also compiled and ran a full hook: a BaseHook subclass deployed to a HookMiner-mined CREATE2 address, pool initialized, liquidity added, real swap executed, beforeSwap callback observed firing. That exercises v4-core src+test, v4-periphery src, solmate, OZ and forge-std together, and confirms PoolManager deploys under these optimizer settings. A second scratch file forced every remapping prefix to resolve. Both proofs lived in test/scratch/ and /tmp and are not in the commit.

solc 0.8.26 was already cached in this image (Feb 2025), not downloaded by me — so the offline re-run should find it. That's the one external thing the build wants, and the README says so.

One limitation I did not paper over: via_ir is off. Hooks and the v4-core harness are fine, but v4-periphery's PositionManager and PositionDescriptor only compile with it on. I tried per-file compilation_restrictions mirroring upstream's profiles and backed it out — a single file importing both contracts fails with "incompatible settings restrictions," which is a worse failure than the one it fixes. It's documented in the README instead, since a later task can't edit foundry.toml.

Done. src/FivePercentFeeHook.sol is the hook.

How it works. Permissions are beforeSwap + beforeSwapReturnDelta, so the address must carry BEFORE_SWAP_FLAG | BEFORE_SWAP_RETURNS_DELTA_FLAG in its low bits; BaseHook's constructor enforces that. On every swap it computes feeAmount = |amountSpecified| * 5 / 100, takes that from the PoolManager in the swap's specified currency, and returns it as a positive deltaSpecified — which nets the hook's currency delta back to zero and shifts the cost onto the swapper. That one expression covers both directions and both swap types:

• exact input — specified currency is the input; the pool swaps 95%, the hook keeps 5%, the swapper still pays the full input.
• exact output — specified currency is the output; the pool is driven for 105% of the request, the hook keeps the extra, the swapper receives exactly what it asked for.

5% is FEE_NUMERATOR / FEE_DENOMINATOR, both constant — no setter, no constructor arg, no per-pool storage. owner is msg.sender at construction (the deployer) and is the only address that can call withdraw, which is bounded by the per-currency accruedFees ledger so stray transfers aren't withdrawable through it. There's a receive() because a native-currency take arrives as a bare value transfer.

Verification. I wrote 14 tests in test/scratch/ (deleted, as intended) covering both directions × exact-in/exact-out, accumulation across swaps, dust rounding to zero, owner/non-owner/over-accrual withdrawal, a native-ETH pool, and a fuzz over the specified amount. All passed. Addresses were mined with HookMiner.find(address(this), flags, type(FivePercentFeeHook).creationCode, abi.encode(manager)). A clean forge build succeeds with no warnings, forge test passes, and forge fmt --check src/ is clean.

Two behaviours the tests surfaced are documented in the _beforeSwap natspec rather than papered over: the 5% is on the specified amount, so a partially-filled swap still pays 5% of what it asked for; and because the take precedes settlement, a swap whose fee exceeds the PoolManager's balance of that currency reverts — that balance is every v4 pool's reserves in it, so it only binds on swaps far larger than the liquidity they trade against.

One thing I could not do. The task says to replace the placeholder, but test/Placeholder.t.sol imports src/Placeholder.sol, and test/ is outside my writable paths. I confirmed by deleting it: forge build fails with Source "src/Placeholder.sol" not found from test/Placeholder.t.sol:7. Since "forge build succeeds" is an acceptance criterion, I left src/Placeholder.sol untouched and added the hook alongside it. Deleting src/Placeholder.sol and test/Placeholder.t.sol together is a one-line follow-up for whoever owns test/.

Done. 50 tests, all passing, forge build clean. Nothing outside test/** was touched (plus the requested .imd-findings.json, which the repo's .git/info/exclude already anticipates alongside test/scratch/).

Test suites

test/FivePercentFeeHookBase.sol — shared wiring. The hook is deployed through a separate HookDeployer contract, so owner is neither the test contract nor an EOA prank; the test contract is itself a non-owner.

FivePercentFeeHookSwapTest (13) — the fee is proved taken, not just plausible: a second pool over the same currency pair with identical liquidity and no hook is the control. An exact-input swap of n through the hook pool must produce exactly the output the hookless pool gives for n − 5%; an exact-output swap for n must cost exactly what the hookless pool charges for n + 5%. Both directions, both exactness modes, plus PoolManager/trader/hook balance deltas, event emission, permission bits, and beforeSwap rejecting a non-PoolManager caller.

FivePercentFeeHookWithdrawTest (13) — deployer withdraws (full, partial, zero, then keeps accruing); stranger / the test contract / the PoolManager / any fuzzed non-owner rejected; over-withdraw, double-withdraw and never-accrued currency all revert on the arithmetic; stray tokens transferred in are unreachable; an invariant fuzz that the ledger stays fully backed across mixed swaps and withdrawals.

FivePercentFeeHookNativeTest (10) — the native path end to end, including withdrawal to an address that reverts on receipt.

FivePercentFeeHookRoundingTest (6) and ...ThinPoolTest (2) — the sub-20-unit rounding threshold in both directions and both modes, and the take-before-settle revert.

Three defects reported rather than blessed

The four test_defect_* tests record current behaviour with comments saying so and pointing at the findings file; they do not assert it is correct.

1. medium — the fee comes off params.amountSpecified, not the executed swap. With sqrtPriceLimitX96 = spot − 1, an exact-input swap of 1e18 has the trader pay 50000000000000002 and receive zero; the exact-output version gives a delta of −5e16 in the currency they were trying to buy. Under an ordinary ~1% slippage guard the effective rate on the executed trade is ~992,000%. Reachability is stated honestly: v4-periphery's router passes non-binding limits, so this bites callers who bind sqrtPriceLimitX96 themselves.
2. low — floor rounding: 10 swaps of 19 units pay 0, the same 190 in one swap pays 9.
3. low — take before settle makes a thin-pool swap revert that plain v4 fills. I verified the counterfactual: the identical hookless pool fills (−1e18 in, 995 out) where the hook pool reverts.

Completed the adversarial review and wrote 3 ranked, reproducible findings to .imd-findings.json.

Findings: 2 high severity, 1 medium severity. No repository files were modified besides the required findings file.

All three reported defects reproduce. forge build and forge test pass — 43 tests, 0 failures.

What was in the tree

The working tree at HEAD contains only src/FivePercentFeeHook.sol and the scaffold's Placeholder test — the reopened test files were not present. So the suite is written fresh rather than edited, and none of the three tests the reviewer named (test_defect_splittingIntoSubThresholdSwapsAvoidsTheFeeEntirely, the partial-fill tests, test_swapLargerThanTheManagerCanCoverReverts) exist in what I'm submitting.

Tests (test/**)

• test/utils/FeeHookFixture.sol — deploys the hook from a HookDeployer contract, so "the deployer" is an identity distinct from the test contract and non-owner withdrawal tests actually mean something. Sets up a deep hooked pool plus an identical hookless pool at the same fee tier as a reference.
• test/FivePercentFeeHook.fees.t.sol (18) — exact input and exact output in both directions, each asserting three things together: the accruedFees book, the hook's real token balance, and how much of the swap reached the pool (read off the hookless mirror, so no hardcoded output amounts). Plus cumulative accrual, two pools sharing one pot, the LP fee tier not being overridden, liquidity/donations charging nothing, beforeSwap rejecting non-PoolManager callers, zero-amount swaps, a fuzz over amounts and directions, and the rounding-to-zero boundary (19 → 0, 20 → 1).
• test/FivePercentFeeHook.withdraw.t.sol (14) — deployer withdrawal, partial and repeated withdrawal, and the failure paths: a non-deployer (fuzzed over callers), the PoolManager itself, withdrawing more than accrued, withdrawing a currency that never accrued, withdrawing twice, and tokens gifted directly to the hook staying unwithdrawable.
• test/FivePercentFeeHook.native.t.sol (9) — the ETH path: fee arriving via receive, native withdrawal, a recipient that rejects ETH reverting the withdrawal, and forced ETH not becoming withdrawable.

Findings (.imd-findings.json)

I reproduced each report before acting on it; all three hold, so none is asserted as correct anywhere in the suite:

1. high — Per-call flooring lets 100 swaps of -19 move 1,900 units for accruedFees[currency0] == 0, where one -1900 swap accrues 95.
2. high — The fee is charged on the requested amount, not the filled one. Exact output 1e18 at sqrtPriceX96 - 1 returns amount1() == -5e16 and the buyer's currency1 balance falls by 5e16. I also confirmed the exact-input twin the reviewer didn't mention: -1e18 at the same limit pays 5.0000000000000002e16 currency0 for zero currency1, and included it in the same finding.
3. medium — take before settlement. On a fresh manager whose only pool holds 3 base units of currency0, a -1e18 exact-input swap reverts (WrappedError wrapping Panic(0x11)); the identical hookless pool fills partially at -5 / +2.

The rounding tests the task asks for are scoped to single-swap truncation and say so in their doc comments — the aggregate consequence is in the findings file, not blessed by an assertion.

forge build is clean (no warnings from src/), and all 20 tests pass.

What I delivered

src/FivePercentFeeHook.sol — a v4 hook charging a fixed 5% on every swap, in both directions, on every pool it's attached to. FEE_NUMERATOR = 5 / FEE_DENOMINATOR = 100 are constant: no constructor arg, no setter, no per-pool value. owner = msg.sender in the constructor; only that address can withdraw(Currency).

src/FivePercentFeeHookDeployer.sol — an internal library that mines the salt with v4-periphery's HookMiner and CREATE2-deploys. Because it's internal it inlines into the caller, so address(this) is the CREATE2 deployer the salt is mined against and msg.sender in the constructor, keeping "whoever deploys can withdraw" true.

The three findings

I could not reproduce them literally — this working tree is the scaffold, not my earlier submission (HEAD is an empty commit; only src/Placeholder.sol exists). So I treated them as design constraints and wrote the hook to satisfy them, then reproduced each reported scenario in test/scratch/:

• Rounding bypass (high) — real. Fee is now ceil(amount * 5 / 100), so every non-zero swap pays ≥ 1 base unit and slicing an order can only raise the total. Measured: 100 × 19-unit swaps pay 100, one 1900-unit swap pays 95. ceil(5a/100) ≤ a for all a, so the fee can never push the trader's side below zero.
• Fee on requested vs filled (high) — real, and inherent to charging in beforeSwap. The fee moved to afterSwap, computed from swapDelta. Since v4 only lets afterSwap keep the unspecified currency, it's 5% of the output on exact-input and 5% of the input on exact-output. Measured on the reported repro: exact-output at sqrtPrice-1 now gives amount0 = -3, amount1 = 0 with the fee in currency0 — the buyer of currency1 never pays currency1. Exact-input at the same limit gives amount0 = -2 and zero fee, not -5e16.
• take before settlement (medium, advisory) — I judged it real. The fee is settled with poolManager.mint of an ERC-6909 claim, which only moves the hook's own delta; withdraw burns the claim and takes real tokens in a separate unlock. Measured on the reported repro (manager holding 3 units of currency0): the swap now partially fills at amount0 = -5, amount1 = 1 instead of reverting with Panic(0x11).

Also covered in scratch: native-ETH pools, two pools sharing one hook, accruedFees[c] == poolManager.balanceOf(hook, c.toId()), owner-only withdrawal, and every swap size from 1 to 25 units.

One thing I did not do

I left src/Placeholder.sol in place. Deleting it, as "replace the placeholder" suggests, breaks forge build: test/Placeholder.t.sol imports it, and test/ is outside my writable paths.

Error (6275): Source "src/Placeholder.sol" not found
 --> test/Placeholder.t.sol:7:1

Since "leave forge build passing" is an acceptance criterion and touching test/ is not permitted, I kept the file. If the harness drops test/Placeholder.t.sol when it pairs my src/ with the test author's tests, src/Placeholder.sol can be deleted at that point — nothing in the hook references it.

Done. Final state: forge build clean, forge test = 58 passed, 0 failed (56 new + the 2 pre-existing placeholder tests), and git status shows only test/ paths added.

The three reported defects do not reproduce

The hook in src/ was rewritten in commit bccd9e8 (after the review that produced those reports). The current code charges the fee in afterSwap on the executed swapDelta, rounds it up, and settles it by minting ERC-6909 claims instead of poolManager.take. I reproduced each report's exact scenario before writing anything:

Report	Reproduction attempt	Result
[high] sub-20-unit swaps bypass the fee	100 × exact-input -19 vs one -1900, same pool	Split pays 100 base units, single order pays 95. Slicing costs more, not zero.
[high] partial exact-output makes the buyer pay the output currency	exact-output 1e18, sqrtPriceLimit = current - 1	delta.amount1() == 0 (never negative), trader's currency1 balance unchanged, fee is 1 unit (5% of the 2 units that executed) — not 5e16 of the request.
[med] take before settlement reverts valid swaps	one-sided pool (liquidity all below price, manager holds 0 currency0), exact-output zeroForOne	Swap succeeds, 5% accrues in currency0 — the currency the manager had none of.

Since none reproduce and I found no new defect, I did not write .imd-findings.json.

Tests

test/utils/FeeHookFixture.sol runs every test against two identical pools — one hooked, one hookless — so the control pool says what the pool alone would have done and the fee is never measured by the hook's own arithmetic (_fivePercentRoundedUp is written out independently rather than calling hook.feeOn). Swaps come from a trader that is neither the LP nor the owner.

• FivePercentFeeHook.t.sol (18) — 5% of output on exact input and 5% of input on exact output, both directions, with balances, deltas, events and claim tokens checked; accrual across swaps and pools; the fee comes off the trader, not the LP; fuzzed rate checks; afterSwap unreachable except from the PoolManager; zero-amount swaps; liquidity/donate accrue nothing.
• FivePercentFeeHookRounding.t.sol (11) — the rounding-toward-zero case: a 1-unit exact-input swap pays no fee and delivers nothing, and fuzzing shows fee == 0 ⇔ nothing delivered. Plus the 19-unit swap, and slicing in three shapes.
• FivePercentFeeHookWithdraw.t.sol (14) — deployer withdraws (per currency, native included, twice → NothingToWithdraw); non-deployer rejected as EOA, as a contract, as the trader/manager/router/hook, with the accrual proven untouched afterwards; ownership follows whoever called the deployer library; unlockCallback unreachable directly.
• FivePercentFeeHookPartialFills.t.sol (8) — the two high-severity scenarios above, partial fills charged on what executed, thin/empty/one-sided pools.
• FivePercentFeeHookInvariants.t.sol (5) — random swap sequences: accrual == claims == what the swaps were seen to pay, and no swap ever pays under the rate.

Evidence the suite isn't vacuous

I rebuilt the three defective implementations in scratch and ran the suite against each: the truncating-fee version fails 19 tests, the charge-on-the-request version 16, the take-before-settle version 28 — each including the specific regression test written for it.

One behaviour worth flagging, which I tested rather than reported: rounding up means a swap whose gross output is 1–19 base units pays more than 5% (at output 1, the fee is that whole unit). It is capped at one base unit and is the deliberate cost of closing the slicing bypass, so test_smallestSwapThatDeliversAnythingIsCharged documents it instead of treating it as a defect.

Review complete. Wrote 3 severity-ranked, reproducible findings to .imd-findings.json:

• 2 medium: deployment gas exhaustion; locked native fees for nonpayable contract deployers.
• 1 low: feeOn(type(uint256).max) overflow.

No implementation, test, or configuration files were changed.