totalValueUsd() multiplies both stablecoin balances by the literal 1e12, i.e. it assumes 6 decimals. The constructor (lines 55-58) checks the usdt/usdc addresses for zero, duplicates, IMD and self, but never checks decimals(), and IMD's decimals are a free uint8 parameter checked only for <= 18. A wrong address or wrong decimal argument therefore does not fail closed: it silently misvalues holdings in either direction.

Over-scaling (an 18-decimal token in a 6-decimal slot) values 1 raw unit at 1e12 WAD so only 1e10 raw units ($0.00000001 nominal) ever fit under the cap and the asset is effectively unusable; under-scaling (a 2-decimal token, or imdDecimals_=18 for a 9-decimal IMD) lets nominal value 1e4..1e9 times the $10,000 cap be accepted.

Mainnet USDT (0xdAC1...1ec7), USDC (0xA0b8...eB48) and IMD (0xD34a...63B7) were read from a public RPC during this review and report 6, 6 and 18 decimals, so the README's expected configuration is correct; the defect is that the contract cannot detect a deviation from it.

Minimal fix that preserves the design: in the constructor require IERC20Metadata(usdt).decimals() == 6, IERC20Metadata(usdc).decimals() == 6 and IERC20Metadata(IMD).decimals() == imdDecimals_ (or read IMD decimals instead of taking a parameter). If the author wants deployment rehearsals without token code present, keep the parameter but add the check behind a code.length > 0 condition.

Boundary x invariant seam: remainingCapacityUsd() tells a depositor exactly how much fits, but the check in _acceptDeposit is made against the live balance, which anyone can raise by a plain ERC20 transfer or forced ETH without going through the vault.

A third party who sees a pending deposit of exactly the remaining capacity can push the total 1e12 WAD (one USDC/USDT unit, $0.000001) over the cap, making the victim's transaction revert and waste its gas; repeated indefinitely at negligible cost while the beneficiary does not withdraw. No funds are lost and the deposit can be retried for a smaller amount, which is why this is low.

The README acknowledges unsolicited balances may exceed the cap but does not describe this griefing path. Mitigations that preserve the design: compute the cap against tracked accepted balances (incremented in _acceptDeposit, decremented in the withdraw paths) so donations cannot consume capacity, or have the frontend recommend leaving a small margin below remainingCapacityUsd().

Trust Gap seam access x economics. The brief asks that the contract "accept maximum of 10K worth of assets". The cap check in _acceptDeposit compares only the vault's present balances (totalValueUsd) against MAX_USD_WAD and the contract keeps no record of cumulative receipts.

Each withdrawal by WITHDRAWER (withdrawETH / withdrawToken / withdrawAll, which carry no pause or policy restriction) therefore frees the same capacity again, and the vault will accept another full $10,000. Across the life of the contract the amount accepted is unbounded; only the instantaneous balance is capped.

The README documents this as the intended reading ("Withdrawals restore deposit capacity"), but the brief's wording is at least as naturally read as a lifetime ceiling. If the requester meant a lifetime cap, a depositor cannot rely on the contract to refuse the 10,001st dollar once the beneficiary has withdrawn.

Fix if a lifetime cap is intended: track a monotonically increasing acceptedUsdWad in _acceptDeposit (acceptedUsdWad += valueOfReceipt; revert if it exceeds MAX_USD_WAD) and leave the withdrawal path unchanged. If the holdings-cap reading is confirmed by the requester, this finding needs no code change.

Access-control scope note, not a permission bypass. upgradeTo only swaps the IDepositPolicy address that is consulted by STATICCALL during deposits. No owner call sequence can alter WITHDRAWER, ETH_PRICE_USD, IMD_PRICE_USD, MAX_USD_WAD, the asset set or the withdrawal logic, and there is no proxy.

This is a deliberate narrowing: the launch policy and the protected runtime check forbid DELEGATECALL/CALLCODE/SELFDESTRUCT and proxies, so a UUPS/transparent upgrade is not deployable here, and the README states the tradeoff. The judge should confirm the requester accepts this reading of "upgrade"; if a fuller upgrade path was expected it requires a scope decision outside this contract (a new reviewed deployment plus funds moved by the beneficiary).

Trust assumptions in this area, recorded for completeness: the owner can pause deposits indefinitely and can install a policy that rejects all or selected depositors; neither power touches withdrawals (verified: withdrawETH/withdrawToken/withdrawAll carry only nonReentrant + onlyWithdrawer and never read depositPolicy or _paused).

