# Task 4 Report: V1 Custody Accounting and Adversarial Unit Tests

## Status

Complete. `BankV1` now supports exact-accounting deposits and CEI withdrawals behind the existing UUPS proxy. The focused custody suite, aggregate V1 suite, and full offline suite pass.

## Implementation

- Added `Deposited` and `Withdrawn` events and the exact `ZeroAmount`, `InsufficientBalance`, and `UnexpectedAssetDelta` errors.
- Added `deposit(uint256)` guarded by `whenNotPaused` and `nonReentrant`. It measures proxy reserves before and after `SafeERC20.safeTransferFrom`, rejects any non-exact received amount, and credits customer balance/liabilities only after the transfer and delta validation succeed.
- Added `withdraw(uint256)` guarded by `whenNotPaused` and `nonReentrant`. It validates zero/available balance, debits customer balance and liabilities before the external `SafeERC20.safeTransfer`, and emits the withdrawal event.
- Preserved application storage exactly: `_asset` slot 0, `_balances` slot 1, `_totalLiabilities` slot 2, and `uint256[47] __gap` slot 3.
- Kept all application behavior proxy-bound and added no sweep/rescue interface.

## Changed Files

- `src/BankV1.sol` — custody API, errors/events, SafeERC20 accounting.
- `test/BankV1.t.sol` — 18 deterministic and 3 fuzz custody tests.
- `test/mocks/FeeOnTransferToken.sol` — six-decimal ERC-20 whose `transferFrom` delivers 99% and burns the remainder.
- `test/mocks/ReentrantToken.sol` — six-decimal ERC-20 with configurable deposit/withdraw callbacks, revert propagation, nested-call result/selector recording, and callback-time accounting observations.

## TDD RED

Tool paths were set separately in a dedicated shell:

```bash
export PATH=/tmp/codius-node-v24.18.0/bin:/home/golem/.foundry/bin:$PATH
```

The required RED command was run verbatim:

```bash
npm_config_offline=true forge test --match-path test/BankV1.t.sol -vvv --force
```

The first invocation found a Solidity tuple-declaration syntax error in the new `ReentrantToken` test double. Only that test-double syntax was corrected; production remained unchanged. The command was rerun verbatim and produced the valid expected RED:

```text
Compiler run failed:
Error (9582): Member "deposit" not found or not visible after argument-dependent lookup in contract BankV1.
  --> test/BankV1.t.sol:28:9:
28 |         bank.deposit(100e6);
   |         ^^^^^^^^^^^^
Error: Compilation failed
```

This RED was expected because the custody tests called the required proxy-bound `deposit`/`withdraw` API before those functions existed. It therefore failed for the missing production behavior rather than a malformed assertion or environment problem.

## GREEN Verification

All upgrade-validation-bearing commands were kept offline.

```bash
forge fmt
```

Output: completed successfully and formatted `test/BankV1.t.sol` on the initial run; the final run completed with no further formatting changes.

```bash
npm_config_offline=true forge test --match-path test/BankV1.t.sol -vvv --force
```

Final output summary:

```text
Ran 21 tests for test/BankV1.t.sol:BankV1CustodyTest
Suite result: ok. 21 passed; 0 failed; 0 skipped
Fuzz tests: 512 runs each
```

```bash
npm_config_offline=true forge test --match-path 'test/BankV1*.t.sol' --force
```

Final output summary:

```text
BankV1AdminTest: 12 passed; 0 failed; 0 skipped
BankV1CustodyTest: 21 passed; 0 failed; 0 skipped
Ran 2 test suites: 33 passed, 0 failed, 0 skipped
```

Full-suite command:

```bash
npm_config_offline=true forge test --force
```

Final output summary:

```text
BankV1AdminTest: 12 passed
MockUSDCTest: 7 passed
BankV1CustodyTest: 21 passed
Ran 3 test suites: 40 passed, 0 failed, 0 skipped
```

Solc emitted only two pre-existing dependency warnings in `openzeppelin-foundry-upgrades` about functions whose mutability could be `pure`; there were no project warnings or test failures.

Storage inspection:

```bash
forge inspect src/BankV1.sol:BankV1 storage-layout
```

Output confirmed `_asset` at slot 0, `_balances` at slot 1, `_totalLiabilities` at slot 2, and `__gap` at slot 3.

## Accounting and Adversarial Coverage

- Exact wallet, reserve, customer balance, and total-liability deltas for deposit and withdrawal.
- Indexed event emission for each custody operation.
- Zero amount and paused-state rejection for both operations.
- Exact ERC-6093 inadequate allowance and wallet-balance failures, with empty bank accounting after revert.
- Independent deposits for Alice and Bob and a withdrawal that leaves the other customer's balance unchanged.
- Fee-on-transfer deposit rejection with `UnexpectedAssetDelta(100e6, 99e6)` and atomic rollback of sender balance, bank reserves, customer credit, and liabilities.
- Deposit callback observes zero customer credit/liabilities before transfer completion, attempts nested deposit, and records `ReentrancyGuardReentrantCall`; swallowing the nested revert credits exactly once, while configured propagation atomically reverts the outer deposit.
- Withdrawal callback observes already-debited customer balance/liabilities, attempts nested withdrawal, records `ReentrancyGuardReentrantCall`, and cannot double debit.
- Direct-transfer surplus: 100 mUSDC liability plus 25 mUSDC direct transfer yields 125 mUSDC reserves; withdrawing the full 100 mUSDC customer balance leaves the 25 mUSDC surplus.
- Deterministic fuzzing uses the existing fixed seed, bounds deposits to `[1, 1_000_000e6]`, bounds withdrawals to `[1, deposited]`, proves exact deltas, and proves every bounded over-withdraw reverts with exact available/requested values.

## Self-Review

- `git diff --check` passed.
- Diff inspection confirmed no persistent storage changes and no owner sweep/rescue behavior.
- Mutation review: tests fail for missing credit/debit/liability changes, incorrect transfer amount or event, removed zero/pause/available checks, fee-token over-credit, credit-before-deposit-transfer, interaction-before-withdraw-effects, absent reentrancy guard, customer cross-account corruption, or surplus consumption.
- Independent read-only review reported no Critical, Important, or Minor findings and assessed the change ready to merge. The reviewer independently saw 21/21 focused and 33/33 aggregate tests pass and confirmed formatting/diff checks.
- Only the four task-listed source/test files are staged for the commit; this report is intentionally outside that scoped commit.

## Concerns

- The bank intentionally supports only standard non-rebasing ERC-20 assets. Outbound transfer-fee/rebasing behavior is outside V1's stated contract.
- `ReentrancyGuardTransient` requires Cancun/EIP-1153, matching the project's configured target.
- The two compiler warnings originate in the pinned `openzeppelin-foundry-upgrades` dependency and are unrelated to this change.