The brief asks for a contract that "should accept maximum of 10K worth of assets". _acceptDeposit() enforces the cap only against totalValueUsd(), i.e. the balances the vault holds at that moment (src/CappedAssetVault.sol:153-154). Nothing records how much value the deposit entry points have already accepted, so every withdrawal by WITHDRAWER re-opens capacity and the same vault accepts another full $10,000 through depositETH()/depositToken()/receive().

Over its lifetime the vault therefore accepts an unbounded amount: N withdraw-and-refill cycles accept N x $10,000.

README.md states this is intentional ("Withdrawals restore deposit capacity"), so this is a specification-interpretation finding rather than an implementation slip: if the requester meant a ceiling on what the vault holds at once, no change is needed; if they meant a ceiling on what it will ever accept (the plainer reading of "accept maximum of 10K"), the current code violates it.

Economic consequence under the lifetime reading: there is no limit to the funds the fixed beneficiary can collect from depositors through this contract.

Minimal design-preserving fix if the lifetime reading is intended: add uint256 public acceptedUsdWad;, increment it in _acceptDeposit() by the USD value of the amount actually received (wei2600, raw1e12, imdRaw*_imdUsdPerUnit) and revert when acceptedUsdWad + delta > MAX_USD_WAD; keep the existing holdings check if unsolicited balances should still count.

Untested edge: no existing test deposits again after a full withdrawal and asserts a limit; test_WithdrawalReopensCapacity asserts the opposite.

Flow-gap seam (periphery x first principles). Every token withdrawal pays the hard-coded constant WITHDRAWER (src/CappedAssetVault.sol:173; ETH at line 164) and there is no recipient parameter, no owner sweep and no migration. Ethereum USDC (Circle FiatTokenV2) and USDT (TetherToken) both revert any transfer whose from or to is on the issuer blacklist.

The brief's guarantee is that the withdrawer "shall be withdraw it any time any amount, full too".

If Circle or Tether blacklists 0x047F606fD5b2BaA5f5C6c4aB8958E45CB6B054B7 (an EOA with about 2,700 sent transactions at mainnet block 26133522, so it has a live compliance footprint) or blacklists the vault address itself, every withdrawToken()/withdrawAll() call for that stablecoin reverts inside safeTransfer, forever: WITHDRAWER cannot redirect to a clean address and the owner has no path at all (UnauthorizedWithdrawer).

Deposits of that stablecoin keep succeeding meanwhile, so the stranded amount can grow to the full cap. The same single-recipient design also means loss of the payee's key freezes everything, which README.md already documents. Precondition is an action by the token issuer, so severity is kept at low; impact when it occurs is permanent loss of the vault's entire holding of that stablecoin (up to $10,000 at fixed prices).

Minimal fix that preserves the brief (only that address may withdraw): let the withdrawer name the recipient, e.g. withdrawToken(address asset, uint256 amount, address to) / withdrawAll(address asset, address to) with to != address(0) and still onlyWithdrawer; if paying only WITHDRAWER is a deliberate hardening, record the blacklist-stranding risk as an accepted trust assumption.

Untested edge: the suite has no blacklisting token mock; MockToken only models revert-all, false-return, fee and no-movement.

The brief only requires that 0x047f...54b7 be the sole caller allowed to withdraw; the implementation additionally forces every payout to that same address (line 164 for ETH, line 173 for tokens). USDC and USDT can blacklist any address, and both revert transfers to a blacklisted recipient.

If the beneficiary is ever blacklisted, every withdrawToken/withdrawAll for that stablecoin reverts forever; the owner has no sweep, no recipient parameter and no way to change WITHDRAWER, so the balance (up to the full $10,000 cap in that asset) is stuck permanently. The same structure makes the vault unrecoverable if the beneficiary's key is lost, which README.md acknowledges.

WITHDRAWER was verified on mainnet (block 26133523) to be an active EOA (nonce 2681), so ETH payouts via call{value} work today. Minimal fix that keeps the brief's rule: keep onlyWithdrawer but let the withdrawer pass a to address (e.g. withdrawETH(uint256 amount, address to)), or add withdrawTo(asset, amount, to) alongside the existing functions.

The brief asks that the owner 'can upgrade'. Because the launch recipe forbids proxies, DELEGATECALL and SELFDESTRUCT, the author implemented upgradeTo() as a swap of the IDepositPolicy contract that is consulted by STATICCALL during deposits.

This is documented in README.md and is a reasonable reading, but the requester should be aware of what it does not cover: ETH_PRICE_USD, IMD_PRICE_USD, MAX_USD_WAD, USDT, USDC, imdDecimals and WITHDRAWER are constants/immutables with no setter. A deployment mistake in a constructor argument (e.g. imdDecimals_ != 18, wrong stablecoin address) or any later change of requirements needs a new deployment and a manual move of funds by the beneficiary.

On mainnet (block 26133523) IMD at 0xD34a...63B7 is a non-proxy LayerZero OFT with 18 decimals and plain OpenZeppelin ERC20 transfers, and USDT/USDC at the README addresses report 6 decimals, so the expected constructor values are correct today. No code change is proposed; this records the scope decision for the requester.

The suite is thorough on cap boundaries, token misbehaviour, pause and policy upgrades, but three paths I traced have no test. (1) withdrawToken(address(0), x) and withdrawToken(, x): both revert because balanceOf on a codeless address fails ABI decoding, which is the intended behaviour but is only implied by the address(0)-means-ETH branch of withdrawAll.

(2) receive() invoked with the 2,300-gas stipend (Solidity transfer/send) reverts because totalValueUsd() makes three external balanceOf calls plus the policy STATICCALL; README documents it but no test pins it, so a future change that makes receive() cheaper or more expensive would go unnoticed.

(3) Because the override rejects transferOwnership(address(0)), the only way to cancel a pending two-step transfer is transferOwnership(currentOwner); this works (pending becomes the owner itself, the old candidate can no longer accept) but is untested and undocumented. All three were verified locally with scratch tests; each behaves as intended today.

Merged from audit_math (info), audit_flow (medium), audit_permissions (low) and audit_economics (low): same root cause. _acceptDeposit compares only totalValueUsd(), i.e. the balances the vault holds right now at fixed prices, against MAX_USD_WAD. No state records how much value the deposit entry points (depositETH, depositToken, receive) have already admitted.

Because WITHDRAWER can withdraw at any time, each withdrawal frees the same $10,000 and the vault admits another $10,000, indefinitely: N refill cycles admit N x $10,000. The brief says the contract 'should accept maximum of 10K worth of assets'.

README.md and launch.json notes document the holdings-ceiling reading ('withdrawals restore capacity'), and the existing test test_WithdrawalReopensCapacity asserts it, so this is a specification interpretation the author chose deliberately, not an implementation slip. If the requester confirms the holdings-ceiling reading, no code change is needed.

If the requester meant a lifetime ceiling on what the vault ever accepts (a plausible reading of 'accept maximum of 10K'), that guarantee is currently not enforced at all. Severity is set to low because the behaviour is documented and intentional; it would be medium if the lifetime reading is confirmed.

Minimal design-preserving fix for the lifetime reading: add uint256 public acceptedUsdWad;, compute the USD value of received in _acceptDeposit with the same multipliers as totalValueUsd (wei2600, raw1e12, imdRaw*_imdUsdPerUnit), revert if acceptedUsdWad + delta > MAX_USD_WAD, then increment; keep the existing holdings check so unsolicited balances still count.

Both specialist proofs (.imd/reads/proofs/Proof_5ae97d07fb75.t.sol, Proof_ac2466046664.t.sol) were run and fail on the current code for the stated reason ('next call did not revert as expected' and 'vault accepted $20,000 of assets over its lifetime'). No proof is attached here because the finding is low and attaching one would bind the fix to the lifetime reading before the requester has chosen it.

Merged from audit_flow and audit_economics (same root cause, same line). The brief requires only that 0x047f...54b7 be the sole party allowed to withdraw. The implementation additionally forces every payout to that same constant address (line 164 for ETH, line 173 for tokens) and offers no recipient parameter, no owner sweep and no migration.

Mainnet USDC (FiatTokenV2) and USDT (TetherToken) revert transfers whose from or to is on the issuer blacklist. If the beneficiary is blacklisted, every withdrawToken/withdrawAll for that stablecoin reverts inside safeTransfer forever; WITHDRAWER cannot redirect and the owner is rejected by onlyWithdrawer. Deposits of that stablecoin continue to be admitted meanwhile, so the stranded amount can grow to the full $10,000 cap.

The same structure means loss of the beneficiary key freezes everything, which README.md documents as accepted. The precondition is an issuer action against a specific address, so severity stays low; the impact when it occurs is permanent loss of the vault's entire holding of that stablecoin. README.md mentions that transfers remain subject to issuer blacklists but does not state that no recovery path exists.

Minimal fix that keeps the brief's rule intact: keep onlyWithdrawer and let the caller name the recipient, e.g. withdrawToken(address asset, uint256 amount, address to) and withdrawAll(address asset, address to) with to != address(0); or record blacklist stranding explicitly as an accepted trust assumption.

Untested edge: the suite has no blacklisting token mock (MockToken models revert-all, false-return, fee and no-movement only).

From audit_math; reproduced. totalValueUsd multiplies both stablecoin balances by the literal 1e12 (assumes 6 decimals) and IMD by 9 * 10**(18 - imdDecimals_), where imdDecimals_ is a constructor argument checked only for <= 18. The constructor (lines 55-58) checks the usdt/usdc arguments for zero, duplicates, IMD and self, but never reads decimals() from any token. A wrong address or wrong decimals argument therefore does not revert: it silently over- or under-values holdings.

Over-scaling (an 18-decimal token in a stablecoin slot) makes the asset unusable (only 1e10 raw units fit under the cap); under-scaling (a low-decimal token, or imdDecimals_=6 for the 18-decimal IMD) lets nominal value far above the cap be admitted or blocks IMD deposits entirely. launch.json carries the correct mainnet values (USDT 0xdAC1...1ec7, USDC 0xA0b8...eB48, imdDecimals 18), and README.md requires live verification before deployment, so the current manifest is not misconfigured; the defect is that the contract cannot detect a deviation and no owner action (pause/unpause/upgradeTo) can correct one after deployment.

README.md explains the constructor deliberately makes no token calls so the protected deployment rehearsal can run without token code at those addresses. A fix compatible with that constraint: in the constructor, when usdt.code.length > 0 require IERC20Metadata(usdt).decimals() == 6, likewise for usdc, and when IMD.code.length > 0 require IERC20Metadata(IMD).decimals() == imdDecimals_. On mainnet all three have code, so the check is live where it matters.

From audit_math; reproduced. remainingCapacityUsd() tells a depositor exactly how much fits, but _acceptDeposit evaluates the cap against the live balance, which anyone can raise with a plain ERC20 transfer or forced ETH without going through the vault. A third party who sees a pending deposit of exactly the remaining capacity can push the total 1e12 WAD (one USDC/USDT unit, $0.000001) over the cap, so the victim's transaction reverts and its gas is wasted.

This can be repeated at negligible cost while the beneficiary does not withdraw. No funds are lost and the depositor can retry with a smaller amount, hence low. The existing test test_DirectTokenDonationsCountAndCanExceedCap covers donations counting toward the cap but not this ordering.

Mitigations that preserve the design: track admitted balances per asset (incremented in _acceptDeposit, decremented in the withdraw paths) and check the cap against those so donations cannot consume capacity; or document that frontends should leave a small margin below remainingCapacityUsd(). The behaviour is inherent to a balance-based cap, so recording it as accepted is also a valid resolution.

Merged from audit_flow and audit_permissions (same observation). Not a permission bypass. upgradeTo swaps only the IDepositPolicy address consulted by STATICCALL during deposits. ETH_PRICE_USD, IMD_PRICE_USD, MAX_USD_WAD, IMD, WITHDRAWER are constants; USDT, USDC and imdDecimals are immutables; there is no proxy and no setter for any of them.

This is a deliberate narrowing forced by the launch recipe and protected runtime check, which forbid proxies, DELEGATECALL, CALLCODE and SELFDESTRUCT; README.md and the launch.json notes state the trade-off. Consequences the requester should accept explicitly: a constructor-argument mistake or any later change of requirements needs a new reviewed deployment plus a manual move of funds by the beneficiary.

Trust assumptions recorded for completeness: the owner can pause deposits indefinitely and can install a policy that rejects all or selected depositors; neither power touches withdrawals (withdrawETH/withdrawToken/withdrawAll carry only nonReentrant + onlyWithdrawer and never read depositPolicy or the pause flag). No code change proposed.

From audit_flow, extended with the blacklist gap from finding 2. The suite covers cap boundaries, token misbehaviour, pause, policy upgrades, two-step ownership and stateful sequences, but four paths have no test: (1) withdrawToken(address(0), x) and withdrawToken(, x) both revert because balanceOf on a codeless address fails ABI decoding; this is intended but only implied by the address(0)-means-ETH branch of withdrawAll.

(2) receive() invoked with the 2,300-gas stipend reverts because totalValueUsd makes three external balanceOf calls plus the policy STATICCALL; README.md documents it but no test pins it. (3) Because the override rejects transferOwnership(address(0)), the only way to cancel a pending handover is transferOwnership(currentOwner); this works but is untested and undocumented. (4) No mock models issuer blacklisting of the payee or vault.

All four were checked with scratch tests and behave as the README intends today; this records what the suite does not pin.